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 Nir Soffer
Recipients Nir Soffer, vstinner, walkhour
Date 2017-07-25.18:58:07
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1501009087.75.0.225361766956.issue30931@psf.upfronthosting.co.za>
In-reply-to
Content
On my PR 2854, Nir added these comments (extract):

> "And now we try to read from close dispatcher. I think we should wait
> for #2804 (...)"

> Sorry, I don't understand. My PR fixes described the bug that you
> described in msg298682:

> "If a dispatchers is closed and and a new dispatcher is created, the
> new dispatcher may get the same file descriptor. If the file
> descriptor was in the ready list returned from poll()/select(),
> asyncore may try to invoke one of the callbacks (e.g.  handle_write)
> on the new dispatcher."

> About reading from closed dispatcher: sorry, I don't know the asyncore
> concurrency model, but I know the asyncio concurrency model. In
> asyncio, I don't see why a "dispatcher" would affect any "unrelated"
> dispatcher. Said differently: if a dispatcher A closes the dispatcher
> B and dispatcher B is "readable", the read event will be handled
> anyway. The "close event" will be handled at the next loop iteration.
> The oversimplified asyncio model is "each iteration is atomic".

I don't know asyncio enough, but I suspect this issue may effect it if
it is using the fd to check if a reader/writer is closed. This is the
main issue in asyncore, assuming that during the loop:

    obj = map.get(fd)

Means that a obj is the dispatcher that owned fd when preparing for
poll()/select(), and we can use obj for calling the I/O callbacks.

The dispatcher owning this fd may have been closed during the loop, and
a new dispatcher may have been created with the *same* fd. The only way
to check if a dispatcher is closed is to check the object.

> Closing a dispatcher calls its del_channel() which removes its file
> descriptor from the asyncore map.

And creating new one at that point will add a new channel using the same
fd.

> If I understand correctly, the *current behaviour* depends on the file
> descriptor order, since select.select() and select.poll.poll() returns
> ordered file descriptors (smallest to largest). If a dispatcher A is
> called and closes a dispatcher B, the dispatcher B is called or not
> depending on the file descriptor. If A.fileno() < B.fileno(), B is not
> called. If A.fileno() > B.fileno(), B was already called. Am I right?

I don't think the order matter, only the fact that closing a dispatcher
and creating a new one is likely to use the same fd.

> What is the use case of a dispatcher closing another one? What is the
> consequence on my PR? Does the dispatcher B expects to not be called
> to read if it was already closed?

I don't know what is the use case of the reporter, but here is one
possible example - you implement a proxy, each proxy connection has 2
legs, each using a dispatcher connected to an endpoint. If one leg is
closed, you close also the other leg (one dispatcher closing another).
Now if you create a new dispatcher during the same loop iteration, it
may get the same fd of the other closed leg, and asyncore may try to
read/write with this dispatcher. This may work or not depending on how
you implement the dispatcher.

This is domonstrated in
https://github.com/python/cpython/pull/2764/commits/5aeb0098d2347984f3a89cf036c305edd2b8382b

> I see that close() also immediately closes the underlying socket. Does
> it mean that my PR can triggered OSError since we try recv() on a
> closed socket?

Yes, this is a typical issue in asyncore if you don't protect your
subclass to handle double close. asyncore.file_wrapper is protected, but
asyncore.dispatcher and its subclasses are not.
History
Date User Action Args
2017-07-25 18:58:07Nir Soffersetrecipients: + Nir Soffer, vstinner, walkhour
2017-07-25 18:58:07Nir Soffersetmessageid: <1501009087.75.0.225361766956.issue30931@psf.upfronthosting.co.za>
2017-07-25 18:58:07Nir Sofferlinkissue30931 messages
2017-07-25 18:58:07Nir Soffercreate