coreutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [coreutils] [PATCH] sort: fix some --compress reaper bugs


From: Jim Meyering
Subject: Re: [coreutils] [PATCH] sort: fix some --compress reaper bugs
Date: Tue, 14 Dec 2010 19:58:27 +0100

Paul Eggert wrote:
> On 14/12/10 09:55, Jim Meyering wrote:
>> It'd be a shame to make everyone sleep even one second for NFS.
>
> Well, it is marked as an _expensive_ test.  :-)
>
> On 12/14/10 02:07, Pádraig Brady wrote:
>> If the subprocesses were reparented to the shell,
>> then one could just `wait`.
>
> I don't know how to do reparenting like that.  But this raises a
> more general issue: is it OK that when 'sort' is interrupted and
> exits, that it does not kill off its children?  That is, it relies on
> compressors nicely exiting (because 'sort' exits and they read EOF),
> and it relies on decompressors nicely exiting (because they write to a
> dangling pipe and get SIGPIPE or a write failure).  If either
> assumption is false, 'sort' will leave behind orphan processes that
> never exit.
>
> I'm sort of inclined to say that's OK, and that it's the invoker's
> responsibility to make sure that compressors and decompressors behave
> 'nicely'.  There are other ways in which they have to be 'nice', too:
> for example, they shouldn't go poking around the file system, fiddle
> with all the file descriptors that they have open, etc.  This is all
> obvious stuff but perhaps it should be documented?

For now, I think the status quo is ok,
though adding documentation would be nice, of course.

> Anyway, to get back to the topic, here's the change I pushed.  It's
> less fancy than reparenting or looping or whatever, but it should
> fix the problem in 99.9% of the time.  I'm not sure the remaining
> 0.1% is worth the tuning effort, given that it's an expensive test
> anyway.

That solves the problem for me.  Thanks!
Thanks for cleaning up the TMPDIR uses, too.

> Subject: [PATCH] tests: default to /tmp as the temporary directory
>
> * tests/check.mk (TESTS_ENVIRONMENT): Default TMPDIR to /tmp,
> rather than to the working directory; this is more common in
> practice, which makes the tests more real-worldish; and it is
> often faster.  Also, it avoids some problems with NFS cleanups.
> * tests/misc/sort-compress: Remove unnecessary code setting TMPDIR.
> * tests/misc/sort-compress-proc: Likewise.  Do the final sleep
> only if TMPDIR is relative, which should be rarely given the
> change to TESTS_ENVIRONMENT.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]