[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

Change by STINNER Victor : -- nosy: -vstinner ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

Kyle Stanley added the comment: Thanks again for working on this @Fantix, and for the continued vigilance on the issue after the test failures occurred in the buildbots. I think this recent PR-20868 will do the trick. At the very least, it will make the failure much more rare than before,

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

miss-islington added the comment: New changeset 83cd968f5cc59ad60b665080087f50fefac17a07 by Miss Islington (bot) in branch '3.9': bpo-30064: Fix slow asyncio sock test (GH-20868) https://github.com/python/cpython/commit/83cd968f5cc59ad60b665080087f50fefac17a07 --

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

Change by miss-islington : -- pull_requests: +20058 pull_request: https://github.com/python/cpython/pull/20869 ___ Python tracker ___

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

miss-islington added the comment: New changeset 8f04a84755babe516ebb5304904ea7c15b865c80 by Fantix King in branch 'master': bpo-30064: Fix slow asyncio sock test (GH-20868) https://github.com/python/cpython/commit/8f04a84755babe516ebb5304904ea7c15b865c80 --

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

Change by Fantix King : -- pull_requests: +20057 pull_request: https://github.com/python/cpython/pull/20868 ___ Python tracker ___

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

Kyle Stanley added the comment: > Last question before PR pls: given the fact that this test is to cover a > fixed case when loop.sock_*() was hanging forever, should I keep the > wait_for(..., timeout=10)? For now, I think we can keep the asyncio.wait_for() w/ 10 the sec timeout. I'll

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

Fantix King added the comment: and yes, updated the SO_SNDBUF too -- ___ Python tracker ___ ___ Python-bugs-list mailing list

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

Fantix King added the comment: OK I think I solved the problem by improving the two loops using smarter buffer sizes: # fill the buffer until sending 5 chars would block size = 8192 while size > 5: with self.assertRaises(BlockingIOError):

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

Kyle Stanley added the comment: No problem at all, and thanks for clarifying. > If enlarging the timeout works in some of your cases, that really inspires me > on a potential cause of the issue in the test. To simulate the race condition > specifically for loop.sock_sendall(), I needed a

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

Fantix King added the comment: Oh thank you for the clue, that's super helpful! Let me try locally. This fix PR was originally done to solve a hanging call in a race condition, so the test would hang forever if the fix is not working as expected. The 10 seconds wait_for() is to make sure

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

Kyle Stanley added the comment: I believe that I might have identified a simple fix for the issue, but first I'd like to clarify on something: What exactly is the reason that we need to use asyncio.wait_for() with a timeout of 10 seconds in these tests? We typically avoid using short

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

Kyle Stanley added the comment: I was able to replicate the failure locally by running the large number of jobs in parallel. We typically use this to test for tricky race conditions, to simulate maximum load: [aeros:~/repos/cpython]$ ./python -m test test_asyncio.test_sock_lowlevel --match

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

Fantix King added the comment: I've been testing on local VMs with Gentoo and Ubuntu, but the issue cannot be reproduced. The error log is not really revealing anything useful, other than that the send was blocked when it was supposed to be successful. As it is not constantly failing on

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

Fantix King added the comment: I'm checking now -- ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

STINNER Victor added the comment: Another kind of failure on x86 Gentoo Installed with X 3.x: https://buildbot.python.org/all/#/builders/128/builds/1030 ERROR: test_sock_client_racing (test.test_asyncio.test_sock_lowlevel.EPollEventLoopTests)

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

miss-islington added the comment: New changeset 6637bd45163024f2187e40d2cc12c473f78bf5da by Miss Islington (bot) in branch '3.9': bpo-30064: Properly skip unstable loop.sock_connect() racing test (GH-20494) https://github.com/python/cpython/commit/6637bd45163024f2187e40d2cc12c473f78bf5da

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

Change by miss-islington : -- pull_requests: +19749 pull_request: https://github.com/python/cpython/pull/20503 ___ Python tracker ___

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

Yury Selivanov added the comment: New changeset dc4eee9e266267498a6b783a0abccc23c06f2b87 by Fantix King in branch 'master': bpo-30064: Properly skip unstable loop.sock_connect() racing test (GH-20494) https://github.com/python/cpython/commit/dc4eee9e266267498a6b783a0abccc23c06f2b87

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

Fantix King added the comment: Thanks for the comments! Added PR 20494 to properly fix/skip the test for (hopefully) all platforms. -- ___ Python tracker ___

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

Change by Fantix King : -- pull_requests: +19743 pull_request: https://github.com/python/cpython/pull/20494 ___ Python tracker ___

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

STINNER Victor added the comment: > bpo-30064: Fix unstable asyncio "racing" socket tests (GH-20485) Sorry. You should read "Skip" tests, not "Fix" tests :-p -- ___ Python tracker

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

miss-islington added the comment: New changeset 1d82f003678816ff8dd822452ec91669844d2d09 by Miss Islington (bot) in branch '3.9': bpo-30064: Fix unstable asyncio "racing" socket tests (GH-20485) https://github.com/python/cpython/commit/1d82f003678816ff8dd822452ec91669844d2d09 --

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

Change by miss-islington : -- pull_requests: +19736 pull_request: https://github.com/python/cpython/pull/20487 ___ Python tracker ___

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

STINNER Victor added the comment: New changeset 84ee7e1573d166fe7a9be676813e12523b62ab24 by Victor Stinner in branch 'master': bpo-30064: Fix unstable asyncio "racing" socket tests (GH-20485) https://github.com/python/cpython/commit/84ee7e1573d166fe7a9be676813e12523b62ab24 --

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

STINNER Victor added the comment: I created https://github.com/python/cpython/pull/20485 to skip the unstable tests until someone fix them. IMO it's better to skip the test than reverting the fix, since the fix does fix a race condition. The test is still failing on buildbots which makes

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

Change by STINNER Victor : -- pull_requests: +19734 pull_request: https://github.com/python/cpython/pull/20485 ___ Python tracker ___

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

Kyle Stanley added the comment: > The new test_sock_client_racing() test seems to have... a race condition... I'm also a bit skeptical about relying on `time.sleep(0.01)` at the end of the loop in `_basetest_sock_recv_racing()`, but I don't have an idea for a better approach at the moment.

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

Fantix King added the comment: Not a simple one - FreeBSD is returning ECONNREFUSED immediately when trying to connect to a non-listening port on the loopback address: https://lists.freebsd.org/pipermail/freebsd-current/2005-May/049876.html Then all following connect attempts on the same

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

Fantix King added the comment: Ouch ... looks like FreeBSD also needs a few more retries than a single retry. I'll test on a FreeBSD and create a PR for that. -- ___ Python tracker

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

STINNER Victor added the comment: The new test_sock_client_racing() test seems to have... a race condition... AMD64 FreeBSD Non-Debug 3.9: https://buildbot.python.org/all/#/builders/750/builds/11 ERROR: test_sock_client_racing (test.test_asyncio.test_sock_lowlevel.KqueueEventLoopTests)

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

miss-islington added the comment: New changeset 3a2667d91e33493ccde113ddf5092afefc3c89fa by Miss Islington (bot) in branch '3.9': bpo-30064: Fix asyncio loop.sock_* race condition issue (GH-20369) https://github.com/python/cpython/commit/3a2667d91e33493ccde113ddf5092afefc3c89fa --

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

Change by miss-islington : -- nosy: +miss-islington nosy_count: 8.0 -> 9.0 pull_requests: +19713 pull_request: https://github.com/python/cpython/pull/20460 ___ Python tracker

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

Yury Selivanov added the comment: New changeset 210a137396979d747c2602eeef46c34fc4955448 by Fantix King in branch 'master': bpo-30064: Fix asyncio loop.sock_* race condition issue (#20369) https://github.com/python/cpython/commit/210a137396979d747c2602eeef46c34fc4955448 --

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

Change by Fantix King : -- pull_requests: +19633 stage: -> patch review pull_request: https://github.com/python/cpython/pull/20369 ___ Python tracker ___

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

Change by Dong-hee Na : -- nosy: +aeros ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

Joongi Kim added the comment: And I suspect that this issue is something simliar to what I did in a recent janus PR: https://github.com/aio-libs/janus/blob/ec8592b91254971473b508313fb91b01623f13d7/janus/__init__.py#L84 to give a chance for specific callbacks to execute via an extra context

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

Joongi Kim added the comment: I just encountered this issue when doing "sys.exit(1)" on a Click-based CLI program that internally uses asyncio event loop via wrapped via a context manager, on Python 3.8.2. Using uvloop or adding "time.sleep(0.1)" before "sys.exit(1)" removes the error.

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

Fantix King added the comment: Following the discussion - looks like `add_reader()` will cancel the previous `handle` on the same `fd` if found, so I think it is reasonable to solve this cancellation-race issue by checking if the `handle` is cancelled as shown in the attached patch (tests

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

Andrew Svetlov added the comment: This is a very interesting question. In asyncio, task cancellation is not executed on `task.cancel()` call immediately but the task is *marked* for cancellation. The actual cancellation requires a context switch (await asyncio.sleep(0) or similar) to

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

Yury Selivanov added the comment: > I very doubt if any sane code is organizing like this test: start delayed > reading, cancel it and read again. Hm, cancellation should work correctly no matter how "sane" or "insane" the user code is. > The worse, neither previous not current sock_read()

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

Łukasz Langa added the comment: Downgrading priority on this one. -- priority: deferred blocker -> high ___ Python tracker ___ ___

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

Andrew Svetlov added the comment: 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. -- ___ Python tracker

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

Łukasz Langa added the comment: 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). -- nosy: +lukasz.langa ___ Python tracker

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

Andrew Svetlov added the comment: 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

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

Ned Deily added the comment: > 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. -- nosy: +ned.deily ___ Python tracker

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

Yury Selivanov added the comment: I'll elevate the status so that we don't forget before 3.8.1 is too close -- priority: normal -> deferred blocker ___ Python tracker ___

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

Yury Selivanov added the comment: 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

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

Change by Andrew Svetlov : -- resolution: -> fixed stage: patch review -> resolved status: open -> closed ___ Python tracker ___

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

Andrew Svetlov added the comment: New changeset 74387926072abf338a4c1cec1bf0501fc65bbee5 by Andrew Svetlov in branch 'master': bpo-30064: Refactor sock_* asyncio API (#10419) https://github.com/python/cpython/commit/74387926072abf338a4c1cec1bf0501fc65bbee5 --

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

Change by Andrew Svetlov : -- keywords: +patch pull_requests: +9699 stage: -> patch review ___ Python tracker ___ ___

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

Change by Andrew Svetlov : -- assignee: -> asvetlov ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

Yury Selivanov added the comment: See also https://bugs.python.org/issue34795 -- nosy: +asvetlov versions: +Python 3.8 -Python 3.5, Python 3.6 ___ Python tracker ___

[issue30064] BaseSelectorEventLoop.sock_{recv, sendall}() don't remove their callbacks when canceled

New submission from Evgeny Kapun: 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)