Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible ABI breakage in 7.3.14 #4816

Closed
mgorny opened this issue Jan 1, 2024 · 13 comments
Closed

Possible ABI breakage in 7.3.14 #4816

mgorny opened this issue Jan 1, 2024 · 13 comments

Comments

@mgorny
Copy link
Contributor

mgorny commented Jan 1, 2024

It seems that some ABI change was introduced between 7.3.13 and 7.3.14, more specifically between 2023-11-29 and 2023-12-07 snapshots. The Pillow extensions built with the prior version crash when used with pikepdf built with the latter version.

To reproduce:

git clone --depth 1 https://github.com/pikepdf/pikepdf/
cd pikepdf
pypy3 -m venv .venv
. .venv/bin/activate
# note that this installs pillow from official wheel
# also note that you need to have qpdf installed on the system
pip install .[test]
python -m pytest -o addopts= -s tests/test_image_access.py::test_image_save_compare

The result is:

========================================================= test session starts =========================================================
platform linux -- Python 3.10.13[pypy-7.3.14-final], pytest-7.4.4, pluggy-1.3.0
rootdir: /tmp/pikepdf
configfile: pyproject.toml
plugins: hypothesis-6.92.2, xdist-3.5.0, timeout-2.2.0, cov-4.1.0
collected 1 item                                                                                                                      

tests/test_image_access.py Fatal Python error: Segmentation fault

Stack (most recent call first, approximate line numbers):
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/PIL/Image.py", line 2980 in frombuffer
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/pikepdf/models/_transcoding.py", line 106 in image_from_byte_buffer
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/pikepdf/models/image.py", line 552 in _extract_transcoded_1248bits
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/pikepdf/models/image.py", line 605 in _extract_transcoded
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/pikepdf/models/image.py", line 635 in _extract_to_stream
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/pikepdf/models/image.py", line 672 in extract_to
  File "/tmp/pikepdf/tests/test_image_access.py", line 301 in test_image_save_compare
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/hypothesis/core.py", line 848 in run
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/hypothesis/core.py", line 728 in default_executor
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/hypothesis/core.py", line 793 in execute_once
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/hypothesis/core.py", line 958 in _execute_once_for_engine
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/hypothesis/internal/conjecture/engine.py", line 162 in __stoppable_test_function
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/hypothesis/internal/conjecture/engine.py", line 183 in test_function
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/hypothesis/internal/conjecture/engine.py", line 982 in cached_test_function
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/hypothesis/internal/conjecture/engine.py", line 586 in generate_new_examples
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/hypothesis/internal/conjecture/engine.py", line 862 in _run
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/hypothesis/internal/conjecture/engine.py", line 457 in run
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/hypothesis/core.py", line 1064 in run_engine
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/hypothesis/core.py", line 1388 in wrapped_test
  File "/tmp/pikepdf/tests/test_image_access.py", line 301 in test_image_save_compare
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/_pytest/python.py", line 187 in pytest_pyfunc_call
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/pluggy/_callers.py", line 27 in _multicall
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/pluggy/_manager.py", line 106 in _hookexec
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/pluggy/_hooks.py", line 479 in __call__
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/_pytest/python.py", line 1790 in runtest
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/_pytest/runner.py", line 160 in pytest_runtest_call
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/pluggy/_callers.py", line 27 in _multicall
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/pluggy/_manager.py", line 106 in _hookexec
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/pluggy/_hooks.py", line 479 in __call__
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/_pytest/runner.py", line 262 in <lambda>
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/_pytest/runner.py", line 318 in from_call
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/_pytest/runner.py", line 247 in call_runtest_hook
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/_pytest/runner.py", line 219 in call_and_report
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/_pytest/runner.py", line 119 in runtestprotocol
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/_pytest/runner.py", line 111 in pytest_runtest_protocol
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/pluggy/_callers.py", line 27 in _multicall
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/pluggy/_manager.py", line 106 in _hookexec
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/pluggy/_hooks.py", line 479 in __call__
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/_pytest/main.py", line 338 in pytest_runtestloop
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/pluggy/_callers.py", line 27 in _multicall
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/pluggy/_manager.py", line 106 in _hookexec
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/pluggy/_hooks.py", line 479 in __call__
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/_pytest/main.py", line 321 in _main
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/_pytest/main.py", line 258 in wrap_session
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/_pytest/main.py", line 317 in pytest_cmdline_main
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/pluggy/_callers.py", line 27 in _multicall
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/pluggy/_manager.py", line 106 in _hookexec
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/pluggy/_hooks.py", line 479 in __call__
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/_pytest/config/__init__.py", line 135 in main
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/_pytest/config/__init__.py", line 185 in console_main
  File "/tmp/pikepdf/.venv/lib/pypy3.10/site-packages/pytest/__main__.py", line 1 in <module>
  File "/usr/lib/pypy3.10/runpy.py", line 63 in _run_code
  File "/usr/lib/pypy3.10/runpy.py", line 174 in _run_module_as_main
  File "<builtin>/app_main.py", line 133 in run_toplevel
  File "<builtin>/app_main.py", line 748 in run_command_line
  File "<builtin>/app_main.py", line 1149 in entry_point
