Issue 31647: asyncio: StreamWriter write_eof() after close raises mysterious AttributeError
Created on 2017-09-30 04:42 by twisteroid ambassador, last changed 2022-04-11 14:58 by admin. This issue is now closed.
Messages (15)
msg303391 - (view)
Author: twisteroid ambassador (twisteroid ambassador) *
Date: 2017-09-30 04:42
Date: 2017-10-16 07:51
Date: 2018-05-25 19:43
Date: 2018-05-28 15:16
Date: 2018-05-28 16:04
Date: 2018-05-28 16:17
Date: 2018-05-28 20:25
Date: 2018-05-28 21:57
Date: 2018-06-27 14:58
Date: 2018-06-27 22:03
Date: 2018-06-28 08:05
Currently, if one attempts to do write_eof() on a StreamWriter after the underlying transport is already closed, an AttributeError is raised:
Traceback (most recent call last):
File "<snip>\scratch_3.py", line 34, in main_coro
writer.write_eof()
File "C:\Program Files\Python36\lib\asyncio\streams.py", line 300, in write_eof
return self._transport.write_eof()
File "C:\Program Files\Python36\lib\asyncio\selector_events.py", line 808, in write_eof
self._sock.shutdown(socket.SHUT_WR)
AttributeError: 'NoneType' object has no attribute 'shutdown'
This is because _SelectorSocketTransport.write_eof() only checks for self._eof before calling self._sock.shutdown(), and self._sock has already been assigned None after _call_connection_lost().
Compare with StreamWriter.write() after close, which either does nothing or logs a warning after 5 attempts; or StreamWriter.drain() after close, which raises a ConnectionResetError; or even StreamWriter.close() after close, which does nothing.
Trying to do write_eof() after close may happen unintentionally, for example when the following sequence of events happen:
* the remote side closes the connection
* the local side attempts to write, so the socket "figures out" the connection is closed, and close this side of the socket. Note the write fails silently, except when loop.set_debug(True) where asyncio logs "Fatal write error on socket transport".
* the local side does write_eof(). An AttributError is raised.
Currently the only way to handle this gracefully is to either catch AttributeError or check StreamWriter.transport.is_closing() before write_eof(). Neither is pretty.
I suggest making write_eof() after close either do nothing, or raise a subclass of ConnectionError. Both will be easier to handle then the current behavior.
Attached repro.
msg303392 - (view)
Author: twisteroid ambassador (twisteroid ambassador) *
Date: 2017-09-30 04:48
This issue is somewhat related to issue27223, in that both are caused by using self._sock after it has already been assigned None when the connection is closed. It seems like Transports in general may need better protection from this kind of behavior.msg304459 - (view) Author: STINNER Victor (vstinner) *
Date: 2017-10-16 07:51
(Sorry, I don't work on asyncio anymore.)msg317707 - (view) Author: Yury Selivanov (yselivanov) *
Date: 2018-05-25 19:43
If this issue isn't yet fixed could you please submit a PR?msg317838 - (view) Author: twisteroid ambassador (twisteroid ambassador) * Date: 2018-05-28 11:02
I was about to write a long comment asking what the appropriate behavior should be, but then saw that _ProactorSocketTransport already handles the same issue properly, so I will just change _SelectorSocketTransport to do the same thing.msg317866 - (view) Author: Yury Selivanov (yselivanov) *
Date: 2018-05-28 15:16
New changeset 23f587e395e41bd5e116312b036183f42bc4159b by Yury Selivanov (twisteroid ambassador) in branch 'master': bpo-31647: Fix write_eof() after close() for SelectorSocketTransport (GH-7149) https://github.com/python/cpython/commit/23f587e395e41bd5e116312b036183f42bc4159bmsg317877 - (view) Author: Yury Selivanov (yselivanov) *
Date: 2018-05-28 16:04
New changeset 1f21ae710d83a37c872355612b58958cef4d5f95 by Yury Selivanov (Miss Islington (bot)) in branch '3.7': bpo-31647: Fix write_eof() after close() for SelectorSocketTransport (GH-7149) (GH-7153) https://github.com/python/cpython/commit/1f21ae710d83a37c872355612b58958cef4d5f95msg317879 - (view) Author: Yury Selivanov (yselivanov) *
Date: 2018-05-28 16:17
Merged, thanks for looking into this! BTW, if you have any other bugfixes in mind, today is pretty much last chance to get them into 3.7.0msg317914 - (view) Author: Yury Selivanov (yselivanov) *
Date: 2018-05-28 20:25
New changeset 7e8819a589c7630db9d440826143ad0a1daf948e by Yury Selivanov (Miss Islington (bot)) in branch '3.6': bpo-31647: Fix write_eof() after close() for SelectorSocketTransport (GH-7149) (#7154) https://github.com/python/cpython/commit/7e8819a589c7630db9d440826143ad0a1daf948emsg317931 - (view) Author: Yury Selivanov (yselivanov) *
Date: 2018-05-28 21:57
Would be great to have this change in 3.7.0 (it's safe to merge it IMO)msg320572 - (view) Author: twisteroid ambassador (twisteroid ambassador) * Date: 2018-06-27 11:23
Turns out my typo when preparing the pull request had another victim: the changelog entries in documentation currently links to the wrong issue. I'll make a PR to fix that typo; since it's just documentation, hopefully it can still get into Python 3.6.6 and 3.7.0.msg320574 - (view) Author: twisteroid ambassador (twisteroid ambassador) * Date: 2018-06-27 12:35
Well, I opened the PR, it shows up here, but there's no reviewer assigned.msg320591 - (view) Author: Ned Deily (ned.deily) *
Date: 2018-06-27 14:58
New changeset 4a8b037d2b9c7199890bc5a01b3a40687a5ce2b1 by Ned Deily (twisteroid ambassador) in branch 'master': bpo-31647: Fix bpo typo in NEWS entry. (GH-7964) https://github.com/python/cpython/commit/4a8b037d2b9c7199890bc5a01b3a40687a5ce2b1msg320631 - (view) Author: Ned Deily (ned.deily) *
Date: 2018-06-27 22:03
New changeset 28c6b94f26661aa6f2bf7d885332c66c85ae4c95 by Ned Deily in branch '3.6': Fix NEWS entry for bpo-31647 https://github.com/python/cpython/commit/28c6b94f26661aa6f2bf7d885332c66c85ae4c95msg320654 - (view) Author: Ned Deily (ned.deily) *
Date: 2018-06-28 08:05
New changeset 0c5918f6f4f113e0c245feac31d00ded29985cce by Ned Deily in branch '3.7': Fix NEWS entry for bpo-31647 https://github.com/python/cpython/commit/0c5918f6f4f113e0c245feac31d00ded29985cce
History
Date
User
Action
Args
2022-04-11 14:58:52adminsetnosy:
+ lukasz.langa
github: 75828
2018-06-28 08:05:30ned.deilysetmessages: + msg320654 2018-06-27 22:03:55ned.deilysetmessages: + msg320631 2018-06-27 14:58:18ned.deilysetmessages: + msg320591 2018-06-27 12:35:28twisteroid ambassadorsetmessages: + msg320574 2018-06-27 11:27:01twisteroid ambassadorsetpull_requests: + pull_request7571 2018-06-27 11:23:02twisteroid ambassadorsetmessages: + msg320572 2018-05-28 21:57:46yselivanovsetpriority: normal -> release blocker
nosy: + ned.deily, larry
messages: + msg317931 2018-05-28 20:25:05yselivanovsetmessages: + msg317914 2018-05-28 16:17:48yselivanovsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved 2018-05-28 16:17:37yselivanovsetmessages: + msg317879 2018-05-28 16:04:11yselivanovsetmessages: + msg317877 2018-05-28 15:18:55miss-islingtonsetpull_requests: + pull_request6790 2018-05-28 15:17:56miss-islingtonsetpull_requests: + pull_request6789 2018-05-28 15:16:47yselivanovsetmessages: + msg317866 2018-05-28 13:23:27twisteroid ambassadorsetkeywords: + patch
stage: patch review
pull_requests: + pull_request6785 2018-05-28 11:02:50twisteroid ambassadorsetmessages: + msg317838 2018-05-25 19:43:11yselivanovsetmessages: + msg317707 2017-10-16 07:52:18vstinnersetnosy: - vstinner
2017-10-16 07:51:58vstinnersetnosy: vstinner, giampaolo.rodola, yselivanov, twisteroid ambassador
messages: + msg304459 2017-10-16 07:08:56twisteroid ambassadorsetnosy: + vstinner, giampaolo.rodola
2017-09-30 04:48:30twisteroid ambassadorsetmessages: + msg303392 2017-09-30 04:42:43twisteroid ambassadorcreate
github: 75828
2018-06-28 08:05:30ned.deilysetmessages: + msg320654 2018-06-27 22:03:55ned.deilysetmessages: + msg320631 2018-06-27 14:58:18ned.deilysetmessages: + msg320591 2018-06-27 12:35:28twisteroid ambassadorsetmessages: + msg320574 2018-06-27 11:27:01twisteroid ambassadorsetpull_requests: + pull_request7571 2018-06-27 11:23:02twisteroid ambassadorsetmessages: + msg320572 2018-05-28 21:57:46yselivanovsetpriority: normal -> release blocker
nosy: + ned.deily, larry
messages: + msg317931 2018-05-28 20:25:05yselivanovsetmessages: + msg317914 2018-05-28 16:17:48yselivanovsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved 2018-05-28 16:17:37yselivanovsetmessages: + msg317879 2018-05-28 16:04:11yselivanovsetmessages: + msg317877 2018-05-28 15:18:55miss-islingtonsetpull_requests: + pull_request6790 2018-05-28 15:17:56miss-islingtonsetpull_requests: + pull_request6789 2018-05-28 15:16:47yselivanovsetmessages: + msg317866 2018-05-28 13:23:27twisteroid ambassadorsetkeywords: + patch
stage: patch review
pull_requests: + pull_request6785 2018-05-28 11:02:50twisteroid ambassadorsetmessages: + msg317838 2018-05-25 19:43:11yselivanovsetmessages: + msg317707 2017-10-16 07:52:18vstinnersetnosy: - vstinner
2017-10-16 07:51:58vstinnersetnosy: vstinner, giampaolo.rodola, yselivanov, twisteroid ambassador
messages: + msg304459 2017-10-16 07:08:56twisteroid ambassadorsetnosy: + vstinner, giampaolo.rodola
2017-09-30 04:48:30twisteroid ambassadorsetmessages: + msg303392 2017-09-30 04:42:43twisteroid ambassadorcreate