asyncio: StreamWriter write_eof() after close raises mysterious AttributeError

[twisteroidambassador]

BPO	31647
Nosy	@larryhastings, @giampaolo, @ned-deily, @1st1, @twisteroidambassador
PRs	bpo-31647: Fix write_eof() after close() for SelectorSocketTransport #7149 [3.7] bpo-31647: Fix write_eof() after close() for SelectorSocketTransport (GH-7149) #7153 [3.6] bpo-31647: Fix write_eof() after close() for SelectorSocketTransport (GH-7149) #7154 bpo-31647: fix typo in blurb's bpo number #7964
Files	asyncio_write_eof_after_close_test.py

^{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 2018-05-28.16:17:48.716>
created_at = <Date 2017-09-30.04:42:43.335>
labels = ['3.8', 'type-bug', '3.7', 'release-blocker', 'expert-asyncio']
title = 'asyncio: StreamWriter write_eof() after close raises mysterious AttributeError'
updated_at = <Date 2018-06-28.08:05:30.146>
user = 'https://github.com/twisteroidambassador'

bugs.python.org fields:

activity = <Date 2018-06-28.08:05:30.146>
actor = 'ned.deily'
assignee = 'none'
closed = True
closed_date = <Date 2018-05-28.16:17:48.716>
closer = 'yselivanov'
components = ['asyncio']
creation = <Date 2017-09-30.04:42:43.335>
creator = 'twisteroid ambassador'
dependencies = []
files = ['47180']
hgrepos = []
issue_num = 31647
keywords = ['patch']
message_count = 15.0
messages = ['303391', '303392', '304459', '317707', '317838', '317866', '317877', '317879', '317914', '317931', '320572', '320574', '320591', '320631', '320654']
nosy_count = 5.0
nosy_names = ['larry', 'giampaolo.rodola', 'ned.deily', 'yselivanov', 'twisteroid ambassador']
pr_nums = ['7149', '7153', '7154', '7964']
priority = 'release blocker'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue31647'
versions = ['Python 3.4', 'Python 3.5', 'Python 3.6', 'Python 3.7', 'Python 3.8']

[twisteroidambassador]

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.

[twisteroidambassador]

This issue is somewhat related to bpo-27223, 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.

[vstinner]

(Sorry, I don't work on asyncio anymore.)

[1st1]

If this issue isn't yet fixed could you please submit a PR?

[twisteroidambassador]

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.

[1st1]

New changeset 23f587e by Yury Selivanov (twisteroid ambassador) in branch 'master':
bpo-31647: Fix write_eof() after close() for SelectorSocketTransport (GH-7149)
23f587e

[1st1]

New changeset 1f21ae7 by Yury Selivanov (Miss Islington (bot)) in branch '3.7':
bpo-31647: Fix write_eof() after close() for SelectorSocketTransport (GH-7149) (GH-7153)
1f21ae7

[1st1]

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.0

[1st1]

New changeset 7e8819a by Yury Selivanov (Miss Islington (bot)) in branch '3.6':
bpo-31647: Fix write_eof() after close() for SelectorSocketTransport (GH-7149) (bpo-7154)
7e8819a

[1st1]

Would be great to have this change in 3.7.0 (it's safe to merge it IMO)

[twisteroidambassador]

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.

[twisteroidambassador]

Well, I opened the PR, it shows up here, but there's no reviewer assigned.

[ned-deily]

New changeset 4a8b037 by Ned Deily (twisteroid ambassador) in branch 'master':
bpo-31647: Fix bpo typo in NEWS entry. (GH-7964)
4a8b037

[ned-deily]

New changeset 28c6b94 by Ned Deily in branch '3.6':
Fix NEWS entry for bpo-31647
28c6b94

[ned-deily]

New changeset 0c5918f by Ned Deily in branch '3.7':
Fix NEWS entry for bpo-31647
0c5918f