classification
Title: BaseSelectorEventLoop.sock_{recv,sendall}() don't remove their callbacks when canceled
Type: behavior Stage:
Components: asyncio Versions: Python 3.8
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: asvetlov Nosy List: abacabadabacaba, asvetlov, lukasz.langa, ned.deily, yselivanov
Priority: deferred blocker Keywords: patch

Created on 2017-04-13 08:55 by abacabadabacaba, last changed 2019-12-09 14:59 by asvetlov.

Pull Requests
URL Status Linked Edit
PR 10419 merged asvetlov, 2018-11-08 22:41
Messages (9)
msg291593 - (view) Author: Evgeny Kapun (abacabadabacaba) Date: 2017-04-13 08:55
Code:

    import asyncio as a
    import socket as s

    @a.coroutine
    def coro():
        s1, s2 = s.socketpair()
        s1.setblocking(False)
        s2.setblocking(False)
        try:
            yield from a.wait_for(loop.sock_recv(s2, 1), 1)
        except a.TimeoutError:
            pass
        yield from loop.sock_sendall(s1, b'\x00')
        yield
        s1.close()
        s2.close()

    loop = a.get_event_loop()
    loop.run_until_complete(coro())

Result:

    Exception in callback BaseSelectorEventLoop._sock_recv(<Future cancelled>, True, <socket.socke...2049, proto=0>, 1)
    handle: <Handle BaseSelectorEventLoop._sock_recv(<Future cancelled>, True, <socket.socke...2049, proto=0>, 1)>
    Traceback (most recent call last):
      File "/usr/lib/python3.6/asyncio/events.py", line 127, in _run
        self._callback(*self._args)
      File "/usr/lib/python3.6/asyncio/selector_events.py", line 378, in _sock_recv
        self.remove_reader(fd)
      File "/usr/lib/python3.6/asyncio/selector_events.py", line 342, in remove_reader
        return self._remove_reader(fd)
      File "/usr/lib/python3.6/asyncio/selector_events.py", line 279, in _remove_reader
        key = self._selector.get_key(fd)
      File "/usr/lib/python3.6/selectors.py", line 189, in get_key
        return mapping[fileobj]
      File "/usr/lib/python3.6/selectors.py", line 70, in __getitem__
        fd = self._selector._fileobj_lookup(fileobj)
      File "/usr/lib/python3.6/selectors.py", line 224, in _fileobj_lookup
        return _fileobj_to_fd(fileobj)
      File "/usr/lib/python3.6/selectors.py", line 41, in _fileobj_to_fd
        raise ValueError("Invalid file descriptor: {}".format(fd))
    ValueError: Invalid file descriptor: -1
msg326294 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-09-24 21:57
See also https://bugs.python.org/issue34795
msg329746 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2018-11-12 17:00
New changeset 74387926072abf338a4c1cec1bf0501fc65bbee5 by Andrew Svetlov in branch 'master':
bpo-30064: Refactor sock_* asyncio API (#10419)
https://github.com/python/cpython/commit/74387926072abf338a4c1cec1bf0501fc65bbee5
msg355272 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2019-10-23 22:42
I think this broke asyncio a bit.  These two uvloop sock_* API tests no longer pass on asyncio 3.8:

* https://github.com/MagicStack/uvloop/blob/master/tests/test_sockets.py#L192

* https://github.com/MagicStack/uvloop/blob/master/tests/test_sockets.py#L238

Andrew, since this was your patch, please take a look.
msg355273 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2019-10-23 22:58
I'll elevate the status so that we don't forget before 3.8.1 is too close
msg357989 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2019-12-07 20:48
> I'll elevate the status so that we don't forget before 3.8.1 is too close

Andrew, Yury: ping. 3.8.1 cutoff is approaching.
msg358011 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2019-12-08 16:37
Not sure if failed uvloop tests are correct.
The scenario is like the following:
1. Suppose we have an unblocking socket connected to peer.
2. Create a task for reading data: 
  task = asyncio.create_task(loop.sock_read(sock, 1))
 Note, the task is not started yet.
3. Just after it, cancel the task without waiting for the actual cancellation
  task.cancel()
4. Finally, read from the socket again:
  data = await loop.sock_read(sock, 1)

If I put context switch (await asyncio.sleep(0)) between 3) and 4) *or* replace direct sock_read() call in 4) with creation a task (data = await asyncio.create_task(loop.sock_read(sock, 1))) the cancellation of former read is performed and test passes.

I very doubt if any sane code is organizing like this test: start delayed reading, cancel it and read again.

The worse, neither previous not current sock_read() implementation doesn't prevent the concurrent reading which basically delivers data in an unpredictable order. Honestly, I'm not sure if we need to provide the concurrency guarantee for such low-level functions.  The code built on top of these low-level primitives should handle the cuncurrent access problem IMHO.
msg358098 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2019-12-09 14:19
Note: this is going to miss Python 3.8.1 as I'm releasing 3.8.1rc1 right now.  Please address this before 3.8.2 (due in February).
msg358108 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2019-12-09 14:59
As I wrote, I don't think that we should fix the code but the failed uvloop test is slightly weird instead.

Maybe I'm wrong, I need to communicate with Yuri anyway.
History
Date User Action Args
2019-12-09 14:59:33asvetlovsetmessages: + msg358108
2019-12-09 14:19:33lukasz.langasetnosy: + lukasz.langa
messages: + msg358098
2019-12-08 16:37:21asvetlovsetmessages: + msg358011
2019-12-07 20:48:28ned.deilysetnosy: + ned.deily
messages: + msg357989
2019-10-23 22:58:45yselivanovsetpriority: normal -> deferred blocker

messages: + msg355273
2019-10-23 22:42:58yselivanovsetstatus: closed -> open
resolution: fixed ->
messages: + msg355272

stage: resolved ->
2018-11-12 17:02:04asvetlovsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2018-11-12 17:00:25asvetlovsetmessages: + msg329746
2018-11-08 22:41:33asvetlovsetkeywords: + patch
stage: patch review
pull_requests: + pull_request9699
2018-10-09 08:57:06asvetlovsetassignee: asvetlov
2018-09-24 21:57:11yselivanovsetnosy: + asvetlov

messages: + msg326294
versions: + Python 3.8, - Python 3.5, Python 3.6
2017-04-13 08:55:09abacabadabacabacreate