Skip to content
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

Jump to bottom

asyncio: __del__() keep reference to warnings.warn #11491

Merged
merged 2 commits into from Jan 10, 2019
Merged

asyncio: __del__() keep reference to warnings.warn #11491

merged 2 commits into from Jan 10, 2019

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Jan 10, 2019

The del() methods of asyncio classes now keep a strong reference
to the warnings.warn() to be able to display the ResourceWarning
warning in more cases. Ensure that the function remains available if
instances are destroyed late during Python shutdown (while module
symbols are cleared).

@vstinner
asyncio: __del__() keep reference to warnings.warn
ac3452f 
The __del__() methods of asyncio classes now keep a strong reference
to the warnings.warn() to be able to display the ResourceWarning
warning in more cases. Ensure that the function remains available if
instances are destroyed late during Python shutdown (while module
symbols are cleared).
@vstinner vstinner requested review from methane, 1st1 and asvetlov Jan 10, 2019
@vstinner vstinner requested a review from gpshead as a code owner Jan 10, 2019
@vstinner
Copy link
Member Author

vstinner commented Jan 10, 2019

I'm not sure that this change fix a real bug, but I had to make a similar change 2 years ago in the subprocess module, after I modified subprocess to emit ResourceWarning in its destructor: commit 9505b03 to fix https://bugs.python.org/issue29174

commit 9505b03bb0d3c5828115fec9306cb976c43f04d9
Author: Victor Stinner <victor.stinner@gmail.com>
Date:   Fri Jan 6 10:44:44 2017 +0100

    Fix subprocess.Popen.__del__() fox Python shutdown
    
    Issue #29174, #26741: subprocess.Popen.__del__() now keeps a strong reference
    to warnings.warn() function.

@vstinner
Rename warn parameter to _warn
0961f09 
"_warn" name is a hint that it's not the regular warnings.warn()
function.
asvetlov
asvetlov approved these changes Jan 10, 2019
View changes
Copy link
Contributor

@asvetlov asvetlov left a comment

I like it.
IIRC we discussed a similar change in tulip/asyncio but decided to do nothing.

@vstinner vstinner merged commit fb2c346 into python:master Jan 10, 2019
@vstinner vstinner deleted the asyncio_del branch Jan 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@asvetlov asvetlov

@methane methane

@1st1 1st1

@gpshead gpshead

Assignees
No one assigned
Labels
skip issue skip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@vstinner @asvetlov @the-knights-who-say-ni @bedevere-bot