Author ncoghlan
Recipients Martin.Teichmann, ncoghlan
Date 2017-05-09.03:22:42
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1494300163.56.0.354453537913.issue30306@psf.upfronthosting.co.za>
In-reply-to
Content
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 with statements: to handle the ContextDecorator case, the original arguments are stored on the _GeneratorContextManager instance, which has the side effect of keeping those arguments referenced for the whole of the with statement body (due to the reference to the instance from the bound __exit__ method).

Martin's initial patch just unconditionally deletes those attributes in __enter__, which is technically correct, but *looks* wrong when reading the code (understanding why it's correct requires understanding the private _recreate_cm API used for collaboration between ContextDecorator and _GeneratorContextManager, and the fact that that relationship is a bit weird and convoluted is the main reason it's private in the first place).

My recommendation on the PR for a more self-obviously correct fix is to do the following:

- add a new "allow_recreation=True" flag parameter to `_GeneratorContextManager.__init__` that controls whether or not the CM recreation attributes get set or not
- pass `False` for that argument in `_recreate_cm` so they get set to None instead
- update `__enter__` to set them to None if they're not already None

The practical effect is the same as Martin's original patch, but the more explicit version should make it easier for readers to see what is going on (i.e. even when recreation is allowed during construction, the intent is that for any given instance, you will only ever call either __enter__() *or* _recreate_cm(), and once you do call __enter__(), that's it - the only permitted call after that point on that particular instance is __exit__()).
History
Date User Action Args
2017-05-09 03:22:43ncoghlansetrecipients: + ncoghlan, Martin.Teichmann
2017-05-09 03:22:43ncoghlansetmessageid: <1494300163.56.0.354453537913.issue30306@psf.upfronthosting.co.za>
2017-05-09 03:22:43ncoghlanlinkissue30306 messages
2017-05-09 03:22:42ncoghlancreate