Skip to content

Big overhaul part 1 - replace PyRc with manual RefCount + WeakRefList #3386

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Nov 21, 2021

Conversation

coolreader18
Copy link
Member

@coolreader18 coolreader18 commented Oct 28, 2021

This resolves #2382, resolves #2381 (afaict), and at the very least it will make it much easier to fix any UB now that we control the internals of all the data structures we use.

@fanninpm
Copy link
Contributor

I'll get to this tonight if someone else doesn't beat me to the punch, but weakref.py needs an update to at least CPython 3.9 (as it hasn't been touched in over two years).

@coolreader18 coolreader18 force-pushed the no-arc branch 5 times, most recently from bfe7aff to 37e1394 Compare October 28, 2021 20:29
@coolreader18
Copy link
Member Author

@fanninpm ooh yea, I noticed that myself, that's really old lol.

@coolreader18 coolreader18 force-pushed the no-arc branch 3 times, most recently from 7cf10e3 to 69f5c80 Compare October 29, 2021 02:48
@fanninpm
Copy link
Contributor

At this point, running Miri without MIRIFLAGS='-Zmiri-ignore-leaks' still reports a memory leak.

@coolreader18
Copy link
Member Author

Well, that's probably to be expected, since we don't implement cycle collection and especially type_type and object_type are codependent on each other

@coolreader18 coolreader18 force-pushed the no-arc branch 3 times, most recently from d41f3e0 to e498933 Compare November 3, 2021 20:11
Copy link
Member

@youknowone youknowone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to review drop part again, but looks great in general.

@coolreader18 coolreader18 force-pushed the no-arc branch 5 times, most recently from 05b3a28 to ed289f7 Compare November 10, 2021 20:40
@coolreader18
Copy link
Member Author

...ok what

@coolreader18
Copy link
Member Author

what is that error

@coolreader18
Copy link
Member Author

did an exception cross an ffi boundary in rustc??

@coolreader18
Copy link
Member Author

Ok phew, just rerunning ci seemed to have fixed it.

The outer struct is only 1 word now, and inside the box it's a byte for
the mutex (which gets padded to a word) + a word for the linked list (we
don't use the tail) + a word for the pointer to the object.
This makes debuggers (or rust-gdb, at least) more pleasant to use, since
you don't have to manually cast `PyRef<T>.obj.ptr as `*const PyObjectView<T>`
Also get rid of PyGenericObject, since it's vestigial
@coolreader18 coolreader18 merged commit a53451b into RustPython:main Nov 21, 2021
@coolreader18 coolreader18 deleted the no-arc branch November 21, 2021 23:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Miri catches some undefined behavior
3 participants