fork and wait

[Alan DeKok]

Joe Maimon wrote:
> code calls rad_fork(1) (such as rad_check_ts())
> 
> This means that no entry is stored in thread_pool.waiters

  It looks like that's a bug in rad_fork.  It should probably be doing

     if (!exec_wait) return fork;

  Oops.

> (why 0 and not -1
> main waitpid.....
> 
>        -1     meaning wait for any child process.
> 
>        0      meaning wait for any child process whose process group ID
> is equal to that of the calling process.

  I don't have strong opinions here.

> The fix? Here are some ideas.

  Try adding the '!' to rad_fork.

  The problem that horrible code in threads.c is trying to solve is
getting the child exit condition to the correct thread.  The waitpid()
function call doesn't have an argument saying "wait only for programs
forked from this thread.".  So one thread calling waitpid() can get the
exit condition from a child forked from another thread.

  The solution (such as it is) is a global list of children.  When a
thread forks a process, and indicates that it cares about the exit
condition, that PID is placed into the global list.  When any thread
calls waitpid(), it looks up the resulting PID in the list.  If it's
found, then the child exit status is updated.

  There's also the case of a thread that wants to call waitpid(), but
another thread has already grabbed the exit code, and updated the global
list.  Hence rad_waitpid().   That looks in the list, and if the PID is
found, clears the entry and returns.  If an entry isn't found, it calls
waitpid (as above), and updates the list with the status of any child
that has exited.

  There's another case, where a child forks a thread, and doesn't care
about the exit status.  In that case, the child PID isn't put into the list.

  I suppose the code could be made a little simpler by removing the
argument to rad_fork().  That way, any thread that cared about the exit
condition would call rad_fork(), and threads that didn't care would call
fork().

  I think that covers all cases, without involving massive code changes.

  So all instances of rad_fork() should be audited, to ensure that
they're being used correctly.  Some can probably be changed to use fork().

  Alan DeKok.
--
  http://deployingradius.com       - The web site of the book
  http://deployingradius.com/blog/ - The blog