bpo-34638: Store a weak reference to stream reader to break strong references loop #9201
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It breaks the strong reference loop between reader and protocol allowing to detect and close the socket if the stream is deleted (garbage collected)
asvetlov
changed the title
1st1
reviewed
Sep 12, 2018
Lib/asyncio/streams.py
Outdated
| context = { | ||
| 'message': ("Close transport. " | ||
| "A stream was destroyed without " | ||
| "stream.close() call") |
There was a problem hiding this comment.
I'd rephrase as:
An open stream is being garbage collected;
call "stream.close()" explicitly.
1st1
reviewed
Sep 12, 2018
Lib/asyncio/streams.py
Outdated
| context = { | ||
| 'message': ("Close transport. " | ||
| "a stream was destroyed " | ||
| "before connection establishment") |
There was a problem hiding this comment.
An open stream was garbage collected prior to
establishing network connection; call "stream.close()" explicitly.
1st1
reviewed
Sep 12, 2018
Lib/asyncio/streams.py
Outdated
| @@ -282,6 +349,9 @@ def can_write_eof(self): | |||
| return self._transport.can_write_eof() | |||
|
|
|||
| def close(self): | |||
| # a reader could be garbage collected / destroyed | |||
| # after connection closing | |||
There was a problem hiding this comment.
# The reader can be safely garbage collected after
# closing the connection.
1st1
reviewed
Sep 12, 2018
Lib/asyncio/streams.py
Outdated
| # server socket | ||
| # we need to keep a strong reference to the reader | ||
| # until connection is made | ||
| self._strong_reader = stream_reader |
There was a problem hiding this comment.
# This is a stream created by the `create_server()` function.
# Keep a strong reference to the reader until a connection
# is established.
|
Notes are fixed |
1st1
reviewed
Sep 12, 2018
Lib/asyncio/subprocess.py
Outdated
| def _untrack_reader(self): | ||
| # a dummy placeholder subprocess protocol doesn't need it but | ||
| # the method is required for regular stream implementation. | ||
| pass |
There was a problem hiding this comment.
`StreamWriter.close()` expects the protocol to have this method defined.
1st1
approved these changes
Sep 12, 2018
seifertm
added a commit
to seifertm/pytest-asyncio
that referenced
this pull request
Mar 11, 2022
When run with Python 3.7 asyncio.subprocess.create_subprocess_exec seems to be affected by an issue that prevents correct cleanup. Tests using pytest-trio will report that signal handling is already performed by another library and fail. [1] This is possibly a bug in CPython 3.7, so we ignore this test for that Python version. CPython 3.7 uses asyncio.streams.StreamReader and asyncio.streams.StreamWriter to implement asyncio.streams.StreamReaderProtocol and asyncio.subprocess.SubprocessStreamProtocol. StreamReaderProtocol contained cyclic references between the reader and the protocol, which prevented garbage collection. While StreamReaderProtocol received a patch [2], SubprocessStreamProtocol, which is used by create_subprocess_exec, possibly has the same problem, but was not patched as part of CPython 3.7. That's why we ignore this test for CPython 3.7. [1] python-trio/pytest-trio#126 [2] python/cpython#9201 Signed-off-by: Michael Seifert <m.seifert@digitalernachschub.de>
seifertm
added a commit
to pytest-dev/pytest-asyncio
that referenced
this pull request
Mar 15, 2022
… in strict mode (#307) * test: Package-scoped event_loop fixture no longer leaks into other tests. The expected behaviour is that the `event_loop` fixture defined in `tests/sessionloop/conftest.py` is torn down when all tests in `tests/sessionloop` are complete. Running the tests with the pytest option --setup-show pointed out that the fixture is torn down at the end of the test session, instead. This is an unintended side effect of the sessionloop test which may affect other tests in the test suite. Reducing the fixture scope from "package" to "module" results in the expected behaviour. The module was renamed to reflect the fact that the tests do not use a session scope. Signed-off-by: Michael Seifert <m.seifert@digitalernachschub.de> * test: Removed test with obsolete "forbid_global_loop". forbid_global_loop was an option to pytest.mark.asyncio which was removed in v0.6.0. The two subprocess tests are otherwise identical. Therefore, one of the tests was removed along with the obsolete option. Signed-off-by: Michael Seifert <m.seifert@digitalernachschub.de> * test: Ignore subprocess tests when running on CPython 3.7. When run with Python 3.7 asyncio.subprocess.create_subprocess_exec seems to be affected by an issue that prevents correct cleanup. Tests using pytest-trio will report that signal handling is already performed by another library and fail. [1] This is possibly a bug in CPython 3.7, so we ignore this test for that Python version. CPython 3.7 uses asyncio.streams.StreamReader and asyncio.streams.StreamWriter to implement asyncio.streams.StreamReaderProtocol and asyncio.subprocess.SubprocessStreamProtocol. StreamReaderProtocol contained cyclic references between the reader and the protocol, which prevented garbage collection. While StreamReaderProtocol received a patch [2], SubprocessStreamProtocol, which is used by create_subprocess_exec, possibly has the same problem, but was not patched as part of CPython 3.7. That's why we ignore this test for CPython 3.7. [1] python-trio/pytest-trio#126 [2] python/cpython#9201 Signed-off-by: Michael Seifert <m.seifert@digitalernachschub.de> * build: Added pytest-trio to the test dependencies. This allows testing compatibility between pytest-trio and pytest-asyncio. Signed-off-by: Michael Seifert <m.seifert@digitalernachschub.de> * fix: Do not try to initialize async fixtures without explicit asyncio mark in strict mode. This fixes a bug that breaks compatibility with pytest_trio. Closes #298 Signed-off-by: Michael Seifert <m.seifert@digitalernachschub.de>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Breaking the strong reference loop between reader and protocol
allows to detect and close the socket if the stream is deleted (garbage collected) without
close()call.https://bugs.python.org/issue34638