From 383b32fe108ea627699cc9c644fba5f8bae95d73 Mon Sep 17 00:00:00 2001 From: Yury Selivanov Date: Fri, 2 Feb 2018 09:31:06 -0500 Subject: [PATCH] Revert "bpo-31356: Add context manager to temporarily disable GC GH-5495 This reverts commit 72a0d218dcc94a3cc409a9ef32dfcd5a7bbcb43c. The reverted commit had a few issues so it was unanimously decided to undo it. See the bpo issue for details. --- Doc/library/gc.rst | 28 ----- Include/internal/mem.h | 1 - Lib/test/test_gc.py | 71 +----------- Misc/NEWS.d/3.7.0b1.rst | 11 -- .../2018-02-02-08-50-46.bpo-31356.MNwUOQ.rst | 2 + Modules/gcmodule.c | 106 ------------------ 6 files changed, 3 insertions(+), 216 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2018-02-02-08-50-46.bpo-31356.MNwUOQ.rst diff --git a/Doc/library/gc.rst b/Doc/library/gc.rst index 92240c76067..153d8fb7045 100644 --- a/Doc/library/gc.rst +++ b/Doc/library/gc.rst @@ -33,34 +33,6 @@ The :mod:`gc` module provides the following functions: Disable automatic garbage collection. -.. class:: ensure_disabled() - - Return a context manager object that disables the garbage collector and reenables the previous - state upon completion of the block. This is basically equivalent to:: - - from gc import enable, disable, isenabled - - @contextmanager - def ensure_disabled(): - was_enabled_previously = isenabled() - gc.disable() - yield - if was_enabled_previously: - gc.enable() - - And lets you write code like this:: - - with ensure_disabled(): - run_some_timing() - - with ensure_disabled(): - # do_something_that_has_real_time_guarantees - # such as a pair trade, robotic braking, etc - - without needing to explicitly enable and disable the garbage collector yourself. - This context manager is implemented in C to assure atomicity, thread safety and speed. - - .. function:: isenabled() Returns true if automatic collection is enabled. diff --git a/Include/internal/mem.h b/Include/internal/mem.h index 4a84b4a78d9..a731e30e6af 100644 --- a/Include/internal/mem.h +++ b/Include/internal/mem.h @@ -116,7 +116,6 @@ struct _gc_runtime_state { int enabled; int debug; - long disabled_threads; /* linked lists of container objects */ struct gc_generation generations[NUM_GENERATIONS]; PyGC_Head *generation0; diff --git a/Lib/test/test_gc.py b/Lib/test/test_gc.py index 246980aa74c..904fc7d88c0 100644 --- a/Lib/test/test_gc.py +++ b/Lib/test/test_gc.py @@ -1,7 +1,7 @@ import unittest from test.support import (verbose, refcount_test, run_unittest, strip_python_stderr, cpython_only, start_threads, - temp_dir, requires_type_collecting,reap_threads) + temp_dir, requires_type_collecting) from test.support.script_helper import assert_python_ok, make_script import sys @@ -9,8 +9,6 @@ import time import gc import weakref import threading -import warnings - try: from _testcapi import with_tp_del @@ -1009,73 +1007,6 @@ class GCTogglingTests(unittest.TestCase): # empty __dict__. self.assertEqual(x, None) - def test_ensure_disabled(self): - original_status = gc.isenabled() - - with gc.ensure_disabled(): - inside_status = gc.isenabled() - - after_status = gc.isenabled() - self.assertEqual(original_status, True) - self.assertEqual(inside_status, False) - self.assertEqual(after_status, True) - - def test_ensure_disabled_with_gc_disabled(self): - gc.disable() - - original_status = gc.isenabled() - - with gc.ensure_disabled(): - inside_status = gc.isenabled() - - after_status = gc.isenabled() - self.assertEqual(original_status, False) - self.assertEqual(inside_status, False) - self.assertEqual(after_status, False) - - @reap_threads - def test_ensure_disabled_thread(self): - - thread_original_status = None - thread_inside_status = None - thread_after_status = None - - def disabling_thread(): - nonlocal thread_original_status - nonlocal thread_inside_status - nonlocal thread_after_status - thread_original_status = gc.isenabled() - - with gc.ensure_disabled(): - time.sleep(0.01) - thread_inside_status = gc.isenabled() - - thread_after_status = gc.isenabled() - - original_status = gc.isenabled() - - with warnings.catch_warnings(record=True) as w, gc.ensure_disabled(): - inside_status_before_thread = gc.isenabled() - thread = threading.Thread(target=disabling_thread) - thread.start() - inside_status_after_thread = gc.isenabled() - - after_status = gc.isenabled() - thread.join() - - self.assertEqual(len(w), 1) - self.assertTrue(issubclass(w[-1].category, RuntimeWarning)) - self.assertEqual("Garbage collector enabled while another thread is " - "inside gc.ensure_enabled", str(w[-1].message)) - self.assertEqual(original_status, True) - self.assertEqual(inside_status_before_thread, False) - self.assertEqual(thread_original_status, False) - self.assertEqual(thread_inside_status, True) - self.assertEqual(thread_after_status, False) - self.assertEqual(inside_status_after_thread, False) - self.assertEqual(after_status, True) - - def test_main(): enabled = gc.isenabled() gc.disable() diff --git a/Misc/NEWS.d/3.7.0b1.rst b/Misc/NEWS.d/3.7.0b1.rst index 4f7bfee85cb..bd3a6111df0 100644 --- a/Misc/NEWS.d/3.7.0b1.rst +++ b/Misc/NEWS.d/3.7.0b1.rst @@ -196,17 +196,6 @@ by Sanyam Khurana. .. -.. bpo: 31356 -.. date: 2017-11-02-00-34-42 -.. nonce: 54Lb8U -.. section: Core and Builtins - -Add a new contextmanager to the gc module that temporarily disables the GC -and restores the previous state. The implementation is done in C to assure -atomicity and speed. - -.. - .. bpo: 31179 .. date: 2017-08-10-17-32-48 .. nonce: XcgLYI diff --git a/Misc/NEWS.d/next/Core and Builtins/2018-02-02-08-50-46.bpo-31356.MNwUOQ.rst b/Misc/NEWS.d/next/Core and Builtins/2018-02-02-08-50-46.bpo-31356.MNwUOQ.rst new file mode 100644 index 00000000000..5022a137060 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2018-02-02-08-50-46.bpo-31356.MNwUOQ.rst @@ -0,0 +1,2 @@ +Remove the new API added in bpo-31356 (gc.ensure_disabled() context +manager). diff --git a/Modules/gcmodule.c b/Modules/gcmodule.c index c057d25a12b..8ba1093c029 100644 --- a/Modules/gcmodule.c +++ b/Modules/gcmodule.c @@ -1067,10 +1067,6 @@ static PyObject * gc_enable_impl(PyObject *module) /*[clinic end generated code: output=45a427e9dce9155c input=81ac4940ca579707]*/ { - if(_PyRuntime.gc.disabled_threads){ - PyErr_WarnEx(PyExc_RuntimeWarning, "Garbage collector enabled while another " - "thread is inside gc.ensure_enabled",1); - } _PyRuntime.gc.enabled = 1; Py_RETURN_NONE; } @@ -1512,102 +1508,6 @@ static PyMethodDef GcMethods[] = { {NULL, NULL} /* Sentinel */ }; -typedef struct { - PyObject_HEAD - int previous_gc_state; -} ensure_disabled_object; - - -static void -ensure_disabled_object_dealloc(ensure_disabled_object *m_obj) -{ - Py_TYPE(m_obj)->tp_free((PyObject*)m_obj); -} - -static PyObject * -ensure_disabled__enter__method(ensure_disabled_object *self, PyObject *args) -{ - PyGILState_STATE gstate = PyGILState_Ensure(); - ++_PyRuntime.gc.disabled_threads; - self->previous_gc_state = _PyRuntime.gc.enabled; - gc_disable_impl(NULL); - PyGILState_Release(gstate); - Py_RETURN_NONE; -} - -static PyObject * -ensure_disabled__exit__method(ensure_disabled_object *self, PyObject *args) -{ - PyGILState_STATE gstate = PyGILState_Ensure(); - --_PyRuntime.gc.disabled_threads; - if(self->previous_gc_state){ - gc_enable_impl(NULL); - }else{ - gc_disable_impl(NULL); - } - PyGILState_Release(gstate); - Py_RETURN_NONE; -} - - - -static struct PyMethodDef ensure_disabled_object_methods[] = { - {"__enter__", (PyCFunction) ensure_disabled__enter__method, METH_NOARGS}, - {"__exit__", (PyCFunction) ensure_disabled__exit__method, METH_VARARGS}, - {NULL, NULL} /* sentinel */ -}; - -static PyObject * -new_disabled_obj(PyTypeObject *type, PyObject *args, PyObject *kwdict){ - ensure_disabled_object *self; - self = (ensure_disabled_object *)type->tp_alloc(type, 0); - return (PyObject *) self; -}; - -static PyTypeObject gc_ensure_disabled_type = { - PyVarObject_HEAD_INIT(NULL, 0) - "gc.ensure_disabled", /* tp_name */ - sizeof(ensure_disabled_object), /* tp_size */ - 0, /* tp_itemsize */ - /* methods */ - (destructor) ensure_disabled_object_dealloc,/* tp_dealloc */ - 0, /* tp_print */ - 0, /* tp_getattr */ - 0, /* tp_setattr */ - 0, /* tp_reserved */ - 0, /* tp_repr */ - 0, /* tp_as_number */ - 0, /*tp_as_sequence*/ - 0, /*tp_as_mapping*/ - 0, /*tp_hash*/ - 0, /*tp_call*/ - 0, /*tp_str*/ - PyObject_GenericGetAttr, /*tp_getattro*/ - 0, /*tp_setattro*/ - 0, /*tp_as_buffer*/ - Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /*tp_flags*/ - 0, /*tp_doc*/ - 0, /* tp_traverse */ - 0, /* tp_clear */ - 0, /* tp_richcompare */ - 0, /* tp_weaklistoffset */ - 0, /* tp_iter */ - 0, /* tp_iternext */ - ensure_disabled_object_methods, /* tp_methods */ - 0, /* tp_members */ - 0, /* tp_getset */ - 0, /* tp_base */ - 0, /* tp_dict */ - 0, /* tp_descr_get */ - 0, /* tp_descr_set */ - 0, /* tp_dictoffset */ - 0, /* tp_init */ - PyType_GenericAlloc, /* tp_alloc */ - new_disabled_obj, /* tp_new */ - PyObject_Del, /* tp_free */ -}; - - static struct PyModuleDef gcmodule = { PyModuleDef_HEAD_INIT, "gc", /* m_name */ @@ -1648,12 +1548,6 @@ PyInit_gc(void) if (PyModule_AddObject(m, "callbacks", _PyRuntime.gc.callbacks) < 0) return NULL; - if (PyType_Ready(&gc_ensure_disabled_type) < 0) - return NULL; - if (PyModule_AddObject(m, "ensure_disabled", (PyObject*) &gc_ensure_disabled_type) < 0) - return NULL; - - #define ADD_INT(NAME) if (PyModule_AddIntConstant(m, #NAME, NAME) < 0) return NULL ADD_INT(DEBUG_STATS); ADD_INT(DEBUG_COLLECTABLE);