[sdiebolt]

Let Python threads run during ITK pipeline updates, and make the regression test check filtering rather than setup. Closes #7000.

Why the GIL stayed locked

The 2019 GIL-release change enabled SWIG threading but also set nothreadallow, citing this SWIG discussion. That advice served a different goal: protecting director callbacks while deliberately avoiding GIL release around native calls. The combination therefore did not provide the concurrency described by the commit.

The later -threads addition in #5776/#6022 left that override in place. Its regression test accepted overlapping setup/filtering intervals as evidence of GIL release. Measured intervals were about 0.81 seconds, but only 0.002–0.003 seconds were inside Update().

What this fixes

• Release the GIL for Update() and UpdateLargestPossibleRegion(), retaining it for unrelated wrappers and respecting ITK_PYTHON_RELEASE_GIL.
• Reacquire the GIL in PyImageFilter callbacks and before PyCommand validates observer callables.
• Keep multi-filter overlap coverage, timing only preconstructed filters' Update() calls, alongside the waiting-thread GIL probe. Both checks suppress periodic Python thread switching.
• Exercise native Update() exceptions instead of errors that could arise in Python-side file-reading setup.
• Document the scope, select an available Python-filter type, and guard the float-dependent GIL tests with ITK_WRAP_float.

Validation

Linux x86_64, CPython 3.13.9, NumPy 2.3.4, Release build:

• Unpatched wrappers: original regression passed; replacement failed.
• Patched wrappers: the GIL test, including the revised multi-filter check, passed 20 consecutive runs; all four targeted tests passed.
• Native-call controls confirmed that the overlap check rejects GIL-held calls and accepts GIL-releasing calls.
• Issue Python Update() appears to hold the GIL despite GIL-release support #7000's ticker workload: 913–927 ticks during 0.97–0.98 seconds of filtering.
• Compiled SWIG probes confirmed enabled/disabled threading behavior; local hooks passed.
• The unset-callable observer test covers error handling, not GIL ownership; it also passes before the PyCommand fix.
• A separate instrumented probe detected PyCallable_Check() without the GIL before that fix and passes afterward; the final safety script also passes with this instrumentation.
• All four float-wrapping/GIL-release combinations were checked for CMake test registration; these were not full native builds.

The initial Linux CI run also passed all three targeted tests. Its separate median type-hint failure is described in the PR comments.

Full local suite, Python 3.14 wheel, and full GIL-disabled native build validation were not run. CI must validate the latest revision.

PR Checklist

• No API changes were made (or the changes have been approved)
• No major design changes were made (or the changes have been approved)
• Added test (or behavior not changed)
• Updated API documentation (or API not changed)
• Added license to new files (if any) — no new files
• Added Python wrapping to new files (if any) as described in ITK Software Guide Section 9.5 — no new classes
• Added ITK examples for all new major features (if any) — bug fix, no new major feature

AI assistance

AI assistance supported diagnosis, implementation, and local testing.

[greptile-apps]

Confidence Score: 4/5

[High impact] The change affects pipeline updates across ITK’s Python wrappers.

Fix the unchecked float requirement in PyImageFilterTest before merging.

Findings

1.  Float-disabled builds fail this test ▶

Summary

The PR enables GIL release for Update() and UpdateLargestPossibleRegion() and adds GIL guards around Python filter callbacks.

• ITK pipeline updates let other Python threads run.
• Python filter callbacks run with the GIL held.

Diagram

sequenceDiagram
  participant P as Python thread
  participant W as Update wrapper
  participant F as Native pipeline
  participant C as Python callback
  P->>W: Update()
  W->>W: Release GIL when enabled
  W->>F: Run pipeline
  F->>C: Acquire GIL and call Python
  C-->>F: Return and release acquired GIL
  F-->>W: Finish update
  W->>W: Reacquire GIL
  W-->>P: Return or raise

Loading

_{Reviews (1) · Last reviewed commit: "BUG: Release the Python GIL during pipel..." · Reviewed by Greptile}

[sdiebolt]

The Linux failure looks separate from this fix: filtering succeeds, then the median test’s type-hint check raises NameError: name 'xr' is not defined. The unchanged parent-commit type alias reproduces it when xarray is available.

All three GIL-related tests passed in that job. The float-wrapping review finding is fixed in 2d8272e; those tests also pass locally.

[hjmjohnson]

@sdiebolt Thanks, this is a careful fix, and I reproduced it locally (results below). The setswitchinterval probe is a real improvement: at the merge base the old overlap test passes while your new test fails with Update did not release the GIL.

Requested before merge:

1. Squash 2d8272e742c and d0a09029620 into 0f6c5a10a70. Both fix something the first commit introduced: it switches PyImageFilterTest.py to itk.F, and releases the GIL in Update() while PyCommand still calls PyCallable_Check without it. One commit keeps every revision bisectable.

2. Two comments still describe the wider release (on lines this PR doesn't touch, so noted here):

   • Wrapping/Generators/Python/CMakeLists.txt:339: "release the GIL during ITK operations"
   • Wrapping/Generators/Python/Tests/test_gil_release_safety.py:27: "SWIG wraps C++ calls in SaveThread/RestoreThread"

   Only Update and UpdateLargestPossibleRegion release it now.

3. The GIL tests hard-code itk.F (inline). Either guard them with ITK_WRAP_float in CMake or select a mandatory wrapped type (ITK.F can be disabled) i.e. do the test with uchar.

4. The unset-callable PyCommand block is an error-path test, not a regression test for the GIL fix (inline).

The END_BLOCK and duplicated-callback notes inline are optional cleanup.

Local verification (macOS arm64, CPython 3.13.9, NumPy 2.3.4, SWIG 4.4.1, abi3)

Minimal wrapped Release build (ITKSmoothing, ITKBridgeNumPy, ITK_PYTHON_RELEASE_GIL=ON), running this PR's three test files against each revision:

Revision	test_gil_release.py	test_gil_release_safety.py	PyImageFilterTest.py
merge base 5e1bd43	FAIL: Update did not release the GIL (the old overlap test passes here)	16/16	pass
2d8272e742c (before the PyCommand fix)	pass	16/16	pass
d0a09029620 (head)	pass, 20/20 repeats on a loaded host	16/16	pass

Not run: a build with ITK_WRAP_float=OFF, and a build with ITK_PYTHON_RELEASE_GIL=OFF.

Checked against the code

• igenerator.py:1565-1570 drops threads="1", keeps the global %feature("nothreadallow"), and adds %threadallow Update; and %threadallow UpdateLargestPossibleRegion;. That answers #7000's question about nothreadallow interacting with -threads.
• ITK_PYTHON_RELEASE_GIL=OFF now really turns the release off.
• C++ exceptions retake the GIL before SWIG converts them to Python errors: SWIG's thread-allow object lives inside $action, within the try in pyBase.i.
• itk.imread, itk.imwrite, and the functional filter calls all go through the patched methods.

Not requested here: UpdateOutputInformation, ImageFileWriter::Write(), and optimizer StartOptimization() still hold the GIL, which matches #7000 and the migration guide. Wider coverage belongs with the thread-safety audit in #6757.

[hjmjohnson]

The SWIG_PYTHON_THREAD_END_BLOCK lines here (and at 206, 237, 272) are redundant. SWIG_PYTHON_THREAD_BEGIN_BLOCK declares a SWIG_Python_Thread_Block object whose destructor calls end(), and END_BLOCK only calls end() early. Each one is the last statement before the closing brace, so the GIL is released at the same point either way. The throw paths skip END_BLOCK already and rely on the destructor, so dropping all four changes nothing.

These four methods now repeat the same "call, decref, PyErr_Print, throw" sequence. A private helper would remove the duplication and give each caller a single GIL scope:

template <class TInputImage, class TOutputImage>
void
PyImageFilter<TInputImage, TOutputImage>::CallPython(PyObject * callable, PyObject * args) const
{
  PyObject * result = PyObject_Call(callable, args, nullptr);
  SWIG_Py_DECREF(args);
  if (result == nullptr)
  {
    PyErr_Print();
    itkExceptionStringMacro("There was an error executing the CommandCallable.");
  }
  SWIG_Py_DECREF(result);
}

template <class TInputImage, class TOutputImage>
void
PyImageFilter<TInputImage, TOutputImage>::GenerateOutputInformation()
{
  Superclass::GenerateOutputInformation();

  SWIG_PYTHON_THREAD_BEGIN_BLOCK;
  if (PyCallable_Check(this->m_GenerateOutputInformationCallable))
  {
    this->CallPython(this->m_GenerateOutputInformationCallable, PyTuple_Pack(1, this->m_Self));
  }
}

In EnlargeOutputRequestedRegion, decref pyDataObject right after PyTuple_Pack(2, ...) (the tuple holds its own reference), so the throw path doesn't leak it.

[sdiebolt]

Indeed, removed all four!

[hjmjohnson]

This is a good error-path check, but it can't detect the defect d0a09029620 fixes. With no callable set, m_Object is NULL, and PyCallable_Check(NULL) returns 0 without touching interpreter state, so the block passes whether or not the GIL is held. I confirmed this locally: at 2d8272e742c (GIL released, before this fix) the safety script passes 16/16, including this block.

I don't think a release-build test can catch a missing GIL before PyCallable_Check; the fix is clearest by reading the code. Could the commit message and PR body describe this block as an error-path test rather than a regression test for the GIL fix?

[sdiebolt]

Indeed, I hadn't tested that properly. Fixed the PR message.

[hjmjohnson]

Nothing in these GIL tests needs float: they run a median filter on a constant image and check an exception for a missing input. Any wrapped scalar pixel type works. make_image() (line 101) and test_gil_release.py:8 also hard-code itk.F, so builds with ITK_WRAP_float=OFF can't run them, even though PyImageFilterTest.py was fixed for the same reason in 2d8272e742c.

Either fix is fine:

• Register the GIL tests only when float is wrapped, as PythonGeometryProtocolTest already does (Tests/CMakeLists.txt:190, if(ITK_WRAP_float AND WRAP_3)). This is the smallest change.
• Reuse the pixel-type selection from PyImageFilterTest.py in both GIL test files.

[sdiebolt]

Thanks for catching this! Both tests are now guarded by ITK_WRAP_float. All four CMake registration cases pass.

[hjmjohnson]

If you take the CMake route for the float dependency, one block can guard both GIL tests. The safety test then needs ITK_WRAP_float only, since it is meant to pass with ITK_PYTHON_RELEASE_GIL on or off:

if(ITK_WRAP_float)
  if(ITK_PYTHON_RELEASE_GIL)
    itk_python_add_test(NAME PythonGILReleaseTest ...)
  endif()
  itk_python_add_test(NAME PythonGILReleaseSafetyTest ...)
endif()

[sdiebolt]

Done, using your suggested nesting. The safety test still runs with GIL release on or off, as long as float wrapping is enabled.

[sdiebolt]

Thanks for the careful review and local reproduction! All required changes are included in the single commit 1f0ae06.

[dzenanz]

Thanks @sdiebolt, and thanks @hjmjohnson for the thorough first pass.
All of Hans's requested items are addressed in 1f0ae063adf (single
commit, comments updated, float guard, error-path wording). LGTM after
a rebase; local Windows results, including ITK_PYTHON_RELEASE_GIL=OFF,
are below.

Requested:

1. Rebase on main. The ITK.Linux.Python / ITK.macOS.Python
   failures are PythonMedianImageFilterTest raising
   NameError: name 'xr' is not defined, already fixed on main by
   c4a04cb ("Resolve optional array types in MedianImageFilterTest
   type hints"). The GIL tests pass in both jobs.

Optional:

2. EnlargeOutputRequestedRegion still leaks pyDataObject when the
   callback fails (pre-existing). Decref it right after PyTuple_Pack.

Local verification (Windows 11, MSVC 19.44, Python 3.13, SWIG/CastXML from pixi)

Minimal wrapped Release build (ITKSmoothing, ITKBridgeNumPy) of 1f0ae063adf:

Test	ITK_PYTHON_RELEASE_GIL=ON	ITK_PYTHON_RELEASE_GIL=OFF
PythonGILReleaseTest	pass, 10/10 repeats	not registered (as intended)
PythonGILReleaseSafetyTest	pass (signal test skipped: no SIGUSR1 on Windows)	pass
PyImageFilterTest	pass	pass
itkPyCommandPythonTest	pass	pass
PythonMedianImageFilterTest	pass	pass

Running test_gil_release.py by hand against the OFF build fails with
Update did not release the GIL, so OFF really keeps the GIL and the
test tells the two modes apart. The OFF build compiles cleanly; in that
mode SWIG_PYTHON_THREAD_BEGIN_BLOCK expands to nothing.

This covers the two configurations not yet tested: Windows/MSVC, and
ITK_PYTHON_RELEASE_GIL=OFF.

[dzenanz]

Manual review

[sdiebolt]

Rebased onto main in 242edca, including the type-hint fix. Thanks for testing both GIL modes on Windows!

[dzenanz]

Looks good on a glance now.

[sdiebolt]

Looks good on a glance now.

Thank you for the review and guidance! 😊

I've tested this locally in a multi-frame registration use-case (registering each frame of a series to the first) and it speeds up parallel calls of ITK python by a big margin (when using threads)!

[hjmjohnson]

Rebased on main (plain rebase, patch unchanged) to pick up #7028, which fixes the itkIndent.h include-cycle error behind the ARMBUILD failure. Pull with git pull --rebase before pushing again.

[dzenanz]

@hjmjohnson Have you taken another look? Should we merge this?

[hjmjohnson]

May want to consider backporting?? Good as is.