gh-136003: Execute pre-finalization callbacks in a loop #136004
Conversation
| /* Each of these functions can start one another, e.g. a pending call | ||
| * could start a thread or vice versa. To ensure that we properly clean | ||
| * call everything, we run these in a loop until none of them run anything. */ | ||
| for (;;) { |
There was a problem hiding this comment.
Would it make sense to add an arbitrary limit to detect infinite loop? For example, log an error after 16 attemps.
There was a problem hiding this comment.
Well, it would prevent deadlocks in rare cases, but it would cause crashes in other equally rare cases. Maybe it would be better to emit a fatal error when there are too many iterations?
|
When you're done making the requested changes, leave the comment: |
…cular-finalization
This solution is much simpler, but will perform a little bit worse. Instead of using a dedicated lock on the interpreter state, we can simply use stop-the-world (and the GIL on the default build) to ensure that no other threads can create pre-finalization callbacks concurrently.
|
The failing CI jobs are due to Ubuntu being weird, and should disappear when I rerun the job. But I'll take that as a sign that I should run the buildbots before merging this. |
ZeroIntensity
added
the
🔨 test-with-buildbots
This comment was marked as outdated.
This comment was marked as outdated.
bedevere-bot
removed
the
🔨 test-with-buildbots
|
Nevermind, I broke FT when I fixed the merge conflicts. Ugh. |
ZeroIntensity
added
the
🔨 test-with-buildbots
|
🤖 New build scheduled with the buildbot fleet by @ZeroIntensity for commit e202848 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F136004%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
bedevere-bot
removed
the
🔨 test-with-buildbots
|
@ericsnowcurrently I'll hold off on merging until we talk tomorrow. We might want to extend the check to subinterpreters, and then disallow creating new interpreters during finalization. |
…cular-finalization
Apparently, Py_AddPendingCall() schedules for the main interpreter.
…ng finalization (GH-140103) This fixes a regression introduced by GH-136004, in which finalization would hang while executing atexit handlers if the system was out of memory. --------- Signed-off-by: yihong0618 <zouzou0208@gmail.com> Co-authored-by: Peter Bierma <zintensitydev@gmail.com> Co-authored-by: Victor Stinner <vstinner@python.org>
5 participants
Uh oh!
There was an error while loading. Please reload this page.