bpo-47015: Update test_os from asyncore to asyncio

[arhadthedev]

PEP 594 – Removing dead batteries from the standard library removes asyncore and asynchat in 3.12 with the following note:

The asyncore module is also used in stdlib tests. The tests for ftplib, logging, smptd, smtplib, and ssl are partly based on asyncore. These tests must be updated to use asyncio or threading.

However, the citation misses the tests for os. So this PR ports test_os.TestSendfile as the only class that uses asyncore to create a dummy sendfile server and communicate with it.

To simplify the review I broke all changes into a few commits to get readable diffs:

1. Preparation:
   1. Change inheritance of the test from TestCase to IsolatedAsyncioTestCase
   2. Make os.sendfile async-friendly by moving it into a worker thread to not block an event loop of IsolatedAsyncioTestCase. As a result, sendfile_wrapper that called it is also made async
   3. Explicitly specify a global event loop policy because an implicit initialization
      by IsolatedAsyncioTestCase is counted as a test failure (this commit is actually the last)
2. Replace SendfileTestServer with asyncio.start_server. The client side is left mostly untouched because os.sendfile works directly with a low-level socket ignoring asyncio message loop anyway. "Mostly" because connect is delegated to an event loop to not block the server in the same thread
3. Cleanup:
   1. Convert server-side buffer population from a loop into a list comprehension to be more Pythonic
   2. Remove redundant waiting of the client for 220 ready. Instead, we use a guarantee that asyncio.loop.sock_connect yields only after establishing a connection
   3. Remove SendfileTestServer with related asyncio imports
   4. Remove threading leftovers (asyncio.to_thread maintains its own threading machinery)

https://bugs.python.org/issue47015

[arhadthedev]

Note to myself: despite asyncio.start_server promises to be IPv4-first if family is undefined:

cpython/Lib/asyncio/base_events.py

Line 976 in 9523c0d

async def create_connection(

calls

cpython/Lib/asyncio/base_events.py

Lines 1034 to 1036 in 9523c0d

	infos = await self._ensure_resolved(
	(host, port), family=family,
	type=socket.SOCK_STREAM, proto=proto, flags=flags, loop=self)

that calls

cpython/Lib/asyncio/base_events.py

Line 1400 in 9523c0d

info = _ipaddr_info(host, port, family, type, proto, *address[2:])

that has the following lines:

cpython/Lib/asyncio/base_events.py

Lines 129 to 134 in 9523c0d

	if family == socket.AF_UNSPEC:
	afs = [socket.AF_INET]
	if _HAS_IPv6:
	afs.append(socket.AF_INET6)
	else:
	afs = [family]

, it creates an IPv6-only listening socket if possible. As a result, not only server.sockets[0].getsockname() returns an IPv6 address, the whole server refuses to accept IPv4 client connections. It took me two force-pushes to find out, so my apologises about flukes in the notifications.

Now, I'm sorting out what and why leaks all connected sockets (as Address sanitizer reports) failing all non-Windows runners:

/home/runner/work/cpython/cpython/Lib/unittest/async_case.py:134: ResourceWarning: unclosed <socket.socket fd=8, family=2, type=1, proto=6, laddr=('127.0.0.1', 32881), raddr=('127.0.0.1', 57190)>

/home/runner/work/cpython/cpython/Lib/unittest/async_case.py:134: ResourceWarning: unclosed <socket.socket fd=10, family=2, type=1, proto=6, laddr=('127.0.0.1', 36545), raddr=('127.0.0.1', 42800)>

/home/runner/work/cpython/cpython/Lib/unittest/async_case.py:134: ResourceWarning: unclosed <socket.socket fd=12, family=2, type=1, proto=6, laddr=('127.0.0.1', 35363), raddr=('127.0.0.1', 49040)>

/home/runner/work/cpython/cpython/Lib/unittest/async_case.py:134: ResourceWarning: unclosed <socket.socket fd=14, family=2, type=1, proto=6, laddr=('127.0.0.1', 39391), raddr=('127.0.0.1', 41928)>

/home/runner/work/cpython/cpython/Lib/unittest/async_case.py:134: ResourceWarning: unclosed <socket.socket fd=16, family=2, type=1, proto=6, laddr=('127.0.0.1', 42821), raddr=('127.0.0.1', 42452)>

[arhadthedev]

The failure was a combination of client-side blocking wait of a server and a missing asyncio loop policy. Both problems are fixed.

[bedevere-bot]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

And if you don't make the requested changes, you will be poked with soft cushions!

[merwok]

The line that set a loop policy is not present any more, so this cleanup step doesn’t seem to apply anymore.

[arhadthedev]

If I understand a failed pre-policy run correctly, the policy must either be manually set to something before any case is runned or be returned back to None before the suit exits. I've chosen the first variant, Andrew proposed to switch to the second.

0:04:29 load avg: 3.56 [300/433/1] test_os failed (env changed) -- running: test_asyncio (1 min 10 sec), test_concurrent_futures (1 min 57 sec)

[a bunch of oks and skips follow]

[...]

423 tests OK.

10 slowest tests:
- test_tools: 4 min 44 sec
- test_concurrent_futures: 3 min 30 sec
- test_peg_generator: 2 min 57 sec
- test_multiprocessing_spawn: 2 min 27 sec
- test_gdb: 1 min 51 sec
- test_asyncio: 1 min 50 sec
- test_multiprocessing_forkserver: 1 min 38 sec
- test_multiprocessing_fork: 1 min 22 sec
- test_regrtest: 1 min 12 sec
- test_statistics: 49.1 sec

1 test altered the execution environment:
    test_os

9 tests skipped:
    test_devpoll test_ioctl test_kqueue test_msilib test_startfile
    test_winconsoleio test_winreg test_winsound test_zipfile64

Total duration: 11 min 50 sec
Tests result: ENV CHANGED
make: *** [Makefile:1795: buildbottest] Error 3
Error: Process completed with exit code 2.

[bedevere-bot]

🤖 New build scheduled with the buildbot fleet by @asvetlov for commit 005ad9a 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

[asvetlov]

LGTM. Let's wait for the buildbots fleet to finish to make sure that everything works smoothly.
Network code can lead to subtle test problems.

[arhadthedev]

@asvetlov I fixed a forgotten self found by AMD64 FreeBSD.

The second runner, ARM64 macOS, failed on test_asyncio that is unrelated:

======================================================================
ERROR: test_get_cancelled (test.test_asyncio.test_queues.QueueGetTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/buildbot/buildarea/pull_request.pablogsal-macos-m1.macos-with-brew/build/Lib/asyncio/tasks.py", line 490, in wait_for
    return fut.result()
           ^^^^^^^^^^^^
  File "/Users/buildbot/buildarea/pull_request.pablogsal-macos-m1.macos-with-brew/build/Lib/asyncio/queues.py", line 158, in get
    await getter
    ^^^^^^^^^^^^
asyncio.exceptions.CancelledError

Network code can lead to subtle test problems.

Considering that the failed test_flags was marked as @requires_headers_trailers (not sys.platform.startswith("linux") and not sys.platform.startswith("solaris") and not sys.platform.startswith("sunos")), GitHub runners had no chance.

[arhadthedev]

GitHub Ubuntu runner failed with Error: Cache service responded with 400 during upload chunk; I reopened the PR to retrigger the workflow.

[asvetlov]

I see. Test failures seem unrelated.
Let's run the buildbot fleet again.

[bedevere-bot]

🤖 New build scheduled with the buildbot fleet by @asvetlov for commit 03cae53 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

[asvetlov]

Merged. Thanks!