diff options
| -rw-r--r-- | Include/object.h | 27 | ||||
| -rw-r--r-- | Include/pystate.h | 3 | ||||
| -rw-r--r-- | Lib/test/test_gc.py | 69 | ||||
| -rw-r--r-- | Misc/NEWS | 5 | ||||
| -rw-r--r-- | Objects/object.c | 37 | ||||
| -rw-r--r-- | Objects/typeobject.c | 5 | ||||
| -rw-r--r-- | Python/pystate.c | 3 |
7 files changed, 140 insertions, 9 deletions
diff --git a/Include/object.h b/Include/object.h index 709a9fff138..3aabb3be68d 100644 --- a/Include/object.h +++ b/Include/object.h @@ -961,24 +961,33 @@ chain of N deallocations is broken into N / PyTrash_UNWIND_LEVEL pieces, with the call stack never exceeding a depth of PyTrash_UNWIND_LEVEL. */ +/* This is the old private API, invoked by the macros before 3.2.4. + Kept for binary compatibility of extensions. */ PyAPI_FUNC(void) _PyTrash_deposit_object(PyObject*); PyAPI_FUNC(void) _PyTrash_destroy_chain(void); PyAPI_DATA(int) _PyTrash_delete_nesting; PyAPI_DATA(PyObject *) _PyTrash_delete_later; +/* The new thread-safe private API, invoked by the macros below. */ +PyAPI_FUNC(void) _PyTrash_thread_deposit_object(PyObject*); +PyAPI_FUNC(void) _PyTrash_thread_destroy_chain(void); + #define PyTrash_UNWIND_LEVEL 50 #define Py_TRASHCAN_SAFE_BEGIN(op) \ - if (_PyTrash_delete_nesting < PyTrash_UNWIND_LEVEL) { \ - ++_PyTrash_delete_nesting; - /* The body of the deallocator is here. */ + do { \ + PyThreadState *_tstate = PyThreadState_GET(); \ + if (_tstate->trash_delete_nesting < PyTrash_UNWIND_LEVEL) { \ + ++_tstate->trash_delete_nesting; + /* The body of the deallocator is here. */ #define Py_TRASHCAN_SAFE_END(op) \ - --_PyTrash_delete_nesting; \ - if (_PyTrash_delete_later && _PyTrash_delete_nesting <= 0) \ - _PyTrash_destroy_chain(); \ - } \ - else \ - _PyTrash_deposit_object((PyObject*)op); + --_tstate->trash_delete_nesting; \ + if (_tstate->trash_delete_later && _tstate->trash_delete_nesting <= 0) \ + _PyTrash_thread_destroy_chain(); \ + } \ + else \ + _PyTrash_thread_deposit_object((PyObject*)op); \ + } while (0); #ifndef Py_LIMITED_API PyAPI_FUNC(void) diff --git a/Include/pystate.h b/Include/pystate.h index 25c2faae925..2017b0276ff 100644 --- a/Include/pystate.h +++ b/Include/pystate.h @@ -114,6 +114,9 @@ typedef struct _ts { PyObject *async_exc; /* Asynchronous exception to raise */ long thread_id; /* Thread id where this tstate was created */ + int trash_delete_nesting; + PyObject *trash_delete_later; + /* XXX signal handlers should also be here */ } PyThreadState; diff --git a/Lib/test/test_gc.py b/Lib/test/test_gc.py index d35f9ed762c..c59b72eacf8 100644 --- a/Lib/test/test_gc.py +++ b/Lib/test/test_gc.py @@ -2,9 +2,15 @@ import unittest from test.support import (verbose, refcount_test, run_unittest, strip_python_stderr) import sys +import time import gc import weakref +try: + import threading +except ImportError: + threading = None + ### Support code ############################################################################### @@ -328,6 +334,69 @@ class GCTests(unittest.TestCase): v = {1: v, 2: Ouch()} gc.disable() + @unittest.skipUnless(threading, "test meaningless on builds without threads") + def test_trashcan_threads(self): + # Issue #13992: trashcan mechanism should be thread-safe + NESTING = 60 + N_THREADS = 2 + + def sleeper_gen(): + """A generator that releases the GIL when closed or dealloc'ed.""" + try: + yield + finally: + time.sleep(0.000001) + + class C(list): + # Appending to a list is atomic, which avoids the use of a lock. + inits = [] + dels = [] + def __init__(self, alist): + self[:] = alist + C.inits.append(None) + def __del__(self): + # This __del__ is called by subtype_dealloc(). + C.dels.append(None) + # `g` will release the GIL when garbage-collected. This + # helps assert subtype_dealloc's behaviour when threads + # switch in the middle of it. + g = sleeper_gen() + next(g) + # Now that __del__ is finished, subtype_dealloc will proceed + # to call list_dealloc, which also uses the trashcan mechanism. + + def make_nested(): + """Create a sufficiently nested container object so that the + trashcan mechanism is invoked when deallocating it.""" + x = C([]) + for i in range(NESTING): + x = [C([x])] + del x + + def run_thread(): + """Exercise make_nested() in a loop.""" + while not exit: + make_nested() + + old_switchinterval = sys.getswitchinterval() + sys.setswitchinterval(1e-5) + try: + exit = False + threads = [] + for i in range(N_THREADS): + t = threading.Thread(target=run_thread) + threads.append(t) + for t in threads: + t.start() + time.sleep(1.0) + exit = True + for t in threads: + t.join() + finally: + sys.setswitchinterval(old_switchinterval) + gc.collect() + self.assertEqual(len(C.inits), len(C.dels)) + def test_boom(self): class Boom: def __getattr__(self, someattribute): diff --git a/Misc/NEWS b/Misc/NEWS index 9a066886395..802d9848322 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -10,6 +10,11 @@ What's New in Python 3.3.0 Release Candidate 2? Core and Builtins ----------------- +- Issue #13992: The trashcan mechanism is now thread-safe. This eliminates + sporadic crashes in multi-thread programs when several long deallocator + chains ran concurrently and involved subclasses of built-in container + types. + - Issue #15784: Modify OSError.__str__() to better distinguish between errno error numbers and Windows error numbers. diff --git a/Objects/object.c b/Objects/object.c index f4c0208bd3b..f41718424d8 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -1954,6 +1954,18 @@ _PyTrash_deposit_object(PyObject *op) _PyTrash_delete_later = op; } +/* The equivalent API, using per-thread state recursion info */ +void +_PyTrash_thread_deposit_object(PyObject *op) +{ + PyThreadState *tstate = PyThreadState_GET(); + assert(PyObject_IS_GC(op)); + assert(_Py_AS_GC(op)->gc.gc_refs == _PyGC_REFS_UNTRACKED); + assert(op->ob_refcnt == 0); + _Py_AS_GC(op)->gc.gc_prev = (PyGC_Head *) tstate->trash_delete_later; + tstate->trash_delete_later = op; +} + /* Dealloccate all the objects in the _PyTrash_delete_later list. Called when * the call-stack unwinds again. */ @@ -1980,6 +1992,31 @@ _PyTrash_destroy_chain(void) } } +/* The equivalent API, using per-thread state recursion info */ +void +_PyTrash_thread_destroy_chain(void) +{ + PyThreadState *tstate = PyThreadState_GET(); + while (tstate->trash_delete_later) { + PyObject *op = tstate->trash_delete_later; + destructor dealloc = Py_TYPE(op)->tp_dealloc; + + tstate->trash_delete_later = + (PyObject*) _Py_AS_GC(op)->gc.gc_prev; + + /* Call the deallocator directly. This used to try to + * fool Py_DECREF into calling it indirectly, but + * Py_DECREF was already called on this object, and in + * assorted non-release builds calling Py_DECREF again ends + * up distorting allocation statistics. + */ + assert(op->ob_refcnt == 0); + ++tstate->trash_delete_nesting; + (*dealloc)(op); + --tstate->trash_delete_nesting; + } +} + #ifndef Py_TRACE_REFS /* For Py_LIMITED_API, we need an out-of-line version of _Py_Dealloc. Define this here, so we can undefine the macro. */ diff --git a/Objects/typeobject.c b/Objects/typeobject.c index a357ced2201..8fb239b3824 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -891,6 +891,7 @@ subtype_dealloc(PyObject *self) { PyTypeObject *type, *base; destructor basedealloc; + PyThreadState *tstate = PyThreadState_GET(); /* Extract the type; we expect it to be a heap type */ type = Py_TYPE(self); @@ -940,8 +941,10 @@ subtype_dealloc(PyObject *self) /* See explanation at end of function for full disclosure */ PyObject_GC_UnTrack(self); ++_PyTrash_delete_nesting; + ++ tstate->trash_delete_nesting; Py_TRASHCAN_SAFE_BEGIN(self); --_PyTrash_delete_nesting; + -- tstate->trash_delete_nesting; /* DO NOT restore GC tracking at this point. weakref callbacks * (if any, and whether directly here or indirectly in something we * call) may trigger GC, and if self is tracked at that point, it @@ -1020,8 +1023,10 @@ subtype_dealloc(PyObject *self) endlabel: ++_PyTrash_delete_nesting; + ++ tstate->trash_delete_nesting; Py_TRASHCAN_SAFE_END(self); --_PyTrash_delete_nesting; + -- tstate->trash_delete_nesting; /* Explanation of the weirdness around the trashcan macros: diff --git a/Python/pystate.c b/Python/pystate.c index 8dc570ab753..cfd61d00986 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -206,6 +206,9 @@ new_threadstate(PyInterpreterState *interp, int init) tstate->c_profileobj = NULL; tstate->c_traceobj = NULL; + tstate->trash_delete_nesting = 0; + tstate->trash_delete_later = NULL; + if (init) _PyThreadState_Init(tstate); |