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.

Sign up for GitHub

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

Jump to bottom

samples_mv is unsafe #4155

Open
jonashaag opened this issue Dec 17, 2024 · 1 comment
Open

samples_mv is unsafe #4155

jonashaag opened this issue Dec 17, 2024 · 1 comment
Labels
bug Fixed in next release

Comments

@jonashaag
Copy link

Description of the bug

We got crashes by using samples_mv when the original fitz handle was already deallocated.

IMO samples_mv should be changes so it should be renamed to something like unsafe_samples_mv.

How to reproduce the bug

Unfortunately I wasn't able to reproduce this outside of our production setting, but it went away by changing this code:

np.frombuffer(pixmap.samples_mv, ...)

to

np.frombuffer(pixmap.samples_mv, ...).copy()

PyMuPDF version

1.25.1

Operating system

Linux

Python version

3.12
@JorjMcKie JorjMcKie added the bug label Dec 17, 2024
julian-smith-artifex-com added a commit that referenced this issue Dec 17, 2024
@julian-smith-artifex-com
src/ tests/ docs/: avoid crash if samples memoryview is used after Pi…
9f2c6ba 
…xmap destruction.

This addresses #4155.

src/__init__.py:
    With .samples_mv, remember the memoryview and call its .release() method in
    Pixmap.__del__().
docs/pixmap.rst:
    Document improved behaviour of Pixmap.samples_mv. Also warn that
    Pixmap.samples_ptr is unsafe after destruction of the pixmap.
tests/test_pixmap.py:
    Added test_4155(), check we get ValueError when accessing memoryview after
    Pixmap is destroyed.
@julian-smith-artifex-com julian-smith-artifex-com added the fix developed release schedule to be determined label Dec 17, 2024
@julian-smith-artifex-com
Copy link
Collaborator

Thanks for the report. We have a fix which will ensure that using the memoryview after the Pixmap has been destroyed will raise a ValueError.

Depending on internal review, this fix will hopefully make it into the next release.

julian-smith-artifex-com added a commit that referenced this issue Dec 17, 2024
@julian-smith-artifex-com
src/ tests/ docs/: avoid crash if samples memoryview is used after Pi…
f877d19 
…xmap destruction.

This addresses #4155.

src/__init__.py:
    With .samples_mv, remember the memoryview and call its .release() method in
    Pixmap.__del__().
docs/pixmap.rst:
    Document improved behaviour of Pixmap.samples_mv. Also warn that
    Pixmap.samples_ptr is unsafe after destruction of the pixmap.
tests/test_pixmap.py:
    Added test_4155(), check we get ValueError when accessing memoryview after
    Pixmap is destroyed.
@julian-smith-artifex-com julian-smith-artifex-com added Fixed in next release and removed fix developed release schedule to be determined labels Dec 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Fixed in next release
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@jonashaag @JorjMcKie @julian-smith-artifex-com