Segmentation fault (core dumped)

Rebuilding Pillow from source or installing the older PyPy version resolves the issue.

@mattip
Copy link
Member

mattip commented Jan 1, 2024

Probably has to do with some of the buffer interface changes I made for HPy. Do the Pillow tests pass, or can I somehow make the reproducer smaller by running a Pillow test?

@mgorny
Copy link
Contributor Author

mgorny commented Jan 1, 2024

I wasn't able to get pillow to crash itself.

@mgorny
Copy link
Contributor Author

mgorny commented Jan 1, 2024

It's possible that it happens only because both pillow and pikepdf have C extensions, and they were built against different versions of PyPy.

@nulano
Copy link
Contributor

nulano commented Jan 1, 2024

FWIW, Pillow 10.2.0 is due to be released tomorrow, built using the versions listed here: https://github.com/pypa/cibuildwheel/blob/v2.16.2/cibuildwheel/resources/build-platforms.toml

Checking the logs, it appears the following PyPy versions will be used:

manylinux_2_28: PyPy3.9-7.3.12, PyPy3.10-7.3.12
manylinux_2_17: PyPy3.9-7.3.12, PyPy3.10-7.3.12
macos (x86_64): PyPy3.9-7.3.13, PyPy3.10-7.3.13
Windows: PyPy3.8-7.3.11, PyPy3.9-7.3.13, PyPy3.10-7.3.13

@nulano
Copy link
Contributor

nulano commented Jan 1, 2024

8d45c2c looks like the likely candidate. Not sure if #4040 can be fixed without breaking the ABI.

@mgorny
Copy link
Contributor Author

mgorny commented Jan 2, 2024

8d45c2c looks like the likely candidate. Not sure if #4040 can be fixed without breaking the ABI.

Thanks. I'm going to test it and let you know in a minute if reverting it helps with the segfault.

@mgorny
Copy link
Contributor Author

mgorny commented Jan 2, 2024

Indeed, I can confirm that reverting it (plus updating pypy/module/cpyext/api.py to list Py_MAX_NDIMS again) resolves the segfault.

@mattip
Copy link
Member

mattip commented Jan 2, 2024

What if you only change this back to 36 but leave the change of Py_MAX_NDIMS -> PyBUF_MAX_NDIM?

-#define Py_MAX_NDIMS 36
+#define PyBUF_MAX_NDIM 64

@mgorny
Copy link
Contributor Author

mgorny commented Jan 2, 2024

Sorry, I'm confused. Did you mean doing:

-#define Py_MAX_NDIMS 36
+#define PyBUF_MAX_NDIM 36

?

@mattip
Copy link
Member

mattip commented Jan 3, 2024

yes, leave 8d45c2c in place and only change the macro to 36

@mgorny
Copy link
Contributor Author

mgorny commented Jan 3, 2024

Yes, this seems to be fix the problem. Thanks!

@mattip
Copy link
Member

mattip commented Jan 3, 2024

cool, thanks

mattip added a commit that referenced this issue Jan 3, 2024
change PyBUF_MAX_NDIM back to 36 for ABI consistancy (issue #4816)
@mattip
Copy link
Member

mattip commented Jan 16, 2024

Closed by #4825

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants