GeezerVison is showing github.com — `test_weakref.test_threaded_weak_key_dict_deepcopy` crash: merged objects should have ob_tid == 0 · Issue #135106 · python/cpython
test_weakref.test_threaded_weak_key_dict_deepcopy crash: merged objects should have ob_tid == 0 #135106
DescriptionBug report
0:08:41 load avg: 0.10 [402/491/1] test_weakref worker non-zero exit code (Exit code 2147483651) -- running (7): test_largefile (2 min 38 sec), test_zipimport (2 min 25 sec), test_compileall (2 min 14 sec), test_dbm (1 min 54 sec), test_free_threading (2 min 32 sec), test_regrtest (1 min 36 sec), test_pathlib (2 min 44 sec)
...
test.test_weakref.MappingTestCase.test_threaded_weak_key_dict_deepcopy) ... C:\Users\Administrator\buildarea\pull_request.itamaro-win64-srv-22-aws.x64.nogil\build\Python\gc_free_threading.c:1081: validate_refcounts: Assertion "op->ob_tid == 0" failed: merged objects should have ob_tid == 0
Enable tracemalloc to get the memory block allocation traceback
object address : 0000020002444810
object refcount : 0
object type : 00007FFC8D8A1620
object type name: weakref.ReferenceType
object repr : <refcnt 0 at 0000020002444810>
Fatal Python error: _PyObject_AssertFailed: _PyObject_AssertFailed
Python runtime state: initialized
Stack (most recent call first):
File "C:\Users\Administrator\buildarea\pull_request.itamaro-win64-srv-22-aws.x64.nogil\build\Lib\test\test_weakref.py", line 1999 in pop_and_collect
File "C:\Users\Administrator\buildarea\pull_request.itamaro-win64-srv-22-aws.x64.nogil\build\Lib\threading.py", line 1016 in run
File "C:\Users\Administrator\buildarea\pull_request.itamaro-win64-srv-22-aws.x64.nogil\build\Lib\threading.py", line 1074 in _bootstrap_inner
File "C:\Users\Administrator\buildarea\pull_request.itamaro-win64-srv-22-aws.x64.nogil\build\Lib\threading.py", line 1036 in _bootstrap
Looks like it happens during a gc.collect() call:
|
def pop_and_collect(lst): |
|
gc_ctr = 0 |
|
while lst: |
|
i = random.randint(0, len(lst) - 1) |
|
gc_ctr += 1 |
|
lst.pop(i) |
|
if gc_ctr % 10000 == 0: |
|
gc.collect() # just in case |
https://buildbot.python.org/#/builders/1295/builds/591
The test passed on retry, but this seems like a bug.
Seen in main (3.15) on Windows debug build with free threading
Linked PRs
Activityvstinner commented on Jun 5, 2025 changed the title [-]`test_weakref.test_threaded_weak_key_dict_deepcopy` crash[/-] [+]`test_weakref.test_threaded_weak_key_dict_deepcopy` crash: merged objects should have ob_tid == 0[/+] on Jun 5, 2025colesbury commented on Jun 16, 2025 ContributorAuthor More actions The trashcan mechanism is broken in the 3.14 free threaded build. _PyTrash_thread_deposit_object overwrites the ob_tid in the free threading build, but that's not safe if the object is still tracked by the GC.
|
void |
|
_Py_Dealloc(PyObject *op) |
|
{ |
|
PyTypeObject *type = Py_TYPE(op); |
|
destructor dealloc = type->tp_dealloc; |
|
PyThreadState *tstate = _PyThreadState_GET(); |
|
intptr_t margin = _Py_RecursionLimit_GetMargin(tstate); |
|
if (margin < 2) { |
|
_PyTrash_thread_deposit_object(tstate, (PyObject *)op); |
|
return; |
|
} |
(from #132280)
We're only seeing this currently on Windows probably because of the smaller stack size.
cc @nascheme @markshannon colesbury commented on Jun 16, 2025 ContributorAuthor More actions I think this is also broken in the default build. If you change _Py_RecursionLimit_GetMargin to occasionally return 1 as in #135589 you get lots of crashes. markshannon commented on Jun 17, 2025 How does the FT GC use ob_tid?
ob_tid should only be overwritten when the refcount reaches zero, so the GC shouldn't be looking at the object. colesbury commented on Jun 17, 2025 ContributorAuthor More actions Another issue is that we now can trigger arbitrary code from Py_DECREF() on types like PyLongObject if there's trash in tstate->delete_later whereas previously we assumed that these destructors had constrained behavior. nascheme commented on Jun 17, 2025
I think we should revert it sooner rather than later unless there are easy fixes to the problems.
I'm not sure there are easy fixes. The objects that previously used Py_TRASHCAN_BEGIN would expect that the object is untracked when executing the rest of their dealloc function. However, now the trashcan applies to all "container" objects, we don't know if the dealloc function expects that or not. Should _PyTrash_thread_destroy_chain should re-track or not?
My old PR (GH-124716) untracked in the deposit and didn't re-track. That made the change a fair bit more complex than GH-132280. I think it is more logical that Py_Dealloc untracks automatically but I suspect that could could break some extension types.
The objects are still visible to the GC and may be treated as cyclic trash, which then corrupts the reference count field.
If we untrack in deposit, this is solved, right?
[a deposited] object can still be accessed via weakrefs
Ugh, I don't think we want to clear the weakrefs when we deposit. If we untrack the object, we could make the trashcan chain use _PyGCHead_SET_PREV(), at least in the default build. All of this doesn't sound very easy, at least in terms of putting it in a beta. nascheme commented on Jun 17, 2025
The objects that previously used Py_TRASHCAN_BEGIN would expect that the object is untracked when executing the rest of their dealloc function. However, now the trashcan applies to all "container" objects, we don't know if the dealloc function expects that or not.
If we allow _PyObject_GC_UNTRACK() to be called on an already untracked object, things should mostly work if deposit untracks. Types that previously used Py_TRASHCAN_BEGIN must call untrack first and will expect that the object is untracked in the body. So those are okay.
Container objects that don't explicitly use the trashcan should untrack before tearing down the object. There could be an issue if a type resurrects an object and therefore doesn't expect that dealloc is called with the object already untracked. markshannon commented on Jun 18, 2025 Weakrefs are a pain, and why we can't thread the destroy chain through objects 😞
I'd like to keep the general approach of using the stack limits to determine when to use the trashcan as that is key for performance.
Once we've hit the trashcan, performance is less important, so adding a _PyObject_IS_GC check should be fine.
With a _PyObject_IS_GC check we can go back to only putting GC-able objects in the trashcan and using the GC fields, not the object header for the linked list.
We'll need to untrack the objects to put them in the list and track them again before cleaning them up.
I'll make a PR later in the week if no one beats me to it. hugovk commented on Jul 7, 2025 Closing, this has been fixed and backported, thanks all. Please re-open if necessary.
MetadataMetadataStatusShow more project fields
DevelopmentNo branches or pull requestsIssue actions
|