classification
Title: Merge StreamWriter and StreamReader into just asyncio.Stream
Type: Stage: resolved
Components: asyncio Versions: Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Michael.Felt, asvetlov, icgood, koobs, miss-islington, xtreak, yselivanov
Priority: normal Keywords: patch

Created on 2019-05-11 18:15 by asvetlov, last changed 2019-09-30 05:30 by yselivanov. This issue is now closed.

Files
File name Uploaded Description Edit
koobs-freebsd-current-3.x-build-168.stdio.txt koobs, 2019-05-29 03:23
Pull Requests
URL Status Linked Edit
PR 13251 merged asvetlov, 2019-05-11 18:15
PR 13790 merged asvetlov, 2019-06-03 22:36
PR 14203 merged xtreak, 2019-06-18 15:48
PR 14349 merged miss-islington, 2019-06-24 18:17
PR 14397 merged asvetlov, 2019-06-26 15:25
PR 14422 merged miss-islington, 2019-06-27 11:40
PR 14488 merged xtreak, 2019-06-30 17:36
PR 16087 merged miss-islington, 2019-09-13 11:03
PR 16482 closed yselivanov, 2019-09-30 04:17
PR 16485 merged yselivanov, 2019-09-30 05:07
Messages (20)
msg342212 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2019-05-11 18:15
The separation was a bad idea, sorry. Both classes are coupled too much.
1. Writer should know about the reader to make `drain()` work
2. Reader has no .close() method.

The idea is: 
1. Merge StreamReader and StreamWriter into just a Stream.
2. The Stream should support three modes: readonly, writeonly, readwrite.
3. Keep StreamReader/StreamWriter but deprecate these classes.
4. Replace all asyncio internals to use Stream class.
msg343678 - (view) Author: miss-islington (miss-islington) Date: 2019-05-27 19:56
New changeset 23b4b697e5b6cc897696f9c0288c187d2d24bff2 by Miss Islington (bot) (Andrew Svetlov) in branch 'master':
bpo-36889: Merge asyncio streams (GH-13251)
https://github.com/python/cpython/commit/23b4b697e5b6cc897696f9c0288c187d2d24bff2
msg343860 - (view) Author: Kubilay Kocak (koobs) (Python triager) Date: 2019-05-29 03:23
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
msg343877 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2019-05-29 09:53
Thanks for letting me know.
Investigating.
BrokenPipeError is not related, I had a plan to fix such messages during the beta phase.
msg343902 - (view) Author: Karthikeyan Singaravelan (xtreak) * (Python triager) Date: 2019-05-29 17:46
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
----------------------------------------------------------------------
msg343903 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2019-05-29 17:54
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
msg344019 - (view) Author: Kubilay Kocak (koobs) (Python triager) Date: 2019-05-31 04:28
@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
msg344055 - (view) Author: Michael Felt (Michael.Felt) * Date: 2019-05-31 10:12
FYI: since:
commit 23b4b697e5b6cc897696f9c0288c187d2d24bff2
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
msg344057 - (view) Author: Michael Felt (Michael.Felt) * Date: 2019-05-31 10:17
hmm - i had just synced with master. must have just missed something since there is a strike-out through GH-13251. If so, please ignore. BBL.
msg344643 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-04 21:05
Can this issue be closed now?
msg344676 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2019-06-05 06:14
Please keep it until new streams will be documented.
msg346425 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2019-06-24 18:17
New changeset 6793cce155f8875b10efd746cb0b34cb72263af7 by Andrew Svetlov (Xtreak) in branch 'master':
bpo-36889: Document asyncio Stream and StreamServer (GH-14203)
https://github.com/python/cpython/commit/6793cce155f8875b10efd746cb0b34cb72263af7
msg346431 - (view) Author: miss-islington (miss-islington) Date: 2019-06-24 18:33
New changeset 7268fd0d212e870103ae052697b32ba54d3f6299 by Miss Islington (bot) in branch '3.8':
bpo-36889: Document asyncio Stream and StreamServer (GH-14203)
https://github.com/python/cpython/commit/7268fd0d212e870103ae052697b32ba54d3f6299
msg346432 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2019-06-24 18:38
Thanks, Karthikeyan!

