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

std::io::Write::write should have an additional example where it handles EINTR #41219

Open
ehiggs opened this Issue Apr 11, 2017 · 4 comments

Comments

Projects
None yet
5 participants
@ehiggs
Copy link

ehiggs commented Apr 11, 2017

Implementations of std::io::Write::write that result in a syscall can be interrupted with EINTR which means it should be tried again immediately. This is inconsistent with the existing example which uses ? to bail if the call failed:

buffer.write(b"some bytes")?;

Any copy-paste coders could be caught out by this.

My (bad) example:

use std::io::prelude::*;
use std::fs::File;

fn run () -> Result<(), ::std::io::Error> {
  let mut buffer = File::create("foo.txt")?;
  loop {
    let written = buffer.write(b"some bytes");
    if let Err(err) = written {
        if err.kind() == ::std::io::ErrorKind::Interrupted {
            break;
        }
        // try again..
    } else {
      break;
    }
  }
  Ok(())
}

fn main() {
  run().unwrap();
}  
@sfackler

This comment has been minimized.

Copy link
Member

sfackler commented Apr 11, 2017

Isn't SA_RESTART set by default?

@ehiggs

This comment has been minimized.

Copy link
Author

ehiggs commented Apr 11, 2017

In general Linux, no, I don't think so. The Linux Programming Interface offers this macro (which exists in gnu libc as TEMP_FAILURE_RETRY):

#define NO_EINTR(stmt) while ((stmt) == -1 && errno == EINTR);

If Rust set some sigactions on startup, I don't know. I wouldn't mind, actually since we could then close this and I would have learned something new. :)

I think SA_RESTART is a default in BSDs since man 2 sigaction says:

Provide behavior compatible with BSD signal semantics by
making certain system calls restartable across signals.
This flag is meaningful only when establishing a signal
handler. See signal(7) for a discussion of system call
restarting.

@Mark-Simulacrum

This comment has been minimized.

Copy link
Member

Mark-Simulacrum commented Jun 20, 2017

Worth pointing out that write_all handles this for the user, by default, though it can be overridden to do otherwise.

@DevQps

This comment has been minimized.

Copy link
Contributor

DevQps commented Apr 3, 2019

@steveklabnik @ehiggs @Mark-Simulacrum @sfackler I did a little bit of research to the SA_RESTART flag and found this:

This flag controls what happens when a signal is delivered during certain primitives (such as open, read or write), and the signal handler returns normally. There are two alternatives: the library function can resume, or it can return failure with error code EINTR.

The choice is controlled by the SA_RESTART flag for the particular kind of signal that was delivered. If the flag is set, returning from a handler resumes the library function. If the flag is clear, returning from a handler makes the function fail. See Interrupted Primitives.

It seems to suggest that that a signal handler that does not set SA_RESTART to true, deliberately does not want the syscall to finish (like kill / hard-termination signals). Otherwise it would just set SA_RESTART to true right? Are there reasons why a signal handler would set SA_RESTART to false but would like the user to try again?

It seems to be that this is quite platform specific as other platforms might handle this differently. Therefore, I think it would be better to not include this particular case as an example because it is likely to confuse less experienced programmers with details they are not likely to encounter. Based on my experience (I have never seen such an error in my years of programming), I'd say that the chances are very small that this error will actually occur. And if it does, a programmer will find out about it due to an error that will be thrown. It can then choose to handle the error accordingly, might that be desired.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.