Jens-G commented on PR #3930: URL: https://github.com/apache/thrift/pull/3930#issuecomment-5847070927
Follow-up to the review above: I have now also run the stalled-body check on the .NET Framework, and there the new read timeout does not work. `ConnectTimeout` no longer bounds a stalled response body on that runtime, on the default path and on the per-call path. The probe (200, `Content-Length: 64`, 8 body bytes, then silence; `ConnectTimeout = 1000`) ran as a net48 program against the netstandard2.0 build, on .NET Framework 4.8 (4.8.3761) under Wine 10: - on master, `FlushAsync` fails with `TTransportException` (`Interrupted`) after about 1 s; - at 12b9ce621, `FlushAsync` returns, and the read then waits until the server closes the connection, 60 s in the probe, before it fails with `Unknown`. Both paths behave the same. On that runtime the pending read of the response stream does not react to the token. In the same probe, both paths fail after about 1.5 s again with this change: register the read token to dispose the stream around the read, in the non-`NET5_0_OR_GREATER` branch of `THttpTransport.ReadAsync` and in `THttpPerCallTransport.ReadAsync`, and report the resulting exception as `Interrupted` when the read token has fired. The normal and slow-header cases stay as they are. https://github.com/apache/thrift/blob/12b9ce6218c78870964089e0ca594d74ae0b136c/lib/netstd/Thrift/Transport/Client/THttpTransport.cs#L216-L222 https://github.com/apache/thrift/blob/12b9ce6218c78870964089e0ca594d74ae0b136c/lib/netstd/Thrift/Transport/Client/THttpPerCallTransport.cs#L226-L232 🤖 Generated with [Claude Code](https://claude.ai/code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
