[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

'svn diff' and 'svn log' not stopping when pipe is closed

From: Nuutti Kotivuori <naked_at_iki.fi>
Date: 2002-12-11 20:05:02 CET

Longish mail, bear with me. This all came up during a discussion with
pretzelgod on IRC.

* Problem

$ svn log http://svn.collab.net/repos/svn/ | ls

Even though 'ls' is not reading it's stdin at all, 'svn log' keeps
writing all it's output there until finished. You might ofcourse say
that "don't do that", but the same thing happens with less if you quit
before the output is finished: svn should stop fetching new entries
when that happens.

The same problem happens also with 'svn diff', but it's a bit
different there since a subprocess is doing all the outputting.

* Cause

If we look at a strace of the above command, we see this:

| ...
| rt_sigaction(SIGPIPE, {SIG_IGN}, {SIG_DFL}, 8) = 0
| ...
| select(4, [3], NULL, NULL, {120, 0}) = 1 (in [3], left {120, 0})
| read(3, "\241\211\312/\343\222\340\255\356\360&cn\240\217Q\367\305"..., 8192) = 4032
| write(1, "--------------------------------"..., 4096) = -1 EPIPE (Broken pipe)
| --- SIGPIPE (Broken pipe) ---
| write(1, "--------------------------------"..., 4096) = -1 EPIPE (Broken pipe)
| --- SIGPIPE (Broken pipe) ---
| ...

Eg. it's reading from the network and writing the output to stdout -
but the write fails with an EPIPE, but we don't care. The SIGPIPE
signal that is delivered is ignored, as seen in the above rt_sigaction
setting. Neon is setting that sigaction for us, so that's a bit out of
our hands there - and it's good that it is set, I'm not sure I'm
comfortable with us just aborting on SIGPIPE.

So, then if we look at the code in 'svn log':

| ...
| printf ("\n"); /* A blank line always precedes the log message. */
| printf ("%s\n", msg_native);
| ...

Simple printf's - and no error checking is done. So even if the
printf's would return decent errors, we wouldn't check them. But
assuming we did - there's another problem. Stdout is supposed to be
line-buffered, but apparently it only is when the output is a
terminal. When piping the output to some program, it's fully
buffered. And it's not even as simple as that - apparently it only
does that on the Linux machine I have here. What that obviously means
that 'printf' cannot return the errors as it is writing to the buffer
and then flushing the buffer only every now and then.

* Solution for 'svn log'

** Using 'fflush' - DOES NOT WORK

The first idea was to add a call to 'fflush' to the end of every log
message and checking it's return value. The error could be processed
there and the loop bailed out if there was an EPIPE.

But 'fflush' will not return an error if the buffer is already empty -
which could be if we hit the right spot, or which it will always be if
stdout indeed is line-buffered.

So, no bonus.

** Using 'apr_file_printf'

Instead of printing with 'printf' directly, we can start using
'apr_file_open_stdout' and running 'apr_file_printf' on
it. 'apr_file_printf' will return an error if the write results in
EPIPE. So if we check all the return values of apr_file_printf calls,
we can detect EPIPE at any point.

There appears to be a bit of a weird behaviour on this in APR
actually. If the file is buffered, then writes only write to the
buffer until it's full. When the buffer is full, 'apr_file_flush' is
called, but it's return value is not checked. So buffered writes will
never return an error. If the file is not buffered, then every
'apr_file_printf' will translate into 'apr_vsnprintf' and 'apr_puts'
which in turn calls 'apr_file_write' whose error is returned all the
way to the caller. Luckily for us, the 'apr_file_open_stdout' does not
make stdout buffered.

But this all will mean that writes to stdout will not be buffered and
that everything is sprintfed first by apr_vformatter - I don't know if
either of these is a good or bad thing.

* Solution for 'svn diff'

This is a bit trickier. First of all, we should make sure that 'diff'
actually gets the SIGPIPE signal. I'm not sure if ignored signal
handlers are inherited by exec'd processes, but if they are, we need
to reset that before invoking diff.

Secondly, we'd have to do the same 'printf' checking as in 'svn log'
to be able to abort running the diffs when the pipe is closed.

* Conclusions

I think I'm missing something major here - this shouldn't be this hard
to do - so please point it out. The main reason why I'm writing this
here is that I do not want the discussion on IRC be in vain - so I'm
putting the conclusions somewhere where they can be seen.

-- Naked

To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Dec 11 20:05:47 2002

This is an archived mail posted to the Subversion Dev mailing list.