You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Reading received data from a closed TCP stream using StreamReader.read might hang forever #79246
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=Noneclosed_at=<Date2018-11-08.12:22:13.649>created_at=<Date2018-10-25.12:32:25.991>labels= ['type-bug', '3.8', 'expert-asyncio']
title='Reading received data from a closed TCP stream using `StreamReader.read` might hang forever'updated_at=<Date2018-11-08.12:22:13.648>user='https://github.com/vxgmichel'
No, the hang is not intended behavior. Thanks for the report.
I'll assign the issue to myself to don't miss the bug; but if you have a patch with a fix -- please don't hesitate to make a Pull Request for it.
vxgmichel commented on Oct 26, 2018
vxgmichelmannequin
MannequinAuthor
More actions
Hi Andrew!
I reverted the commit associated with the following PR, and the hanging issue disappeared: #9201
The call to _untrack_reader is performed too soon. Closing the transport causes protocol.connection_lost() to be "called soon" but by the time it is actually executed, the stream reader has been "untracked". Since the protocol doesn't know the stream reader anymore, it has not way to feed it the EOF.
The fix attached removes the _untrack_reader call and definition altogether. I don't really see the point of this method since one has to wait for connection_lost to be executed before untracking the reader, but connection_lost already gets rid of the reader reference.
With this fix, calling writer.close then awaiting writer.wait_closed (or awaiting writer.aclose) should:
close the transport
schedule protocol.connection_lost
wait for the protocol to be closed
run protocol.connection_lost
feed the EOF to the reader
set the protocol as closed
get rid of the reader reference in the protocol
return (making aclose causal and safe)
the reader can then be safely garbage collected
But maybe I'm missing something about _untrack_reader?
New changeset fd512d7 by Andrew Svetlov (Vincent Michel) in branch 'master': bpo-35065: Remove StreamReaderProtocol._untrack_reader (bpo-10212) fd512d7
Activity
vxgmichel commentedon Oct 25, 2018
I'm not sure whether it is intended or not, but I noticed a change in the behavior of
StreamReaderbetween version 3.7 and 3.8.Basically, reading some received data from a closed TCP stream using
StreamReader.readmight hang forever, under certain conditions.I'm not sure what those conditions are but I managed to reproduce the issue consistently with the following workflow:
The test attached implements the behavior. It fails on 3.8 but passes on 3.7
asvetlov commentedon Oct 25, 2018
Hi Vincent!
No, the hang is not intended behavior. Thanks for the report.
I'll assign the issue to myself to don't miss the bug; but if you have a patch with a fix -- please don't hesitate to make a Pull Request for it.
vxgmichel commentedon Oct 26, 2018
Hi Andrew!
I reverted the commit associated with the following PR, and the hanging issue disappeared:
#9201
I'll look into it.
vxgmichel commentedon Oct 26, 2018
I found the culprit:
cpython/Lib/asyncio/streams.py
Line 350 in a05bef4
The call to
_untrack_readeris performed too soon. Closing the transport causesprotocol.connection_lost()to be "called soon" but by the time it is actually executed, the stream reader has been "untracked". Since the protocol doesn't know the stream reader anymore, it has not way to feed it the EOF.The fix attached removes the
_untrack_readercall and definition altogether. I don't really see the point of this method since one has to wait forconnection_lostto be executed before untracking the reader, butconnection_lostalready gets rid of the reader reference.With this fix, calling
writer.closethen awaitingwriter.wait_closed(or awaitingwriter.aclose) should:protocol.connection_lostprotocol.connection_lostBut maybe I'm missing something about
_untrack_reader?asvetlov commentedon Nov 8, 2018
New changeset fd512d7 by Andrew Svetlov (Vincent Michel) in branch 'master':
bpo-35065: Remove
StreamReaderProtocol._untrack_reader(bpo-10212)fd512d7