[johng]

In both uncollectable and collectables we would
iterate through gc items using GC_NEXT(gc).
However in the case the current item was cleaned
during iteration this would reuslt in UAF when
fetching the next step in the loop.

The fix is to move each item to a temporary seen list
before printing it, and always take the next
item from the head of the original list, so the
loop never reads from a freed item. This takes
a similar approach to finalize_garbage().

• Issue: gcmodule: GIL drop during collection in debug mode may result in unexpected behavior with multiple threads #112564

[johng]

Wonder if we just call PyUnstable_GC_VisitObjects to avoid maintaining two copies of the traversing code here

[sergey-miryanov]

Same notes about tests and list traversal.

[bedevere-app]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

[sergey-miryanov]

@johng Could you please fix a cast warning in debug_cycles?

[sergey-miryanov]

Thanks! LGTM.

[sergey-miryanov]

@nascheme Could you please take a look?

[nascheme]

As mentioned on the other issue, my inclination would be to have a way to output debug info without the GIL drop. Likely we can't be using sys.stderr and have to write directly to the stderr file descriptor. It would be nicer if we could output to whatever sys.stderr is bound to, more user friendly, but I'm afraid that could make the GC implementation a fair bit more complex.

OTOH, if we look what's done for resource warnings, they do get written to sys.stderr. PyErr_ResourceWarning takes extra trouble to try to write to it.

Another possible solution: write the debugging output to a buffer while GC is in the middle of its work. Then, write that buffer to sys.stderr at a safe place. Buffering the output should be fine.

[johng]

Happy to explore those suggestions, thanks @nascheme !

[johng]

I think the buffer is the cleanest approach, it does result in iterating twice through the loop since we need to know the length ahead of time for buffer allocations unless we have something quite a bit more complex (I believe)