Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork33.7k
gh-140287: HandlePYTHONSTARTUP 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.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
gh-140287: HandlePYTHONSTARTUP script exceptions in the asyncio REPL#140288
Conversation
johnslavik commentedOct 23, 2025
Please merge#140298 first. |
| # Isolated mode implies -E, -P and -s, purifies sys.path and ignores PYTHON* | ||
| # variables. | ||
| ifisolated: | ||
| cmd_line.append('-I') |
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.
To reuse this routine andspawn_asyncio_repl below, we need to have a knob to not pass-I since that ignoresPYTHONSTARTUP completely.
The asyncio REPL currently doesn't comply to that, which is a separate issue I'll report having learnt this.
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.
Tracking the-I edge case in#140648.
| deftest_asyncio_repl_reaches_python_startup_script(self): | ||
| withos_helper.temp_dir()astmpdir: | ||
| script=os.path.join(tmpdir,"pythonstartup.py") | ||
| withopen(script,"w")asf: | ||
| 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, | ||
| ) |
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.
Covered byTestPythonStartup now, so not needed here.
johnslavik commentedNov 8, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
It seems that the asyncio REPL on Windows leaks the event loop: Definitely a bug, we'll track it elsewhere soon. EDIT: Tracking in#141262 |
b85a31b to0a50a50Comparejohnslavik commentedDec 5, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
This is ready, but Windows doesn't pass because ofGH-141262. |
johnslavik commentedDec 16, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
It seems that these unclosed loop warnings regressed (stopped appearing) in the recent releases -- this is bad, but we can fix them separately and we can also fix the asyncio REPL in this regard by actually closing the loop (in a separate PR). In the meantime, updating this branch should make the CI green. |
johnslavik commentedDec 16, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Looks like I was confused about the regression -- that's good news! This will have to wait until the loop is properly closed in the asyncio REPL. |
johnslavik commentedDec 16, 2025
johnslavik commentedDec 16, 2025
It does! Cool! |
This reverts commit875fd2a.
| exceptSystemExit: | ||
| raise |
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.
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.
This is also why I didn't add a test for this.
Uh oh!
There was an error while loading.Please reload this page.
This is generally a work in progress; tests are needed.
The asyncio patch is simple and ready.
PYTHONSTARTUPexceptions #140287