Fix segfault when a get_cdrawings() callback raises

[lifrary]

Problem

If a callback passed to Page.get_cdrawings(callback=...) raises, the Python process dies with SIGSEGV instead of the failure being reported:

import pymupdf

document = pymupdf.open()
page = document.new_page()
page.draw_line((10, 10), (100, 100))

def callback(path):
    raise ValueError("callback failed")

page.get_cdrawings(callback=callback)   # SIGSEGV
print("returned")

Reproduced with the released wheels of 1.27.1, 1.28.0 and 1.28.2 on macOS arm64 (Python 3.14). The code involved is unchanged on main.

Root cause

jm_append_merge() reports a failed callback with messagef("calling cdrawings callback function/method failed!") and only then calls PyErr_Clear(), so messagev() runs with the callback's exception still set. messagev() initialises two function-local statics on its first call:

static PyObject* pymupdf_module = PyImport_ImportModule("pymupdf");
static PyObject* message_fn = PyObject_GetAttrString(pymupdf_module, "message");

With an exception set, PyImport_ImportModule() returns NULL (CPython raises SystemError: ... returned a result with an exception set), and the NULL is passed to PyObject_GetAttrString(). The crash report shows PyObject_GetAttrString+24 under messagef() under jm_append_merge(), faulting at address 0x8.

It only happens when there has been no earlier messagef() call in the process. Once the statics have been initialised by a call with no exception pending, the same callback failure does not crash, but pymupdf.message() is still called with the exception set, and only part of the message is printed (the trailing newline is lost).

Fix

messagev() now puts any pending exception aside with PyErr_Fetch() before its static variables are initialised, and restores it with PyErr_Restore() after the message is output, discarding any error from outputting the message in that case. With no exception pending, an error from the output is left set, as before. (Simplified as suggested in review.)

Otherwise the behaviour of a failing callback is unchanged: the failure is reported through pymupdf.message() and the exception is cleared, and extraction continues. Whether the callback's exception (including KeyboardInterrupt) should instead propagate to the caller of get_cdrawings() is a separate question that I have left alone.

Testing

tests/test_drawings.py:test_cdrawings_callback_exception() runs the example above in a child process, because the crash depends on there having been no earlier messagef() call in the process, and asserts that the child exits 0, printed the whole message line, and printed returned. Like the other child-process tests it is skipped on Pyodide.

Built locally from the first version of this branch with the default MuPDF 1.28.2 on macOS arm64, Python 3.14.7, PYMUPDF_SETUP_MUPDF_TESSERACT=0:

• test_cdrawings_callback_exception fails with src/extra.i reverted to main (the child dies with SIGSEGV) and passes with the fix.
• tests/test_drawings.py, tests/test_insertpdf.py and tests/test_cluster_drawings.py: 26 passed.

For the current, simplified version I ran this repo's test_quick workflow on my fork (Linux, Python 3.12):

• Without the fix (a branch with only the new test), the new test fails, the child dying with SIGSEGV, against MuPDF master, and against 1.28.x when it is run on its own (run).
• In whole-suite runs against 1.28.x, though, the new test passed without the fix, in both runs (run 1, run 2): the child exited normally and printed the whole message. I have not found out why, so a whole-suite job against 1.28.x may not catch a regression here; against master it does.
• With the fix, all 490 tests pass against both MuPDF master and 1.28.x (run).

Unrelated to this change, but in case it saves someone time: the build used SWIG 4.4.1, because SWIG 4.5.0 (what pip installs by default on macOS with Python 3.13+) fails to compile MuPDF 1.28.2's generated mupdfcpp_swig code with use of undeclared identifier 'PyString_FromString'.

🤖 Generated with Claude Code

[github-actions]

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

[lifrary]

I have read the CLA Document and I hereby sign the CLA

[julian-smith-artifex-com]

Many thanks for figuring this out and creating this PR.

I wonder whether we could simplify the new code though. Once we call PyErr_Fetch(), i think we can assume that PyImport_ImportModule("pymupdf") and PyObject_GetAttrString(pymupdf_module, "message") succeed - if they don't then something has gone badly wrong and it's arguably better to crash out immediately rather than struggle on.

So we'd end up with a smaller patch and simpler code:

 static void messagev(const char* format, va_list va)
 {
+    PyObject* exc_type;
+    PyObject* exc_value;
+    PyObject* exc_traceback;
+    PyErr_Fetch(&exc_type, &exc_value, &exc_traceback);
+    
     static PyObject* pymupdf_module = PyImport_ImportModule("pymupdf");
     static PyObject* message_fn = PyObject_GetAttrString(pymupdf_module, "message");
     char* text;
@@ -232,6 +237,13 @@ static void messagev(const char* format, va_list va)
     Py_XDECREF(args);
     Py_XDECREF(text_py);
     free(text);
+    
+    if (exc_type)
+    {
+        // The caller's exception takes precedence; PyErr_Restore() discards
+        // any error from outputting the message.
+        PyErr_Restore(exc_type, exc_value, exc_traceback);
+    }    
 }

[lifrary]

Thanks, that makes sense. I've updated the PR to your version (plus a two-line comment on why PyErr_Fetch() has to come before the static variables) and dropped the caching and vasprintf() changes; test_quick passes with it on my fork against MuPDF master and 1.28.x. One oddity, with the runs linked in the description: without the fix, the new test catches the crash against master, but in whole-suite runs against 1.28.x it passes, for a reason I haven't found.

[julian-smith-artifex-com]

Thanks for updating.

We've just pushed some quite extensive changes, so could you rebase on top of latest branch main? I don't think this will have any conflicts.

Then we can run the test_quick workflow here and merge.

[julian-smith-artifex-com]

[I've fixed Aptest so that test_quick only needs https access to pymupdf repositories, so tests have now run successfully.]

I'll merge the PR now.

Thanks again for your contribution!