Title: BaseSelectorEventLoop.sock_{recv,sendall}() don't remove their callbacks when canceled
Type: behavior Stage:
Components: asyncio Versions: Python 3.8
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

    import asyncio as a
    import socket as s

    def coro():
        s1, s2 = s.socketpair()
            yield from a.wait_for(loop.sock_recv(s2, 1), 1)
        except a.TimeoutError:
        yield from loop.sock_sendall(s1, b'\x00')

    loop = a.get_event_loop()


    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/", line 127, in _run
      File "/usr/lib/python3.6/asyncio/", line 378, in _sock_recv
      File "/usr/lib/python3.6/asyncio/", line 342, in remove_reader
        return self._remove_reader(fd)
      File "/usr/lib/python3.6/asyncio/", line 279, in _remove_reader
        key = self._selector.get_key(fd)
      File "/usr/lib/python3.6/", line 189, in get_key
        return mapping[fileobj]
      File "/usr/lib/python3.6/", line 70, in __getitem__
        fd = self._selector._fileobj_lookup(fileobj)
      File "/usr/lib/python3.6/", line 224, in _fileobj_lookup
        return _fileobj_to_fd(fileobj)
      File "/usr/lib/python3.6/", 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
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)
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:



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
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.
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