Ensure close and connection_lost do not race in Win32#101
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #101 +/- ##
==========================================
- Coverage 92.20% 92.15% -0.05%
==========================================
Files 22 22
Lines 3654 3660 +6
==========================================
+ Hits 3369 3373 +4
- Misses 285 287 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Aligns the Win32 async transport close lifecycle with the POSIX behavior by ensuring handle-release work completes before protocol.connection_lost() is dispatched, and strengthens lifecycle regression coverage by running the suite under both lazy and eager task execution.
Changes:
- Win32: defer
connection_lostdispatch until after the underlying handle-close completes, stashing the loss reason meanwhile. - Tests: run async lifecycle tests twice (default vs eager task factory) and add a “slow close” regression test to assert
connection_lostdoes not fire before the releasing syscall returns.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
tests/test_async_lifecycle.py |
Adds eager-task parametrization and a slow-close lifecycle regression test. |
serialx/platforms/serial_win32.py |
Reorders Win32 close/loss signaling to avoid races between closing and connection_lost. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The earlier fix we made for POSIX was also required for Win32. I've added a slow close test to catch more errors like this in the future, in addition to running the lifecycle test suite twice: once with eager task execution, once without.