ASDF segment faults sometimes under some circumstances when `tags` is not a list/tuple
WilliamJamieson opened this issue · 3 comments
WilliamJamieson commented
The small bug introduced in: https://github.com/WilliamJamieson/asdf-astropy/blob/9568c042baa16f0d447df777a22f31aff9f13a80/asdf_astropy/converters/coordinates/angle.py#L5 causes a segment fault originating somewhere in ASDF or its pytest plugin.
That if some converter lists a * tag without wrapping it in a tuple or list, causes a segment fault to occur during test collection. See:
❯ pytest
=========================================================================== test session starts ============================================================================
platform darwin -- Python 3.11.4, pytest-7.4.0, pluggy-1.2.0
Running tests with asdf_astropy version 0.4.1.dev25+gc6608be.
Running tests in asdf_astropy docs.
Date: 2023-07-13T15:38:00
Platform: macOS-13.4.1-arm64-arm-64bit
Executable: /Users/wjamieson/.pyenv/versions/3.11.4/envs/asdf-astropy/bin/python3.11
Full Python Version:
3.11.4 (main, Jun 26 2023, 16:08:50) [Clang 14.0.3 (clang-1403.0.22.14.1)]
encodings: sys: utf-8, locale: UTF-8, filesystem: utf-8
byteorder: little
float info: dig: 15, mant_dig: 15
Package versions:
Numpy: 1.25.1
Scipy: 1.11.1
Matplotlib: 3.7.2
h5py: not available
scikit-image: not available
Using Astropy options: remote_data: none.
rootdir: /Users/wjamieson/workspaces/asdf-astropy/asdf-astropy
configfile: pyproject.toml
testpaths: asdf_astropy, docs
plugins: hypothesis-6.81.1, remotedata-0.4.0, asdf-2.15.0, doctestplus-0.13.0, cov-4.1.0, filter-subpackage-0.1.2, mock-3.11.1, astropy-header-0.2.2, astropy-0.10.0, openfiles-0.5.0, arraydiff-0.5.0
collected 6257 items
asdf_astropy/converters/coordinates/tests/test_angle.py Fatal Python error: Segmentation fault
Current thread 0x00000001ea4c1e00 (most recent call first):
File "/Users/wjamieson/.pyenv/versions/3.11.4/envs/asdf-astropy/lib/python3.11/site-packages/numpy/core/arrayprint.py", line 936 in fillFormat
File "/Users/wjamieson/.pyenv/versions/3.11.4/envs/asdf-astropy/lib/python3.11/site-packages/numpy/core/arrayprint.py", line 932 in __init__
File "/Users/wjamieson/.pyenv/versions/3.11.4/envs/asdf-astropy/lib/python3.11/site-packages/numpy/core/arrayprint.py", line 411 in <lambda>
File "/Users/wjamieson/.pyenv/versions/3.11.4/envs/asdf-astropy/lib/python3.11/site-packages/numpy/core/arrayprint.py", line 472 in _get_format_function
File "/Users/wjamieson/.pyenv/versions/3.11.4/envs/asdf-astropy/lib/python3.11/site-packages/numpy/core/arrayprint.py", line 539 in _array2string
File "/Users/wjamieson/.pyenv/versions/3.11.4/envs/asdf-astropy/lib/python3.11/site-packages/numpy/core/arrayprint.py", line 513 in wrapper
File "/Users/wjamieson/.pyenv/versions/3.11.4/envs/asdf-astropy/lib/python3.11/site-packages/numpy/core/arrayprint.py", line 736 in array2string
File "/Users/wjamieson/.pyenv/versions/3.11.4/envs/asdf-astropy/lib/python3.11/site-packages/numpy/core/arrayprint.py", line 1508 in _array_repr_implementation
File "/Users/wjamieson/.pyenv/versions/3.11.4/envs/asdf-astropy/lib/python3.11/site-packages/_pytest/_io/saferepr.py", line 76 in repr_instance
File "/Users/wjamieson/.pyenv/versions/3.11.4/lib/python3.11/reprlib.py", line 63 in repr1
File "/Users/wjamieson/.pyenv/versions/3.11.4/lib/python3.11/reprlib.py", line 72 in <listcomp>
File "/Users/wjamieson/.pyenv/versions/3.11.4/lib/python3.11/reprlib.py", line 72 in _repr_iterable
File "/Users/wjamieson/.pyenv/versions/3.11.4/lib/python3.11/reprlib.py", line 81 in repr_tuple
File "/Users/wjamieson/.pyenv/versions/3.11.4/lib/python3.11/reprlib.py", line 61 in repr1
File "/Users/wjamieson/.pyenv/versions/3.11.4/lib/python3.11/reprlib.py", line 53 in repr
File "/Users/wjamieson/.pyenv/versions/3.11.4/envs/asdf-astropy/lib/python3.11/site-packages/_pytest/_io/saferepr.py", line 64 in repr
File "/Users/wjamieson/.pyenv/versions/3.11.4/envs/asdf-astropy/lib/python3.11/site-packages/_pytest/_io/saferepr.py", line 115 in saferepr
File "/Users/wjamieson/.pyenv/versions/3.11.4/envs/asdf-astropy/lib/python3.11/site-packages/_pytest/_code/code.py", line 763 in repr_args
File "/Users/wjamieson/.pyenv/versions/3.11.4/envs/asdf-astropy/lib/python3.11/site-packages/_pytest/_code/code.py", line 859 in repr_traceback_entry
File "/Users/wjamieson/.pyenv/versions/3.11.4/envs/asdf-astropy/lib/python3.11/site-packages/_pytest/_code/code.py", line 914 in <listcomp>
File "/Users/wjamieson/.pyenv/versions/3.11.4/envs/asdf-astropy/lib/python3.11/site-packages/_pytest/_code/code.py", line 913 in repr_traceback
File "/Users/wjamieson/.pyenv/versions/3.11.4/envs/asdf-astropy/lib/python3.11/site-packages/_pytest/_code/code.py", line 989 in repr_excinfo
File "/Users/wjamieson/.pyenv/versions/3.11.4/envs/asdf-astropy/lib/python3.11/site-packages/_pytest/_code/code.py", line 701 in getrepr
File "/Users/wjamieson/.pyenv/versions/3.11.4/envs/asdf-astropy/lib/python3.11/site-packages/_pytest/nodes.py", line 485 in _repr_failure_py
File "/Users/wjamieson/.pyenv/versions/3.11.4/envs/asdf-astropy/lib/python3.11/site-packages/_pytest/python.py", line 1829 in repr_failure
File "/Users/wjamieson/.pyenv/versions/3.11.4/envs/asdf-astropy/lib/python3.11/site-packages/_pytest/reports.py", line 362 in from_item_and_call
File "/Users/wjamieson/.pyenv/versions/3.11.4/envs/asdf-astropy/lib/python3.11/site-packages/_pytest/runner.py", line 368 in pytest_runtest_makereport
File "/Users/wjamieson/.pyenv/versions/3.11.4/envs/asdf-astropy/lib/python3.11/site-packages/pluggy/_callers.py", line 80 in _multicall
File "/Users/wjamieson/.pyenv/versions/3.11.4/envs/asdf-astropy/lib/python3.11/site-packages/pluggy/_manager.py", line 112 in _hookexec
File "/Users/wjamieson/.pyenv/versions/3.11.4/envs/asdf-astropy/lib/python3.11/site-packages/pluggy/_hooks.py", line 433 in __call__
File "/Users/wjamieson/.pyenv/versions/3.11.4/envs/asdf-astropy/lib/python3.11/site-packages/_pytest/runner.py", line 224 in call_and_report
File "/Users/wjamieson/.pyenv/versions/3.11.4/envs/asdf-astropy/lib/python3.11/site-packages/_pytest/runner.py", line 133 in runtestprotocol
File "/Users/wjamieson/.pyenv/versions/3.11.4/envs/asdf-astropy/lib/python3.11/site-packages/_pytest/runner.py", line 114 in pytest_runtest_protocol
File "/Users/wjamieson/.pyenv/versions/3.11.4/envs/asdf-astropy/lib/python3.11/site-packages/pluggy/_callers.py", line 80 in _multicall
File "/Users/wjamieson/.pyenv/versions/3.11.4/envs/asdf-astropy/lib/python3.11/site-packages/pluggy/_manager.py", line 112 in _hookexec
File "/Users/wjamieson/.pyenv/versions/3.11.4/envs/asdf-astropy/lib/python3.11/site-packages/pluggy/_hooks.py", line 433 in __call__
File "/Users/wjamieson/.pyenv/versions/3.11.4/envs/asdf-astropy/lib/python3.11/site-packages/_pytest/main.py", line 349 in pytest_runtestloop
File "/Users/wjamieson/.pyenv/versions/3.11.4/envs/asdf-astropy/lib/python3.11/site-packages/pluggy/_callers.py", line 80 in _multicall
File "/Users/wjamieson/.pyenv/versions/3.11.4/envs/asdf-astropy/lib/python3.11/site-packages/pluggy/_manager.py", line 112 in _hookexec
File "/Users/wjamieson/.pyenv/versions/3.11.4/envs/asdf-astropy/lib/python3.11/site-packages/pluggy/_hooks.py", line 433 in __call__
File "/Users/wjamieson/.pyenv/versions/3.11.4/envs/asdf-astropy/lib/python3.11/site-packages/_pytest/main.py", line 324 in _main
File "/Users/wjamieson/.pyenv/versions/3.11.4/envs/asdf-astropy/lib/python3.11/site-packages/_pytest/main.py", line 270 in wrap_session
File "/Users/wjamieson/.pyenv/versions/3.11.4/envs/asdf-astropy/lib/python3.11/site-packages/_pytest/main.py", line 317 in pytest_cmdline_main
File "/Users/wjamieson/.pyenv/versions/3.11.4/envs/asdf-astropy/lib/python3.11/site-packages/pluggy/_callers.py", line 80 in _multicall
File "/Users/wjamieson/.pyenv/versions/3.11.4/envs/asdf-astropy/lib/python3.11/site-packages/pluggy/_manager.py", line 112 in _hookexec
File "/Users/wjamieson/.pyenv/versions/3.11.4/envs/asdf-astropy/lib/python3.11/site-packages/pluggy/_hooks.py", line 433 in __call__
File "/Users/wjamieson/.pyenv/versions/3.11.4/envs/asdf-astropy/lib/python3.11/site-packages/_pytest/config/__init__.py", line 166 in main
File "/Users/wjamieson/.pyenv/versions/3.11.4/envs/asdf-astropy/lib/python3.11/site-packages/_pytest/config/__init__.py", line 189 in console_main
File "/Users/wjamieson/.pyenv/versions/asdf-astropy/bin/pytest", line 8 in <module>
Extension modules: numpy.core._multiarray_umath, numpy.core._multiarray_tests, numpy.linalg._umath_linalg, numpy.fft._pocketfft_internal, numpy.random._common, numpy.random.bit_generator, numpy.random._bounded_integers, numpy.random._mt19937, numpy.random.mtrand, numpy.random._philox, numpy.random._pcg64, numpy.random._sfc64, numpy.random._generator, yaml._yaml, erfa.ufunc, pvectorc, scipy._lib._ccallback_c, matplotlib._c_internal_utils, PIL._imaging, matplotlib._path, kiwisolver._cext, astropy.time._parse_times, astropy.table._column_mixins, astropy.table._np_utils, astropy.io.ascii.cparser, astropy.utils.xml._iterparser, astropy.io.fits._utils, astropy.io.fits._tiled_compression._compression, astropy.io.votable.tablewriter, numpy.linalg.lapack_lite, astropy.wcs._wcs, scipy.special._ufuncs_cxx, scipy.special._ufuncs, scipy.special._specfun, scipy.special._comb, scipy.linalg._fblas, scipy.linalg._flapack, scipy.linalg.cython_lapack, scipy.linalg._cythonized_array_utils, scipy.linalg._solve_toeplitz, scipy.linalg._decomp_lu_cython, scipy.linalg._matfuncs_sqrtm_triu, scipy.linalg.cython_blas, scipy.linalg._matfuncs_expm, scipy.linalg._decomp_update, scipy.sparse._sparsetools, _csparsetools, scipy.sparse._csparsetools, scipy.sparse.linalg._isolve._iterative, scipy.sparse.linalg._dsolve._superlu, scipy.sparse.linalg._eigen.arpack._arpack, scipy.sparse.csgraph._tools, scipy.sparse.csgraph._shortest_path, scipy.sparse.csgraph._traversal, scipy.sparse.csgraph._min_spanning_tree, scipy.sparse.csgraph._flow, scipy.sparse.csgraph._matching, scipy.sparse.csgraph._reordering, scipy.linalg._flinalg, scipy.special._ellip_harm_2, scipy.interpolate._fitpack, scipy.interpolate.dfitpack, scipy.optimize._minpack2, scipy.optimize._group_columns, scipy._lib.messagestream, scipy.optimize._trlib._trlib, scipy.optimize._lbfgsb, _moduleTNC, scipy.optimize._moduleTNC, scipy.optimize._cobyla, scipy.optimize._slsqp, scipy.optimize._minpack, scipy.optimize._lsq.givens_elimination, scipy.optimize._zeros, scipy.optimize.__nnls, scipy.optimize._highs.cython.src._highs_wrapper, scipy.optimize._highs._highs_wrapper, scipy.optimize._highs.cython.src._highs_constants, scipy.optimize._highs._highs_constants, scipy.linalg._interpolative, scipy.optimize._bglu_dense, scipy.optimize._lsap, scipy.spatial._ckdtree, scipy.spatial._qhull, scipy.spatial._voronoi, scipy.spatial._distance_wrap, scipy.spatial._hausdorff, scipy.spatial.transform._rotation, scipy.optimize._direct, scipy.interpolate._bspl, scipy.interpolate._ppoly, scipy.interpolate.interpnd, scipy.interpolate._rbfinterp_pythran, scipy.interpolate._rgi_cython, scipy.integrate._odepack, scipy.integrate._quadpack, scipy.integrate._vode, scipy.integrate._dop, scipy.integrate._lsoda, astropy.cosmology.flrw.scalar_inv_efuncs, charset_normalizer.md (total: 101)
[1] 42088 segmentation fault pytest
Note that this works for any of the * tags listed for asdf-astropy that I tried.
I think this should be addressed because a segment fault is rather harsh. In many cases, a Converter is written such that it supports a single tag pattern. If one uses a tag pattern (i.e. using * for the version number) for tags in the Converter something in ASDF causes a segmentation fault. Since the * pattern is usually used to represent multiple tags (or account for multiple tags in the future) it naturally implies that there are multiple tags involved. This seems like an easy mistake to make.
I think ASDF should either:
- Catch this issue before the segmentation fault, and raise some useful error.
- Or wrap these cases automatically in a list/tuple and continue (maybe raise a warning in this case).
This was on Python 3.11.4 with:
❯ pip list
Package Version Editable project location
----------------------------- -------------------- -----------------------------------------------------
alabaster 0.7.13
asdf 2.15.0
asdf-astropy 0.4.1.dev25+gc6608be /Users/wjamieson/workspaces/asdf-astropy/asdf-astropy
asdf-coordinates-schemas 0.2.0
asdf-standard 1.0.3
asdf-transform-schemas 0.3.0
asdf-unit-schemas 0.1.0
astropy 5.3.1
astropy-sphinx-theme 1.1
attrs 23.1.0
Babel 2.12.1
certifi 2023.5.7
charset-normalizer 3.2.0
contourpy 1.1.0
coverage 7.2.7
cycler 0.11.0
docutils 0.18.1
fonttools 4.41.0
graphviz 0.20.1
hypothesis 6.81.1
idna 3.4
imagesize 1.4.1
importlib-metadata 6.8.0
iniconfig 2.0.0
Jinja2 3.1.2
jmespath 1.0.1
jsonschema 4.17.3
kiwisolver 1.4.4
MarkupSafe 2.1.3
matplotlib 3.7.2
mistune 3.0.1
numpy 1.25.1
numpydoc 1.5.0
packaging 23.1
Pillow 10.0.0
pip 23.1.2
pluggy 1.2.0
psutil 5.9.5
pyerfa 2.0.0.3
Pygments 2.15.1
pyparsing 3.0.9
pyrsistent 0.19.3
pytest 7.4.0
pytest-arraydiff 0.5.0
pytest-astropy 0.10.0
pytest-astropy-header 0.2.2
pytest-cov 4.1.0
pytest-doctestplus 0.13.0
pytest-filter-subpackage 0.1.2
pytest-mock 3.11.1
pytest-openfiles 0.5.0
pytest-remotedata 0.4.0
python-dateutil 2.8.2
PyYAML 6.0
requests 2.31.0
scipy 1.11.1
semantic-version 2.10.0
setuptools 65.5.0
six 1.16.0
snowballstemmer 2.2.0
sortedcontainers 2.4.0
Sphinx 6.2.1
sphinx-asdf 0.2.0
sphinx-astropy 1.9.1
sphinx-automodapi 0.15.0
sphinx-bootstrap-theme 0.8.1
sphinx-gallery 0.13.0
sphinx-rtd-theme 1.2.2
sphinxcontrib-applehelp 1.0.4
sphinxcontrib-devhelp 1.0.2
sphinxcontrib-htmlhelp 2.0.1
sphinxcontrib-jquery 4.1
sphinxcontrib-jsmath 1.0.1
sphinxcontrib-qthelp 1.0.3
sphinxcontrib-serializinghtml 1.1.5
toml 0.10.2
tomli 2.0.1
urllib3 2.0.3
zipp 3.16.1
braingram commented
Thanks for the details!
Testing the modification you shared (on python 3.10), I'm not seeing the segfault on test collection but during test_serialization in test_angle.py.
The issue here appears to be (somewhat unsurprisingly given it's a segfault) due to the default memmapping of arrays. If I add copy_arrays=True to the asdf.open call here:https://github.com/astropy/asdf-astropy/blob/main/asdf_astropy/converters/coordinates/tests/test_angle.py#L26
I no longer get a segfault but all test_serialization tests fail (as the converter incorrectly serializes the angle). I've seen this once before but dug into it a bit more to learn more about the cause.
It comes down to when the with statement is closed relative to when the exception is inspected.
With the * tag, asdf saves the angle as an ndarray which when loaded is by default memmapped. When the comparison with the angle fails, the exception cascades through astropy and finally makes it back to pytest. The exception does not prevent the with context from exiting (closing the asdf file, leaving any references to the memmap vulnerable to segfaults). pytest, during generating the report output, will by default use a 'long' traceback which will result in an attempt to display the memmapped array from the closed file resulting in the segfault (running the segfaulting run with --tb=native or other non-long options prevents the segfault).
These segfaults should be avoided and appears to be an issue with how memmaps are generated and passed to non-NDArrayType objects (see the related #1334). However I don't see any issue specific to the Converter tags.
WilliamJamieson commented
This is fair, it's just a surprisingly small "mistake" to induce a segment fault, and points to issues in how we are handling something (in this case the memory map).