SerialPortBuilder.timeout(Duration::MAX) gives "Invalid argument"

[wxifze]

When serial port is created through SerialPortBuilder and .timeout() is called with the argument set to Duration::MAX, any attempt to read from the port returns an error Custom { kind: Other, error: "Invalid argument" }.
Apparently there is no other way to set blocking read without a timeout.

Code to reproduce:

use std::{time::Duration, error::Error};

fn main() -> Result<(), Box<dyn Error>> {
    let mut serial = serialport::new("/dev/ttyUSB0", 9600)
        .timeout(Duration::MAX)
        .open()?;
    let mut tmp = vec![0];
    serial.read(&mut tmp)?;
    dbg!(tmp);
    Ok(())
}

Environment:

• Fedora 40
• 6.9.11-200.fc40.x86_64
• rustc 1.79.0 (129f3b996 2024-06-10)
• serialport = "4.4.0"
• /dev/ttyUSB0: ID 1a86:7523 QinHeng Electronics CH340 serial converter
• also reproducible with /dev/tty

[sirhcel]

Thank you for spotting this issue @wxifze! At a first glance with a new test it looks like Windows is affected as well while macOS is not.

On Linux, the error is caused by an actually negative timeout timespan computed from Duration::MAX. Clamping the timeout to the maximum representable timespan (like sirhcel@55f5dd0) could fix the behavior by actually using a shorter but still longish timeout.

[sirhcel]

It looks conversions which do not work for large timeouts like Duration::MAX are causing the errors seen with the example from #207 and the new test case test_timeout_max.

This test fails on Linux when reading data:

$ export RUST_BACKTRACE=1
$ export SERIALPORT_TEST_PORT_1=/dev/ttyUSB1
$ export SERIALPORT_TEST_PORT_2=/dev/ttyUSB2
$ cargo test -- test_timeout_max
[...]
     Running tests/test_timeout.rs (target/debug/deps/test_timeout-8bf8e0b21317bbde)

running 1 test
test test_timeout_max ... FAILED

failures:

---- test_timeout_max stdout ----
-------------- TEST START --------------
thread 'test_timeout_max' panicked at tests/test_timeout.rs:211:43:
called `Result::unwrap()` on an `Err` value: Custom { kind: Other, error: "Invalid argument" }
stack backtrace:
   0: rust_begin_unwind
             at /rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/std/src/panicking.rs:652:5
[...]
   3: core::result::Result<T,E>::unwrap
             at /rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/core/src/result.rs:1102:23
   4: test_timeout::test_timeout_max::test_timeout_max
             at ./tests/test_timeout.rs:211:16
[...]

This seems to be caused by the conversion of Duration::MAX into negative milliseconds here.

And it fails on Windows at configuring the timeout when opening the serial port:

>cargo test -- test_timeout_max
[...]
     Running tests\test_timeout.rs (target\debug\deps\test_timeout-1088926ab4a2b0a3.exe)

running 1 test
test test_timeout_max ... FAILED

failures:

---- test_timeout_max stdout ----
-------------- TEST START --------------
thread 'test_timeout_max' panicked at tests\test_timeout.rs:193:10:
called `Result::unwrap()` on an `Err` value: Error { kind: Io(Other), description: "Falscher Parameter." }
stack backtrace:
   0: std::panicking::begin_panic_handler
[...]
   3: enum2$<core::result::Result<alloc::boxed::Box<dyn$<serialport::SerialPort>,alloc::alloc::Global>,serialport::Error> >::unwrap
             at /rustc/129f3b9964af4d4a709d1383930ade12dfe7c081\library\core\src\result.rs:1077
   4: test_timeout::test_timeout_max::test_timeout_max
             at .\tests\test_timeout.rs:190
[...]

Which seems to be caused by casting Duration::MAX.as_millis() into DWORD resulting in MAXDWORD which gets rejected (see remarks on COMMTIMEOUTS)

As we don't document any limits on the duration and recommend using Duration::MAX for emulating "no timeout" with the current API, I would expect this case to result in a working and sufficiently long timeout.

[sirhcel]

Using a timeout of Duration::MAX did not give an error on macOS but would have waited longer in this corner case due to timeout milliseconds of -1 being interpreted as blocking indefinitely by poll. On the one hand this is technically wrong as this would be longer than Duration::MAX - and on the other hand and in practice this is hairsplitting.