brolley wrote:
Regarding commit 9021460e, which nathans already reviewed(?) / merged:
> Implement overall timeout for service discovery.
>
> -t=N.N, --timeout=N.N for pmfind. N.N is the timeout period
> in seconds.
I have one concern: the new timeout parameter passed to the
pmDiscover* function doesn't actually work! What i mean is that it is
not sufficient to perform the timeout functionality by itself: it only
sets up a posix timer to send a SIGALARM sometime. It does not
consume that signal, but requires the client to register a SIGALRM
handler, and let it flip the options->interrupt flag. This embryonic
a service the pmfind app could have done for itself, by just calling
alarm(2)/setitimer(2).
A better approach would be to avoid presuming anything about the
signal handling state of the enveloping application (and thus not use
SIGALRM-generating facilities). For example, libpcp could spawn an
internal thread that just sleeps awhile, then sets the currently
operating context/interrupt flag. (The code would have to be careful
about races between that thread's activities vs. lifecycle, but it
doesn't sound too bad.)
- FChE
|