Merge StreamWriter and StreamReader into just asyncio.Stream #81070

asvetlov · 2019-05-11T18:15:14Z

BPO	36889
Nosy	@asvetlov, @1st1, @koobs, @aixtools, @icgood, @miss-islington, @tirkarthi
PRs	bpo-36889: Merge asyncio streams #13251 bpo-36889: Make StreamServer.close() tests more robust #13790 bpo-36889: Document Stream and StreamServer. #14203 [3.8] bpo-36889: Document asyncio Stream and StreamServer (GH-14203) #14349 bpo-36889: Replace deprecation warning with RuntimeError #14397 [3.8] bpo-36889: Replace deprecation warning with RuntimeError (GH-14397) #14422 bpo-36889: Document Stream class and add docstrings #14488 [3.8] bpo-36889: Document Stream class and add docstrings (GH-14488) #16087 bpo-38242: Revert "bpo-36889: Merge asyncio streams (GH-13251)" #16482 [3.8] bpo-38242: Revert "bpo-36889: Merge asyncio streams (GH-13251)" (#16482) #16485
Files	koobs-freebsd-current-3.x-build-168.stdio.txt

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2019-09-13.11:32:11.525>
created_at = <Date 2019-05-11.18:15:14.485>
labels = ['3.8', 'expert-asyncio']
title = 'Merge StreamWriter and StreamReader into just asyncio.Stream'
updated_at = <Date 2019-09-30.05:30:20.394>
user = 'https://github.com/asvetlov'

bugs.python.org fields:

activity = <Date 2019-09-30.05:30:20.394>
actor = 'yselivanov'
assignee = 'none'
closed = True
closed_date = <Date 2019-09-13.11:32:11.525>
closer = 'asvetlov'
components = ['asyncio']
creation = <Date 2019-05-11.18:15:14.485>
creator = 'asvetlov'
dependencies = []
files = ['48376']
hgrepos = []
issue_num = 36889
keywords = ['patch']
message_count = 20.0
messages = ['342212', '343678', '343860', '343877', '343902', '343903', '344019', '344055', '344057', '344643', '344676', '346425', '346431', '346432', '346435', '346498', '352284', '352289', '353535', '353538']
nosy_count = 7.0
nosy_names = ['asvetlov', 'yselivanov', 'koobs', 'Michael.Felt', 'icgood', 'miss-islington', 'xtreak']
pr_nums = ['13251', '13790', '14203', '14349', '14397', '14422', '14488', '16087', '16482', '16485']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue36889'
versions = ['Python 3.8']

asvetlov · 2019-05-11T18:15:14Z

The separation was a bad idea, sorry. Both classes are coupled too much.

Writer should know about the reader to make drain() work
Reader has no .close() method.

The idea is:

Merge StreamReader and StreamWriter into just a Stream.
The Stream should support three modes: readonly, writeonly, readwrite.
Keep StreamReader/StreamWriter but deprecate these classes.
Replace all asyncio internals to use Stream class.

miss-islington · 2019-05-27T19:56:28Z

New changeset 23b4b69 by Miss Islington (bot) (Andrew Svetlov) in branch 'master':
bpo-36889: Merge asyncio streams (GH-13251)
23b4b69

koobs · 2019-05-29T03:23:39Z

Seeing a test_ayncio failure on koobs-freebsd-current:

Timeout (0:25:00)!
Thread 0x0000000800abb000 (most recent call first):
File "/usr/home/buildbot/python/3.x.koobs-freebsd-current/build/Lib/selectors.py", line 558 in select
File "/usr/home/buildbot/python/3.x.koobs-freebsd-current/build/Lib/asyncio/base_events.py", line 1813 in _run_once
File "/usr/home/buildbot/python/3.x.koobs-freebsd-current/build/Lib/asyncio/base_events.py", line 563 in run_forever
File "/usr/home/buildbot/python/3.x.koobs-freebsd-current/build/Lib/asyncio/base_events.py", line 595 in run_until_complete
File "/usr/home/buildbot/python/3.x.koobs-freebsd-current/build/Lib/test/test_asyncio/test_streams.py", line 1531 in test_stream_server_abort

