fix(stdio): handle ObjectDisposedException from disposed NetworkStream during domain reload#1251
Conversation
…m during domain reload During domain reload, Stop() closes all active TcpClient connections. Any pending ReadAsync/WriteAsync on the disposed NetworkStream throws ObjectDisposedException. The existing top-level catch in HandleClientAsync already treats this as benign (ex is ObjectDisposedException), but the I/O methods should also handle it defensively so the exception type is consistent throughout the stack. Fix: - In ReadExactAsync: catch ObjectDisposedException and throw IOException with a clear 'Connection closed' message — the standard .NET pattern. - In WriteFrameAsync: wrap the WriteAsync calls in try/catch for ObjectDisposedException, same treatment. This ensures the disposed-stream scenario is handled uniformly as an IOException at every level, not just one catch block. Fixes CoplayDev#1232
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough
ChangesDisposed Connection Error Handling
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
Suggested labels: bug, transport Suggested reviewers: none 🐰 A stream once closed, an object gone, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Problem
After domain reload or compilation,
StdioBridgeHostlogs an error:Cannot access a disposed object. Object name: 'System.Net.Sockets.NetworkStream'. While theHandleClientAsyncouter catch already treatsObjectDisposedExceptionas benign, the I/O methods (ReadExactAsync,WriteFrameAsync) throw it raw, making the code depend on a single catch block for correctness.Fixes #1232
Root Cause
During domain reload,
Stop()closes all activeTcpClientconnections viac.Close(). Any pending async read/write on the underlyingNetworkStreamthrowsObjectDisposedException. While this is caught and treated as benign at theHandleClientAsyncouter catch, the exception type is inconsistent —ReadExactAsyncandWriteFrameAsyncthrowObjectDisposedExceptionwhile every other connection-closed scenario throwsIOException.Solution
Two defensive changes at the I/O boundary:
ReadExactAsync: Addedcatch (ObjectDisposedException)that throwsIOException("Connection closed before reading expected bytes")— matching the existing pattern whenstream.ReadAsyncreturns 0 bytes.WriteFrameAsync: Wrapped theWriteAsynccalls in a try/catch that convertsObjectDisposedExceptiontoIOException("Connection closed before writing frame").This follows the standard .NET pattern where disposed network resources throw
IOExceptionrather thanObjectDisposedException, and ensures consistent handling at every level of the stack.Verification
|| ex is IOException) now catches disposed-stream scenarios uniformly — previously it only caughtObjectDisposedExceptionwhich was fragile.Notes
WriteFrameAsynctry/catch adds a small indentation change for the preprocessor conditional body, but no logic changes beyond the new catch block.Summary by CodeRabbit