pwait: Add a -t flag to specify a timeout in seconds before exiting.

[bdrewery]

Summary

The exit status will be 124, as the timeout(1) utility uses.

I plan to use this in poudriere to replace a chained 'timeout $n pwait' usage
in some critical code. I could easily just commit this to poudriere only
but this seemed useful in the default utility as well.

MFC after: 2 weeks

[jilles]

bin/pwait/pwait.c

94 ↗

(On Diff #25455)

Hmm, given that you are discarding the fractional part anyway, why use doubles in parse_duration()?

144 ↗

(On Diff #25455)

The timeout applies to waiting for each process, so if N processes are waited for it may take N times the given timeout.

This needs some more complicated code, either with clock_gettime() or with SIGALRM.

[bdrewery]

bin/pwait/pwait.c

94 ↗

(On Diff #25455)

Yes double makes no sense here. It was because I copied it from the timeout.c file and removed the support for suffixes.

144 ↗

(On Diff #25455)

Yeah I suspected that may be the case. It does what I expect with a simple case of all timing out which is a timeout for *pwait*, regardless of how many processes, but not if one exits:

~ # sleep 300 &
[1] 28293
~ # sleep 300 &
[2] 28299
~ # sleep 300 &
[3] 28337
~ # sleep 300 &
[4] 28345
~ # sleep 300 &
[5] 28358
~ # date
Fri Feb 24 09:03:24 PST 2017
~ # ./pwait -t 5 28293 28299 28337 28345 28358
~ # date
Fri Feb 24 09:03:31 PST 2017

But if one of them returns then the timeout is all wrong:

~ # date
Fri Feb 24 09:05:04 PST 2017
~ # ./pwait -t 5 28293 28299 28337 28345 28358
[5]  + terminated  sleep 300
~ # date
Fri Feb 24 09:05:34 PST 2017

[bdrewery]

Rework to use SIGALRM, support fractional seconds, and add some tests.

[jilles]

I like the tests, even though they take a fair bit of time to execute.

I would like to see the admittedly unlikely race fixed where SIGALRM occurs between checking got_alarm and calling kevent(). This could be either using EVFILT_SIGNAL or by writing the -v message (using write(2)) and exiting from the signal handler. I think the code will be simpler either way.

Also, signal() or sigaction() should be called before setitimer() to eliminate any window where you may get a SIGALRM exit.

[bdrewery]

In D9697#204786, @jilles wrote:

I like the tests, even though they take a fair bit of time to execute.

I would like to see the admittedly unlikely race fixed where SIGALRM occurs between checking got_alarm and calling kevent(). This could be either using EVFILT_SIGNAL or by writing the -v message (using write(2)) and exiting from the signal handler. I think the code will be simpler either way.

Admittedly I wanted to avoid exiting in the signal handler since I have been making some utilities into builtins in poudriere and want to control the exit status. Exiting from and using write(2) in the handler definitely would be simpler. I am going to explore using EVFILT_SIGNAL first.

Also, signal() or sigaction() should be called before setitimer() to eliminate any window where you may get a SIGALRM exit.

Woops.

[bdrewery]

In D9697#204796, @bdrewery wrote:

In D9697#204786, @jilles wrote:

I like the tests, even though they take a fair bit of time to execute.

I would like to see the admittedly unlikely race fixed where SIGALRM occurs between checking got_alarm and calling kevent(). This could be either using EVFILT_SIGNAL or by writing the -v message (using write(2)) and exiting from the signal handler. I think the code will be simpler either way.

Admittedly I wanted to avoid exiting in the signal handler since I have been making some utilities into builtins in poudriere and want to control the exit status. Exiting from and using write(2) in the handler definitely would be simpler. I am going to explore using EVFILT_SIGNAL first.

Meant to link https://github.com/freebsd/poudriere/blob/master/src/libexec/poudriere/pwait/pwait.c

Also, signal() or sigaction() should be called before setitimer() to eliminate any window where you may get a SIGALRM exit.

Woops.

[bdrewery]

Use EVFILT_SIGNAL rather than a signal handler function.

[jilles]

Looks good to me as a separate process.

For usage as a builtin, you may want shell-specific handling to allow processing traps while waiting in some way.

[bdrewery]

In D9697#205046, @jilles wrote:

Looks good to me as a separate process.

For usage as a builtin, you may want shell-specific handling to allow processing traps while waiting in some way.

Yeah, the only one I've cared to handle so far for poudriere is SIGINFO. The rest I am fine with causing an EINTR and ceasing the wait. For SIGINFO I just disable it while doing operations in builtins that may EINTR since poudriere's SIGINFO support is not worth all of the extra complexity to handle ambiguous EINTR, and still halt on SIGINT/SIGTERM/etc, in tools like this. That's what the 'siginfo_push' and 'siginfo_pop' calls are doing.