Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rollup of 7 pull requests #73635

Merged
merged 21 commits into from
Jun 23, 2020
Merged

Rollup of 7 pull requests #73635

merged 21 commits into from
Jun 23, 2020

Conversation

Dylan-DPC-zz
Copy link

Successful merges:

Failed merges:

r? @ghost

Carsten Andrich and others added 21 commits May 1, 2020 17:52
Fixes rust-lang#46099

Previously, we would check the 'move' and 'use' spans to see if we
should emit this message. However, this can give false positives when
macros are involved, since two distinct expressions may end up with the
same span.

Instead, we check the actual MIR `Location`, which eliminates false
positives.
add Windows system error codes that should map to io::ErrorKind::TimedOut

closes rust-lang#71646

**Disclaimer:** The author of this pull request has a negligible amount of experience (i.e., kinda zero) with the Windows API. This PR should _definitely_ be reviewed by someone familiar with the API and its error handling.

While porting POSIX software using serial ports to Windows, I found that for many Windows system error codes, an `io::Error` created via `io::Error::from_raw_os_error()` or `io::Error::last_os_error()` is not `io::ErrorKind::TimedOut`. For example, when a (non-overlapped) write to a COM port via [`WriteFile()`](https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-readfile) times out, [`GetLastError()`](https://docs.microsoft.com/en-us/windows/win32/api/errhandlingapi/nf-errhandlingapi-getlasterror) returns `ERROR_SEM_TIMEOUT` ([121](https://docs.microsoft.com/en-us/windows/win32/debug/system-error-codes--0-499-)). However, an `io::Error` created from this error code will have `io::ErrorKind::Other`.

Currently, only the error codes `ERROR_OPERATION_ABORTED` and `WSAETIMEDOUT` will instantiate `io::Error`s with kind `io::ErrorKind::TimedOut`.
This makes `io::Error::last_os_error()` unsuitable for error handling of syscalls that could time out, because timeouts can not be caught by matching the error's kind against `io::ErrorKind::TimedOut`.

Downloading the [list of Windows system error codes](https://gist.github.com/carstenandrich/c331d557520b8a0e7f44689ca257f805) and grepping anything that sounds like a timeout (`egrep -i "timed?.?(out|limit)"`), I've identified the following error codes that should also have `io::ErrorKind::TimedOut`, because they could be I/O-related:

Name | Code | Description
--- | --- | ---
`ERROR_SEM_TIMEOUT` | [121](https://docs.microsoft.com/en-us/windows/win32/debug/system-error-codes--0-499-) | The semaphore timeout period has expired.
`WAIT_TIMEOUT` | [258](https://docs.microsoft.com/en-us/windows/win32/debug/system-error-codes--0-499-) | The wait operation timed out.
`ERROR_DRIVER_CANCEL_TIMEOUT` | [594](https://docs.microsoft.com/en-us/windows/win32/debug/system-error-codes--500-999-) | The driver %hs failed to complete a cancelled I/O request in the allotted time.
`ERROR_COUNTER_TIMEOUT` | [1121](https://docs.microsoft.com/en-us/windows/win32/debug/system-error-codes--1000-1299-) | A serial I/O operation completed because the timeout period expired. The IOCTL_SERIAL_XOFF_COUNTER did not reach zero.)
`ERROR_TIMEOUT` | [1460](https://docs.microsoft.com/en-us/windows/win32/debug/system-error-codes--1300-1699-) | This operation returned because the timeout period expired.
`ERROR_CTX_MODEM_RESPONSE_TIMEOUT` | [7012](https://docs.microsoft.com/en-us/windows/win32/debug/system-error-codes--6000-8199-) | The modem did not respond to the command sent to it. Verify that the modem is properly cabled and powered on.
`ERROR_CTX_CLIENT_QUERY_TIMEOUT` | [7040](https://docs.microsoft.com/en-us/windows/win32/debug/system-error-codes--6000-8199-) | The client failed to respond to the server connect message.
`ERROR_DS_TIMELIMIT_EXCEEDED` | [8226](https://docs.microsoft.com/en-us/windows/win32/debug/system-error-codes--8200-8999-) | The time limit for this request was exceeded.
`DNS_ERROR_RECORD_TIMED_OUT` | [9705](https://docs.microsoft.com/en-us/windows/win32/debug/system-error-codes--9000-11999-) | DNS record timed out.
`ERROR_IPSEC_IKE_TIMED_OUT` | [13805](https://docs.microsoft.com/en-us/windows/win32/debug/system-error-codes--12000-15999-) | Negotiation timed out.

The following errors are also timeouts, but they don't seem to be directly related to I/O or network operations:

Name | Code | Description
--- | --- | ---
`ERROR_SERVICE_REQUEST_TIMEOUT` | [1053](https://docs.microsoft.com/en-us/windows/win32/debug/system-error-codes--1000-1299-) | The service did not respond to the start or control request in a timely fashion.
`ERROR_RESOURCE_CALL_TIMED_OUT` | [5910](https://docs.microsoft.com/en-us/windows/win32/debug/system-error-codes--4000-5999-) | The call to the cluster resource DLL timed out.
`FRS_ERR_SYSVOL_POPULATE_TIMEOUT` | [8014](https://docs.microsoft.com/en-us/windows/win32/debug/system-error-codes--6000-8199-) | The file replication service cannot populate the system volume because of an internal timeout. The event log may have more information.
`ERROR_RUNLEVEL_SWITCH_TIMEOUT` | [15402](https://docs.microsoft.com/en-us/windows/win32/debug/system-error-codes--12000-15999-) | The requested run level switch cannot be completed successfully since one or more services will not stop or restart within the specified timeout.
`ERROR_RUNLEVEL_SWITCH_AGENT_TIMEOUT` | [15403](https://docs.microsoft.com/en-us/windows/win32/debug/system-error-codes--12000-15999-) | A run level switch agent did not respond within the specified timeout.

Please note that `ERROR_SEM_TIMEOUT` is the only timeout error I have [seen in action](https://gist.github.com/carstenandrich/10b3962fa1abc9e50816b6460010900b). The remainder of the error codes listed above is based purely on reading documentation.

This pull request adds all of the errors listed in both tables, but I'm not sure whether adding all of them makes sense. Someone with actual Windows API experience should decide that.

I expect these changes to be fairly backwards compatible, because only the error's [`.kind()`](https://doc.rust-lang.org/std/io/struct.Error.html#method.kind) will change, but matching the error's code via [`.raw_os_error()`](https://doc.rust-lang.org/std/io/struct.Error.html#method.raw_os_error) will not be affected.
However, code expecting these errors to be `io::ErrorKind::Other` would break. Even though I personally do not think such an implementation would make sense, after all the docs say that `io::ErrorKind` is _intended to grow over time_, a residual risk remains, of course. I took the liberty to ammend the docstring of `io::ErrorKind::Other` with a remark that discourages matching against it.

As per the contributing guidelines I'm adding @steveklabnik due to the changed documentation. Also @retep998 might have some valuable insights on the error codes.

r? @steveklabnik
cc @retep998
cc @Mark-Simulacrum
Converted all platform-specific stdin/stdout/stderr implementations to use io:: traits

Currently, some of the platform-specific standard streams (`src/libstd/sys/*/stdio.rs`) manually implement parts of the `io::Write` interface directly as methods on the struct, rather than by actually implementing the trait. There doesn't seem to be any reason for this, other than an unused advantage of `fn write(&self, ...)` instead of `fn write(&mut self, ...)`.

Unfortunately, this means that those implementations don't have the default-implemented io methods, like `read_exact` and `write_all`. This caused rust-lang#72705, which adds forwarding methods to the user-facing standard stream implementations, to fail to compile on those platforms.

This change converts *all* such standard stream structs to use the standard library traits. This change should not cause any breakages, because the changed types are not publicly exported, and in fact are only ever used in `src/libstd/io/stdio.rs`.
Make is_freeze and is_copy_modulo_regions take TyCtxtAt

Make is_freeze and is_copy_modulo_regions take TyCtxtAt instead of separately taking TyCtxt and Span. This is consistent with is_sized.
switch_ty is redundant

This field is redundant, but we cannot remove it currently as pretty-printing relies on it (and it does not have access to `mir::Body` to compute the type itself).

Cc @oli-obk @matthewjasper @jonas-schievink
…atic-morse

Fix spurious 'value moved here in previous iteration of loop' messages

Fixes rust-lang#46099

Previously, we would check the 'move' and 'use' spans to see if we
should emit this message. However, this can give false positives when
macros are involved, since two distinct expressions may end up with the
same span.

Instead, we check the actual MIR `Location`, which eliminates false
positives.
@Dylan-DPC-zz
Copy link
Author

@bors r+ rollup=never p=7

@bors
Copy link
Contributor

bors commented Jun 23, 2020

📌 Commit e979392 has been approved by Dylan-DPC

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jun 23, 2020
@bors
Copy link
Contributor

bors commented Jun 23, 2020

⌛ Testing commit e979392 with merge 3b1c08c...

@bors
Copy link
Contributor

bors commented Jun 23, 2020

☀️ Test successful - checks-azure
Approved by: Dylan-DPC
Pushing 3b1c08c to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 23, 2020
@bors bors merged commit 3b1c08c into rust-lang:master Jun 23, 2020
@cuviper cuviper added this to the 1.46 milestone May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants