Skip to content

gh-121621: clear running loop early in asyncio #128004

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Dec 18, 2024

Conversation

kumaraditya303
Copy link
Contributor

Clearing the loop at late in finalization can run arbitrarily run finalizers at shutdown which is bad so we clear the ref to it when clearing the module to avoid running finalizers late.

@colesbury
Copy link
Contributor

PyThreadState_Clear() seems like the right place to me. We can already execute arbitrary code from there (unlike PyThreadState_Delete()).

For example, if you have a threading.local containing an object with a Python __del__ function, that will get called from PyThreadState_Clear(), at least for non-main threads.

@kumaraditya303
Copy link
Contributor Author

PyThreadState_Clear() seems like the right place to me. We can already execute arbitrary code from there (unlike PyThreadState_Delete()).

The issue is that the event loop has references to lot of objects with finalizers and running those later while interp is finalizing restricts what can be done. For example there are workarounds to only capture tracebacks when interp isn't finalizing in asyncio.

if (is_true && !_Py_IsInterpreterFinalizing(_PyInterpreterState_GET())) {
/* Only try to capture the traceback if the interpreter is not being
finalized. The original motivation to add a `Py_IsFinalizing()`
call was to prevent SIGSEGV when a Future is created in a __del__
method, which is called during the interpreter shutdown and the
traceback module is already unloaded.

@colesbury
Copy link
Contributor

It may be okay to also clear asyncio_running_loop in module_clear, but I don't think it should be removed from PyThreadState_Clear(). Threads may have been created and destroyed before module_clear is invoked, so without the call in PyThreadState_Clear() we will leak memory for non-main threads.

@kumaraditya303
Copy link
Contributor Author

It may be okay to also clear asyncio_running_loop in module_clear, but I don't think it should be removed from PyThreadState_Clear(). Threads may have been created and destroyed before module_clear is invoked, so without the call in PyThreadState_Clear() we will leak memory for non-main threads.

yeah, that sounds good to me

@kumaraditya303 kumaraditya303 enabled auto-merge (squash) December 18, 2024 06:55
@kumaraditya303 kumaraditya303 merged commit 8a433b6 into python:main Dec 18, 2024
38 of 39 checks passed
@kumaraditya303 kumaraditya303 deleted the clear-ts branch December 27, 2024 08:22
srinivasreddy pushed a commit to srinivasreddy/cpython that referenced this pull request Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants