bpo-36820: Break unnecessary cycle in socket.py, codeop.py and dyld.py #13135

mariocj89 · 2019-05-06T19:37:04Z

Break cycle generated when saving an exception in socket.py, codeop.py and dyld.py as they keep alive not only the exception but user objects through the __traceback__ attribute.

https://bugs.python.org/issue36820

Automerge-Triggered-By: @pablogsal

mariocj89 · 2019-10-20T09:44:52Z

From offline conversation @pablogsal, this was waiting on trying to get a generic way to detect these kinds of cycles. It seemed to be much harder than initially thought.
As you suggested back in the day, do you want me to remove the tests here and land this?
We can look at detecting cycles in a generic way in a separate effort/PR.


        Remove binding of captured exceptions when not used to reduce the cha…

…nces of creating cycles (GH-17246) Capturing exceptions into names can lead to reference cycles though the __traceback__ attribute of the exceptions in some obscure cases that have been reported previously and fixed individually. As these variables are not used anyway, we can remove the binding to reduce the chances of creating reference cycles. See for example GH-13135


        Remove binding of captured exceptions when not used to reduce the cha…

…nces of creating cycles (pythonGH-17246) Capturing exceptions into names can lead to reference cycles though the __traceback__ attribute of the exceptions in some obscure cases that have been reported previously and fixed individually. As these variables are not used anyway, we can remove the binding to reduce the chances of creating reference cycles. See for example pythonGH-13135


        Fix cycle generated in `socket.create_connection` on error

The cycle happens when an exception is raised in `socket.create_connection` as it saves the exception in a variable and then re-raise it. By removing the name when the exception is raised we remove this cycle. We cannot just remove the name before as in the happy path since we need the exception to be raised.


        Remove exceptions saved in variables in codeop.py

As the exceptions were being saved in a different name it was generating a cycle since it was own through the `__traceback__`. By unsetting them we can collect the object earlier without the need of waiting for the gc.


        Remove exceptions saved in variables in dyld.py

As the exceptions were being saved in a different name it was generating a cycle since it was own through the `__traceback__`. By unsetting them we can collect the object earlier without the need of waiting for the gc.


        Add news entry


        tests: Remove tests as required on review

pablogsal

LGTM 🚀

miss-islington · 2019-12-06T14:27:36Z

@mariocj89: Status check is done, and it's a success ✅ .

miss-islington · 2019-12-06T14:42:46Z

Thanks @mariocj89 for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

miss-islington · 2019-12-06T14:42:46Z

Thanks @mariocj89 for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

bedevere-bot · 2019-12-06T14:43:19Z

GH-17485 is a backport of this pull request to the 3.7 branch.


        bpo-36820: Break unnecessary cycle in socket.py, codeop.py and dyld.py (

@pablogsal

pythonGH-13135) Break cycle generated when saving an exception in socket.py, codeop.py and dyld.py as they keep alive not only the exception but user objects through the ``__traceback__`` attribute. https://bugs.python.org/issue36820 Automerge-Triggered-By: @pablogsal (cherry picked from commit b64334c) Co-authored-by: Mario Corchero <mcorcherojim@bloomberg.net>

bedevere-bot · 2019-12-06T14:43:26Z

GH-17486 is a backport of this pull request to the 3.8 branch.


        bpo-36820: Break unnecessary cycle in socket.py, codeop.py and dyld.py (

@pablogsal

pythonGH-13135) Break cycle generated when saving an exception in socket.py, codeop.py and dyld.py as they keep alive not only the exception but user objects through the ``__traceback__`` attribute. https://bugs.python.org/issue36820 Automerge-Triggered-By: @pablogsal (cherry picked from commit b64334c) Co-authored-by: Mario Corchero <mcorcherojim@bloomberg.net>


        bpo-36820: Break unnecessary cycle in socket.py, codeop.py and dyld.py (

@pablogsal

GH-13135) Break cycle generated when saving an exception in socket.py, codeop.py and dyld.py as they keep alive not only the exception but user objects through the ``__traceback__`` attribute. https://bugs.python.org/issue36820 Automerge-Triggered-By: @pablogsal (cherry picked from commit b64334c) Co-authored-by: Mario Corchero <mcorcherojim@bloomberg.net>


        bpo-36820: Break unnecessary cycle in socket.py, codeop.py and dyld.py (

@pablogsal

GH-13135) Break cycle generated when saving an exception in socket.py, codeop.py and dyld.py as they keep alive not only the exception but user objects through the ``__traceback__`` attribute. https://bugs.python.org/issue36820 Automerge-Triggered-By: @pablogsal (cherry picked from commit b64334c) Co-authored-by: Mario Corchero <mcorcherojim@bloomberg.net>

