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.

Sign up for GitHub

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

Jump to bottom

Server fails to shutdown while persistent connections exist #180

Closed
ciscorn opened this issue Jul 6, 2018 · 6 comments · Fixed by #181
Closed

Server fails to shutdown while persistent connections exist #180

ciscorn opened this issue Jul 6, 2018 · 6 comments · Fixed by #181
Labels
bug

Comments

@ciscorn
Copy link
Contributor

ciscorn commented Jul 6, 2018 •
edited
Loading

I noticed that my aiohttp server with uvloop==0.11.0 awaits until all connections (WebSocket, streaming, etc.) are closed. This behavior prevents us from performing aiohttp's graceful shutdown.

This problem doesn't occur with uvloop==0.10.2.

I'm suspecting the commit 124e981 because this change seems not to conform to the following spec:

asyncio.Server.close()
Stop serving: close listening sockets and set the sockets attribute to None.
The sockets that represent existing incoming client connections are left open.
@ciscorn ciscorn changed the title Server.wait_closed shouldn't wait for incoming connections closed Server.wait_closed shouldn't wait until all connections are closed Jul 6, 2018
@1st1
Copy link
Member

1st1 commented Jul 6, 2018

Can you submit a PR to revert that commit and maybe write a unittest?

@1st1 1st1 added the bug label Jul 6, 2018
@ciscorn
Copy link
Contributor Author

ciscorn commented Jul 7, 2018 •
edited
Loading

I wrote a test and then reverted the 124e981, but the test still fails with uvloop.
I'll investigate further and write a PR.

@ciscorn ciscorn changed the title Server.wait_closed shouldn't wait until all connections are closed Server failed to shutdown when incoming connections exist Jul 7, 2018
@1st1
Copy link
Member

1st1 commented Jul 7, 2018

Thanks!

ciscorn added a commit to ciscorn/uvloop that referenced this issue Jul 8, 2018
@ciscorn
Add a test for aiohttp graceful shutdown (MagicStack#180)
933cf92
ciscorn added a commit to ciscorn/uvloop that referenced this issue Jul 8, 2018
@ciscorn
Revert "Fix Server to wait in wait_closed() until all transports are …
90d7e06 
…done" (MagicStack#180)

This reverts commit 124e981.
@ciscorn ciscorn changed the title Server failed to shutdown when incoming connections exist Server fails to shutdown while persistent connections exist Jul 8, 2018
@ciscorn
Copy link
Contributor Author

ciscorn commented Jul 8, 2018 •
edited
Loading

Sorry, I noticed I hadn't removed the generated .c file.

After running python3 setup.py bulid_ext --cython-always, the reverted revision passed the test.

@ciscorn ciscorn mentioned this issue Jul 8, 2018
Merged
ciscorn added a commit to ciscorn/uvloop that referenced this issue Jul 8, 2018
@ciscorn
Add a test for aiohttp graceful shutdown (MagicStack#180)
2aeb4dc
ciscorn added a commit to ciscorn/uvloop that referenced this issue Jul 8, 2018
@ciscorn
Revert "Fix Server to wait in wait_closed() until all transports are …
451110e 
…done" (MagicStack#180)

This reverts commit 124e981.
@pvizeli
Copy link

pvizeli commented Jul 24, 2018

Can we have a bugfix release?

@pvizeli pvizeli mentioned this issue Jul 24, 2018
Merged
ciscorn added a commit to ciscorn/uvloop that referenced this issue Jul 31, 2018
@ciscorn
Fix MagicStack#180 (Revert commit 124e981 and add a comment)
474d9ae 
Revert "Fix Server to wait in wait_closed() until all transports are done"
This reverts commit 124e981.
@1st1 1st1 closed this as completed in #181 Aug 1, 2018
1st1 pushed a commit that referenced this issue Aug 1, 2018
@ciscorn @1st1
Fix #180: Server unable to shutdown when persistent connections exist (…
486f82d 
…#181)

Revert "Fix Server to wait in wait_closed() until all transports are done"
This reverts commit 124e981.
1st1 pushed a commit that referenced this issue Aug 1, 2018
@ciscorn @1st1
Fix #180: Server unable to shutdown when persistent connections exist (…
5f71e29 
…#181)

Revert "Fix Server to wait in wait_closed() until all transports are done"
This reverts commit 124e981.
@1st1
Copy link
Member

1st1 commented Aug 2, 2018

Fixed in uvloop 0.11.1. Thanks for working on this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix #180: Server unable to shutdown while persistent connections exist ciscorn/uvloop
3 participants
@1st1 @ciscorn @pvizeli