gh-75537: Add gc.ensure_disabled() context manager - #154999
gh-75537: Add gc.ensure_disabled() context manager#154999SakshamKapoor2911 wants to merge 5 commits into
Conversation
Add a context manager to the gc module that temporarily disables the cyclic garbage collector and reliably restores it to its previous state on exit. Uses PyGC_Disable() which returns the old state, making nesting and restoration atomic under the GIL. The feature was originally merged in 2018 (PR python#4224) but reverted due to edge cases around GIL release, debug build crashes, and unclear nesting semantics. The modern C API (PyGC_Disable returning old state) resolves the state-tracking issues that caused the revert.
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
Documentation build overview
9 files changed ·
|
3dc9842 to
060cb64
Compare
060cb64 to
53e93bc
Compare
There was a problem hiding this comment.
In my opinion, there much, much more subtlety around pausing (defering):
#153946 (comment)
#153946 (comment)
#154188 (comment)
#154188 (comment)
Perhaps the right course of action is reusing internals that'd be added by #154188.
|
|
||
|
|
||
| PyDoc_STRVAR(gc_ensure_disabled__doc__, | ||
| "ensure_disabled() -> context manager\n" |
There was a problem hiding this comment.
gc.ensure_disabled() without with would be irreversible?
There was a problem hiding this comment.
That's correct, and consistent with all Python context managers. threading.Lock().acquire() without with also leaves the lock held indefinitely. This is a known limitation: exit only fires via the with statement. Adding a tp_dealloc fallback to re-enable GC on garbage collection would cause surprising behavior (the GC silently re-enabling at an unpredictable time). The with statement is the only supported usage.
There was a problem hiding this comment.
@maurycy Do you think this is a reasonable tradeoff, or would you suggest I made modifications to prevent this?
|
Thanks for the pointer to #154188 and #153946. I think these are complementary layers of the same capability:
They differ by design: the private API preserves gc.isenabled() because internals shouldn't change user-visible state. The public API should change gc.isenabled() because users need to see the effect. The 2018 merged implementation (PR #4224) also used disable/enable, and the revert was for unrelated issues (GIL release, debug crash) that PyGC_Disable() returning old state now resolves. |
53e93bc to
5a55da3
Compare
|
@SakshamKapoor2911 just small suggestion please avoid |
|
This PR is premature. The discussion did not advance since 3y ago and I believe a newer discussion is necessary first. |
picnixz
left a comment
There was a problem hiding this comment.
I do not understand why we could not have a pure Python function for that, left to users themselves. What is wrong with manually disabling/enabling the GC first?
|
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 |
Fixes: #75537
Summary
Add
gc.ensure_disabled()— a context manager that temporarily disables the cyclic garbage collector and reliably restores it to its previous state on exit.Background
This feature was originally implemented by @pablogsal in 2018 (PR #4224), merged by @rhettinger, and subsequently reverted (PR #5495/#5496) due to edge cases around GIL release, debug build assertions, and threading concerns during the 3.7 beta window.
The modern CPython C API (
PyGC_Disable()returns the old state) resolves the key state-tracking issue. The implementation:PyGC_Disable()(returns old state) for atomic state save/restoreTests
Four new tests in
Lib/test/test_gc.py(GCTogglingTests):test_ensure_disabled,test_ensure_disabled_nesting,test_ensure_disabled_already_disabled,test_ensure_disabled_exceptionExisting test suite: 59 tests pass, 0 regressions.