[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.