Skip to content

Acquire GIL around every Python API call - #1097

Draft
topolarity wants to merge 3 commits into
JuliaPy:masterfrom
topolarity:ct/with-gil
Draft

Acquire GIL around every Python API call#1097
topolarity wants to merge 3 commits into
JuliaPy:masterfrom
topolarity:ct/with-gil

Conversation

@topolarity

@topolarity topolarity commented Mar 10, 2026

Copy link
Copy Markdown
Contributor

This should resolve #1096 by wrapping every Python API usage in a GIL acquire-release.

If this leads to overhead, we can probably re-export a "manual GIL" version of the API that allows consumers to manage the GIL themselves. In any case, it's worth having proper support for multi-threading.

Fixes #1096. Fixes #1095. Fixes #1090. Fixes #1088.
Fixes #1078. Fixes #1077. Fixes #1072. Fixes #1083.
Fixes timholy/Revise.jl#989.

@topolarity
topolarity force-pushed the ct/with-gil branch 5 times, most recently from b3aea39 to dd3cc6d Compare March 10, 2026 18:49
@topolarity

topolarity commented Mar 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Before this can be considered complete, it needs an equivalent to JuliaMath/FFTW.jl#160 to handle:

  1. Task A holds the GIL lock.
  2. Task A waits for Task B to make "progress".
  3. Task B waits in GC to obtain the GIL lock to run finalizers. (deadlocks)

edit: Done!

@topolarity
topolarity marked this pull request as draft March 10, 2026 19:16
@topolarity
Acquire GIL around every Python API call
12243fd 
Majority of the API is covered by updating the `@pycheck(v/n)` macros.
The rest is covered manually by `@with_GIL`. Care should be taken to
avoid yielding to the Julia scheduler, which could cause task migration
and lead to deadlocks.
@topolarity
topolarity force-pushed the ct/with-gil branch 3 times, most recently from a395a25 to 25038c7 Compare March 11, 2026 21:09
@topolarity
Add deferred "release" queue for PyObject finalizers
19a20ec 
This is required to avoid deadlocking in the GC as it executes
finalizers.
@topolarity
Wrap GIL acquire / release in ReentrantLock
bd8d220 
Required to avoid starving the scheduler or deadlocking against the GC.
@yuyichao

yuyichao commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Is there any known issue with this currently? Are we going to use this approach?

I've rebased this on master at https://github.com/JuliaPy/PyCall.jl/tree/ct/with-gil-rebase with a few fixes/tweaks.

  1. pydecref_ needs to hold a reference to the julia object to avoid potential race with the GC. This is really hard to happen in practice but not impossible.

    As part of it, stops returning the pointer from it since that pointer should never be used again. This PR already changed the signature of this function. The only other reference I've found for this function is in some precompilation statements which shouldn't cause any breakage.

  2. Still make the finalizer clear the pointer in PyObject. I can't think of a case right now that would crash because of this but it would follow

  3. Minor optimization in _drain_release_queues to use one atomic sub rather than two, and to only copy the vector if it's not empty.

One of the few issues I could think of is maybe if there are a lot of object that became garbage they can only be free'd the next time the user calls a PyCall API, which may not happen for a while. For example, the following code can place unbound number of objects in the deferred list,

[PyObject(i) for i in 1:100000];

Maybe the finalizer should detect some cases where the GIL is safe to acquire (e.g. try to acquire without waiting) and clear the queue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment