Re: [PATCH v2] udevd: don't use alarm() for timeouts

[Alan Jenkins <alan-jenkins@tuffmail.co.uk>]

Kay Sievers wrote:
> On Tue, May 26, 2009 at 12:31, Alan Jenkins <alan-jenkins@tuffmail.co.uk> wrote:
>   
>> alarm() is per-process; we can't use it for timeouts in a multi-threaded
>> udevd.  We need to explicitly track the total time spent waiting for
>> each event.
>>
>> There is an issue here if the timeout expires in run_program().  If
>> run_program() returns without calling wait(), the process it forked
>> will become a zombie when it finally exits.  Currently, udev-event is
>> implemented as a process, so the zombie will be reparented to init and
>> reaped once the event is finished.  But that won't work for threads.
>>
>> The solution is to fork twice:
>>
>> udevd (event thread)
>>  udevd child process
>>  command
>>
>> When the timeout expires, the event thread can return immediately.
>> The child process will stay blocked in wait(), until the command finally
>> finishes (or is killed).  We'll get a nice process tree showing that the
>> the hung process was started by udevd :-).
>>     
>
> The event runs all programs serialized, one after the other, can't we
> just kill the program that does not return in time, and wait for it to
> cleanup the process, instead of just exiting the event process?
>   

That makes sense if you also change the timeout model - from per-event
timeouts, to per-command timeouts.

The second fork is cheap, because it's a vfork.  But if you like
per-command timeouts, I would be happy to see it go away.

I'm not sure about killing.  Do we need to escalate to SIGKILL?  Do we
e.g. allow half the timeout before sending SIGTERM, then another half
before sending SIGKILL?  The process could even be unkillable - is it ok
to block after SIGKILL, or do we need another timeout?

I could keep a list of timed out processes instead, and reap them on
SIGCHLD.  Would that be better?

There are other workarounds for the lack of a timeout in sys_wait(), so
I don't think that's a problem.  We can require that commands close
stdout & stderr pipes when they exit - i.e. do not pass them on to a
long-running child.  (At the moment there's a debian script "net.agent"
which has to do this for debug mode - it would need fixing to always do it).

Alan