the-knights-who-say-ni added the CLA signed label May 6, 2019

bedevere-bot added the awaiting review label May 6, 2019

matrixise changed the title ~~bpo-36820: Break unnecesary cycle in socket.py, codeop.py and dyld.py~~ bpo-36820: Break unnecessary cycle in socket.py, codeop.py and dyld.py May 6, 2019

pablogsal self-assigned this May 6, 2019

pablogsal self-requested a review May 6, 2019

auvipy approved these changes May 31, 2019

View changes

bedevere-bot added awaiting core review and removed awaiting review labels May 31, 2019

pablogsal mentioned this pull request Nov 19, 2019

Remove binding of captured exceptions when not used to reduce the chances of creating cycles #17246

Merged

pablogsal added the 🤖 automerge label Dec 6, 2019

mariocj89 added 5 commits May 6, 2019

Remove exceptions saved in variables in codeop.py

4fee725

As the exceptions were being saved in a different name it was generating a cycle since it was own through the `__traceback__`. By unsetting them we can collect the object earlier without the need of waiting for the gc.

Remove exceptions saved in variables in dyld.py

2fc95b9

As the exceptions were being saved in a different name it was generating a cycle since it was own through the `__traceback__`. By unsetting them we can collect the object earlier without the need of waiting for the gc.

Add news entry

fb86cbe

tests: Remove tests as required on review

Loading status checks…

be7ebe0

mariocj89 force-pushed the mariocj89:pu/socket-cycle-on-err branch from adfd53a to be7ebe0 Dec 6, 2019

pablogsal approved these changes Dec 6, 2019

View changes

bedevere-bot added awaiting merge and removed awaiting core review labels Dec 6, 2019

bedevere-bot removed the awaiting merge label Dec 6, 2019

pablogsal added needs backport to 3.7 needs backport to 3.8 labels Dec 6, 2019

bedevere-bot removed the needs backport to 3.7 label Dec 6, 2019

bedevere-bot removed the needs backport to 3.8 label Dec 6, 2019

sthagen mentioned this pull request Dec 6, 2019

bpo-36820: Break unnecessary cycle in socket.py, codeop.py and dyld.p… #76

Merged

python/cpython

bpo-36820: Break unnecessary cycle in socket.py, codeop.py and dyld.py #13135

bpo-36820: Break unnecessary cycle in socket.py, codeop.py and dyld.py #13135

mariocj89 commented May 6, 2019 •

edited by miss-islington

This comment has been minimized.

mariocj89 commented Oct 20, 2019 •

edited

pablogsal left a comment

This comment has been minimized.

miss-islington commented Dec 6, 2019

This comment has been minimized.

miss-islington commented Dec 6, 2019

This comment has been minimized.

miss-islington commented Dec 6, 2019

This comment has been minimized.

bedevere-bot commented Dec 6, 2019

This comment has been minimized.

bedevere-bot commented Dec 6, 2019

Sponsor python/cpython

python/cpython

Join GitHub today

bpo-36820: Break unnecessary cycle in socket.py, codeop.py and dyld.py #13135

bpo-36820: Break unnecessary cycle in socket.py, codeop.py and dyld.py #13135

Conversation

mariocj89 commented May 6, 2019 • edited by miss-islington

This comment has been minimized.

mariocj89 commented Oct 20, 2019 • edited

pablogsal left a comment

This comment has been minimized.

miss-islington commented Dec 6, 2019

This comment has been minimized.

miss-islington commented Dec 6, 2019

This comment has been minimized.

miss-islington commented Dec 6, 2019

This comment has been minimized.

bedevere-bot commented Dec 6, 2019

This comment has been minimized.

bedevere-bot commented Dec 6, 2019

mariocj89 commented May 6, 2019 •

edited by miss-islington

mariocj89 commented Oct 20, 2019 •

edited