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
Stepan Sindelar (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 1 commit
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
6 changes: 4 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 @@ -12,6 +12,7 @@
# WARNING: debugpy and submodules must not be imported on top level in this module,
# and should be imported locally inside main() instead.

_CAN_DAEMONIZE = os.name == "posix" and hasattr(os, "fork")

def main():
args = _parse_argv(sys.argv)
Expand All @@ -31,12 +32,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 _CAN_DAEMONIZE and os.fork() != 0:

@rchiodo Rich Chiodo (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.

The extra variable outside of the if statement seems like overkill. Why not just have this:

if hasattr(os, "fork") and os.fork() != 0:

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 Heejae Chang (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.

@heejaechang Heejae Chang (heejaechang) Jul 2, 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.

process_group=0 requires the subprocess.Popen(process_group=...) parameter, which only exists in Python 3.11+. GraalPy has shipped 3.8/3.10-compatible releases; on such a build this raises TypeError, and spawn() re-raises it as MessageHandlingError, so every launch fails — converting the fork fix into a spawn crash. Guard it (e.g. check "process_group" in inspect.signature(subprocess.Popen).parameters) or wrap in try/except with a graceful fallback. At minimum confirm the minimum supported GraalPy exposes process_group.

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

@rchiodo Rich Chiodo (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 Stepan Sindelar (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 Heejae Chang (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.

@rchiodo Rich Chiodo (rchiodo) Jul 16, 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.

Released GraalPy 23.1.3 and 24.0.2 reject Popen(process_group=...) with TypeError, preventing debuggee launch. Capability-check this argument and preserve the existing fallback and termination behavior for unsupported releases.

kwargs.update(process_group=0)

@StellaHuang95 Stella Huang (StellaHuang95) Aug 6, 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.

Warning · Non-blocking recommendation

process_group=0 replaces only the new-process-group portion of the CPython preexec_fn; it does not perform the foreground-terminal handoff. Please clarify that terminal foreground/job-control behavior is unsupported on GraalPy so this is not mistaken for full parity.

@StellaHuang95 Stella Huang (StellaHuang95) Aug 6, 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.

Warning · Non-blocking recommendation

process_group requires Python 3.11+. A GraalPy build targeting an older Python version will fail every launch with an unsupported Popen keyword. Please guard this path or document the required GraalPy/Python version.


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

@heejaechang Heejae Chang (heejaechang) Jul 2, 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.

The two if sys.platform != "win32" and sys.implementation.name == / != "graalpy" blocks are mutually-exclusive arms of one decision, but written as independent top-level ifs they hide that relationship and re-evaluate sys.platform != "win32" twice. Consider collapsing into a single if sys.platform != "win32": with an inner if/else on the implementation, so the either/or intent is visible and a future edit to one arm prompts an edit to the other.

def preexec_fn():
try:
# Start the debuggee in a new process group, so that the launcher can
Expand Down
9 changes: 6 additions & 3 deletions src/debugpy/server/api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from debugpy.common.util import hide_debugpy_internals

_tls = threading.local()
_CAN_DAEMONIZE = os.name == "posix" and hasattr(os, "fork")

# TODO: "gevent", if possible.
_config = {
Expand Down Expand Up @@ -218,11 +219,13 @@ def listen(address, settrace_kwargs, in_process_debug_adapter=False):
creationflags=creationflags,
env=python_env,
)
if os.name == "posix":
if _CAN_DAEMONIZE:

@rchiodo Rich Chiodo (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.

Same thing here. It would be

if os.name == "posix" and hasattr(os, "fork")

Makes for fewer code changes.

# 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.

@StellaHuang95 Stella Huang (StellaHuang95) Aug 6, 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.

Warning · Non-blocking recommendation

This wait guard must remain synchronized with the adapter daemonization behavior in adapter/__main__.py: waiting is correct only when the adapter forks and its spawned parent exits. Please add a cross-reference so future changes do not introduce a hang or unreaped child.

@StellaHuang95 Stella Huang (StellaHuang95) Aug 13, 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.

Warning · Non-blocking recommendation

Please add regression coverage for the forkless POSIX path, verifying that listen() reaches endpoint acceptance without blocking when the adapter cannot daemonize via fork().

_adapter_process.wait()
else:
elif os.name != "posix":
# Suppress misleading warning about child process still being alive when
# this process exits (https://bugs.python.org/issue38890).
_adapter_process.returncode = 0
Expand Down