PythonCall.jl icon indicating copy to clipboard operation
PythonCall.jl copied to clipboard

Segfault in `_jl_call_nogil` with jupyter extension or passing pytypes

Open PhilReinhold opened this issue 1 year ago • 4 comments

Affects: JuliaCall

Describe the bug

If I run this in a jupyter notebook

from juliacall import Main as jl
jl.println._jl_call_nogil("foo") # segfaus

I get a segfault.

It goes away if I disable the extension:

import os
os.environ["PYTHON_JULIACALL_AUTOLOAD_IPYTHON_EXTENSION"] = "no"
from juliacall import Main as jl
jl.println._jl_call_nogil("foo") # works

Also I get segfaults if I pass a python type that I would expect to be converted, e.g.

import os
os.environ["PYTHON_JULIACALL_AUTOLOAD_IPYTHON_EXTENSION"] = "no"
from juliacall import Main as jl
jl.println._jl_call_nogil({})  # segfaults

This goes away if I manually convert

import os
os.environ["PYTHON_JULIACALL_AUTOLOAD_IPYTHON_EXTENSION"] = "no"
from juliacall import Main as jl
jl.println._jl_call_nogil(jl.pyconvert(jl.Dict, {}))  # works

Your system

  • macos 13.6.8, M2
  • Julia 1.10.4, with PythonCall 0.9.22

versioninfo()

Julia Version 1.10.4
Commit 48d4fd48430 (2024-06-04 10:41 UTC)
Build Info:
  Official https://julialang.org/ release
Platform Info:
  OS: macOS (arm64-apple-darwin22.4.0)
  CPU: 12 × Apple M2 Pro
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-15.0.7 (ORCJIT, apple-m1)
Threads: 1 default, 0 interactive, 1 GC (on 8 virtual cores)
Environment:
  JULIA_PKG_USE_CLI_GIT = true
  JULIA_PYTHONCALL_EXE = /Users/pcrein/.pyenv/versions/3.11.4/envs/test-juliacall/bin/python3.11

Pkg.status()

Status `~/.pyenv/versions/3.11.4/envs/test-juliacall/julia_env/Project.toml`
  [6099a3de] PythonCall v0.9.22` 

CondaPkg.status() I don't seem to have condapkg

PhilReinhold avatar Aug 20 '24 14:08 PhilReinhold

is it https://juliapy.github.io/PythonCall.jl/dev/juliacall/#py-multi-threading-signal-handling?

Can you include the full segfault text (if it includes a stacktrace etc)?

ericphanson avatar Aug 21 '24 21:08 ericphanson

is it https://juliapy.github.io/PythonCall.jl/dev/juliacall/#py-multi-threading-signal-handling?

No, this happens when Julia is handling its signals. It seems likely it is manipulation of Python while the GIL is released.

In the first case, this is probably the IPython extension's handling of stdio. That extension should probably reacquire the GIL for the work it does.

In the second case, this is because some python objects are passed wrapped by default, e.g. dict -> PyDict, so that access to those objects within _nogil is a bug. This might be expected, but should at least be clearly documented. I wonder if the PyX wrappers could also acquire the GIL as needed?

amilsted avatar Aug 22 '24 10:08 amilsted

The two issues are sort of the same. As you realised, the second issue is because python dicts are converted to Julia PyDicts, which require the GIL to be held to manipulate. The first issue is because the IPython extension forwards Julia stdout to Python stdout, which is wrapped as a Julia PyIO, which again requires the GIL to be held.

I'm quite reluctant to make PythonCall functions/types automatically lock the GIL because there's a runtime cost, but also it would require adding GIL.@lock in hundreds of places around the code. I'd rather it was up to the user to apply GIL.@lock as necessary at a higher level around larger blocks of code than in each small PythonCall function.

That said, I agree that it is surprising that println("some string") causes an interaction with Python. I think for this specific case, we should make the task which forwards Julia stdout to Python stdout lock the GIL each time there is something to forward.

And certainly the documentation for _jl_call_nogil could document this pitfall.

cjdoris avatar Aug 22 '24 20:08 cjdoris

That seems reasonable to me! I took a look at the IPython extension but I couldn't see quite where to put the locking.

PS: It's great to see support for threading! Appreciate the work that went into this.

amilsted avatar Aug 23 '24 13:08 amilsted