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.

Sign up for GitHub

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

Jump to bottom

os/exec: broken pipe writing into djpeg #9173

Closed
bradfitz opened this issue Nov 26, 2014 · 4 comments
Closed

os/exec: broken pipe writing into djpeg #9173

bradfitz opened this issue Nov 26, 2014 · 4 comments
Labels
FrozenDueToAge
Milestone
Go1.5

Comments

@bradfitz
Copy link
Contributor 

Go's os/exec seems to have a problem piping into the djpeg program.

To reproduce, at least on Linux:

1) $ apt-get install libjpeg-turbo-progs
2) Download the attached image (7.8 MB, which seems to matter, or its JPEG structure
matters)
3) Run boat.go: http://play.golang.org/p/cHra0dvPr6

ante:tmp $ go run boat.go
2014/11/26 14:26:46 Process state: exit status 0 (success = true)
2014/11/26 14:26:46 Stdout: 11943953 bytes
2014/11/26 14:26:46 Stderr: ""
2014/11/26 14:26:46 cmd.Wait = write |1: broken pipe

Of note:

The djpeg process ends successfully, and outputs its resultant image to stdout.

But djpeg seems to end before reading all of its input, causing a SIGPIPE to the
goroutine trying to write all the data to it. strace says:

21770 <... write resumed> )             = 4096
21772 read(5,  <unfinished ...>
21770 write(1,
"2\10\0002\10\0007\t\0007\t\0008\t\0007\10\0009\n\0008\7\0008\7\0<\t\0=\n\0>\v\0@\f\0?\v\0E\r\0F\16\0J\16\0M\16\0T\22\0Z\24\0]\24\0]\23\0^
\23\0f\30\0o\36\0n\32\0{$\0\205)\0\2212\0\236<\0\2369\0\2405\0\2423\0\254:\0\272C\0\317W\0\351m\0\342g\0\344h\0\336e\0\334d\0\356w\0\363}\0\377\207\0\377
\213\0\377\220\0\377\221\0\377\222\0\377\221\0\377\224\0\377\224\0\377\220\0\377\217\0\377\223\0\377\227\0\377\234\0\377\244\0\377\264\0\377\276\0\377\30
5\0\377\311\0\377\312\0\377\301\0\377\266\0\377\243\0\377\226\0\377\201"..., 4096
<unfinished ...>
21772 <... read resumed>
"\4\v\2\5\v\2\5\v\2\5\v\2\5\n\1\4\n\1\4\v\0\4\v\0\4\f\1\5\r\2\6\17\5\6\21\5\7\21\5\7\23\4\7\22\3\10\22\3\10\22\3\10\21\2\7\21\2\
7\21\2\7\17\3\7\20\4\10\17\5\6\20\6\7\20\6\7\20\6\7\20\6\7\20\6\7\22\6\6\22\6\6\24\6\6\24\6\6\22\6\6\23\7\7\21\5\7\20\4\6\17\3\5\17\3\5\r\3\4\f\2\3\r\3\4
\f\2\3\v\2\3\n\1\2\n\1\2\n\1\2\v\2\3\f\3\4\f\3\4\r\4\5\17\5\6\20\6\7\22\6\6\24\6\6\25\7\6\31\7\7\33\10\4\37\7\5!\10\3#\7\3&\t\3'\10\3*\t\4-\n\6,\t\5*\t\4
'"..., 4840960) = 4096
21770 <... write resumed> )             = 4096
21768 <... select resumed> )            = 0 (Timeout)
21770 write(1, "\7\3\32\7\3\31\7\3\31\7\3\27\10\5\27\10\5", 17 <unfinished
...>
21772 read(5,  <unfinished ...>
21770 <... write resumed> )             = 17
21772 <... read resumed>
"2\10\0002\10\0007\t\0007\t\0008\t\0007\10\0009\n\0008\7\0008\7\0<\t\0=\n\0>\v\0@\f\0?\v\0E\r\0F\16\0J\16\0M\16\0T\22\0Z\24\0]\2
4\0]\23\0^\23\0f\30\0o\36\0n\32\0{$\0\205)\0\2212\0\236<\0\2369\0\2405\0\2423\0\254:\0\272C\0\317W\0\351m\0\342g\0\344h\0\336e\0\334d\0\356w\0\363}\0\377
\207\0\377\213\0\377\220\0\377\221\0\377\222\0\377\221\0\377\224\0\377\224\0\377\220\0\377\217\0\377\223\0\377\227\0\377\234\0\377\244\0\377\264\0\377\27
6\0\377\305\0\377\311\0\377\312\0\377\301\0\377\266\0\377\243\0\377\226\0\377\201"...,
4836864) = 4113
21770 exit_group(0)                     = ?
21772 read(5,  <unfinished ...>
21768 select(0, NULL, NULL, NULL, {0, 20} <unfinished ...>
21772 <... read resumed> "", 4832751)   = 0
21771 <... write resumed> )             = 16384
21772 --- SIGCHLD (Child exited) @ 0 (0) ---
21771 --- SIGPIPE (Broken pipe) @ 0 (0) ---
21772 rt_sigreturn(0x11 <unfinished ...>
21771 rt_sigreturn(0xd <unfinished ...>
21772 <... rt_sigreturn resumed> )      = 0
21771 <... rt_sigreturn resumed> )      = 16384
21769 <... read resumed> "", 512)       = 0
21772 futex(0xc208020b58, FUTEX_WAIT, 0, NULL <unfinished ...>
21771 write(4,
"\320\272\232\376k\3375]\303\23\222s\232\350-\265\375B\35>8e-*\216\273\2115\340Q\251\316\365=\372\270w\rN\206\312\366K\233r\321)R\6p\5G|\3
42\342\300\227\373\303\257=k*\362J\3461\2475Qhqo\2435\345\336-\321\244'?*\212\307}\"t\274uTl\251
\203\317\326\276S\33\213\214*8\263\356\362\374\33\2355;\
31\222Gq\25\323\0\33\203\3152k\233\270\234lc\327\247\255c\32\221\234N\207Ft\246\333?\377\325\374\264\203R\270\23\3(\301\34\327Og\251\31\\n\373\247\257\32
6\277\25\305PR\367\217\333\360\265\332vf\270\322WUF).$\7\345^\231\253\326\336\27\271\213L/\""...,
16384 <unfinished ...>
21769 futex(0xc208020458, FUTEX_WAIT, 0, NULL <unfinished ...>
21771 <... write resumed> )             = -1 EPIPE (Broken pipe)
21768 <... select resumed> )            = 0 (Timeout)
21771 --- SIGPIPE (Broken pipe) @ 0 (0) ---
21768 futex(0x593ac0, FUTEX_WAIT, 0, {60, 0} <unfinished ...>
21771 rt_sigreturn(0xd <unfinished ...>
21767 <... wait4 resumed> [{WIFEXITED(s) && WEXITSTATUS(s) == 0}], 0,
{ru_utime={0, 496000}, ru_stime={0, 36000}, ...}) = 21770
21771 <... rt_sigreturn resumed> )      = -1 EPIPE (Broken pipe)


... note you can see the djpeg exit_group(0), then the SIGPIPE and the wait4 call return.

We can work around this in Go by knowing that this program does this and ignoring the
return value from cmd.Run() or cmd.Wait() and only looking at the
cmd.ProcessState.Success. But is that best?

Or are we doing something wrong?

Should Go ignore the Stdin copy error if the program ended successfully?

Camlistore bug: http://camlistore.org/issue/550
Upstream djpeg bug from Mathieu: http://sourceforge.net/p/libjpeg-turbo/bugs/80/

Attachments:

  1. PA190470.JPG (7781878 bytes)
@bradfitz
Copy link
Contributor Author

Comment 1:

I mentioned the JPEG structure but didn't elaborate: I have a theory that the JPEG has
some extra AppMarker segments at the end which don't contribute pixels to the image. So
djpeg stops reading when it's done with the pixels, does its resizing and writes its
output images and ends the process, without reading the extra junk at the end of the
JPEG.

@minux minux mentioned this issue Dec 14, 2014
Closed
@bradfitz bradfitz modified the milestone: Go1.5 Dec 16, 2014
@rsc rsc removed accepted labels Apr 14, 2015
@bradfitz
Copy link
Contributor Author

@ianlancetaylor, any wisdom here?

@ianlancetaylor
Copy link
Contributor

Don't be misled by the SIGPIPE in the strace output. Nothing is happening because of that signal. It is caught and ignored.

In the os/exec package, the io.Copy call in Cmd.stdin is returning an EPIPE error. That is the error being reported by the call to the Wait method. The name of the write side of the pipe is "|1" (from os.Pipe). The error is an os.PathError generated by os.File.Write, called by io.Copy.

I think that your suggestion of ignoring an EPIPE error in os/exec.stdin is reasonable. Cmd.Wait will already only return that error if the program exited successfully. In that scenario, returning EPIPE conveys little useful information.

Or, we could document the behaviour.

Most C programs in this situation will receive the SIGPIPE signal, fail to handle it, and exit. In a shell pipeline, nothing much will happen, because the shell will use the exit status of the final process in the pipeline.

If you do this, using bash:

set -o pipefail
cat boat.jpg | /usr/bin/djpeg > /dev/null
echo $?

you will see that the cat program died due to a SIGPIPE signal.

If you do this:

trap "" PIPE
cat boat.jpg | /usr/bin/djpeg > /dev/null

then the cat program will not get a SIGPIPE signal, but will instead fail because write returned EPIPE.

cat: write error: Broken pipe

Anyhow, the only real question is whether we should return the EPIPE error from os/exec, or just drop it. Personally I think it would be fine to drop it. That is the natural implementation of the default behaviour of shell pipelines.

@bradfitz
Copy link
Contributor Author

Excellent, thanks. I'd prefer to drop the error then. I'd prefer people go out of their way to get the failure for the weird case.

@dcbw dcbw mentioned this issue Sep 25, 2015
Merged
dcbw added a commit to dcbw/cni that referenced this issue Sep 25, 2015
@dcbw
Ignore EPIPE errors
b3a7ceb 
See golang/go#9173 which has been added
to Go >= 1.5.  If the subprocess for whatever reason (like exiting)
closes stdin without reading it the caller (cni) will get an EPIPE
error even if there was valid output and the subprocess finished
successfully.  Since Go >= 1.5 will be ignoring this error, we need
to compensate for older Go versions.
@golang golang locked and limited conversation to collaborators Jul 13, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.5
Development

No branches or pull requests
4 participants
@bradfitz @rsc @ianlancetaylor @gopherbot