Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Listener removes wrong guy #645

Merged
merged 2 commits into from Sep 23, 2020
Merged

Listener removes wrong guy #645

merged 2 commits into from Sep 23, 2020

Conversation

clayg
Copy link
Contributor

@clayg clayg commented Sep 23, 2020

I really only intended this to be a followup to @charmster 's comment on #498

I'm not actually sure if this is better, but I do think it would also work

smerritt and others added 2 commits September 23, 2020 10:16
When in hubs.trampoline(fd, ...), a greenthread registers itself as a
listener for fd, switches to the hub, and then calls
hub.remove(listener) to deregister itself. hub.remove(listener)
removes the primary listener. If the greenthread awoke because its fd
became ready, then it is the primary listener, and everything is
fine. However, if the greenthread was a secondary listener and awoke
because a Timeout fired then it would remove the primary and promote a
random secondary to primary.

This commit makes hub.remove(listener) check to make sure listener is
the primary, and if it's not, remove the listener from the
secondaries.
There was some concern in fixing hub.remove for secondary listeners
because tests seemed to rely on the implicit robustness of hub.remove
when a listener wasn't being tracked.

It was discovered that socket errors can bubble up in poll and select
hubs which result in all listeners being removed for that fileno before
the listener was alerted (which would then *also* triggered a remove).

Rather than having remove be robust to being called twice this change
makes it be called only once.
@codecov-commenter
Copy link

codecov-commenter commented Sep 23, 2020

Codecov Report

Merging #645 into master will increase coverage by 0%.
The diff coverage is 83%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master    #645   +/-   ##
======================================
  Coverage      44%     44%           
======================================
  Files          87      87           
  Lines       11831   11839    +8     
  Branches     1774    1776    +2     
======================================
+ Hits         5254    5277   +23     
+ Misses       6180    6167   -13     
+ Partials      397     395    -2     
Flag Coverage Δ
#ipv6 16% <5%> (?)
#py27epolls 56% <83%> (+<1%) ⬆️
#py27poll 56% <83%> (+<1%) ⬆️
#py27selects 55% <83%> (?)
#py35epolls 49% <83%> (+<1%) ⬆️
#py35poll 49% <83%> (+<1%) ⬆️
#py35selects 49% <83%> (?)
#py36epolls 49% <83%> (+<1%) ⬆️
#py36poll 49% <83%> (+<1%) ⬆️
#py36selects 49% <83%> (+<1%) ⬆️
#py37epolls 49% <83%> (+<1%) ⬆️
#py37poll 49% <83%> (+<1%) ⬆️
#py37selects 49% <83%> (+<1%) ⬆️
#py38epolls 41% <83%> (+<1%) ⬆️
#py38poll 41% <83%> (+<1%) ⬆️
#py38selects 40% <83%> (+<1%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
eventlet/hubs/hub.py 89% <83%> (+1%) ⬆️
eventlet/greenio/py3.py 78% <0%> (-2%) ⬇️
eventlet/green/ssl.py 47% <0%> (-1%) ⬇️
eventlet/greenio/base.py 80% <0%> (-1%) ⬇️
eventlet/queue.py 78% <0%> (ø)
eventlet/green/zmq.py 79% <0%> (ø)
eventlet/support/greendns.py 63% <0%> (ø)
eventlet/green/http/client.py 32% <0%> (ø)
eventlet/wsgi.py 70% <0%> (+<1%) ⬆️
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 004cc4e...af253ad. Read the comment docs.

Copy link
Member

@temoto temoto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks a lot

@temoto temoto merged commit af253ad into eventlet:master Sep 23, 2020
clrpackages pushed a commit to clearlinux-pkgs/eventlet that referenced this pull request Sep 25, 2020
… 0.28.0

Clay Gerrard (1):
      Make remove more explicit

Samuel Merritt (1):
      Always remove the right listener from the hub

Sergey Shepelev (1):
      v0.28.0 release

0.28.0
======
* Always remove the right listener from the hub eventlet/eventlet#645
bors bot added a commit to duckinator/parts.horse that referenced this pull request Sep 25, 2020
64: Update eventlet to 0.28.0 r=duckinator a=pyup-bot


This PR updates [eventlet](https://pypi.org/project/eventlet) from **0.27.0** to **0.28.0**.



<details>
  <summary>Changelog</summary>
  
  
   ### 0.28.0
   ```
   ======
* Always remove the right listener from the hub eventlet/eventlet#645
   ```
   
  
</details>


 

<details>
  <summary>Links</summary>
  
  - PyPI: https://pypi.org/project/eventlet
  - Changelog: https://pyup.io/changelogs/eventlet/
  - Homepage: http://eventlet.net
</details>



Co-authored-by: pyup-bot <github-bot@pyup.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants