Skip to content

Fix GraalPy compatibility #2040

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.

Sign up for GitHub

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

Open
steve-s wants to merge 3 commits into
base: main
Choose a base branch
from
+12 −6
Open

Fix GraalPy compatibility #2040

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9eb0cde
Fix GraalPy compatibility
steve-s Jun 17, 2026
7317cc7
Handle non-demonized GraalPy children correctly in api.py
steve-s Jun 19, 2026
81d199b
Inline _CAN_DEMONIZE
steve-s Jun 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/debugpy/adapter/__main__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,13 @@ def main():
if os.name == "posix":
# On POSIX, we need to leave the process group and its session, and then
# daemonize properly by double-forking (first fork already happened when
# this process was spawned).
# this process was spawned). Some POSIX runtimes, such as GraalPy, do not
# implement fork(), so in that case we settle for a single detached child.
# NOTE: if process is already the session leader, then
# setsid would fail with `operation not permitted`
if os.getsid(os.getpid()) != os.getpid():
os.setsid()
if os.fork() != 0:
if hasattr(os, "fork") and os.fork() != 0:

@rchiodo rchiodo Jun 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📍 src/debugpy/adapter/main.py:38
This PR adds a hasattr(os, "fork") probe but leaves the sibling syscalls os.getsid/os.setsid (a few lines up in the same if os.name == "posix": block) gated only on os.name == "posix". A forkless-POSIX runtime that also lacks setsid would raise AttributeError/OSError here and crash before reaching the new fork guard — defeating the PR's goal. Please confirm GraalPy exposes setsid/getsid, or guard with hasattr(os, "setsid") / wrap in try/except (AttributeError, OSError).

@steve-s steve-s Jun 23, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, GraalPy exposes those functions

sys.exit(0)

for stdio in sys.stdin, sys.stdout, sys.stderr:
Expand Down
7 changes: 5 additions & 2 deletions src/debugpy/launcher/debuggee.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,12 @@ def spawn(process_name, cmdline, env, redirect_output):
else:
kwargs = {}

if sys.platform != "win32" and sys.implementation.name != 'graalpy':
# GraalPy does not support running code between fork and exec
# GraalPy does not support running code between fork and exec, but supports the
# process_group argument for Popen

@heejaechang heejaechang Jun 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📍 src/debugpy/launcher/debuggee.py:62
The process_group argument to subprocess.Popen was only added in Python 3.11. On GraalPy builds targeting CPython ≤ 3.10 (e.g. GraalPy 23.x / 24.0), kwargs.update(process_group=0) causes Popen to raise TypeError, which is caught at the spawn site and re-raised as MessageHandlingError — so every debuggee launch fails. This is a regression: pre-PR, GraalPy hit neither branch and spawned fine (only kill() was unreliable). Guard the kwarg, e.g. if sys.version_info >= (3, 11): kwargs.update(process_group=0) (or check inspect.signature(subprocess.Popen).parameters), with a documented fallback for older runtimes.

if sys.platform != "win32" and sys.implementation.name == "graalpy":

@rchiodo rchiodo Jun 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📍 src/debugpy/launcher/debuggee.py:62
The process_group keyword for subprocess.Popen was only added in Python 3.11. A GraalPy build targeting a pre-3.11 stdlib (e.g. GraalPy 23.x → 3.10) will raise TypeError: got an unexpected keyword argument 'process_group', which is caught and surfaced as "Couldn't spawn debuggee" — making launch fail entirely rather than degrading. Guard it, e.g. if ... graalpy and sys.version_info >= (3, 11): kwargs.update(process_group=0), or wrap in try/except TypeError.

@steve-s steve-s Jun 19, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would also require adding another branch to kill so that we use os.kill and not os.killpg for older GraalPy versions. Moreover, support for current and older GraalPy releases depends on fabioz/PyDev.Debugger#325, and unless that's merged and propagated to debugpy I think it's better to keep the changes in this file simple - it will work on GraalPy master (and some future release) for which fixes in fabioz/PyDev.Debugger#325 aren't necessary.

@heejaechang heejaechang Jun 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📍 src/debugpy/launcher/debuggee.py:60
process_group=0 reproduces only the os.setpgrp() half of the original preexec_fn; it does not replicate the os.tcsetpgrp() call that makes the debuggee the foreground process group of the controlling terminal. Interactive GraalPy debuggees that read stdin or use job control may behave differently (e.g. SIGTTIN/SIGTTOU on terminal access). This is likely acceptable since the block is best-effort, but add a one-line comment noting the lost foreground-terminal behavior is a known, intentional limitation.

kwargs.update(process_group=0)

if sys.platform != "win32" and sys.implementation.name != 'graalpy':
def preexec_fn():
try:
# Start the debuggee in a new process group, so that the launcher can
Expand Down
6 changes: 4 additions & 2 deletions src/debugpy/server/api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,9 +218,11 @@ def listen(address, settrace_kwargs, in_process_debug_adapter=False):
creationflags=creationflags,
env=python_env,
)
if os.name == "posix":
if os.name == "posix" and hasattr(os, "fork"):
# It's going to fork again to daemonize, so we need to wait on it to
# clean it up properly.
# clean it up properly. If we did not fork, we cannot take this path
# because it cannot perform that extra fork; waiting there would just
# preserve the broken assumption that a daemonized grandchild exists.
_adapter_process.wait()
else:
# Suppress misleading warning about child process still being alive when
Expand Down
Loading