[issue30306] release arguments of contextmanager

Nick Coghlan added the comment: Thanks for the issue report and patch Martin, and sorry for the long delay in getting it merged! -- resolution: -> fixed stage: patch review -> resolved status: open -> closed ___ Python tracker

[issue30306] release arguments of contextmanager

Nick Coghlan added the comment: New changeset a278ad2faa76ae61569a58f36e06ba8745f4a9b7 by Nick Coghlan in branch 'master': bpo-30306: Add missing NEWS entry (GH-5374) https://github.com/python/cpython/commit/a278ad2faa76ae61569a58f36e06ba8745f4a9b7 --

[issue30306] release arguments of contextmanager

Change by Nick Coghlan : -- pull_requests: +5215 stage: -> patch review ___ Python tracker ___

[issue30306] release arguments of contextmanager

Nick Coghlan added the comment: New changeset dd0e087edc8f1e4d2c0913236b1a62a77d9db6d8 by Nick Coghlan (Martin Teichmann) in branch 'master': bpo-30306: release arguments of contextmanager (GH-1500)

[issue30306] release arguments of contextmanager

Nick Coghlan added the comment: Ouch, this clearly slipped off my review radar last year - I just picked it up again now while going through all my currently assigned issues before 3.7b1. While I still think the suggested refactoring above would be a good way to go,

[issue30306] release arguments of contextmanager

Nick Coghlan added the comment: Re-reading my previous comment, I now realise it wasn't particularly clear. To be more explicit, I agree with Serhiy that since the internals of how @contextmanager works are a private API, it makes sense to switch to a two-class design that doesn't rely on the

[issue30306] release arguments of contextmanager

Martin Teichmann added the comment: Hi, so where do we go from here? I could write the proposed allow_recreation flag, but IMHO this adds only dead code, as it is of no need at all. I don't think code gets more clear by adding dead code... I opted for one line more of comment, I think that

[issue30306] release arguments of contextmanager

Nick Coghlan added the comment: Noting something I picked up in the PR review that apparently isn't covered by the test suite: the whole _recreate_cm() dance is necessary to make recursive and concurrent invocation of decorated functions work correctly. Otherwise the original CM instance ends

[issue30306] release arguments of contextmanager

Martin Teichmann added the comment: I personally prefer the current situation. The problem is the following: when an @contextmanager is first called, we don't know yet whether the user wants to use it directly, or as a decorator, so we have to have some kind of hybrid class. Once it's used as

[issue30306] release arguments of contextmanager

Nick Coghlan added the comment: You're right, there's no actual requirement that _recreate_cm() call self.__class__, so it can use a different type internally. -- ___ Python tracker

[issue30306] release arguments of contextmanager

Serhiy Storchaka added the comment: Wouldn't be better to split _GeneratorContextManager on two classes rather than add a boolean argument? Currently _GeneratorContextManager used two different functions -- an one-shot context manager and a decorator that recreates context managers.

[issue30306] release arguments of contextmanager

Nick Coghlan added the comment: Nice catch. This behaviour is an artifact of the ContextDecorator support in _GeneratorContextManager, where context managers created with `@contextmanager` can be used as function decorators (implicitly wrapping them in a with statement) as well as directly in

[issue30306] release arguments of contextmanager

Changes by Martin Teichmann : -- pull_requests: +1602 ___ Python tracker ___ ___

[issue30306] release arguments of contextmanager

Changes by Raymond Hettinger : -- assignee: -> ncoghlan nosy: +ncoghlan ___ Python tracker ___

[issue30306] release arguments of contextmanager

New submission from Martin Teichmann: The arguments of a function which was decorated to be a context manager are stored inside the context manager, and are thus kept alive. This is a memory leak. Example: @contextmanager def f(a): do_something_with(a) a = None #