classification
Title: Add context manager to temporarily disable GC
Type: enhancement Stage: needs patch
Components: Interpreter Core, Library (Lib) Versions: Python 3.8
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: YoSTEALTH, eric.snow, gregory.p.smith, josh.r, lisroach, ncoghlan, ned.deily, pablogsal, pitrou, pmpp, rhettinger, serhiy.storchaka, yselivanov
Priority: normal Keywords: patch

Created on 2017-09-05 23:47 by rhettinger, last changed 2018-02-05 06:29 by serhiy.storchaka.

Pull Requests
URL Status Linked Edit
PR 4224 merged pablogsal, 2017-11-02 00:24
PR 5456 closed lisroach, 2018-01-31 06:16
PR 5462 closed yselivanov, 2018-01-31 16:14
PR 5495 merged yselivanov, 2018-02-02 13:40
PR 5496 merged miss-islington, 2018-02-02 14:31
Messages (50)
msg301407 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2017-09-05 23:47
Used like this:

    with gc_disabled():
        run_some_timing()

    with gc_disabled():
        # do_something_that_has_real_time_guarantees
        # such as a pair trade, robotic braking, etc

This would be implemented in C for atomicity and speed.  It would be roughly equivalent to:

    @contextmanager
    def gc_disabled():
        if gc.isenabled():
            gc.disable()
            yield
            gc.enable()
        else:
            yield
msg301633 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2017-09-07 21:36
+1 from me for the general idea. As far as where to put it goes, I think the `gc` module would be the most appropriate home.
msg301636 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-09-07 21:55
Note that threads can break this. Even without calling gc.enable() explicitly, "with gc_disabled()" in different thread can enable GC inside other "with gc_disabled()" block.
msg301644 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2017-09-07 22:31
Yes, this will be in the same category as the standard stream redirection context managers - multi-threaded applications either won't be able to use it, or else will need to manage access to it somehow.
msg301645 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2017-09-07 22:43
I believe Raymond is aware of the thread issue.  We discussed it.

If gc.disable() would return the previous state of the gc instead of None and an API to enable based on a passed in bool, both of which are written in C and executed with the GIL (or merely another lock protecting changing the gc state rather than the GIL) held and this Python would works with multiple threads all fighting to control the gc state:

@contextmanager
def gc_disabled():
  previous_enable_state = gc.disable()
  yield
  gc.set_enable(previous_enable_state)

But we don't need to modify gc.disable's return value or add a set_enable() if gc_disabled() itself is not implemented in Python.  That's up to the implementer.
msg301710 - (view) Author: Josh Rosenberg (josh.r) * Date: 2017-09-08 18:02
I'd be -1 on this without a demonstrated broad need for this in at least some context outside of microbenchmarking utilities (which presumably have already implemented similar stuff).

If a minimum bar for applicability isn't applied, we'll end up with dozens of these special purpose managers justified by the last limited applicability one. Stuff like contextlib.closing is justifiable (though I wish it allowed you to replace the method name called, so it could call terminate, kill, release, what-have-you) since cleanup close is so common, but disabling and reenabling gc is usually for timing purposes, and there aren't *that* many tools that need it.

It doesn't seem all that useful for real time purposes either; sure, it disables gc, but it doesn't disable other forms of implicit non-local code execution that are surprisingly hard to predict (e.g. object destruction, including __del__, for non-cyclic cases is going to depend on whether the stuff being decref-ed is still owned outside the block). Disabling GC means a lot in Java, because *all* cleanup is GC; in Python, it's just cycle collection, so you're not giving much in the way of guarantees, as the code in question has to be written with a *lot* of assumptions that Python usually can't support (it's hardly a realtime language).

