From 4ca239a799db44a4f890df1a0ae8e147ffb6ef16 Mon Sep 17 00:00:00 2001 From: Matthias Urlichs Date: Wed, 18 Jan 2023 20:51:03 +0100 Subject: [PATCH 1/4] ConflictDetector: kill both tasks It's not always obvious which task is the "bad guy" when a conflict occurs. If the "legitimate" task crashes and the buggy one, i.e. the one that forgot to take the lock, does not, the backtrace is not helpful. Thus, kill them both. --- trio/_util.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/trio/_util.py b/trio/_util.py index b60e0104e8..ffe45949d3 100644 --- a/trio/_util.py +++ b/trio/_util.py @@ -180,15 +180,20 @@ class ConflictDetector: def __init__(self, msg): self._msg = msg self._held = False - + self._conflicted = False + def __enter__(self): if self._held: + self._conflicted = True raise trio.BusyResourceError(self._msg) else: self._held = True - + def __exit__(self, *args): self._held = False + if self._conflicted: + self._conflicted = False + raise trio.BusyResourceError(self._msg) def async_wraps(cls, wrapped_cls, attr_name): From 39d0f6fff1cc5575d5a78dfea66fa345c5f85f5b Mon Sep 17 00:00:00 2001 From: Matthias Urlichs Date: Wed, 18 Jan 2023 20:54:57 +0100 Subject: [PATCH 2/4] add newsfragment --- newsfragments/2540.feature.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 newsfragments/2540.feature.rst diff --git a/newsfragments/2540.feature.rst b/newsfragments/2540.feature.rst new file mode 100644 index 0000000000..4b7bb537ba --- /dev/null +++ b/newsfragments/2540.feature.rst @@ -0,0 +1 @@ +ConflictDetector now kills all tasks involved in a conflict. From 44f655975ff979fc0c7421b2d76149e7498ad03e Mon Sep 17 00:00:00 2001 From: Matthias Urlichs Date: Wed, 18 Jan 2023 21:16:28 +0100 Subject: [PATCH 3/4] Test fixups --- trio/testing/_check_streams.py | 12 ++++++------ trio/tests/test_signals.py | 2 +- trio/tests/test_ssl.py | 16 ++++++++-------- trio/tests/test_testing.py | 6 +++--- trio/tests/test_util.py | 2 +- 5 files changed, 19 insertions(+), 19 deletions(-) diff --git a/trio/testing/_check_streams.py b/trio/testing/_check_streams.py index 0206f1f737..211a8e3dce 100644 --- a/trio/testing/_check_streams.py +++ b/trio/testing/_check_streams.py @@ -113,7 +113,7 @@ async def send_empty_then_y(): nursery.start_soon(do_send_all, b"x") assert await do_receive_some(None) == b"x" - with _assert_raises(_core.BusyResourceError): + with _assert_raises((_core.BusyResourceError, _core._multierror.NonBaseMultiError)): async with _core.open_nursery() as nursery: nursery.start_soon(do_receive_some, 1) nursery.start_soon(do_receive_some, 1) @@ -307,7 +307,7 @@ async def receiver(): async with _ForceCloseBoth(await clogged_stream_maker()) as (s, r): # simultaneous wait_send_all_might_not_block fails - with _assert_raises(_core.BusyResourceError): + with _assert_raises(_core._multierror.NonBaseMultiError): async with _core.open_nursery() as nursery: nursery.start_soon(s.wait_send_all_might_not_block) nursery.start_soon(s.wait_send_all_might_not_block) @@ -316,7 +316,7 @@ async def receiver(): # this test might destroy the stream b/c we end up cancelling # send_all and e.g. SSLStream can't handle that, so we have to # recreate afterwards) - with _assert_raises(_core.BusyResourceError): + with _assert_raises(_core._multierror.NonBaseMultiError): async with _core.open_nursery() as nursery: nursery.start_soon(s.wait_send_all_might_not_block) nursery.start_soon(s.send_all, b"123") @@ -324,7 +324,7 @@ async def receiver(): async with _ForceCloseBoth(await clogged_stream_maker()) as (s, r): # send_all and send_all blocked simultaneously should also raise # (but again this might destroy the stream) - with _assert_raises(_core.BusyResourceError): + with _assert_raises(_core._multierror.NonBaseMultiError): async with _core.open_nursery() as nursery: nursery.start_soon(s.send_all, b"123") nursery.start_soon(s.send_all, b"123") @@ -496,7 +496,7 @@ async def expect_x_then_eof(r): if clogged_stream_maker is not None: async with _ForceCloseBoth(await clogged_stream_maker()) as (s1, s2): # send_all and send_eof simultaneously is not ok - with _assert_raises(_core.BusyResourceError): + with _assert_raises((_core.BusyResourceError, _core._multierror.NonBaseMultiError)): async with _core.open_nursery() as nursery: nursery.start_soon(s1.send_all, b"x") await _core.wait_all_tasks_blocked() @@ -505,7 +505,7 @@ async def expect_x_then_eof(r): async with _ForceCloseBoth(await clogged_stream_maker()) as (s1, s2): # wait_send_all_might_not_block and send_eof simultaneously is not # ok either - with _assert_raises(_core.BusyResourceError): + with _assert_raises((_core.BusyResourceError, _core._multierror.NonBaseMultiError)): async with _core.open_nursery() as nursery: nursery.start_soon(s1.wait_send_all_might_not_block) await _core.wait_all_tasks_blocked() diff --git a/trio/tests/test_signals.py b/trio/tests/test_signals.py index 235772f900..992350f4e8 100644 --- a/trio/tests/test_signals.py +++ b/trio/tests/test_signals.py @@ -65,7 +65,7 @@ async def naughty(): async def test_open_signal_receiver_conflict(): - with pytest.raises(trio.BusyResourceError): + with pytest.raises(_core._multierror.NonBaseMultiError): with open_signal_receiver(signal.SIGILL) as receiver: async with trio.open_nursery() as nursery: nursery.start_soon(receiver.__anext__) diff --git a/trio/tests/test_ssl.py b/trio/tests/test_ssl.py index 26e107e08f..ab5b211218 100644 --- a/trio/tests/test_ssl.py +++ b/trio/tests/test_ssl.py @@ -354,28 +354,28 @@ async def test_PyOpenSSLEchoStream_gives_resource_busy_errors(): # PyOpenSSLEchoStream will notice and complain. s = PyOpenSSLEchoStream() - with pytest.raises(_core.BusyResourceError) as excinfo: + with pytest.raises(_core._multierror.NonBaseMultiError) as excinfo: async with _core.open_nursery() as nursery: nursery.start_soon(s.send_all, b"x") nursery.start_soon(s.send_all, b"x") assert "simultaneous" in str(excinfo.value) s = PyOpenSSLEchoStream() - with pytest.raises(_core.BusyResourceError) as excinfo: + with pytest.raises(_core._multierror.NonBaseMultiError) as excinfo: async with _core.open_nursery() as nursery: nursery.start_soon(s.send_all, b"x") nursery.start_soon(s.wait_send_all_might_not_block) assert "simultaneous" in str(excinfo.value) s = PyOpenSSLEchoStream() - with pytest.raises(_core.BusyResourceError) as excinfo: + with pytest.raises(_core._multierror.NonBaseMultiError) as excinfo: async with _core.open_nursery() as nursery: nursery.start_soon(s.wait_send_all_might_not_block) nursery.start_soon(s.wait_send_all_might_not_block) assert "simultaneous" in str(excinfo.value) s = PyOpenSSLEchoStream() - with pytest.raises(_core.BusyResourceError) as excinfo: + with pytest.raises(_core._multierror.NonBaseMultiError) as excinfo: async with _core.open_nursery() as nursery: nursery.start_soon(s.receive_some, 1) nursery.start_soon(s.receive_some, 1) @@ -732,28 +732,28 @@ async def do_wait_send_all_might_not_block(): await s.wait_send_all_might_not_block() s, _ = ssl_lockstep_stream_pair(client_ctx) - with pytest.raises(_core.BusyResourceError) as excinfo: + with pytest.raises(_core._multierror.NonBaseMultiError) as excinfo: async with _core.open_nursery() as nursery: nursery.start_soon(do_send_all) nursery.start_soon(do_send_all) assert "another task" in str(excinfo.value) s, _ = ssl_lockstep_stream_pair(client_ctx) - with pytest.raises(_core.BusyResourceError) as excinfo: + with pytest.raises(_core._multierror.NonBaseMultiError) as excinfo: async with _core.open_nursery() as nursery: nursery.start_soon(do_receive_some) nursery.start_soon(do_receive_some) assert "another task" in str(excinfo.value) s, _ = ssl_lockstep_stream_pair(client_ctx) - with pytest.raises(_core.BusyResourceError) as excinfo: + with pytest.raises(_core._multierror.NonBaseMultiError) as excinfo: async with _core.open_nursery() as nursery: nursery.start_soon(do_send_all) nursery.start_soon(do_wait_send_all_might_not_block) assert "another task" in str(excinfo.value) s, _ = ssl_lockstep_stream_pair(client_ctx) - with pytest.raises(_core.BusyResourceError) as excinfo: + with pytest.raises(_core._multierror.NonBaseMultiError) as excinfo: async with _core.open_nursery() as nursery: nursery.start_soon(do_wait_send_all_might_not_block) nursery.start_soon(do_wait_send_all_might_not_block) diff --git a/trio/tests/test_testing.py b/trio/tests/test_testing.py index a2dba728d5..3a9e2f6ef6 100644 --- a/trio/tests/test_testing.py +++ b/trio/tests/test_testing.py @@ -288,7 +288,7 @@ async def getter(expect): nursery.start_soon(putter, b"xyz") # Two gets at the same time -> BusyResourceError - with pytest.raises(_core.BusyResourceError): + with pytest.raises(_core._multierror.NonBaseMultiError): async with _core.open_nursery() as nursery: nursery.start_soon(getter, b"asdf") nursery.start_soon(getter, b"asdf") @@ -359,7 +359,7 @@ async def do_send_all_count_resourcebusy(): nursery.start_soon(do_send_all_count_resourcebusy) nursery.start_soon(do_send_all_count_resourcebusy) - assert resource_busy_count == 1 + assert resource_busy_count == 2 with assert_checkpoints(): await mss.aclose() @@ -422,7 +422,7 @@ async def do_receive_some(max_bytes): mrs.put_data(b"abc") assert await do_receive_some(None) == b"abc" - with pytest.raises(_core.BusyResourceError): + with pytest.raises(_core._multierror.NonBaseMultiError): async with _core.open_nursery() as nursery: nursery.start_soon(do_receive_some, 10) nursery.start_soon(do_receive_some, 10) diff --git a/trio/tests/test_util.py b/trio/tests/test_util.py index 15ab09a80b..dc212777c5 100644 --- a/trio/tests/test_util.py +++ b/trio/tests/test_util.py @@ -53,7 +53,7 @@ async def wait_with_ul1(): with ul1: await wait_all_tasks_blocked() - with pytest.raises(_core.BusyResourceError) as excinfo: + with pytest.raises(_core._multierror.NonBaseMultiError) as excinfo: async with _core.open_nursery() as nursery: nursery.start_soon(wait_with_ul1) nursery.start_soon(wait_with_ul1) From 6adbaf0dfd23ea88bae1c6b75eeaec1b0599aae3 Mon Sep 17 00:00:00 2001 From: Matthias Urlichs Date: Wed, 18 Jan 2023 21:38:23 +0100 Subject: [PATCH 4/4] Fix formatting --- trio/_util.py | 4 ++-- trio/testing/_check_streams.py | 12 +++++++++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/trio/_util.py b/trio/_util.py index ffe45949d3..eb29badbf6 100644 --- a/trio/_util.py +++ b/trio/_util.py @@ -181,14 +181,14 @@ def __init__(self, msg): self._msg = msg self._held = False self._conflicted = False - + def __enter__(self): if self._held: self._conflicted = True raise trio.BusyResourceError(self._msg) else: self._held = True - + def __exit__(self, *args): self._held = False if self._conflicted: diff --git a/trio/testing/_check_streams.py b/trio/testing/_check_streams.py index 211a8e3dce..3e5abcc2c9 100644 --- a/trio/testing/_check_streams.py +++ b/trio/testing/_check_streams.py @@ -113,7 +113,9 @@ async def send_empty_then_y(): nursery.start_soon(do_send_all, b"x") assert await do_receive_some(None) == b"x" - with _assert_raises((_core.BusyResourceError, _core._multierror.NonBaseMultiError)): + with _assert_raises( + (_core.BusyResourceError, _core._multierror.NonBaseMultiError) + ): async with _core.open_nursery() as nursery: nursery.start_soon(do_receive_some, 1) nursery.start_soon(do_receive_some, 1) @@ -496,7 +498,9 @@ async def expect_x_then_eof(r): if clogged_stream_maker is not None: async with _ForceCloseBoth(await clogged_stream_maker()) as (s1, s2): # send_all and send_eof simultaneously is not ok - with _assert_raises((_core.BusyResourceError, _core._multierror.NonBaseMultiError)): + with _assert_raises( + (_core.BusyResourceError, _core._multierror.NonBaseMultiError) + ): async with _core.open_nursery() as nursery: nursery.start_soon(s1.send_all, b"x") await _core.wait_all_tasks_blocked() @@ -505,7 +509,9 @@ async def expect_x_then_eof(r): async with _ForceCloseBoth(await clogged_stream_maker()) as (s1, s2): # wait_send_all_might_not_block and send_eof simultaneously is not # ok either - with _assert_raises((_core.BusyResourceError, _core._multierror.NonBaseMultiError)): + with _assert_raises( + (_core.BusyResourceError, _core._multierror.NonBaseMultiError) + ): async with _core.open_nursery() as nursery: nursery.start_soon(s1.wait_send_all_might_not_block) await _core.wait_all_tasks_blocked()