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

siginfo_init should maybe set SA_RESTART flag on sigaction (EINTR) #336

Open
cemeyer opened this issue Nov 7, 2018 · 4 comments
Open

siginfo_init should maybe set SA_RESTART flag on sigaction (EINTR) #336

cemeyer opened this issue Nov 7, 2018 · 4 comments

Comments

@cemeyer
Copy link

cemeyer commented Nov 7, 2018

sigaction() in siginfo_init() sets sa_flags = 0. The SA_RESTART flag has the nice property of resuming some typical system calls without returning -1 / errno=EINTR to the application.

Alternatively, tarsnap should handle EINTR gracefully. I see some checks for EINTR in the source code.
But it does not appear to be consistently handled gracefully:

tarsnap: Can't read -: Interrupted system call

(Given this is EINTR and not ENOMEM, I think the printed line can only come from one location in the source code: https://github.com/Tarsnap/tarsnap/blob/master/tar/util.c#L358-L360 . I'm not sure what the right interaction between EINTR and FILE streams is. You could take the minimal approach of clearerror(3) if errno == EINTR, I guess.)

@cperciva
Copy link
Member

cperciva commented Nov 7, 2018

What's your command line, what OS are you running on, and what are you doing to trigger a signal? It seems like fread should handle EINTR by itself.

@cemeyer
Copy link
Author

cemeyer commented Nov 7, 2018

  1. tarsnap -xpkf XXX --null -T - with a extract list provided via stdin (modified redsnapper)
  2. FreeBSD 13-CURRENT as of this week.
  3. ^T (SIGINFO)
  4. I agree — it seems odd that fread(3) would not be handling the EINTR.

@cemeyer
Copy link
Author

cemeyer commented Nov 7, 2018

Re: fread, it can be blamed on lib/libc/stdio/refill.c:

  136         fp->_r = _sread(fp, (char *)fp->_p, fp->_bf._size);
  137         fp->_flags &= ~__SMOD;  /* buffer contents are again pristine */
  138         if (fp->_r <= 0) {
  139                 if (fp->_r == 0)
  140                         fp->_flags |= __SEOF;
  141                 else {
  142                         fp->_r = 0;
  143                         fp->_flags |= __SERR;
  144                 }
  145                 return (EOF);
  146         }

(_sread does not appear to do anything magic like re-start EINTR operations.)

Edit: In fact, it seems like very little of FreeBSD libc (and none of libc/stdio) handles EINTR in any explicit way.

@cemeyer
Copy link
Author

cemeyer commented Nov 17, 2018

FWIW, POSIX 2017 specifically calls out EINTR as a valid error for fgetc(3), and by extension, fread(3). And e.g. musl libc does not do any special handling of EINTR either. It seems that stdio FILE-using programs need to be EINTR-aware in the same ways that programs using bare syscalls do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants