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.

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

bpo-30775: Clear potential ref cycle between Process and Process target #2470

Merged
merged 2 commits into from
Jun 28, 2017
Merged

bpo-30775: Clear potential ref cycle between Process and Process target #2470

merged 2 commits into from
Jun 28, 2017

Conversation

pitrou
Copy link
Member

@pitrou pitrou commented Jun 28, 2017

Besides Process.join() not being called, this was an indirect cause of bpo-30775.
The threading module already does this.

@pitrou
Clear potential ref cycle between Process and Process target
e69b02a 
Besides Process.join() not being called, this was an indirect cause of bpo-30775.
The threading module already does this.
@pitrou pitrou added trivial type-bug An unexpected behavior, bug, or error labels Jun 28, 2017
@pitrou
Copy link
Member Author

pitrou commented Jun 28, 2017

@Haypo @applio

vstinner
vstinner reviewed Jun 28, 2017
View reviewed changes
Lib/multiprocessing/process.py Outdated
@@ -110,6 +110,9 @@ def start(self):
_cleanup()
self._popen = self._Popen(self)
self._sentinel = self._popen.sentinel
# Avoid a refcycle if the target function holds an indirect
# reference to the process object
del self._target, self._args, self._kwargs
Copy link
Member

Choose a reason for hiding this comment

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

I dislike magic attributes which disappear. Would it be possible to assign them to None instead? Please add a reference to bpo-30775 in the comment.

Copy link
Member Author

Choose a reason for hiding this comment

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

No preference from me, but the deletion is what threading does.

vstinner
vstinner reviewed Jun 28, 2017
View reviewed changes
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

Does it work if run() is called "late" after start()?

Maybe we clear clear the attributes in join() instead?

It's a open question, I don't understand how these things work :-)

@pitrou
Copy link
Member Author

pitrou commented Jun 28, 2017

Does it work if run() is called "late" after start()?

run() is called in the child process, so it's safe to clear variables in the parent once the child is spawned (which happens in start()).

I don't know what happens if people call run() themselves. They shouldn't :-)

@vstinner
Copy link
Member

run() is called in the child process, so it's safe to clear variables in the parent once the child is spawned (which happens in start())

Oh ok. In this case, it makes sense and it's safe :-)

@vstinner vstinner merged commit 79d37ae into python:master Jun 28, 2017
@pitrou pitrou deleted the process_lose_target_ref branch June 28, 2017 10:29
@vstinner
Copy link
Member

Hum, how do you feel about backporting this change to other branches? IMHO it's a bug and it should be backported up to 2.7.

@pitrou
Copy link
Member Author

pitrou commented Jun 28, 2017

Yes, that sounds reasonable.

pitrou added a commit to pitrou/cpython that referenced this pull request Jun 28, 2017
@pitrou
[3.6] Clear potential ref cycle between Process and Process target (p…
f1f57eb 
…ythonGH-2470)

* Clear potential ref cycle between Process and Process target

Besides Process.join() not being called, this was an indirect cause of bpo-30775.
The threading module already does this.

* Add issue reference.
(cherry picked from commit 79d37ae)
@bedevere-bot
Copy link

GH-2471 is a backport of this pull request to the 3.6 branch.

pitrou added a commit to pitrou/cpython that referenced this pull request Jun 28, 2017
@pitrou
[3.5] Clear potential ref cycle between Process and Process target (p…
5f2ab58 
…ythonGH-2470)

* Clear potential ref cycle between Process and Process target

Besides Process.join() not being called, this was an indirect cause of bpo-30775.
The threading module already does this.

* Add issue reference.
(cherry picked from commit 79d37ae)
@bedevere-bot
Copy link

GH-2472 is a backport of this pull request to the 3.5 branch.

pitrou added a commit to pitrou/cpython that referenced this pull request Jun 28, 2017
@pitrou
[2.7] Clear potential ref cycle between Process and Process target (p…
a4e9d76 
…ythonGH-2470)

* Clear potential ref cycle between Process and Process target

Besides Process.join() not being called, this was an indirect cause of bpo-30775.
The threading module already does this.

* Add issue reference.
(cherry picked from commit 79d37ae)
@bedevere-bot
Copy link

GH-2473 is a backport of this pull request to the 2.7 branch.

pitrou added a commit that referenced this pull request Jun 28, 2017
@pitrou
[3.6] Clear potential ref cycle between Process and Process target (G…
38d6a40 
…H-2470) (#2471)

* Clear potential ref cycle between Process and Process target

Besides Process.join() not being called, this was an indirect cause of bpo-30775.
The threading module already does this.

* Add issue reference.
(cherry picked from commit 79d37ae)
pitrou added a commit that referenced this pull request Jun 28, 2017
@pitrou
[3.5] Clear potential ref cycle between Process and Process target (G…
a9705b7 
…H-2470) (#2472)

* Clear potential ref cycle between Process and Process target

Besides Process.join() not being called, this was an indirect cause of bpo-30775.
The threading module already does this.

* Add issue reference.
(cherry picked from commit 79d37ae)
@vstinner vstinner changed the title Clear potential ref cycle between Process and Process target bpo-30775: Clear potential ref cycle between Process and Process target Jun 28, 2017
vstinner pushed a commit that referenced this pull request Jun 28, 2017
@pitrou @vstinner
[2.7] Clear potential ref cycle between Process and Process target (G…
12536bd 
…H-2470) (#2473)

* Clear potential ref cycle between Process and Process target

Besides Process.join() not being called, this was an indirect cause of bpo-30775.
The threading module already does this.

* Add issue reference.
(cherry picked from commit 79d37ae)
@vstinner vstinner mentioned this pull request Sep 15, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vstinner vstinner vstinner left review comments

Assignees
No one assigned
Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@pitrou @vstinner @bedevere-bot @the-knights-who-say-ni