Some errors (warnings?) later in the re-run (which passes), may be related/indicative:

Future exception was never retrieved
future: <Future finished exception=BrokenPipeError()>
Traceback (most recent call last):
  File "/usr/home/buildbot/python/3.x.koobs-freebsd-current/build/Lib/asyncio/subprocess.py", line 173, in _feed_stdin
    await self.stdin.drain()
  File "/usr/home/buildbot/python/3.x.koobs-freebsd-current/build/Lib/asyncio/streams.py", line 1415, in drain
    await self._protocol._drain_helper()
  File "/usr/home/buildbot/python/3.x.koobs-freebsd-current/build/Lib/asyncio/streams.py", line 594, in _drain_helper
    await waiter
BrokenPipeError

Full log attached

asvetlov · 2019-05-29T09:53:31Z

Thanks for letting me know.
Investigating.
BrokenPipeError is not related, I had a plan to fix such messages during the beta phase.

tirkarthi · 2019-05-29T17:46:52Z

test_stream_shutdown_hung_task_prevents_cancellation was added as part of this issue. Seems to be a random error : https://ci.appveyor.com/project/python/cpython/builds/24901585

======================================================================
ERROR: test_stream_shutdown_hung_task_prevents_cancellation (test.test_asyncio.test_streams.StreamTests)
----------------------------------------------------------------------

Traceback (most recent call last):
  File "C:\projects\cpython\lib\asyncio\windows_events.py", line 453, in finish_recv
    return ov.getresult()
OSError: [WinError 64] The specified network name is no longer available
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
  File "C:\projects\cpython\lib\test\test_asyncio\test_streams.py", line 1605, in test_stream_shutdown_hung_task_prevents_cancellation
    self.loop.run_until_complete(test())
  File "C:\projects\cpython\lib\asyncio\base_events.py", line 608, in run_until_complete
    return future.result()
  File "C:\projects\cpython\lib\test\test_asyncio\test_streams.py", line 1601, in test
    await task
  File "C:\projects\cpython\lib\test\test_asyncio\test_streams.py", line 1586, in client
    self.assertEqual(b'', await stream.readline())
  File "C:\projects\cpython\lib\asyncio\streams.py", line 1545, in readline
    line = await self.readuntil(sep)
  File "C:\projects\cpython\lib\asyncio\streams.py", line 1638, in readuntil
    await self._wait_for_data('readuntil')
  File "C:\projects\cpython\lib\asyncio\streams.py", line 1521, in _wait_for_data
    await self._waiter
  File "C:\projects\cpython\lib\asyncio\proactor_events.py", line 279, in _loop_reading
    data = fut.result()
  File "C:\projects\cpython\lib\asyncio\windows_events.py", line 808, in _poll
    value = callback(transferred, key, ov)
  File "C:\projects\cpython\lib\asyncio\windows_events.py", line 457, in finish_recv
    raise ConnectionResetError(*exc.args)
ConnectionResetError: [WinError 64] The specified network name is no longer available

asvetlov · 2019-05-29T17:54:54Z

Wow!
Thanks for the report.
Internally it is pretty close to freebsd problem.
The test is unstable, not asyncio code itself.
The test uses future objects to synchronize client and server socket processing but looks like there are race conditions still.

Please let me work on it.
If the problem is very annoying I can temporarily disable these problematic tests while working on their improvements

koobs · 2019-05-31T04:28:54Z

@andrew That would be great, as having failing buildbots can hide new regressions that end up taking much longer to identify and fix as they are hidden, and is better for you than reverting implementation code

aixtools · 2019-05-31T10:12:13Z

FYI: since:
commit 23b4b69
Author: Andrew Svetlov <andrew.svetlov@gmail.com>
Date: Mon May 27 22:56:22 2019 +0300

bpo-36889: Merge asyncio streams (GH-13251)



https://bugs.python.org/issue36889

AIX bot 'hangs' during
test_sendfile (test.test_asyncio.test_streams.StreamTests)

