kpumuk commented on code in PR #3399:
URL: https://github.com/apache/thrift/pull/3399#discussion_r3157895508
##########
lib/rs/src/transport/socket.rs:
##########
@@ -141,9 +196,13 @@ impl TIoChannel for TTcpChannel {
.map(|cloned| {
let read_half = ReadHalf::new(TTcpChannel {
stream: s.stream.take(),
+ read_timeout: s.read_timeout,
Review Comment:
There is a weird issue caused by the `split()` creating two `TTcpChannel`
wrappers for the same underlying TCP socket. The socket timeout is shared OS
state, but the new `read_timeout` / `write_timeout` fields are copied by value
into each wrapper. After one half changes a timeout, the other half still
reports the old cached value. If that stale half later calls
`set_timeouts(...)`, it writes the old timeout back to the shared socket.
```rust
let initial = Some(Duration::from_millis(80));
let updated = Some(Duration::from_millis(250));
let updated_write = Some(Duration::from_millis(500));
let (mut channel, _server) = wrapped_channel();
channel.set_timeouts(initial, None).unwrap();
let (mut read_half, mut write_half) = channel.split().unwrap();
read_half.set_read_timeout(updated).unwrap();
let stale = write_half.read_timeout();
write_half.set_timeouts(stale, updated_write).unwrap();
assert_eq!(
(stale, read_half.stream.as_ref().unwrap().read_timeout().unwrap()),
(updated, updated)
);
```
--
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]
