Repository navigation
asyncio.base_futures.InvalidStateError: invalid state #169
Description
- uvloop version: 0.10.1
- Python version: 3.6.5
- Platform: Ubuntu 16.04
- Can you reproduce the bug with
PYTHONASYNCIODEBUGin env?: No
I work with a raw sockets using asyncio.Lock to isolate blocks reading and sock_recv to communicate inside a Lock section.
In a random time first of all I get the following stacktrace:
Exception in callback Loop._sock_recv
handle: <Handle Loop._sock_recv>
Traceback (most recent call last):
File "uvloop/cbhandles.pyx", line 76, in uvloop.loop.Handle._run
File "uvloop/loop.pyx", line 888, in uvloop.loop.Loop._sock_recv
asyncio.base_futures.InvalidStateError: invalid state
After that there is a problem with acquiring the Lock: it becomes pass any simultaneously calls.
Now very small information, but I'll try to investigate and reproduce it. And give you more details. But maybe you have a thoughts already?
Activity
andr-04 commentedon Jun 15, 2018
Ok, I compare the implementation with base loop one and see you don't check does the future is cancelled or not yet. It's bad because it's a real usecase to wrap
sock_recvbyasyncio.waitwith a timeout.1st1 commentedon Jun 15, 2018
Would you mind making a PR fixing this?
andr-04 commentedon Jun 15, 2018
Well, I've understood the behaviour and Lock is not a problem there at all; the problem is shifting in the source data structure: before exception we read the data which already is related with other caller.
I didn't use Cython in real life yet and I'm aware it will take a lot of time to make a fix, check it and write unit tests for it. Especially I've not done it there.
But I can to try later.
Fix MagicStack#169: asyncio.base_futures.InvalidStateError: invalid s…
1st1 commentedon Jun 19, 2018
Regarding #170: I don't think that copying the approach of
asyncio/base_events.pyis the right strategy here. IIRC I decided to go with a different implementation ofloop.sock_*methods because asyncio has many other problems with cancellation. In uvloop, cancellation happens in_new_reader_futureand_new_writer_futuremethods, maybe they should be adjusted to handle your case?BTW, I don't see a unittest to reproduce the bug you're trying to fix. It's absolutely crucial to have a unittest, can you please try to come up with one?
andr-04 commentedon Jun 21, 2018
Well, it more difficult to reproduce than I thought before. I've investigated the problem by write the sequence of operations to the log.
The normal pipeline with timeouts is:
await sock_recv;_on_cancelcallback from previously timeoutsock_recv;_remove_readercalled by_on_canclecallback;_add_readercalled by the lastawait sock_recv;await sock_recvby timeout with callingfut.cancel().In the InvalidStateError the sequence a little bit differ: before
_on_cancelwe get_sock_recv. Well, it's really not guaranteed the data must not be received at this time. So, due to the previous future fromawait sock_recvis cancelled, but the data from socket has read, so it is completely lost.Another issue which I have caught -- even if I add a check the result future must not be cancelled (i.e. if it cancelled just return from
_sock_recvwithout reading), the new_add_readerdoes not raise_sock_recvfor old data which was given, but not read yet. I suppose It is also because of_on_cancelcallback is async and can happens after_add_reader: it removes the last reader which is actual. It's more seldom behavior, I've no fair experiment for it.As a solution I suggest to make a subclass from the base
Eventand redefinecancelmethod of it to make this part of code be synced. What the name you would like I give for it and what are your views for this solution at all?Yes, I've reproduced main situation and can add a unit test for it. The second case is related, but more difficult to be clearly reproduced and can be eliminated by suggested solution as well.
Experimental fix for #169
Experimental fix for #169
Merge branch 'master' of https://github.com/MagicStack/uvloop
#1: Merge branch 'master' of https://github.com/MagicStack/uvloop
Experimental fix for #169