Issue 28212: Closing server in asyncio is not efficient

When you close asyncio socket server it closes all connection sockets, remove its readers and do "cancel" for readers. Problem is that, that after this reader tasks are leaved alone in "cancelling" state. They wouldn`t be really cancelled, because they need a loop step for that. But they are not in loop now(they suggest execution throught selector, but they not presented there after removal). Cancelling step woldn`t be done and tasks wouldn`t be really finished, and no finishing actions can be done.
I think that It is good idea such tasks to "ready" queue, so they can be executed in normal task way after removing from selector.

You have to call wait_closed to complete the shutdown.

This should probably be mentioned directly in the socket server docs (at least in the examples) (assuming I'm not wrong...but I'm pretty sure I'm right) rather than by implicit reference to Server via the create_server comment.

Thats not a thing I talking about. But thanks for "wait_closed", I will be
use it in future )

Im talking about reading tasks.When reader removed from selector, there is
reading task for it. Loop do "cancel" for it, but it not really cancelling
task. Its only marking task for be cancelled on next loop step. It must
raise Cancel exception, so task may do corresponding actions inside it (for
exemple try..except Cancel). But it will not happen, because there will be
no next step - task removed from selector and now is missed from loop. It
will freese in "cancelling" state forever if protocol implementation
wouldn`t take care about it itself. Some protocol implementations make
"_step" for such tasks manually to loop them for correct finishing, someone
makes "set_result(None)", but its all looks bad.
Seems it will be better put it in loop after removing from selector, so it
will go "cancelling way" itself as every task do.

2016-09-20 17:50 GMT+03:00 R. David Murray <report@bugs.python.org>:

>
> R. David Murray added the comment:
>
> You have to call wait_closed to complete the shutdown.
>
> This should probably be mentioned directly in the socket server docs (at
> least in the examples) (assuming I'm not wrong...but I'm pretty sure I'm
> right) rather than by implicit reference to Server via the create_server
> comment.
>
> ----------
> assignee:  -> docs@python
> components: +Documentation
> nosy: +docs@python, r.david.murray
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue28212>
> _______________________________________
>

Can you please supply a complete example?

--Guido (mobile)

Yes? may be a in a hour,

2016-09-20 18:12 GMT+03:00 Guido van Rossum <report@bugs.python.org>:

>
> Guido van Rossum added the comment:
>
> Can you please supply a complete example?
>
> --Guido (mobile)
>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue28212>
> _______________________________________
>

Attached file with test example. There in "ping" task "except" and "finally" sections never called as expected (but code inside "try" will work).
Server is closed while client connection is active, and it just removes reading task from loop. You can still find it through Task.all_tasks(), but it is weakref array. So, sometime later GC will simply delete this task and no finalization will be performed.

It's a known annoying issue.
Honestly I don't know how to fix it properly.
`transport.close()` is affected also because `protocol.connection_lost()` is called on next loop iteration only.

Adding a small `asyncio.sleep()` between finishing all worn and loop closing solves the problem for me but the solution is not obvious for newbies.

Seems that its not so hard - in loop.remove_reader add

self._ready.append(reader)

after reader.cancel()

May be its needed to check that its not already there, but I cant imagine
how it can be.

2016-09-20 23:16 GMT+03:00 Andrew Svetlov <report@bugs.python.org>:

>
> Andrew Svetlov added the comment:
>
> It's a known annoying issue.
> Honestly I don't know how to fix it properly.
> `transport.close()` is affected also because `protocol.connection_lost()`
> is called on next loop iteration only.
>
> Adding a small `asyncio.sleep()` between finishing all worn and loop
> closing solves the problem for me but the solution is not obvious for
> newbies.
>
> ----------
> nosy: +asvetlov
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue28212>
> _______________________________________
>

> Seems that its not so hard - in loop.remove_reader add

If you have a patch in mind, please create a PR on github.com/python/asyncio

Seems that my example wasn`t good. Real reason in it was that closing server is not closing already established connections, and seems that it is not expected to do.
Andrew, can you provide your example? I catched some problems but now I think it was because of asyncio internal logic misunderstanding. May be if you provide your example there will be some ideas.

From my perspective the problem is: many asyncio calls schedules a delayed activity internally.
E.g. `task.cancel()` doesn't cancels immediately but requires at least one extra loop iteration.
The same is true for `transport.close()` -- it doesn't close socket in the call.
This behavior is encouraged by asyncio design and many third-party libraries just call `transp.close()` without waiting for upcoming `protocol.connection_lost()` callback.

I don't think it's a big problem, especially for server code.
But when users write small client tool they need to do extra no-op loop iterations before `loop.close()` call.

Waiting for no scheduled by `loop.call_soon()` callbacks makes no sense I believe. I could open a can of worms by introducing another weird side effects.

Superseded by https://bugs.python.org/issue32391

resolution: duplicate
stage: resolved

messages: + msg277041

assignee: docs@python
components: + Documentation