Message299164
> 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 ;-) |
|
Date |
User |
Action |
Args |
2017-07-25 23:21:08 | vstinner | set | recipients:
+ vstinner, giampaolo.rodola, Nir Soffer, walkhour |
2017-07-25 23:21:08 | vstinner | set | messageid: <1501024868.69.0.235256198967.issue30931@psf.upfronthosting.co.za> |
2017-07-25 23:21:08 | vstinner | link | issue30931 messages |
2017-07-25 23:21:08 | vstinner | create | |
|