Seems like if you want a speed up and reliability for this case (and all other context managers intended to be low overhead), it would be better to move parts of contextlib to the C layer (e.g. contextmanager and the classes it's implemented in terms of), so the simple and correct throwaway implementations for arbitrary custom use cases are fast enough; atomicity clearly doesn't actually matter here (it can't be made atomic in any meaningful sense given the lack of thread safety), so any real problem with the provided implementations (assuming they had try/finally added to make them robust) is overhead, not atomicity; the code posted so far would be fine (after adding try/finally) without the speed issues.
msg301720 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2017-09-08 19:07
This is being proposed for the gc module, not contextlib, so I'm not sure how the previous comment applies.

This is just a convenience API for folks already doing this kind of manipulation themselves.
msg302903 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2017-09-25 01:58
Assigning this to Lisa for a C implementation, docs, and tests.
msg305402 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2017-11-02 00:26
I have prepared a PR in GitHub with an initial implementation of the context manager trying to fulfil the discussed requirements: https://github.com/python/cpython/pull/3980
msg306053 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2017-11-10 22:30
I just realize that the link I provided is incorrect. This is the correct one (also is the one that appears in this issue anyway):

https://github.com/python/cpython/pull/4224
msg306054 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-11-10 22:46
What is the name of the context manager? gc_disabled, disabled, or Disabled? I see different names in the PR and this confuses me.
msg306071 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2017-11-11 00:18
Sorry about that. The context manager is "gc.Disabled()", which I admit is probably a bad name. The documentation is an example of the "equivalent" Python code as stated by Raymond in the first comment but I see now that this will raise confusion. Please, can you indicate what will be a correct name for the context manager so I can update the PR?
msg306072 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2017-11-11 00:22
Just to clarify the current situation: At this point, the contextmanager is referred as "disabled" in the C code but is exported as "Disabled" to the garbage collection module. The "gc_disabled" is the Python "equivalent" given by Raymond that is included in the documentation
msg306076 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2017-11-11 06:04
Given the existing "gc.enable", "gc.disable" and "gc.isenabled" APIs, I'd suggest calling this one "gc.ensure_disabled": it ensures the GC is disabled in the with statement body, but only turns it back on at the end if it was previously enabled.

That same name would then be used all the way through the code and documentation.
msg311167 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2018-01-29 20:37
New changeset 72a0d218dcc94a3cc409a9ef32dfcd5a7bbcb43c by Raymond Hettinger (Pablo Galindo) in branch 'master':
bpo-31356: Add context manager to temporarily disable GC (GH-4224)
https://github.com/python/cpython/commit/72a0d218dcc94a3cc409a9ef32dfcd5a7bbcb43c
msg311300 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-01-30 20:32
A bug found by coverity:

(PyErr_WarnEx might error out; please update the code to handle that)


________________________________________________________________________________________________________ *** CID 1428756: Error handling issues (CHECKED_RETURN)
/Modules/gcmodule.c: 1071 in gc_enable_impl()
1065
1066 static PyObject *
1067 gc_enable_impl(PyObject *module)
1068 /*[clinic end generated code: output=45a427e9dce9155c input=81ac4940ca579707]*/
1069 {
1070 if(_PyRuntime.gc.disabled_threads){
1071 PyErr_WarnEx(PyExc_RuntimeWarning, "Garbage collector enabled while another "
1072 "thread is inside gc.ensure_enabled",1);
1073 }
1074 _PyRuntime.gc.enabled = 1;
1075 Py_RETURN_NONE;
1076 }
msg311305 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2018-01-30 23:39
> (PyErr_WarnEx might error out; please update the code to handle that)

Lisa, would you like to take a crack at fixing this one?
msg311312 - (view) Author: Lisa Roach (lisroach) * (Python committer) Date: 2018-01-31 06:17
I gave it a shot- looks like we need to ensure that we catch any errors that could be thrown by the warning itself.

I wasn't entirely sure how to create a test for this, if anyone knows how I'll definitely add it!
msg311331 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2018-01-31 13:57
@lisroach A possible test for this is repeat `test_ensure_disabled_thread` with warnings as errors:

```python
warnings.filterwarnings('error')
```

and then checking for a `RuntimeWarning` exception instead of the warning. I think this will work because without the changes in this PR, this proposed test will produce this error:

```python

======================================================================
ERROR: test_ensure_disabled_thread (Lib.test.test_gc.GCTogglingTests)
----------------------------------------------------------------------
RuntimeWarning: Garbage collector enabled while another thread is inside gc.ensure_enabled

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/pgalindo3/cpython/Lib/test/support/__init__.py", line 2083, in decorator
    return func(*args)
  File "/home/pgalindo3/cpython/Lib/test/test_gc.py", line 1063, in test_ensure_disabled_thread
    inside_status_after_thread = gc.isenabled()
SystemError: <built-in method __exit__ of gc.ensure_disabled object at 0x7f0253d22de0> returned a result with an error se

```

Another posible test is checking that SystemError is not raised / in stderr.
msg311337 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-01-31 15:54
Actually raising an exception in PyErr_WarnEx() is not an error, but a standard behavior with corresponding configuration.

For now running tests with -We in debug build crashes.

$ ./python -We -m test test_gc
Run tests sequentially
0:00:00 load avg: 0.33 [1/1] test_gc
Fatal Python error: a function returned a result with an error set
RuntimeWarning: Garbage collector enabled while another thread is inside gc.ensure_enabled

The above exception was the direct cause of the following exception:

SystemError: <built-in method __exit__ of gc.ensure_disabled object at 0x7fb1b62a08f8> returned a result with an error set

Thread 0x00007fb1b1773700 (most recent call first):
  File "/home/serhiy/py/cpython/Lib/test/test_gc.py", line 1050 in disabling_thread
  File "/home/serhiy/py/cpython/Lib/threading.py", line 865 in run
  File "/home/serhiy/py/cpython/Lib/threading.py", line 917 in _bootstrap_inner
  File "/home/serhiy/py/cpython/Lib/threading.py", line 885 in _bootstrap

Current thread 0x00007fb1b824f040 (most recent call first):
  File "/home/serhiy/py/cpython/Lib/test/test_gc.py", line 1061 in test_ensure_disabled_thread
  File "/home/serhiy/py/cpython/Lib/test/support/__init__.py", line 2083 in decorator
  File "/home/serhiy/py/cpython/Lib/unittest/case.py", line 615 in run
  File "/home/serhiy/py/cpython/Lib/unittest/case.py", line 663 in __call__
  File "/home/serhiy/py/cpython/Lib/unittest/suite.py", line 122 in run
  File "/home/serhiy/py/cpython/Lib/unittest/suite.py", line 84 in __call__
  File "/home/serhiy/py/cpython/Lib/unittest/suite.py", line 122 in run
  File "/home/serhiy/py/cpython/Lib/unittest/suite.py", line 84 in __call__
  File "/home/serhiy/py/cpython/Lib/test/support/__init__.py", line 1760 in run
  File "/home/serhiy/py/cpython/Lib/test/support/__init__.py", line 1861 in _run_suite
  File "/home/serhiy/py/cpython/Lib/test/support/__init__.py", line 1951 in run_unittest
  File "/home/serhiy/py/cpython/Lib/test/test_gc.py", line 1088 in test_main
  File "/home/serhiy/py/cpython/Lib/test/libregrtest/runtest.py", line 176 in runtest_inner
  File "/home/serhiy/py/cpython/Lib/test/libregrtest/runtest.py", line 140 in runtest
  File "/home/serhiy/py/cpython/Lib/test/libregrtest/main.py", line 379 in run_tests_sequential
  File "/home/serhiy/py/cpython/Lib/test/libregrtest/main.py", line 458 in run_tests
  File "/home/serhiy/py/cpython/Lib/test/libregrtest/main.py", line 536 in _main
  File "/home/serhiy/py/cpython/Lib/test/libregrtest/main.py", line 510 in main
  File "/home/serhiy/py/cpython/Lib/test/libregrtest/main.py", line 585 in main
  File "/home/serhiy/py/cpython/Lib/test/__main__.py", line 2 in <module>
  File "/home/serhiy/py/cpython/Lib/runpy.py", line 85 in _run_code
  File "/home/serhiy/py/cpython/Lib/runpy.py", line 193 in _run_module_as_main
Aborted (core dumped)
msg311340 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-01-31 16:18
Just noting here: the original PR was a little bit under-reviewed: return values of C functions were not checked, and the code style was very far from PEP 7.
msg311342 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-01-31 16:43
IMO this needs to be pulled from 3.7.  Ned?
msg311343 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2018-01-31 17:09
I would not remove this.  It is a new feature, leave it in for 3.7.

The style issues and missing error checks are easy to address.  We're at 3.7beta1 stage, it is normal to have issues to be cleaned up in a beta release.
msg311344 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-01-31 17:11
> The style issues and missing error checks are easy to address.  We're at 3.7beta1 stage, it is normal to have issues to be cleaned up in a beta release.


I don't understand the code. Please take a look in my PR.
msg311347 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-01-31 17:56
I don't understand why PyGILState_Ensure() and PyGILState_Release() are used at all. I think it is better to revert PR 4224 until we understood this code.

Currently the code contains a bug which leads to a crash in some circumstances. Since several third-party projects try to support warning-free code, this can prevent running tests with 3.7.
msg311348 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-01-31 18:00
A few thoughts:

1. The merged PR releases GIL for any Python code run in `with gc.ensure_disabled()`.  This is just plain wrong.

2. The merged PR crashes on debug build of CPython.

3. I don't actually understand this feature.  Our GC is not per OS thread, which means that inside `with gc.ensure_disabled()` the GC can become suddenly **enabled**, if another thread enables it.  This API is just misleading (maybe the name of the new context manager is bad—it cannot ensure anything).  There's even a unittest that tests this!

4. This new API can be trivially implemented in pure Python:

   @contextmanager
   def disable_gc():
       gc.disable()
       try:
           yield
       finally:
           gc.enable()

5. While such pure Python version shares the problem discussed in (3), the currently committed C implementation doesn't fix them either.

IMO this entire issue needs to be properly debated on python-ideas first and the PR should be reverted from beta-1.  If the latter is not possible, OK, let's revert it for beta-2 and for 3.8.
msg311350 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-01-31 18:03
I concur with Yury.
msg311351 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2018-01-31 18:15
> 2. The merged PR crashes on debug build of CPython.

Actually, for me, it only crashes on a debug build when using -We with tests, an option I don't normally use, otherwise, I would have noticed it before.  And, as noted, few downstream users use debug builds.

But, more importantly, this is a new feature so it doesn't break any existing code.  If that is the case, I think we don't need to expedite a release; it can wait four weeks for 3.7.0b2 and in that time we need to decide what the proper actions for 3.7 (fix, reimplement, remove) and for 3.8.

Please chime in now if you strongly disagree.  In any case, thanks all for your efforts on this!
msg311352 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-01-31 18:22
> But, more importantly, this is a new feature so it doesn't break any existing code.

I imagine you're almost done with the beta-1 release, Ned, so I'd hate to create more work for you here.  Let's release beta-1 as is.

> and in that time we need to decide what the proper actions for 3.7 (fix, reimplement, remove) and for 3.8.

I have a different opinion on this though.  The committed PR is **fundamentally** broken (releases GIL for Python code, refleaks, SIGABRT, nonsensical test) and the feature itself is very questionable.  As soon as 3.7 branch appears, I'll revert the commit from 3.7 and from master.

If we want this feature to be kept in 3.7beta2, someone should write a **new** patch from scratch.  But I strongly suggest to first discuss it on python-dev, because as is, not only the commit is broken, the proposed API is broken too from my standpoint.
msg311356 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2018-01-31 18:36
I also think this should be reverted if that is still possible.  I didn't put in sufficient review effort at the outset.  No need to publish embarrassing code, especially for such a minor feature.
msg311357 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2018-01-31 18:46
OK, it sounds like we have enough of a consensus to remove it for 3.7.0b2.  Unfortunately, 3.7.0b1 is already finished.  We are not going to do an emergency brown bag b2 just for this.  We could add a warning note to the release notice that is about to got out but OTOH the feature isn't very visible and the documentation for it will disappear from the docs web site once the revert is made (and please wait for the imminent announcement of the 3.7 branch to do that).
msg311360 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2018-01-31 18:54
(If anyone downstream is having problems with test aborts, a workaround until b2 is to simply skip test_gc by adding -x test_gc, like:

python3.7 -m test -We -x test_gc

)
msg311362 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2018-01-31 19:13
Agree that this is invisible enough that no release note is warranted.

Can I go ahead and do a reversion on the master branch or I should I wait a day or so?
msg311363 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2018-01-31 19:17
> Agree that this is invisible enough that no release note is warranted.

OK

> Can I go ahead and do a reversion on the master branch or I should I wait a day or so?

Please hold off for the moment until the 3.7 branch is announced.  It will make life easier for me.  Thanks!
msg311366 - (view) Author: (YoSTEALTH) * Date: 2018-01-31 20:34
Since "ensure_disabled" is a class https://docs.python.org/3.7/library/gc.html#gc.ensure_disabled it should be "EnsureDisabled" according to https://www.python.org/dev/peps/pep-0008/#class-names
msg311372 - (view) Author: (YoSTEALTH) * Date: 2018-01-31 21:00
ps, maybe a better name "DisabledZone" ?
msg311396 - (view) Author: Josh Rosenberg (josh.r) * Date: 2018-02-01 01:14
YoSTEALTH: I think this is one of those cases where the fact of it being a class is incidental; it's used the same as if it were a function, and just happens to be implemented as a class (the docs actually describe it in terms of a function). PEP8 guidance on CapWords for class names isn't ironclad, particularly when being a class is more implementation detail than design goal.
msg311397 - (view) Author: (YoSTEALTH) * Date: 2018-02-01 01:34
Actually i don't remember the last time where i saw anyone call a function using a "with" statement. This is very sloppy, sure PEP8 isn't ironclad but this is going to lead to confusion and we might have another case of datetime model (where you don't know whats what!!!).
msg311402 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2018-02-01 03:55
A useful heuristic is that if something is named with CapWords, then class-like *usage* is explicitly supported (inheritance, isinstance checks, etc).

If it's named with snake_case or wordsruntogether, then calling it is OK, but you may run into quirky behaviour in class-style usage (e.g. it may not be a class at all in some implementations, or it may not be friendly to subclassing even when it is a full class).

So for something like this, snake_case is appropriate - you're meant to just use it, not subclass it. The fact that it's a type instance is an implementation detail.

(In other cases like contextlib.contextmanager we've made that implementation details status completely explicit in the code by having the public API be a wrapper function that returns an instance of a private class)
msg311444 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-02-01 16:05
Raymond, do you need help with reverts?
msg311491 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-02-02 13:44
Since I'm going to be unavailable for the next 10 days, and I don't want this to be accidentally forgotten, I'll do the revert myself.  Opened a PR for that.
msg311494 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-02-02 14:31
New changeset 383b32fe108ea627699cc9c644fba5f8bae95d73 by Yury Selivanov in branch 'master':
Revert "bpo-31356: Add context manager to temporarily disable GC GH-5495
https://github.com/python/cpython/commit/383b32fe108ea627699cc9c644fba5f8bae95d73
msg311495 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-02-02 15:04
New changeset 29fd9eae432a54c963262e895b46f081f238539a by Yury Selivanov (Miss Islington (bot)) in branch '3.7':
Revert "bpo-31356: Add context manager to temporarily disable GC GH-5495 (#5496)
https://github.com/python/cpython/commit/29fd9eae432a54c963262e895b46f081f238539a
msg311508 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2018-02-02 19:07
The idea which this issue represents is not rejected.  It is a good one, we found a need for it during the dev sprint last September.
msg311512 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-02-02 20:22
> The idea which this issue represents is not rejected.  It is a good one, we found a need for it during the dev sprint last September.

Well, not everybody thinks it is a good one.  I, for instance, don't think it's a good idea, so it is at least one "-1".  I saw Serhiy was unsure about this new feature too, so maybe there are two "-1"s; I don't know.  So I kindly ask(-ed) for it to be openly discussed on python-dev, instead of making a unilateral decision.

The problem with the current design is that GC can become enabled inside the 'with' block at any time:

    with gc.ensure_disabled():
        # gc.is_enabled() might be True !

The GC can become enabled from another OS thread, as we have one GC per process.  Adding a locking mechanism might be tricky in terms of implementation, and risky in terms of allowing people to accidentally have a deadlock or something.  In short, I don't see any way to make this context manager to work reliably in CPython.

Therefore, I think that the best location for a helper like this would be some unittesting helpers collection, as it can work only under some very specific conditions.  The core 'gc' module is currently a collection of low-level primitives.  I think we need a very solid motivation to add a core GC function that works unreliably and is suitable only for unittesting (at best).  

Maybe I'm completely wrong here, in which case I would love to be proved wrong and not just ignored.
msg311513 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-02-02 20:43
I concur with Yury in every point.

The idea looks good for three core developers, but I just don't understand this. This feature looks useless and misleading to me. It is not atomic and doesn't ensure anything, despite its name. If it will be added in the stdlib, there should be very good explanation of its purpose and limitations in the documentation. And I agree that unittest may be better place than gc if the purpose of it is testing.
msg311514 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2018-02-02 20:45
Indeed if unit testing is the main use case, I don't really see the point of adding C code.  People can code a simple helper using `contextlib.contextmanager` in a couple of lines.
msg311566 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2018-02-03 17:36
Nick and Raymond: do you remember what our original motivating use cases were for this idea during the sprint?
msg311635 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2018-02-05 02:35
If I recall the discussion correctly, it was:

1. That this was worth doing precisely because the naive approach is likely to be broken in the presence of multiple threads;
2. It was only worth doing either as a true global disable that accounted for multi-threading (e.g. backed by a counted semaphore or the functional equivalent), or else by making gc enable/disable status have a thread local toggle in addition to the global one (so the context manager can ensure "GC is off *in this thread*, regardless of the global status").

Either of those two options requires changes to the main GC machinery though, as otherwise you basically *can't* write a correct context manager for this use case, since a direct call to gc.enable() in another thread would always be problematic.
msg311646 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-02-05 06:29
1. The used approach was broken in the presence of multiple threads too. It didn't guarantee even that GC will be disabled in the next line.

2. What is a sense of disabling GC in a single thread? Objects in Python are not thread local, they are accessible from all threads, and collecting garbage in one thread affects other threads.

For truly disabling GC globally you need to use a counted semaphore or other synchronization primitives, and this can be implemented at Python level. But what are use cases for this context manager? Isn't naive approach enough?
History
Date User Action Args
2018-02-05 06:29:02serhiy.storchakasetmessages: + msg311646
2018-02-05 02:35:03ncoghlansetmessages: + msg311635
2018-02-03 21:10:59pmppsetnosy: + pmpp
2018-02-03 17:36:28gregory.p.smithsetnosy: rhettinger, gregory.p.smith, ncoghlan, pitrou, ned.deily, eric.snow, serhiy.storchaka, yselivanov, josh.r, YoSTEALTH, lisroach, pablogsal
messages: + msg311566
2018-02-02 20:45:03pitrousetmessages: + msg311514
2018-02-02 20:43:18serhiy.storchakasetmessages: + msg311513
2018-02-02 20:22:26yselivanovsetmessages: + msg311512
2018-02-02 19:07:43gregory.p.smithsetstage: resolved -> needs patch
2018-02-02 19:07:36gregory.p.smithsetstatus: closed -> open
priority: release blocker -> normal
versions: + Python 3.8, - Python 3.7
messages: + msg311508

resolution: rejected ->
2018-02-02 15:04:57yselivanovsetstatus: open -> closed
resolution: rejected
stage: patch review -> resolved
2018-02-02 15:04:35yselivanovsetmessages: + msg311495
2018-02-02 14:31:25miss-islingtonsetpull_requests: + pull_request5329
2018-02-02 14:31:08yselivanovsetmessages: + msg311494
2018-02-02 13:44:43yselivanovsetassignee: rhettinger ->
messages: + msg311491
2018-02-02 13:40:06yselivanovsetpull_requests: + pull_request5328
2018-02-01 16:05:44yselivanovsetmessages: + msg311444
2018-02-01 03:55:52ncoghlansetmessages: + msg311402
2018-02-01 01:34:01YoSTEALTHsetmessages: + msg311397
2018-02-01 01:14:45josh.rsetmessages: + msg311396
2018-01-31 21:00:13YoSTEALTHsetmessages: + msg311372
2018-01-31 20:34:17YoSTEALTHsetnosy: + YoSTEALTH
messages: + msg311366
2018-01-31 19:17:07ned.deilysetmessages: + msg311363
2018-01-31 19:13:36rhettingersetassignee: lisroach -> rhettinger
messages: + msg311362
2018-01-31 18:54:32ned.deilysetmessages: - msg311359
2018-01-31 18:54:19ned.deilysetmessages: + msg311360
2018-01-31 18:53:20ned.deilysetmessages: + msg311359
2018-01-31 18:46:36ned.deilysetmessages: + msg311357
2018-01-31 18:36:04rhettingersetmessages: + msg311356
2018-01-31 18:22:21yselivanovsetmessages: + msg311352
2018-01-31 18:15:40ned.deilysetmessages: + msg311351
2018-01-31 18:03:19serhiy.storchakasetmessages: + msg311350
2018-01-31 18:00:16yselivanovsetmessages: + msg311348
2018-01-31 17:56:27serhiy.storchakasetmessages: + msg311347
2018-01-31 17:11:23yselivanovsetmessages: + msg311344
2018-01-31 17:09:19gregory.p.smithsetmessages: + msg311343
2018-01-31 16:43:55yselivanovsetmessages: + msg311342
2018-01-31 16:18:19yselivanovsetmessages: + msg311340
2018-01-31 16:14:45yselivanovsetpull_requests: + pull_request5288
2018-01-31 15:54:18serhiy.storchakasetpriority: normal -> release blocker
nosy: + ned.deily
messages: + msg311337

2018-01-31 13:57:42pablogsalsetmessages: + msg311331
2018-01-31 06:17:36lisroachsetmessages: + msg311312
2018-01-31 06:16:23lisroachsetstage: patch review
pull_requests: + pull_request5284
2018-01-30 23:39:49rhettingersetmessages: + msg311305
2018-01-30 20:33:02yselivanovsetstatus: closed -> open
resolution: fixed -> (no value)
stage: resolved -> (no value)
2018-01-30 20:32:53yselivanovsetnosy: + yselivanov
messages: + msg311300
2018-01-29 20:38:01rhettingersetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2018-01-29 20:37:12rhettingersetmessages: + msg311167
2017-11-11 06:04:55ncoghlansetmessages: + msg306076
2017-11-11 00:22:21pablogsalsetmessages: + msg306072
2017-11-11 00:18:45pablogsalsetmessages: + msg306071
2017-11-10 22:46:16serhiy.storchakasetmessages: + msg306054
2017-11-10 22:30:43pablogsalsetmessages: + msg306053
2017-11-02 00:26:33pablogsalsetnosy: + pablogsal
messages: + msg305402
2017-11-02 00:24:53pablogsalsetkeywords: + patch
stage: needs patch -> patch review
pull_requests: + pull_request4192
2017-09-25 01:58:28rhettingersetassignee: lisroach

messages: + msg302903
nosy: + lisroach
2017-09-08 19:07:17ncoghlansetmessages: + msg301720
2017-09-08 18:02:46josh.rsetnosy: + josh.r
messages: + msg301710
2017-09-07 22:43:23gregory.p.smithsetmessages: + msg301645
2017-09-07 22:31:50ncoghlansetmessages: + msg301644
2017-09-07 21:55:34serhiy.storchakasetnosy: + pitrou, serhiy.storchaka
messages: + msg301636
2017-09-07 21:36:02ncoghlansetassignee: ncoghlan -> (no value)
messages: + msg301633
components: + Library (Lib)
stage: needs patch
2017-09-05 23:47:23rhettingercreate