bigfile/py: Properly untrack PyVMA from GC before dealloc
On a testing instance we started to see segfaults in pyvma_dealloc() with inside calls to vma_unmap but with NULL pyvma->fileh. That was strange, becuse before calling vma_unmap(), the code explicitly checks whether pyvma->fileh is !NULL.
That was, as it turned out, due to pyvma_dealloc being called twice at the same time from two python threads. Here is how that was possible:
T1 decrefs pyvma and finds its reference count drops to zero. It calls pyvma_dealloc. From there vma_unmap() is called, which calls virt_lock() and that releases GIL first. Another thread T2 was waiting for GIL, it acquires it, does some work at python level and somehow triggers GC. Since PyVMA supports cyclic GC, it was on GC list and thus GC calls dealloc for the same vma again. Here is how it look in the backtraces:
#0 0x00007f6aefc57827 in futex_abstimed_wait_cancelable (private=0, abstime=0x0, expected=0, futex_word=0x1e011d0) at ../sysdeps/unix/sysv/linux/futex-internal.h:205 #1 do_futex_wait (sem=sem@entry=0x1e011d0, abstime=0x0) at sem_waitcommon.c:111 #2 0x00007f6aefc578d4 in __new_sem_wait_slow (sem=0x1e011d0, abstime=0x0) at sem_waitcommon.c:181 #3 0x00007f6aefc5797a in __new_sem_wait (sem=<optimized out>) at sem_wait.c:29 #4 0x00000000004ffbc4 in PyThread_acquire_lock () #5 0x00000000004dbe8a in PyEval_RestoreThread () #6 0x00007f6ac6d3b8fc in py_gil_retake_if_waslocked (arg=0x4f18f00) at bigfile/_bigfile.c:1048 #7 0x00007f6ac6d3dcfc in virt_gil_retake_if_waslocked (gilstate=0x4f18f00) at bigfile/virtmem.c:78 #8 0x00007f6ac6d3dd30 in virt_lock () at bigfile/virtmem.c:92 #9 0x00007f6ac6d3e724 in vma_unmap (vma=0x7f6a7e0c4100) at bigfile/virtmem.c:271 #10 0x00007f6ac6d3a0bc in pyvma_dealloc (pyvma0=0x7f6a7e0c40e0) at bigfile/_bigfile.c:284 ... #13 0x00000000004d76b0 in PyEval_EvalFrameEx ()
#5 0x00007f6ac6d3a081 in pyvma_dealloc (pyvma0=0x7f6a7e0c40e0) at bigfile/_bigfile.c:276 #6 0x0000000000500450 in ?? () #7 0x00000000004ffd82 in _PyObject_GC_New () #8 0x0000000000485392 in PyList_New () #9 0x00000000004d3bff in PyEval_EvalFrameEx ()
T2 does the work of vma_unmap and clears C-level vma. Then when T1 wakes up and returns to vma_unmap, it sees vma->file and all other fields cleared -> oops segfault.
Fix it by removing pyvma from GC list before going to do actual destruction. This way if a concurrent GC triggers, it won't see the vma object on its list, and thus won't have a chance to invoke its destructor the second time.
The bug was introduced in 450ad804 (bigarray: ArrayRef support for BigArray) when PyVMA was changed to be cyclic-GC aware. However at that time, even Python documentation itself was not saying PyObject_GC_UnTrack is needed, as it was added only in 2.7.15 after finding that many types in CPython itself are vulnerable for similar segfaults:
It is pity, that CPython took the approach to force all type authors to care to invoke PyObject_GC_UnTrack explicitly, instead of doing that automatically in Python runtime before calling tp_dealloc.
This should be ok to go; I'm merging.
closedToggle commit list