Move the ThreadChecker field in front of dict and weakref.

[sebpuetz]

Addresses #1022

Changing the order doesn't seem to cause any other issues. If changing the order seems risky, we can also calculate the initial offsets based on offset -= std::mem::size_of::<T::ThreadChecker>() as isize;

---

Offsets for dict and weakref are calculated from the end of the PyCell struct. When using the non-dummy ThreadChecker, the offsets were invalid since the ThreadCheckerImpl is not zero-sized.

---

Thank you for contributing to pyo3!

Here are some things you should check for submitting your pull request:

• Run cargo fmt (This is checked by travis ci)
• Run cargo clippy and check there are no hard errors (There are a bunch of existing warnings; This is also checked by travis)
• If applicable, add an entry in the changelog.
• If applicable, add documentation to all new items and extend the guide.
• If applicable, add tests for all new or fixed functions
• If you changed any python code, run black .. You can install black with pip install black)

You might want to run tox (pip install tox) locally to check compatibility with all supported python versions. If you're using linux or mac you might find the Makefile helpful for testing.

[davidhewitt]

That sounds like the correct fix to me, thanks!

Could you please add a CHANGELOG entry and also a test?

"Fix segfault with #[pyclass(dict, unsendable)] is probably good enough message

[sebpuetz]

I keep forgetting about the changelog! Thanks for the reminder.

The amended commit has a (hopefully correct) test case and the changelog entry.

[davidhewitt]

Looks great, many thanks for this!

[kngwyu]

@sebpuetz Thanks!
@davidhewitt Thank you for your review, but could you please wait for my review if a PR is related to what I solely wrote ?
In such cases, I want to understand what I misunderstood.

[davidhewitt]

but could you please wait for my review if a PR is related to what I solely wrote ?

👍 apologies for going too fast here!

[sebpuetz]

Since there's still some life in here:

It probably would be nice to add an in-line comment on PyCell mentioning that the dict and weakref fields must be at the end of the struct. I'm also wondering if there is a cleaner solution for this that doesn't depend on the order of struct fields in one place and subtracting the correct offsets in a different place.

[kngwyu]

I'm also wondering if there is a cleaner solution for this that doesn't depend on the order of struct fields

I think it's difficult since we don't have the concrete PyCell at that time, though we can do some cleanups around that.