[issue30931] Race condition in asyncore may access the wrong dispatcher

Change by Irit Katriel : -- resolution: -> wont fix stage: -> resolved status: open -> closed superseder: -> Close asyncore/asynchat/smtpd issues and list them here ___ Python tracker

[issue30931] Race condition in asyncore may access the wrong dispatcher

Changes by Giampaolo Rodola' : -- assignee: -> giampaolo.rodola ___ Python tracker ___

[issue30931] Race condition in asyncore may access the wrong dispatcher

Giampaolo Rodola' added the comment: +1, I would push https://github.com/python/cpython/pull/2854/ first and fix the race condition only. -- ___ Python tracker

[issue30931] Race condition in asyncore may access the wrong dispatcher

Nir Soffer added the comment: Victor, I mostly agree with you, but I think we have here several bugs, and we should do the minimal fix for each of them. Your PR is too big, trying to fix too much. (Bug 1) The dispatcher A closes the dispatcher B. Currently, asyncore calls the handlers of the

[issue30931] Race condition in asyncore may access the wrong dispatcher

STINNER Victor added the comment: Much shorter summary: * Handlers of dispatcher must not be called if the dispatcher was called. * If a dispatcher is closed between the execution of two of his handlers, the next handlers should not be called My PR 2854 adds 3 unit tests to verify this

[issue30931] Race condition in asyncore may access the wrong dispatcher

STINNER Victor added the comment: Ok, wow, the discussion on this issue, bpo-30985, PR 2854, PR 2804, etc. was very productive. We identified much more race conditions than the first one that I spotted. (Bug 1) The dispatcher A closes the dispatcher B. Currently, asyncore calls the handlers

[issue30931] Race condition in asyncore may access the wrong dispatcher

STINNER Victor added the comment: > 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

[issue30931] Race condition in asyncore may access the wrong dispatcher

Jaume added the comment: There's an alternative fix which follows a similar approach to the one you mention: https://github.com/python/cpython/pull/2707/. I personally don't like too much to reuse again again an old variable as it's true that we don't know how people are using it (although I

[issue30931] Race condition in asyncore may access the wrong dispatcher

STINNER Victor added the comment: If I understood correctly, to fix this race condition, there are two main constraints: (A) We must call handlers of dispatcher registered before calling select()/poll(): dispatchers can be closed and their file descriptor can be reused while we execute other

[issue30931] Race condition in asyncore may access the wrong dispatcher

STINNER Victor added the comment: I'm not sure that it's a good idea to compare asyncore and asyncio. While their name are similar, their design are *very* different. I'm only talking about the kernel, the core event loop checking for file descriptors. In asyncio, when you close a transport,

[issue30931] Race condition in asyncore may access the wrong dispatcher

Nir Soffer added the comment: > I use the same trick all over the place in pyftpdlib: > https://github.com/giampaolo/pyftpdlib/blob/1268bb185cd63c657d78bc33309041628e62360a/pyftpdlib/handlers.py#L537 This allow detection of closed sockets, but does not fix the issue of accessing the wrong

[issue30931] Race condition in asyncore may access the wrong dispatcher

Giampaolo Rodola' added the comment: > Yes, this is a typical issue in asyncore if you don't protect your > subclass to handle double close. I use the same trick all over the place in pyftpdlib:

[issue30931] Race condition in asyncore may access the wrong dispatcher

Nir Soffer added the comment: 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

[issue30931] Race condition in asyncore may access the wrong dispatcher

STINNER Victor added the comment: 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

[issue30931] Race condition in asyncore may access the wrong dispatcher

Changes by STINNER Victor : -- pull_requests: +2905 ___ Python tracker ___ ___

[issue30931] Race condition in asyncore may access the wrong dispatcher

Changes by STINNER Victor : -- versions: -Python 3.3, Python 3.4 ___ Python tracker ___

[issue30931] Race condition in asyncore may access the wrong dispatcher

Nir Soffer added the comment: Adding more info after discussion in github. After polling readable/writeable dispatchers, asyncore.poll(2) receive a list of ready file descriptors, and invoke callbacks on the dispatcher objects. If a dispatchers is closed and and a new dispatcher is created,