Conversation
This currently deadlocks
this is nothing compared to what we have a few lines above
| We listen to all future messages from this Comm. | ||
| """ | ||
| assert client is not None | ||
| assert client not in self.clients, f"Client {client!r} already exists" |
There was a problem hiding this comment.
The changes from fb8f1c9 are a driveby (@graingert and I discussed how the current logic is messed up in potentially allowing the wrong client batchedsend to be deleted and closed). I'm happy to pull them out to a separate PR.
and try to print their tracebacks
| def teardown(self): | ||
| self.bcomm.close() | ||
| async def close(self) -> None: | ||
| await self.bcomm.close() |
There was a problem hiding this comment.
I didn't do anything about my concern in #6272 (comment) because I don't know if it's valid or not
Unit Test Results 15 files - 1 15 suites - 1 7h 5m 29s ⏱️ - 18m 38s For more details on these failures, see this check. Results for commit d49d5ca. ± Comparison against base commit 9f02e7a. ♻️ This comment has been updated with latest results. |
Introduced in dask#6146. We don't know why this code path is only being triggered now.
|
This is currently plagued by a few reliably failing tests: I can't reproduce these failures locally. Additionally, when I restrict CI to only run these tests, they pass: https://github.com/dask/distributed/runs/6428523276?check_suite_focus=true Interestingly, |
|
Update: the |
This reverts commit 641f975.
|
Thanks to 40e37fa, we can now at least see that the asyncio.Task being leaked from |
|
This should be no longer relevant after #6361 |
This is #6272, with my PR comments addressed.
Closes #5481, #6228, #5480
pre-commit run --all-files