Author pablogsal
Recipients Windson Yang, benjamin.peterson, davin, docs@python, mattip, ned.deily, pablogsal, pitrou, tzickel, vstinner, zach.ware
Date 2018-12-03.22:59:57
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1543877997.33.0.788709270274.issue34172@psf.upfronthosting.co.za>
In-reply-to
Content
> I'm not comfortable with the fix. I cannot explain why but I feel like adding a strong dependency from a child to its parent is going to lead to more bugs, not less. It sounds like a recipe for reference cycles. Maybe I'm just plain wrong.

The pool child objects (imap iterators, async results...etc) need to keep a reference to the parent because if not, the caller is in charge of doing that and that may lead to bugs. Is the same scenario as if I get a dictionary iterator and then I destroy my reference to the dictionary: if the iterator does not keep a reference to the parent (the dictionary) it will not be possible to be used in the future. Indeed, we can see that this is what happens:

>>> x = {1:2}
>>> y = iter(x)
>>> gc.get_referrents(x)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: module 'gc' has no attribute 'get_referrents'
>>> gc.get_referrers(x)
[<dict_keyiterator object at 0x0000024447D6D598>, 
{'__name__': '__main__', '__doc__': None, '__package__': None, '__loader__': <class '_frozen_importlib.BuiltinImporter'>, '__spec__': None, '__annotations__': {}, '__builtins__': <module 'builtins' (built-in)>, 'y': <dict_keyiterator object at 0x0000024447D6D598>, 'gc': <module 'gc' (built-in)>, 'x': {1: 2}}
]

We can see that the dict_keyiterator refers to the dictionary, keeping it alive.

Here we have the same situation: if we do not keep the pool alive, the iterator will hang when iterating because the jobs won't get finished.

>At this point, I would like that someone explains to me what the problem is. https://github.com/python/cpython/pull/10852 is a solution, ok, but what is the problem? What does the code hangs, whereas >previously it was fine? Is the example code really correct? Do we want to support such usage?

The code hangs because the pool was not being destroyed before due to the reference cycle between the pool and an internal object (a Thread). Now it hangs because the worker thread is destroyed with the pool as no references are kept to it and the job that the iterator is waiting on is never finished.

>I understand that msg330864 rely on black magic to expect that it's going to be fine. The lifetime of the pool is implicit and it sounds like a bad design. I don't want to endorse that.

I found the weird code in the example in several projects. I have corrected it to use the pool as a context manager or to call close(), but this means that users are doing this and it used to work and not it does not: technically is a regression.
History
Date User Action Args
2018-12-03 23:00:15pablogsalunlinkissue34172 messages
2018-12-03 22:59:57pablogsalsetrecipients: + pablogsal, pitrou, vstinner, benjamin.peterson, ned.deily, docs@python, zach.ware, mattip, davin, tzickel, Windson Yang
2018-12-03 22:59:57pablogsalsetmessageid: <1543877997.33.0.788709270274.issue34172@psf.upfronthosting.co.za>
2018-12-03 22:59:57pablogsallinkissue34172 messages
2018-12-03 22:59:57pablogsalcreate