ctrl-c gives this (in case useful)
Warning -- 'test_asyncio' left behind file '@test_7012552_tmp'
Warning -- asyncio.events._event_loop_policy was modified by test_asyncio
Before: None
After: <asyncio.unix_events._UnixDefaultEventLoopPolicy object at 0x307f8aa8>

== Tests result: INTERRUPTED ==
Test suite interrupted by signal SIGINT.

1 test omitted:
test_asyncio

Total duration: 13 min 36 sec
Tests result: INTERRUPTED
/data/prj/python/git/python3-3.8/Lib/asyncio/base_events.py:646: ResourceWarning: unclosed event loop <_UnixSelectorEventLoop running=False closed=False debug=False>
  _warn(f"unclosed event loop {self!r}", ResourceWarning, source=self)
ResourceWarning: Enable tracemalloc to get the object allocation traceback
Exception ignored in: <coroutine object StreamTests.test_sendfile.<locals>.test at 0x30576ed0>
Traceback (most recent call last):
  File "/data/prj/python/git/python3-3.8/Lib/test/test_asyncio/test_streams.py", line 1643, in test
    await do_connect(*srv.sockets[0].getsockname())
  File "/data/prj/python/git/python3-3.8/Lib/asyncio/streams.py", line 314, in __aexit__
    await self.close()
  File "/data/prj/python/git/python3-3.8/Lib/asyncio/streams.py", line 292, in close
    await tasks.wait([stream.close() for stream in streams])
  File "/data/prj/python/git/python3-3.8/Lib/asyncio/streams.py", line 292, in <listcomp>
    await tasks.wait([stream.close() for stream in streams])
  File "/data/prj/python/git/python3-3.8/Lib/asyncio/streams.py", line 1382, in close
    self._transport.close()
  File "/data/prj/python/git/python3-3.8/Lib/asyncio/selector_events.py", line 680, in close
    self._loop.call_soon(self._call_connection_lost, None)
  File "/data/prj/python/git/python3-3.8/Lib/asyncio/base_events.py", line 711, in call_soon
    self._check_closed()
  File "/data/prj/python/git/python3-3.8/Lib/asyncio/base_events.py", line 504, in _check_closed
    raise RuntimeError('Event loop is closed')
