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.

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

Segfault in _jl_call_nogil with jupyter extension or passing pytypes #542

Open
PhilReinhold opened this issue Aug 20, 2024 · 4 comments
Open
Labels
bug Something isn't working

Comments

@PhilReinhold
Copy link

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 PhilReinhold added the bug Something isn't working label Aug 20, 2024
@ericphanson
Copy link
Contributor

ericphanson commented Aug 21, 2024

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)?

@amilsted
Copy link

amilsted commented Aug 22, 2024

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?

@cjdoris
Copy link
Collaborator

cjdoris commented Aug 22, 2024

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.

@amilsted
Copy link

amilsted commented Aug 23, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants