Repository navigation
Windows: io::Error::from_raw_os_error(ERROR_TIMEOUT) is not io::ErrorKind::TimedOut #71646
Description
Activity
- addedO-windowsOperating system: WindowsOperating system: WindowsT-libs-api[DEPRECATED; DO NOT USE][DEPRECATED; DO NOT USE]
on Apr 28, 2020 This issue is not limited to the system error code
ERROR_TIMEOUT, as Windows appears to use various error codes for what would all beETIMEDOUTon POSIX. For example, I'm getting anERROR_SEM_TIMEOUT(121) if a write operation on a COM port times out (on Windows 10 version 1809).Looking at the list of Windows system error codes, I found the following that contain TIME and appear to be timeouts:
ERROR_SEM_TIMEOUT(121)WAIT_TIMEOUT(258)ERROR_DRIVER_CANCEL_TIMEOUT(594)ERROR_SERVICE_REQUEST_TIMEOUT(1053)ERROR_COUNTER_TIMEOUT(1121)ERROR_TIMEOUT(1460)Thanks to @retep998 for pointing out that this error code indicates an invalid timeout and not that a timeout occurred.RPC_S_INVALID_TIMEOUT(1709)ERROR_RESOURCE_CALL_TIMED_OUT(5910)ERROR_CTX_MODEM_RESPONSE_TIMEOUT(7012)ERROR_CTX_CLIENT_QUERY_TIMEOUT(7040)FRS_ERR_SYSVOL_POPULATE_TIMEOUT(8014)ERROR_DS_TIMELIMIT_EXCEEDED(8226)DNS_ERROR_RECORD_TIMED_OUT(9705)WSAETIMEDOUT(10060)ERROR_IPSEC_IKE_TIMED_OUT(13805)ERROR_RUNLEVEL_SWITCH_TIMEOUT(15402)ERROR_RUNLEVEL_SWITCH_AGENT_TIMEOUT(15403)
One concern with merging these all into
ErrorKind::TimedOutis that the caller would no longer know which of the timeout occurred -- are there cases where that's important?I don't think that's the case, but please correct me if I'm wrong. The following example
use std::io; const ERROR_OPERATION_ABORTED: i32 = 995; const ERROR_TIMEOUT: i32 = 1460; fn main() { println!("{:?}", io::Error::from_raw_os_error(ERROR_OPERATION_ABORTED)); println!("{:?}", io::Error::from_raw_os_error(ERROR_TIMEOUT)); }
will produce this on Windows:
Os { code: 995, kind: TimedOut, message: "The I/O operation has been aborted because of either a thread exit or an application request." } Os { code: 1460, kind: Other, message: "This operation returned because the timeout period expired." }Currently, only
ERROR_OPERATION_ABORTEDwill yield anio::Errorofio::ErrorKind::TimedOut. All other timeout errors will be ofio::ErrorKind::Other, which is counter-intuitive, because matching an error's kind againstio::ErrorKind::TimedOutwill not catch all timeout errors.I would suggest to only change the kind of the errors listed above to
io::ErrorKind::TimedOut. The original OS error code can still be accessed via raw_os_error() if the programmer is interested in the OS-specific timeout reason. Therefore, this should not break backwards-compatibility, unless someone expects these errors to beio::ErrorKind::Other, which should be discouraged.Ah, okay, right. I would in that case not be personally opposed to making this change, I think a PR that does so would be the best way to do so (and that can be FCP'd to libs team).
All right. Thanks for the quick response. I'll prepare a pull request.
Note that
RPC_S_INVALID_TIMEOUTdoes not indicate the operation timed out, but rather that the timeout you specified was invalid.@retep998 Thanks for checking that. I'll revise the list of errors before submitting the PR, but I'm afraid I have very close to zero experience with the Windows API. I just stumbled over this issue while porting POSIX software to Windows.
- added 2 commits that reference this issue
on Jun 21, 2020 - added a commit that references this issue
on Jun 22, 2020 - added a commit that references this issue
on Jun 22, 2020 - added a commit that references this issue
on Jun 23, 2020
On Windows an io::Error created via
from_raw_os_error()with the Windows system error codeERROR_TIMEOUT(1460) is not of the kindio::ErrorKind::TimedOut, butio::ErrorKind::Other.This is counter-intuitive considering both (system error code and kind) have (almost) the same symbolic name.
I've tested this with Rust 1.42 on x86_64-pc-windows-gnu, but this behavior is obviously still present in git master. Unless I'm overlooking sth., this should be fairly easy to fix by defining the
c::ERROR_TIMEOUTconstant and adding the following after this line:If you'd like me to, I can prepare a merge request implementing the suggested fix.
I tried this code:
I expected to see this happen: Assertion passes because (should be):
Instead, this happened: Assertion fails because:
Meta
rustc --version --verbose:Backtrace