The only thing that needs to be documented is Stream class itself.
Would you finish this job, please?
msg346435 - (view) Author: Karthikeyan Singaravelan (xtreak) * (Python triager) Date: 2019-06-24 18:46
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?
msg346498 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2019-06-25 09:31
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.
msg352284 - (view) Author: miss-islington (miss-islington) Date: 2019-09-13 10:52
New changeset d31b31516c71890e8735606aec1dbf2bfb8fd6be by Miss Islington (bot) (Xtreak) in branch 'master':
bpo-36889: Document Stream class and add docstrings (GH-14488)
https://github.com/python/cpython/commit/d31b31516c71890e8735606aec1dbf2bfb8fd6be
msg352289 - (view) Author: miss-islington (miss-islington) Date: 2019-09-13 11:23
New changeset b9bfe143d151d184615fa3890f78874c5d4ed4c6 by Miss Islington (bot) in branch '3.8':
bpo-36889: Document Stream class and add docstrings (GH-14488)
https://github.com/python/cpython/commit/b9bfe143d151d184615fa3890f78874c5d4ed4c6
msg353535 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2019-09-30 04:59
New changeset 6758e6e12a71ef5530146161881f88df1fa43382 by Yury Selivanov in branch 'master':
bpo-38242: Revert "bpo-36889: Merge asyncio streams (GH-13251)" (#16482)
https://github.com/python/cpython/commit/6758e6e12a71ef5530146161881f88df1fa43382
msg353538 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2019-09-30 05:30
New changeset 1c19d656a79a00f58361ceb61c0a6d1faf90c686 by Yury Selivanov in branch '3.8':
bpo-38242: Revert "bpo-36889: Merge asyncio streams (GH-13251)" (#16482) (#16485)
https://github.com/python/cpython/commit/1c19d656a79a00f58361ceb61c0a6d1faf90c686
History
Date User Action Args
2019-09-30 05:30:20yselivanovsetmessages: + msg353538
2019-09-30 05:07:49yselivanovsetpull_requests: + pull_request16071
2019-09-30 04:59:58yselivanovsetmessages: + msg353535
2019-09-30 04:17:30yselivanovsetpull_requests: + pull_request16067
2019-09-13 11:32:11asvetlovsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2019-09-13 11:23:51miss-islingtonsetmessages: + msg352289
2019-09-13 11:03:18miss-islingtonsetpull_requests: + pull_request15708
2019-09-13 10:52:41miss-islingtonsetmessages: + msg352284
2019-09-11 17:54:00xtreaklinkissue34634 superseder
2019-09-01 13:33:35THRlWiTisetnosy: - THRlWiTi
2019-09-01 13:26:16THRlWiTisetnosy: + THRlWiTi
2019-06-30 17:36:22xtreaksetpull_requests: + pull_request14306
2019-06-27 14:26:35vstinnersetnosy: - vstinner
2019-06-27 11:40:07miss-islingtonsetpull_requests: + pull_request14238
2019-06-26 15:25:17asvetlovsetpull_requests: + pull_request14211
2019-06-25 09:31:25asvetlovsetmessages: + msg346498
2019-06-24 18:46:21xtreaksetmessages: + msg346435
2019-06-24 18:38:19asvetlovsetmessages: + msg346432
2019-06-24 18:33:14miss-islingtonsetmessages: + msg346431
2019-06-24 18:17:13miss-islingtonsetpull_requests: + pull_request14168
2019-06-24 18:17:05asvetlovsetmessages: + msg346425
2019-06-18 15:48:02xtreaksetpull_requests: + pull_request14041
2019-06-05 06:14:36asvetlovsetmessages: + msg344676
2019-06-04 21:05:39vstinnersetnosy: + vstinner
messages: + msg344643
2019-06-03 22:36:47asvetlovsetstage: resolved -> patch review
pull_requests: + pull_request13674
2019-05-31 10:17:36Michael.Feltsetmessages: + msg344057
2019-05-31 10:12:13Michael.Feltsetnosy: + Michael.Felt
messages: + msg344055
2019-05-31 04:28:54koobssetmessages: + msg344019
2019-05-29 17:54:54asvetlovsetmessages: + msg343903
2019-05-29 17:46:52xtreaksetnosy: + xtreak
messages: + msg343902
2019-05-29 09:53:30asvetlovsetmessages: + msg343877
2019-05-29 03:23:41koobssetstatus: closed -> open
files: + koobs-freebsd-current-3.x-build-168.stdio.txt

nosy: + koobs
messages: + msg343860

resolution: fixed -> (no value)
2019-05-27 20:25:11asvetlovsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2019-05-27 20:14:40asvetlovlinkissue34655 superseder
2019-05-27 20:01:30asvetlovlinkissue36840 superseder
2019-05-27 19:59:57asvetlovlinkissue34975 superseder
2019-05-27 19:56:27miss-islingtonsetnosy: + miss-islington
messages: + msg343678
2019-05-17 02:26:04icgoodsetnosy: + icgood
2019-05-11 18:15:46asvetlovsetkeywords: + patch
stage: patch review
pull_requests: + pull_request13162
2019-05-11 18:15:14asvetlovcreate