segfault raised by AlignmentFilePileup_test.py with python3.11

[emollier]

Greetings,

With the introduction of python3.11 in Debian sid, Graham Inggs
noticed in Debian Bug#1024425 that in pysam 0.19.1, the
tests/AlignmentFilePileup_test.py::TestPileupObjects::testIteratorOutOfScope
raises a segmentation fault when running on top of the
python3.11 interpreter. I could reproduce a similar behavior
while trying to upgrade the pysam package to version 0.20.0.

I attempted to pinpoint the issue by running the faulty test in
pysam 0.20.0 in gdb. Here is what I could get out of the
debugger:

(gdb) run
Starting program: /usr/bin/python3.11 tests/AlignmentFilePileup_test.py
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
[Detaching after vfork from child process 1504815]
/build/python-pysam-dNO1D7/python-pysam-0.20.0+ds/tests/PileupTestUtils.py:35: ResourceWarning: unclosed file <_io.TextIOWrapper name='/dev/null' mode='w' encoding='UTF-8'>
  with subprocess.Popen(["samtools", "mpileup", fn],
ResourceWarning: Enable tracemalloc to get the object allocation traceback
.[Detaching after vfork from child process 1504816]
/build/python-pysam-dNO1D7/python-pysam-0.20.0+ds/tests/PileupTestUtils.py:134: ResourceWarning: unclosed file <_io.TextIOWrapper name='/dev/null' mode='w' encoding='UTF-8'>
  return data
ResourceWarning: Enable tracemalloc to get the object allocation traceback
.[Detaching after vfork from child process 1504817]
/build/python-pysam-dNO1D7/python-pysam-0.20.0+ds/tests/PileupTestUtils.py:60: ResourceWarning: unclosed file <_io.TextIOWrapper name='/dev/null' mode='w' encoding='UTF-8'>
  with subprocess.Popen(["samtools", "mpileup", fn] + list(args),
ResourceWarning: Enable tracemalloc to get the object allocation traceback
.[Detaching after vfork from child process 1504818]
/build/python-pysam-dNO1D7/python-pysam-0.20.0+ds/tests/PileupTestUtils.py:60: ResourceWarning: unclosed file <_io.TextIOWrapper name='/dev/null' mode='w' encoding='UTF-8'>
  with subprocess.Popen(["samtools", "mpileup", fn] + list(args),
ResourceWarning: Enable tracemalloc to get the object allocation traceback
.[Detaching after vfork from child process 1504819]
/build/python-pysam-dNO1D7/python-pysam-0.20.0+ds/tests/PileupTestUtils.py:116: ResourceWarning: unclosed file <_io.TextIOWrapper name='/dev/null' mode='w' encoding='UTF-8'>
  return data
ResourceWarning: Enable tracemalloc to get the object allocation traceback
.[Detaching after vfork from child process 1504820]
/build/python-pysam-dNO1D7/python-pysam-0.20.0+ds/tests/PileupTestUtils.py:116: ResourceWarning: unclosed file <_io.TextIOWrapper name='/dev/null' mode='w' encoding='UTF-8'>
  return data
ResourceWarning: Enable tracemalloc to get the object allocation traceback
..........
Program received signal SIGSEGV, Segmentation fault.
__pyx_f_5pysam_18libcalignedsegment_pileup_base_qual_skip (__pyx_v_threshold=13, __pyx_v_p=0x100000000) at pysam/libcalignedsegment.c:5658
5658	  __pyx_t_1 = ((__pyx_v_p->qpos < __pyx_v_p->b->core.l_qseq) != 0);
(gdb) up
#1  __pyx_pf_5pysam_18libcalignedsegment_12PileupColumn_10get_num_aligned (__pyx_v_self=0x7ffff690cba0) at pysam/libcalignedsegment.c:36098
36098	    __pyx_t_1 = (__pyx_f_5pysam_18libcalignedsegment_pileup_base_qual_skip(__pyx_v_p, __pyx_v_self->min_base_quality) != 0);
(gdb) backtrace
#0  __pyx_f_5pysam_18libcalignedsegment_pileup_base_qual_skip (__pyx_v_threshold=13, __pyx_v_p=0x100000000) at pysam/libcalignedsegment.c:5658
#1  __pyx_pf_5pysam_18libcalignedsegment_12PileupColumn_10get_num_aligned (__pyx_v_self=0x7ffff690cba0) at pysam/libcalignedsegment.c:36098
#2  __pyx_pw_5pysam_18libcalignedsegment_12PileupColumn_11get_num_aligned (__pyx_v_self=0x7ffff690cba0, unused=<optimized out>) at pysam/libcalignedsegment.c:35939
#3  0x000000000051e20b in ?? ()
#4  0x0000000000533ae5 in _PyEval_EvalFrameDefault ()
#5  0x00000000005822e4 in ?? ()
#6  0x0000000000581c6e in ?? ()
#7  0x000000000052e612 in _PyEval_EvalFrameDefault ()
#8  0x000000000051dcf7 in _PyObject_FastCallDictTstate ()
#9  0x0000000000563e6d in _PyObject_Call_Prepend ()
#10 0x00000000005e1b72 in ?? ()
#11 0x0000000000516ae3 in _PyObject_MakeTpCall ()
#12 0x000000000052a690 in _PyEval_EvalFrameDefault ()
#13 0x00000000005822e4 in ?? ()
#14 0x0000000000581c6e in ?? ()
#15 0x000000000052e612 in _PyEval_EvalFrameDefault ()
#16 0x000000000051dcf7 in _PyObject_FastCallDictTstate ()
#17 0x0000000000563e6d in _PyObject_Call_Prepend ()
#18 0x00000000005e1b72 in ?? ()
#19 0x0000000000516ae3 in _PyObject_MakeTpCall ()
#20 0x000000000052a690 in _PyEval_EvalFrameDefault ()
#21 0x00000000005822e4 in ?? ()
#22 0x0000000000581c6e in ?? ()
#23 0x000000000052e612 in _PyEval_EvalFrameDefault ()
#24 0x000000000051dcf7 in _PyObject_FastCallDictTstate ()
#25 0x0000000000563e6d in _PyObject_Call_Prepend ()
#26 0x00000000005e1b72 in ?? ()
#27 0x0000000000516ae3 in _PyObject_MakeTpCall ()
#28 0x000000000052a690 in _PyEval_EvalFrameDefault ()
#29 0x000000000051dcf7 in _PyObject_FastCallDictTstate ()
#30 0x00000000005635a3 in ?? ()
#31 0x0000000000516a85 in _PyObject_MakeTpCall ()
#32 0x000000000052a690 in _PyEval_EvalFrameDefault ()
#33 0x00000000005222bb in PyEval_EvalCode ()
#34 0x0000000000648957 in ?? ()
#35 0x000000000064621f in ?? ()
#36 0x0000000000652390 in ?? ()
#37 0x00000000006520db in _PyRun_SimpleFileObject ()
#38 0x0000000000651f04 in _PyRun_AnyFileObject ()
#39 0x0000000000650c9f in Py_RunMain ()
#40 0x0000000000625347 in Py_BytesMain ()
#41 0x00007ffff7cdc18a in ?? () from /lib/x86_64-linux-gnu/libc.so.6
#42 0x00007ffff7cdc245 in __libc_start_main () from /lib/x86_64-linux-gnu/libc.so.6
#43 0x00000000006251e1 in _start ()

All I managed to see is that the address stored in the pileup
object seems dubious. With python3.10 it lands in the range
of addressable virtual memory, but with python3.11 it is not
NULL but unlikely valid:

(gdb) print __pyx_v_self.plp[0]
$1 = (const bam_pileup1_t *) 0x100000000

It is also quite possible that I am not looking at the right
thing, and that for the moment the python3.11 interpreter in
Debian sid is buggy. But to be honest, I do not know whether
this is the case or not, and thought you might want to be aware
of the issue in any case.

If that can be reassurring, the problem is not particularly
urgent: we intend to ship Debian 12 with python3.10, bonus
points if python3.11 makes it too. In the meantime, I can
disable python3.11 support for pysam in Debian.

Have a nice day, :)
Étienne.

[nileshpatra]

@AndreasHeger @jmarshall any idea? :-)

[emollier]

Apparently there may be an attempt to ship Debian 12 with python3.11 (transition will be tracked in Debian bug#1024425), so chance are this issue needs to be adequately addressed sooner than later.

Two days ago, I had a look back at this issue, trying to fix or work around it, but didn't manage to get very far. Upon closer inspection, I noticed that the way the test is run during the packaging raises the segmentation fault earlier in the test:

$ python3.11 -m pytest tests

The Debian build log says:

  File "/<<PKGBUILDDIR>>/.pybuild/cpython3_3.11_pysam/build/tests/AlignmentFilePileup_test.py", line 220 in testIteratorOutOfScope

where line 220 corresponds to:

self.assertRaises(ValueError, max_col.get_query_sequences)

But, when I run my reproducer:

$ python3.11 tests/AlignmentFilePileup_test.py

get_query_sequences does raise the ValueError as expected, and then my segmentation fault occurs on the next test, about get_num_aligned, and it is reflected by symbols shown in gdb output.

I'm not sure whether that helps, but the way tests are started seems to be a data point.

[jmarshall]

This is a fairly ridiculous test case. “Please verify that my invalid code results in a pleasing exception rather than a segfault.” Doctor, it hurts when I do this — so don’t do that, then! 😄

So if this cannot be fixed, an appropriate approach would be to disable this particular test or remove it altogether.

I have reproduced this with Python 3.11, which has presumably changed the way unreferenced objects call Cython deallocators or something. I will compare what happens with earlier Pythons and see if it can be improved with 3.11. But as the code tested is essentially invalid C usage, I don’t think it is guaranteed to be particularly fixable.

[emollier]

Hi John, John Marshall, on 2022-12-26:

So if this cannot be fixed, an appropriate approach would be to disable this particular test or remove it altogether.

Sounds good, will proceed this way for now then. Thanks for your time! Have nice solstice festivities, :) -- Étienne Mollier ***@***.***> Fingerprint: 8f91 b227 c7d6 f2b1 948c 8236 793c f67e 8f0d 11da Sent from /dev/pts/1, please excuse my verbosity.

[jmarshall]

For the time being, I have suppressed the test case (for Python 3.11) on the master branch.

Let's leave this issue open in the hope that someone will be able to analyse what has changed in 3.11 and hence fix it properly.