[PATCH] Processes that spawn other processes should wait for their children to exit before exiting due to a signal.

[Chris Bieneman]

> On Apr 30, 2015, at 3:17 PM, Daniel Dunbar <daniel at zuster.org> wrote:
> 
> It doesn't look like ChildPIDs is ever added to?

Doh… I had un-comitted changes locally that I left out of the diff. Will update in a minute with your other feedback incorporated too.

> 
> This implementation can possibly call free() from ChildPIDs->clear(), and
> while sys::Wait() looks safe as invoked, I could see how someone might
> change it in a way that would cause it to break. It might be safer to call
> waitpid() directly, or at least to add a comment to sys::Wait() noting that
> it needs to stay signal safe when WaitUntilTerminates == 0 && ErrMsg ==
> nullptr.

I prefer calling sys::Wait because it is a bit more complex than just calling waitpid(), so I’ll comment it.

I didn’t think about the free. I can not call clear if I ensure that WaitForChildren() isn’t multiply registered. I didn’t bother to prevent multiple registration initially because there really isn’t much cost associated with calling it if ChildPIDs is empty.

> 
> Also, I think you should take care not to register WaitForChildren()
> multiple times.

Will do.

> 
> Probably also good to include a note about why we are waiting for children.

Will do.

-Chris

> 
> - Daniel
> 
> 
> http://reviews.llvm.org/D9420
> 
> EMAIL PREFERENCES
>  http://reviews.llvm.org/settings/panel/emailpreferences/
> 
>