Skip to content

gh-140287: Handle PYTHONSTARTUP script exceptions in the asyncio REPL #140288

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

Jump to bottom
Draft
wants to merge 29 commits into
base: main
Choose a base branch
from
Draft

gh-140287: Handle PYTHONSTARTUP script exceptions in the asyncio REPL #140288

wants to merge 29 commits into from

Conversation

@johnslavik
Copy link
Contributor

@johnslavik johnslavik commented Oct 18, 2025 •
edited by bedevere-app bot
Loading

This is generally a work in progress; tests are needed.
The asyncio patch is simple and ready.

@bedevere-app bedevere-app bot mentioned this pull request Oct 18, 2025
Open
@johnslavik
Copy link
Contributor Author

johnslavik commented Oct 23, 2025

Please merge #140298 first.

@johnslavik johnslavik marked this pull request as ready for review October 23, 2025 14:57
@johnslavik johnslavik marked this pull request as draft October 23, 2025 15:00
johnslavik
johnslavik commented Oct 27, 2025
View reviewed changes
Lib/test/test_repl.py
# Isolated mode implies -E, -P and -s, purifies sys.path and ignores PYTHON*
# variables.
if isolated:
cmd_line.append('-I')
Copy link
Contributor Author

@johnslavik johnslavik Oct 27, 2025

Choose a reason for hiding this comment

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

To reuse this routine and spawn_asyncio_repl below, we need to have a knob to not pass -I since that ignores PYTHONSTARTUP completely.

The asyncio REPL currently doesn't comply to that, which is a separate issue I'll report having learnt this.

Copy link
Contributor Author

@johnslavik johnslavik Oct 27, 2025

Choose a reason for hiding this comment

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

Tracking the -I edge case in #140648.

Lib/test/test_repl.py
Comment on lines -295 to -311
def test_asyncio_repl_reaches_python_startup_script(self):
with os_helper.temp_dir() as tmpdir:
script = os.path.join(tmpdir, "pythonstartup.py")
with open(script, "w") as f:
f.write("print('pythonstartup done!')" + os.linesep)
f.write("exit(0)" + os.linesep)

env = os.environ.copy()
env["PYTHON_HISTORY"] = os.path.join(tmpdir, ".asyncio_history")
env["PYTHONSTARTUP"] = script
subprocess.check_call(
[sys.executable, "-m", "asyncio"],
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
env=env,
timeout=SHORT_TIMEOUT,
)
Copy link
Contributor Author

@johnslavik johnslavik Oct 27, 2025

Choose a reason for hiding this comment

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

Covered by TestPythonStartup now, so not needed here.

@johnslavik johnslavik marked this pull request as ready for review October 27, 2025 01:15
@johnslavik johnslavik mentioned this pull request Oct 27, 2025
Open
@johnslavik johnslavik marked this pull request as draft October 30, 2025 07:07
johnslavik
johnslavik commented Oct 30, 2025
View reviewed changes
johnslavik
johnslavik commented Oct 30, 2025
View reviewed changes
Copy link
Contributor Author

@johnslavik johnslavik left a comment

Choose a reason for hiding this comment

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

I'm also thinking that ideally we would test stdout and stderr separately.

@johnslavik
Copy link
Contributor Author

johnslavik commented Nov 8, 2025 •
edited
Loading

This is likely it. I can't think of anything simpler but equally complete at the same time.
This suite seems intentionally straightforward, as are the new tests proposed here.
I'll give this a few more days in a daemon thread of my mind.

We can abstract as needed in the future. We could use some regexes.
However, I don't think this is so much needed now, and the environments differ enough to justify some repetition.

johnslavik
johnslavik commented Nov 8, 2025
View reviewed changes
Lib/test/test_repl.py
self.assertEqual(traceback_lines, expected_lines)


@support.force_not_colorized_test_class
Copy link
Contributor Author

@johnslavik johnslavik Nov 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not sure this is needed, but I think it's OK to keep it as is.

@johnslavik
Copy link
Contributor Author

johnslavik commented Nov 8, 2025 •
edited
Loading

It seems that the asyncio REPL on Windows leaks the event loop:

❯ PYTHONSTARTUP='' python -m asyncio
Running Debug|x64 interpreter...
asyncio REPL 3.15.0a1+ (heads/asyncio-repl-handle-python-startup-dirty:ce03ccef689, Nov  8 2025, 02:06:1) [MSC v.1944 64 bit (AMD64)] on win32
Use "await" directly instead of "asyncio.run()".
Type "help", "copyright", "credits" or "license" for more information.
>>> import asyncio
>>> exit
exiting asyncio REPL...
C:\Users\zenon\Python\cpython\Lib\asyncio\base_events.py:758: ResourceWarning: unclosed event loop <ProactorEventLoop running=False closed=False debug=False>

Definitely a bug, we'll track it elsewhere soon.

EDIT: Tracking in #141262

@johnslavik johnslavik force-pushed the asyncio-repl-handle-python-startup branch from b85a31b to 0a50a50 Compare November 8, 2025 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@1st1 1st1 Awaiting requested review from 1st1 1st1 is a code owner

@asvetlov asvetlov Awaiting requested review from asvetlov asvetlov is a code owner

@kumaraditya303 kumaraditya303 Awaiting requested review from kumaraditya303 kumaraditya303 is a code owner

@willingc willingc Awaiting requested review from willingc willingc is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@johnslavik