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.

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

asyncio.base_futures.InvalidStateError: invalid state #169

Closed
andr-04 opened this issue Jun 15, 2018 · 5 comments
Closed

asyncio.base_futures.InvalidStateError: invalid state #169

andr-04 opened this issue Jun 15, 2018 · 5 comments

Comments

@andr-04
Copy link
Contributor

andr-04 commented Jun 15, 2018

  • uvloop version: 0.10.1
  • Python version: 3.6.5
  • Platform: Ubuntu 16.04
  • Can you reproduce the bug with PYTHONASYNCIODEBUG in 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?

@andr-04
Copy link
Contributor Author

andr-04 commented 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_recv by asyncio.wait with a timeout.

@1st1
Copy link
Member

1st1 commented Jun 15, 2018

Would you mind making a PR fixing this?

@andr-04
Copy link
Contributor Author

andr-04 commented 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.

andr-04 added a commit to andr-04/uvloop that referenced this issue Jun 17, 2018
@andr-04 andr-04 closed this as completed Jun 18, 2018
@1st1 1st1 reopened this Jun 19, 2018
@1st1
Copy link
Member

1st1 commented Jun 19, 2018

Regarding #170: I don't think that copying the approach of asyncio/base_events.py is the right strategy here. IIRC I decided to go with a different implementation of loop.sock_* methods because asyncio has many other problems with cancellation. In uvloop, cancellation happens in _new_reader_future and _new_writer_future methods, 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
Copy link
Contributor Author

andr-04 commented 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:

  1. await sock_recv;
  2. _on_cancel callback from previously timeout sock_recv;
  3. _remove_reader called by _on_cancle callback;
  4. _add_reader called by the last await sock_recv;
  5. interrupting await sock_recv by timeout with calling fut.cancel().

In the InvalidStateError the sequence a little bit differ: before _on_cancel we get _sock_recv. Well, it's really not guaranteed the data must not be received at this time. So, due to the previous future from await sock_recv is 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_recv without reading), the new _add_reader does not raise _sock_recv for old data which was given, but not read yet. I suppose It is also because of _on_cancel callback 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 Event and redefine cancel method 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.

1st1 added a commit that referenced this issue Jun 22, 2018
Co-authored-by: Andrey Egorov <andr06@gmail.com>
@1st1 1st1 closed this as completed in #173 Jun 22, 2018
1st1 added a commit that referenced this issue Jun 22, 2018
Co-authored-by: Andrey Egorov <andr06@gmail.com>
andr-04 added a commit to andr-04/uvloop that referenced this issue Jun 23, 2018
* 'master' of https://github.com/MagicStack/uvloop:
  Experimental fix for MagicStack#169
  Use ssize_t instead of int where necessary
  Use a proper type for the thread indent.
  Ignore linting errors in includes/__init__
  setup.py: Use raw string for regexp
  setup.py: Detect if libuv submodule has not been checked out

# Conflicts (their side resolved):
#	tests/test_sockets.py
#	uvloop/loop.pyx
andr-04 added a commit to andr-04/uvloop that referenced this issue Jun 23, 2018
* 'master' of https://github.com/MagicStack/uvloop:
  Experimental fix for MagicStack#169
  Use ssize_t instead of int where necessary
  Use a proper type for the thread indent.
  Ignore linting errors in includes/__init__
  setup.py: Use raw string for regexp
  setup.py: Detect if libuv submodule has not been checked out

# Conflicts (their side resolved):
#	tests/test_sockets.py
#	uvloop/loop.pyx
1st1 added a commit that referenced this issue Jun 25, 2018
Co-authored-by: Andrey Egorov <andr06@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants