net: ignore UV_EALREADY in tryReadStart - #64530
Conversation
|
Review requested:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #64530 +/- ##
==========================================
+ Coverage 90.21% 90.24% +0.02%
==========================================
Files 739 739
Lines 241650 241699 +49
Branches 45542 45544 +2
==========================================
+ Hits 218013 218118 +105
+ Misses 15155 15109 -46
+ Partials 8482 8472 -10
🚀 New features to boost your workflow:
|
Signed-off-by: harjoth <harjoth.khara@gmail.com>
51e7fce to
09c630a
Compare
|
The repro in that issue requires manually patching socket handle internals. Is this actually reachable in practice? In theory I think we already check for Notably there are zero reports of Node issues that mention |
|
You're right. I checked how I looked at every place in core that touches the flag, and that mismatch doesn't seem possible with current socket types. Everywhere that sets So I agree this doesn't look reachable in normal Node code today. It would only show up with a handle type that starts reading on its own, like the AF_CAN work in #62398. Given that, it probably makes more sense to close this and revisit if #62398 lands. Also fine keeping it open until then if you think the guard is worth having. Whatever you prefer. |
Yeah, I'm leaning that way. I'll close this. The code does look correct and it's not totally unreasonable, but for now I don't think we should silently handle this, given that currently it implies a serious internal error somewhere. If we do reach a point in future where there's a good reason why this might happen I'd be happy to reconsider. |
tryReadStart()treated every non-zeroreadStart()result as fatal. Thatincluded
UV_EALREADY, which means the handle is already reading and noadditional action is required.
Ignore
UV_EALREADYwhile preserving the existing socket destruction path forall other read-start errors. Add a loopback regression test that forces this
return value and verifies that the socket remains usable.
Fixes: #62400
Tests:
make -j8tools/test.py test/parallel/test-net-read-start-ealready.jstools/test.py test/parallel/test-net-*.jsmake lint-jsAI assistance: OpenAI Codex assisted with implementation and validation. Local
Claude and Codex review passes reported no actionable findings.