GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Fix UnboundLocalError when system() is interrupted during spawn 🤖🤖 by anyingiit · Pull Request #15448 · ipython/ipython

Conversation

anyingiit commented Oct 22, 2026

Copy link

Copy Markdown

Requested by winshyon

Description

🤖🤖 This PR was prepared with an AI coding agent (see disclosure below).

ProcessHandler.system() in IPython/utils/_process_posix.py binds child inside the try: block (child = pexpect.spawn(...)). If the KeyboardInterrupt arrives while pexpect is still spawning the child (as seen on the conda-forge aarch64/ppc64le builds in test_system_interrupt), child is never assigned and the except KeyboardInterrupt: handler fails with:

UnboundLocalError: cannot access local variable 'child' where it is not associated with a value

Fix:

initialise child = None before the try:;
in the except KeyboardInterrupt: handler, if there is no child yet there is nothing to forward ^C to or terminate, so print ^C (same as getoutput() does) and return -signal.SIGINT. That keeps the existing convention of system() (a negative number for "terminated by signal N"), so callers such as %sx/!cmd see a normal interrupted status instead of a crash.

The normal path and the "interrupted after spawn" path are unchanged.

A regression test, tests/test_process.py::test_system_interrupt_during_spawn, monkeypatches pexpect.spawn to raise KeyboardInterrupt, so the race is reproduced deterministically. It fails with the UnboundLocalError above without the fix.

Motivation / disclosure: I had some spare AI-assistant quota (Claude Code) and am using it to try to help projects with open good-first-issues. The change was prepared with Claude Code and reviewed by me. If it doesn't fit, isn't up to your bar, or you'd simply rather not take it — please feel free to close it, no hard feelings at all 🙂

Related issue

Closes #12164

Checklist

Regression test added (tests/test_process.py::test_system_interrupt_during_spawn)
CHANGELOG.md / whatsnew entry — n/a (bug fix; docs/source/whatsnew/pr/ is for new features and incompatible changes only)
Documentation is updated — n/a (no user-facing behaviour change besides not crashing)

🤖 Generated with Claude Code

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Fix UnboundLocalError when system() is interrupted during spawn 🤖🤖

e1661fd

If ^C arrives while pexpect is still spawning the child, `child` is never
bound and the KeyboardInterrupt handler in ProcessHandler.system() crashed
with UnboundLocalError. Initialise `child` to None and, when there is no
child yet, print ^C and return -SIGINT like an interrupted command.

Closes ipython#12164

chatgpt-codex-connector Bot commented Oct 22, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-22T18:53:38.826180Z	`e1661fd`	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

chatgpt-codex-connector Bot reviewed

Oct 22, 2026

View reviewed changes

chatgpt-codex-connector Bot left a comment

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e1661fd353

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

IPython/utils/_process_posix.py

		# Interrupted while spawning: there is no child process to
		# forward ^C to, so report it as killed by SIGINT.
		print("^C", file=sys.stderr, end="")
		return -signal.SIGINT

chatgpt-codex-connector Bot Oct 22, 2026

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Retain control of a child started during spawn

When Ctrl-C lands after pexpect's internal fork has created the shell but before pexpect.spawn() returns, child is still None; this new return skips termination/reaping, records -SIGINT, and lets the rest of the cell proceed while the child can remain running. Delay or guard the interrupt through spawn (or otherwise retain the child handle), and make the regression test interrupt after process creation rather than having the stub raise before one.

AGENTS.md reference: AGENTS.md:L30-L30

Useful? React with 👍 / 👎.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

tests/test_process.py Outdated

		monkeypatch.delattr(pexpect, "spawnb", raising=False)
		monkeypatch.setattr(pexpect, "spawn", interrupted_spawn)

		status = ProcessHandler().system("true")

chatgpt-codex-connector Bot Oct 22, 2026

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Make the regression fail at the final assertion

With the production hunk removed, interrupted_spawn enters the old handler, which raises UnboundLocalError while dereferencing child before the final assertion is reached; the regression therefore errors rather than failing on an assertion. Catch and record that old exception and let the status assertion encode the failure, as required for bug-fix tests.

AGENTS.md reference: AGENTS.md:L32-L32

Useful? React with 👍 / 👎.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

tests/test_process.py



		@dec.skip_win32
		def test_system_interrupt_during_spawn(monkeypatch):

chatgpt-codex-connector Bot Oct 22, 2026

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Provide a failing-test revision before the fixed revision

This test is introduced in the same commit as the implementation, so neither the parent nor this revision provides the required checkout where the new test fails before the fix. Separate the regression and implementation in the mandated order; because this history is already pushed, preserve it and use follow-up commits rather than rebasing it.

AGENTS.md reference: AGENTS.md:L33-L33

Useful? React with 👍 / 👎.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

anyingiit added 2 commits

October 23, 2026 06:40


          TST: fail on the assertion, and cover ^C after the child is forked

e35f2f4

The interrupt-during-spawn test now records the old UnboundLocalError so it
fails on its assertion without the fix. Add a test where ^C arrives after
pexpect has created the child but before spawn() returns; the child must not
be left running. It fails on this commit.


          Hold back ^C while pexpect spawns the child in system()

9bddbb8

A ^C that arrived after pexpect had forked the shell but before spawn()
returned left system() without a handle on the child, so it kept running.
Defer SIGINT until spawn() returns and raise it then, so the usual handler
forwards ^C to the child and terminates it. Also cope with child.before
still being None in that handler.

anyingiit commented Oct 22, 2026

Copy link

Copy Markdown

Author

Thanks for the review notes. I pushed two follow-up commits (no rebase, per AGENTS.md):

Tests first: the interrupt-during-spawn test now records the old UnboundLocalError, so without the fix it fails on its assertion instead of erroring. A second test sends SIGINT after pexpect has created the child but before spawn() returns, and checks the child isn't left running. It fails on that commit.
The fix: system() holds back SIGINT while pexpect spawns, and raises KeyboardInterrupt again right after spawn() returns, so the existing handler forwards ^C to the child and terminates it. Outside the main thread it doesn't touch the handler, since Python only raises ^C in the main thread anyway. The handler also copes with child.before still being None at that point.

tests/test_process.py passes locally (22 passed, 5 skipped), and ruff is clean.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.


          Apply ruff formatting to the spawn block

a5b9e79

This branch has not been deployed

No deployments

Reviewers

chatgpt-codex-connector[bot]

chatgpt-codex-connector[bot] left review comments

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Unhandled reference to child in utils._process_posix interrupt handler

Uh oh!

There was an error while loading. Please reload this page.

1 participant

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

anyingiit commented Oct 22, 2026

Description

Related issue

Checklist

Uh oh!

chatgpt-codex-connector Bot commented Oct 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Oct 22, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Oct 22, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Oct 22, 2026

Choose a reason for hiding this comment

Uh oh!

anyingiit commented Oct 22, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

chatgpt-codex-connector Bot commented Oct 22, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.