diff --git a/CHANGELOG.md b/CHANGELOG.md
index e2f3b57f..1550890d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -23,6 +23,10 @@ project adheres to [Semantic Versioning](https://semver.org/).
### Fixed
* Fix looking up `UsbPortInfo::interface` on macOS.
[#193](https://github.com/serialport/serialport-rs/pull/193)
+* Fix issues with very long timeout values like `Duration::MAX` by clamping to
+ maximum supported value for underlying platform.
+ [#207](https://github.com/serialport/serialport-rs/issues/207),
+ [#208](https://github.com/serialport/serialport-rs/pull/208)
### Removed
diff --git a/Cargo.toml b/Cargo.toml
index 1608f758..32bbad0f 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -16,7 +16,6 @@ categories = ["hardware-support"]
[target."cfg(unix)".dependencies]
bitflags = "2.4.0"
-cfg-if = "1.0.0"
nix = { version = "0.26", default-features = false, features = ["fs", "ioctl", "poll", "signal", "term"] }
[target.'cfg(all(target_os = "linux", not(target_env = "musl")))'.dependencies]
@@ -36,6 +35,7 @@ features = [
]
[dependencies]
+cfg-if = "1.0.0"
scopeguard = "1.1"
serde = { version = "1.0", features = ["derive"], optional = true }
diff --git a/src/lib.rs b/src/lib.rs
index 200374e8..25242d1b 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -47,6 +47,9 @@ mod windows;
#[cfg(windows)]
pub use windows::COMPort;
+#[cfg(test)]
+pub(crate) mod tests;
+
/// A type for results generated by interacting with serial ports
///
/// The `Err` type is hard-wired to [`serialport::Error`](struct.Error.html).
@@ -375,6 +378,14 @@ impl SerialPortBuilder {
}
/// Set the amount of time to wait to receive data before timing out
+ ///
+ ///
+ ///
+ /// The accuracy is limited by the underlying platform's capabilities. Longer timeouts will be
+ /// clamped to the maximum supported value which is expected to be in the magnitude of a few
+ /// days.
+ ///
+ ///
#[must_use]
pub fn timeout(mut self, timeout: Duration) -> Self {
self.timeout = timeout;
@@ -487,6 +498,14 @@ pub trait SerialPort: Send + io::Read + io::Write {
fn set_stop_bits(&mut self, stop_bits: StopBits) -> Result<()>;
/// Sets the timeout for future I/O operations.
+ ///
+ ///
+ ///
+ /// The accuracy is limited by the underlying platform's capabilities. Longer timeouts will be
+ /// clamped to the maximum supported value which is expected to be in the magnitude of a few
+ /// days.
+ ///
+ ///
fn set_timeout(&mut self, timeout: Duration) -> Result<()>;
// Functions for setting non-data control signal pins
diff --git a/src/posix/enumerate.rs b/src/posix/enumerate.rs
index 44c44a33..209e7cd0 100644
--- a/src/posix/enumerate.rs
+++ b/src/posix/enumerate.rs
@@ -696,7 +696,7 @@ cfg_if! {
not(target_env = "musl"),
feature = "libudev"
))]
-mod test {
+mod tests {
use super::*;
use quickcheck_macros::quickcheck;
diff --git a/src/posix/poll.rs b/src/posix/poll.rs
index e45226ea..e193ac0e 100644
--- a/src/posix/poll.rs
+++ b/src/posix/poll.rs
@@ -5,11 +5,12 @@ use std::os::unix::io::RawFd;
use std::slice;
use std::time::Duration;
+use nix::libc::c_int;
use nix::poll::{PollFd, PollFlags};
#[cfg(target_os = "linux")]
use nix::sys::signal::SigSet;
-#[cfg(target_os = "linux")]
-use nix::sys::time::{TimeSpec, TimeValLike};
+#[cfg(any(target_os = "linux", test))]
+use nix::sys::time::TimeSpec;
pub fn wait_read_fd(fd: RawFd, timeout: Duration) -> io::Result<()> {
wait_fd(fd, PollFlags::POLLIN, timeout)
@@ -24,23 +25,12 @@ fn wait_fd(fd: RawFd, events: PollFlags, timeout: Duration) -> io::Result<()> {
let mut fd = PollFd::new(fd, events);
- let milliseconds =
- timeout.as_secs() as i64 * 1000 + i64::from(timeout.subsec_nanos()) / 1_000_000;
- #[cfg(target_os = "linux")]
- let wait_res = {
- let timespec = TimeSpec::milliseconds(milliseconds);
- nix::poll::ppoll(
- slice::from_mut(&mut fd),
- Some(timespec),
- Some(SigSet::empty()),
- )
- };
- #[cfg(not(target_os = "linux"))]
- let wait_res = nix::poll::poll(slice::from_mut(&mut fd), milliseconds as nix::libc::c_int);
-
- let wait = match wait_res {
+ let wait = match poll_clamped(&mut fd, timeout) {
Ok(r) => r,
- Err(e) => return Err(io::Error::from(crate::Error::from(e))),
+ Err(e) => {
+ dbg!(e);
+ return Err(io::Error::from(crate::Error::from(e)));
+ }
};
// All errors generated by poll or ppoll are already caught by the nix wrapper around libc, so
// here we only need to check if there's at least 1 event
@@ -63,3 +53,102 @@ fn wait_fd(fd: RawFd, events: PollFlags, timeout: Duration) -> io::Result<()> {
Err(io::Error::new(io::ErrorKind::Other, EIO.desc()))
}
+
+/// Poll with a duration clamped to the maximum value representable by the `TimeSpec` used by
+/// `ppoll`.
+#[cfg(target_os = "linux")]
+fn poll_clamped(fd: &mut PollFd, timeout: Duration) -> nix::Result {
+ let spec = clamped_time_spec(timeout);
+ nix::poll::ppoll(slice::from_mut(fd), Some(spec), Some(SigSet::empty()))
+}
+
+#[cfg(any(target_os = "linux", test))]
+// The type time_t is deprecaten on musl. The nix crate internally uses this type and makes an
+// exeption for the deprecation for musl. And so do we.
+//
+// See https://github.com/rust-lang/libc/issues/1848 which is referenced from every exemption used
+// in nix.
+#[cfg_attr(target_env = "musl", allow(deprecated))]
+fn clamped_time_spec(duration: Duration) -> TimeSpec {
+ use nix::libc::c_long;
+ use nix::sys::time::time_t;
+
+ // We need to clamp manually as TimeSpec::from_duration translates durations with more than
+ // i64::MAX seconds to negative timespans. This happens due to casting to i64 and is still the
+ // case as of nix 0.29.
+ let secs_limit = time_t::MAX as u64;
+ let secs = duration.as_secs();
+ if secs <= secs_limit {
+ TimeSpec::new(secs as time_t, duration.subsec_nanos() as c_long)
+ } else {
+ TimeSpec::new(time_t::MAX, 999_999_999)
+ }
+}
+
+// Poll with a duration clamped to the maximum millisecond value representable by the `c_int` used
+// by `poll`.
+#[cfg(not(target_os = "linux"))]
+fn poll_clamped(fd: &mut PollFd, timeout: Duration) -> nix::Result {
+ let millis = clamped_millis_c_int(timeout);
+ nix::poll::poll(slice::from_mut(fd), millis)
+}
+
+#[cfg(any(not(target_os = "linux"), test))]
+fn clamped_millis_c_int(duration: Duration) -> c_int {
+ let secs_limit = (c_int::MAX as u64) / 1000;
+ let secs = duration.as_secs();
+
+ if secs <= secs_limit {
+ secs as c_int * 1000 + duration.subsec_millis() as c_int
+ } else {
+ c_int::MAX
+ }
+}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+ use crate::tests::timeout::MONOTONIC_DURATIONS;
+
+ #[test]
+ fn clamped_millis_c_int_is_monotonic() {
+ let mut last = clamped_millis_c_int(Duration::ZERO);
+
+ for (i, d) in MONOTONIC_DURATIONS.iter().enumerate() {
+ let next = clamped_millis_c_int(*d);
+ dbg!((i, d));
+ assert!(
+ next >= last,
+ "{next} >= {last} failed for {d:?} at index {i}"
+ );
+ last = next;
+ }
+ }
+
+ #[test]
+ fn clamped_millis_c_int_zero_is_zero() {
+ assert_eq!(0, clamped_millis_c_int(Duration::ZERO));
+ }
+
+ #[test]
+ fn clamped_time_spec_is_monotonic() {
+ let mut last = clamped_time_spec(Duration::ZERO);
+
+ for (i, d) in MONOTONIC_DURATIONS.iter().enumerate() {
+ let next = clamped_time_spec(*d);
+ dbg!((i, d));
+ assert!(
+ next >= last,
+ "{next} >= {last} failed for {d:?} at index {i}"
+ );
+ last = next;
+ }
+ }
+
+ #[test]
+ fn clamped_time_spec_zero_is_zero() {
+ let spec = clamped_time_spec(Duration::ZERO);
+ assert_eq!(0, spec.tv_sec());
+ assert_eq!(0, spec.tv_nsec());
+ }
+}
diff --git a/src/tests/mod.rs b/src/tests/mod.rs
new file mode 100644
index 00000000..deb6b6ae
--- /dev/null
+++ b/src/tests/mod.rs
@@ -0,0 +1,7 @@
+use cfg_if::cfg_if;
+
+cfg_if! {
+ if #[cfg(test)] {
+ pub(crate) mod timeout;
+ }
+}
diff --git a/src/tests/timeout.rs b/src/tests/timeout.rs
new file mode 100644
index 00000000..dd53c172
--- /dev/null
+++ b/src/tests/timeout.rs
@@ -0,0 +1,41 @@
+use std::time::Duration;
+
+/// A sequence of strongly monotonic inrceasing durations. Introduced for testing conversions from
+/// `Duration` to platform-specific types.
+pub(crate) const MONOTONIC_DURATIONS: [Duration; 17] = [
+ Duration::ZERO,
+ Duration::from_nanos(1),
+ Duration::from_millis(1),
+ Duration::from_secs(1),
+ Duration::from_secs(i16::MAX as u64 - 1),
+ Duration::from_secs(i16::MAX as u64),
+ Duration::from_secs(i16::MAX as u64 + 1),
+ Duration::from_secs(i32::MAX as u64 - 1),
+ Duration::from_secs(i32::MAX as u64),
+ Duration::from_secs(i32::MAX as u64 + 1),
+ Duration::from_secs(i64::MAX as u64 - 1),
+ Duration::from_secs(i64::MAX as u64),
+ Duration::from_secs(i64::MAX as u64 + 1),
+ Duration::from_secs(u64::MAX - 1),
+ Duration::from_secs(u64::MAX),
+ Duration::new(u64::MAX, 1_000_000),
+ Duration::MAX,
+];
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+
+ #[test]
+ fn basic_durations_properties() {
+ assert_eq!(Duration::ZERO, *MONOTONIC_DURATIONS.first().unwrap());
+ assert_eq!(Duration::MAX, *MONOTONIC_DURATIONS.last().unwrap());
+
+ // Check that this array is monotonic.
+ let mut last = MONOTONIC_DURATIONS[0];
+ for next in MONOTONIC_DURATIONS {
+ assert!(last <= next);
+ last = next;
+ }
+ }
+}
diff --git a/src/windows/com.rs b/src/windows/com.rs
index 20e243fb..21ed0486 100644
--- a/src/windows/com.rs
+++ b/src/windows/com.rs
@@ -154,6 +154,18 @@ impl COMPort {
port_name: None,
}
}
+
+ fn timeout_constant(duration: Duration) -> DWORD {
+ let milliseconds = duration.as_millis();
+ // In the way we are setting up COMMTIMEOUTS, a timeout_constant of MAXDWORD gets rejected.
+ // Let's clamp the timeout constant for values of MAXDWORD and above. See remarks at
+ // https://learn.microsoft.com/en-us/windows/win32/api/winbase/ns-winbase-commtimeouts.
+ //
+ // This effectively throws away accuracy for really long timeouts but at least preserves a
+ // long-ish timeout. But just casting to DWORD would result in presumably unexpected short
+ // and non-monotonic timeouts from cutting off the higher bits.
+ u128::min(milliseconds, MAXDWORD as u128 - 1) as DWORD
+ }
}
impl Drop for COMPort {
@@ -248,14 +260,14 @@ impl SerialPort for COMPort {
}
fn set_timeout(&mut self, timeout: Duration) -> Result<()> {
- let milliseconds = timeout.as_millis();
+ let timeout_constant = Self::timeout_constant(timeout);
let mut timeouts = COMMTIMEOUTS {
ReadIntervalTimeout: MAXDWORD,
ReadTotalTimeoutMultiplier: MAXDWORD,
- ReadTotalTimeoutConstant: milliseconds as DWORD,
+ ReadTotalTimeoutConstant: timeout_constant,
WriteTotalTimeoutMultiplier: 0,
- WriteTotalTimeoutConstant: milliseconds as DWORD,
+ WriteTotalTimeoutConstant: timeout_constant,
};
if unsafe { SetCommTimeouts(self.handle, &mut timeouts) } == 0 {
@@ -442,3 +454,29 @@ impl SerialPort for COMPort {
}
}
}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+ use crate::tests::timeout::MONOTONIC_DURATIONS;
+
+ #[test]
+ fn timeout_constant_is_monotonic() {
+ let mut last = COMPort::timeout_constant(Duration::ZERO);
+
+ for (i, d) in MONOTONIC_DURATIONS.iter().enumerate() {
+ let next = COMPort::timeout_constant(*d);
+ dbg!((i, d));
+ assert!(
+ next >= last,
+ "{next} >= {last} failed for {d:?} at index {i}"
+ );
+ last = next;
+ }
+ }
+
+ #[test]
+ fn timeout_constant_zero_is_zero() {
+ assert_eq!(0, COMPort::timeout_constant(Duration::ZERO));
+ }
+}
diff --git a/src/windows/enumerate.rs b/src/windows/enumerate.rs
index 57333ec2..c303e246 100644
--- a/src/windows/enumerate.rs
+++ b/src/windows/enumerate.rs
@@ -562,7 +562,7 @@ pub fn available_ports() -> Result> {
}
#[cfg(test)]
-mod test {
+mod tests {
use super::*;
use quickcheck_macros::quickcheck;
diff --git a/tests/test_timeout.rs b/tests/test_timeout.rs
index fc51c9aa..a0627d29 100644
--- a/tests/test_timeout.rs
+++ b/tests/test_timeout.rs
@@ -59,7 +59,7 @@ fn test_read_returns_available_data_before_timeout(
);
received_message.extend_from_slice(&buffer[..read]);
}
- _ => assert!(false),
+ e => panic!("unexpected error {:?}", e),
}
if received_message.len() >= expected_message.len() {
@@ -79,7 +79,7 @@ fn test_read_returns_available_data_before_timeout(
println!("send: {} bytes", chunk.len());
- next = next + send_period;
+ next += send_period;
thread::sleep(next - Instant::now());
}
});
@@ -178,3 +178,50 @@ fn test_timeout_greater_zero(hw_config: HardwareConfig, #[case] timeout: Duratio
assert!(flushed_at + timeout + margin > read_at);
assert_eq!(buffer[..read], message[..read]);
}
+
+/// Checks that reading data with a timeout of `Duration::MAX` returns some data and no error. It
+/// does not check the actual timeout for obvious reason.
+#[rstest]
+#[cfg_attr(feature = "ignore-hardware-tests", ignore)]
+fn test_timeout_max(hw_config: HardwareConfig) {
+ let sleep = Duration::from_millis(3000);
+ let margin = Duration::from_millis(500);
+ let mut sender = serialport::new(hw_config.port_1, 115200).open().unwrap();
+ let mut receiver = serialport::new(hw_config.port_2, 115200)
+ .timeout(Duration::MAX)
+ .open()
+ .unwrap();
+
+ let message =
+ b"0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~";
+ let mut buffer: [u8; 1024] = [0xff; 1024];
+
+ sender.clear(ClearBuffer::All).unwrap();
+ receiver.clear(ClearBuffer::All).unwrap();
+
+ let started_at = Instant::now();
+
+ let sender_thread = thread::spawn(move || {
+ thread::sleep(sleep);
+
+ sender.write_all(message).unwrap();
+ sender.flush().unwrap();
+ });
+
+ let read = receiver.read(&mut buffer).unwrap();
+ let read_at = Instant::now();
+
+ println!(
+ "read: {} bytes of {} after {} ms",
+ read,
+ message.len(),
+ (Instant::now() - started_at).as_millis()
+ );
+
+ assert!(read > 0);
+ assert!(read_at > started_at + sleep);
+ assert!(read_at < started_at + sleep + margin);
+ assert_eq!(buffer[..read], message[..read]);
+
+ sender_thread.join().unwrap();
+}