RuntimeError: Event loop is closed
/data/prj/python/git/python3-3.8/Lib/asyncio/streams.py:352: ResourceWarning: unclosed stream server <StreamServer>
  _warn(f"unclosed stream server {self!r}",
ResourceWarning: Enable tracemalloc to get the object allocation traceback
/data/prj/python/git/python3-3.8/Lib/asyncio/selector_events.py:684: ResourceWarning: unclosed transport <_SelectorSocketTransport fd=7>
  _warn(f"unclosed transport {self!r}", ResourceWarning, source=self)
ResourceWarning: Enable tracemalloc to get the object allocation traceback
/data/prj/python/git/python3-3.8/Lib/asyncio/selector_events.py:684: ResourceWarning: unclosed transport <_SelectorSocketTransport closing fd=8>
  _warn(f"unclosed transport {self!r}", ResourceWarning, source=self)
ResourceWarning: Enable tracemalloc to get the object allocation traceback

aixtools · 2019-05-31T10:17:37Z

hmm - i had just synced with master. must have just missed something since there is a strike-out through #57460. If so, please ignore. BBL.

vstinner · 2019-06-04T21:05:39Z

Can this issue be closed now?

asvetlov · 2019-06-05T06:14:37Z

Please keep it until new streams will be documented.

asvetlov · 2019-06-24T18:17:05Z

New changeset 6793cce by Andrew Svetlov (Xtreak) in branch 'master':
bpo-36889: Document asyncio Stream and StreamServer (GH-14203)
6793cce

miss-islington · 2019-06-24T18:33:15Z

New changeset 7268fd0 by Miss Islington (bot) in branch '3.8':
bpo-36889: Document asyncio Stream and StreamServer (GH-14203)
7268fd0

asvetlov · 2019-06-24T18:38:20Z

Thanks, Karthikeyan!

The only thing that needs to be documented is Stream class itself.
Would you finish this job, please?

tirkarthi · 2019-06-24T18:46:21Z

Sure, I think I missed it somehow though you mentioned it's around combining StreamReader and StreamWriter and to have appropriate signatures for the methods. I will try to raise a PR this week. I also realize there are no docstrings for the new functions like connect, connect_read_pipe, connect_write_pipe etc. Is it worthy enough of adding them too in the PR?

asvetlov · 2019-06-25T09:31:26Z

Yes, Stream doc should be a combination of StreamReader and StreamWriter with small changes/additions.
StreamReader and StreamWriter should be kept in the doc.

Regarding docstrings: there is no strong requirement (we don't provide docstrings for all asyncio public API) but please don't hesitate to write them.

miss-islington · 2019-09-13T10:52:41Z

New changeset d31b315 by Miss Islington (bot) (Xtreak) in branch 'master':
bpo-36889: Document Stream class and add docstrings (GH-14488)
d31b315

miss-islington · 2019-09-13T11:23:52Z

New changeset b9bfe14 by Miss Islington (bot) in branch '3.8':
bpo-36889: Document Stream class and add docstrings (GH-14488)
b9bfe14

1st1 · 2019-09-30T04:59:58Z

New changeset 6758e6e by Yury Selivanov in branch 'master':
bpo-38242: Revert "bpo-36889: Merge asyncio streams (GH-13251)" (bpo-16482)
6758e6e

1st1 · 2019-09-30T05:30:20Z

New changeset 1c19d65 by Yury Selivanov in branch '3.8':
bpo-38242: Revert "bpo-36889: Merge asyncio streams (GH-13251)" (bpo-16482) (bpo-16485)
1c19d65

asvetlov added 3.8 only security fixes topic-asyncio labels May 11, 2019

asvetlov closed this as completed May 27, 2019

koobs reopened this May 29, 2019

asvetlov closed this as completed Sep 13, 2019

ezio-melotti transferred this issue from another repository Apr 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Merge StreamWriter and StreamReader into just asyncio.Stream #81070

Merge StreamWriter and StreamReader into just asyncio.Stream #81070

asvetlov commented May 11, 2019

asvetlov commented May 11, 2019

miss-islington commented May 27, 2019

koobs commented May 29, 2019

asvetlov commented May 29, 2019

tirkarthi commented May 29, 2019

asvetlov commented May 29, 2019

koobs commented May 31, 2019

aixtools commented May 31, 2019

aixtools commented May 31, 2019

vstinner commented Jun 4, 2019

asvetlov commented Jun 5, 2019

asvetlov commented Jun 24, 2019

miss-islington commented Jun 24, 2019

asvetlov commented Jun 24, 2019

tirkarthi commented Jun 24, 2019

asvetlov commented Jun 25, 2019

miss-islington commented Sep 13, 2019

miss-islington commented Sep 13, 2019

1st1 commented Sep 30, 2019

1st1 commented Sep 30, 2019

Merge StreamWriter and StreamReader into just asyncio.Stream #81070

Merge StreamWriter and StreamReader into just asyncio.Stream #81070

Comments

asvetlov commented May 11, 2019

asvetlov commented May 11, 2019

miss-islington commented May 27, 2019

koobs commented May 29, 2019

asvetlov commented May 29, 2019

tirkarthi commented May 29, 2019

asvetlov commented May 29, 2019

koobs commented May 31, 2019

aixtools commented May 31, 2019

aixtools commented May 31, 2019

vstinner commented Jun 4, 2019

asvetlov commented Jun 5, 2019

asvetlov commented Jun 24, 2019

miss-islington commented Jun 24, 2019

asvetlov commented Jun 24, 2019

tirkarthi commented Jun 24, 2019

asvetlov commented Jun 25, 2019

miss-islington commented Sep 13, 2019

miss-islington commented Sep 13, 2019

1st1 commented Sep 30, 2019

1st1 commented Sep 30, 2019