This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author vstinner
Recipients Nir Soffer, giampaolo.rodola, vstinner, walkhour
Date 2017-07-25.23:21:08
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1501024868.69.0.235256198967.issue30931@psf.upfronthosting.co.za>
In-reply-to
Content
> There's an alternative fix which follows a similar approach to the one you mention: https://github.com/python/cpython/pull/2707/.

Sorry, I'm slow to understand. Now that the bug and expected behaviour is better explained, I can now review correctly your PR and I like it :-)

While you wrote you previous comment, I updated my PR to implement a similar idea:
https://github.com/python/cpython/pull/2854

My code now detects if a dispatcher was closed, but also if it's file descriptor was reused by a newly registered dispatcher.

Our PR are similar:

* create a copy before starting to run handler
* check if our copy is still consistent to the current map: if not, skip the dispatcher handler

My PR is different:

* It only creates a list of ready objects: it might be cheaper if only few file descriptors are ready, whereas the map is large... I don't think that it's a real performance bottleneck in practice, but well, I'm just trying to compare our implementations :-) ... To be honest, my PR is a copy of your other PR, not you wrote both versions in fact ;-) (that's why I credited you as co-author in my PR)

* My PR has a simpler and more specific unit test: only testing the poll() function, no server, no client: just handlers.

* My PR also tests when a dispatcher was only closed, not replaced.

--

Good! It seems like slowly we converge to a solution, no? Now, it's more bikeshedding on the exact implementation ;-)
History
Date User Action Args
2017-07-25 23:21:08vstinnersetrecipients: + vstinner, giampaolo.rodola, Nir Soffer, walkhour
2017-07-25 23:21:08vstinnersetmessageid: <1501024868.69.0.235256198967.issue30931@psf.upfronthosting.co.za>
2017-07-25 23:21:08vstinnerlinkissue30931 messages
2017-07-25 23:21:08vstinnercreate