> On 29 Sep 2016, at 18:57, Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Torsten Bögershausen <tboegi@xxxxxx> writes: > >>> 1) Git exits >>> 2) The filter process receives EOF and prints "STOP" to the log >>> 3) t0021 checks the content of the log >>> >>> Sometimes 3 happened before 2 which makes the test fail. >>> (Example: https://travis-ci.org/git/git/jobs/162660563 ) >>> >>> I added a this to wait until the filter process terminates: >>> >>> +wait_for_filter_termination () { >>> + while ps | grep -v grep | grep -F "/t0021/rot13-filter.pl" >/dev/null 2>&1 >>> + do >>> + echo "Waiting for /t0021/rot13-filter.pl to finish..." >>> + sleep 1 >>> + done >>> +} >>> >>> Does this look OK to you? >> Do we need the ps at all ? >> How about this: >> >> +wait_for_filter_termination () { >> + while ! grep "STOP" LOGFILENAME >/dev/null >> + do >> + echo "Waiting for /t0021/rot13-filter.pl to finish..." >> + sleep 1 >> + done >> +} > > Running "ps" and grepping for a command is not suitable for script > to reliably tell things, so it is out of question. Compared to > that, your version looks slightly better, but what if the machinery > that being tested, i.e. the part that drives the filter process, is > buggy or becomes buggy and causes the filter process that writes > "STOP" to die before it actually writes that string? > > I have a feeling that the machinery being tested needs to be fixed > so that the sequence is always be: > > 0) Git spawns the filter process, as it needs some contents to > be filtered. > > 1) Git did everything it needed to do and decides that is time > to go. > > 2) Filter process receives EOF and prints "STOP" to the log. > > 3) Git waits until the filter process finishes. > > 4) t0021, after Git finishes, checks the log. > > Repeated sleep combined with grep is probably just sweeping the real > problem under the rug. Do we have enough information to do the > above? > > An inspiration may be in the way we centrally clean all tempfiles > and lockfiles before exiting. We have a central registry of these > files that need cleaning up and have a single atexit(3) handler to > clean them up. Perhaps we need a registry that filter processes > spawned by the mechanism Lars introduces in this series, and have an > atexit(3) handler that closes the pipe to them (which signals the > filters that it is time for them to go) and wait(2) on them, or > something? I do not think we want any kill(2) to be involved in > this clean-up procedure, but I do think we should wait(2) on what we > spawn, as long as these processes are meant to be shut down when the > main process of Git exits (this is different from things like > credential-cache daemon where they are expected to persist and meant > to serve multiple Git processes). We discussed that issue in v4 and v6: http://public-inbox.org/git/20160803225313.pk3tfe5ovz4y3i7l@xxxxxxxxxxxxxxxxxxxxx/ http://public-inbox.org/git/xmqqbn0a3wy3.fsf@xxxxxxxxxxxxxxxxxxxxxxxxxxx/ My impression was that you don't want Git to wait for the filter process. If Git waits for the filter process - how long should Git wait? Thanks, Lars