Re: [Xen-devel] Re: [PATCH] setsid() exception in xend

[Horms <horms@xxxxxxxxxxxx>]

On Mon, Nov 28, 2005 at 01:37:13PM +0000, Ewan Mellor wrote:
> On Mon, Nov 28, 2005 at 12:53:17PM +0000, Horms wrote:
> 
> > Ewan Mellor <ewan@xxxxxxxxxxxxx> wrote:
> > > On Mon, Nov 28, 2005 at 04:29:04AM +0000, Horms wrote:
> > > 
> > >> [Re: forking / setsid'ing patch]
> > > 
> > > Hi Horms,
> > > 
> > > How are you running Xend?  There's a call to fork in fork_pid, and 
> > > no-one's
> > > had a problem with this or setsid before.
> > 
> > Hi Ewan,
> > 
> > at the time that I noticed the problem, my machine was doing some very
> > strange things that I won't go into. I can't actually reproduce the
> > exception now that I fixed the box up.
> > 
> > However, I do think that there is a minor problem. My previous patch
> > seemed to solve it, but I think it was slightly wrong, as you point out
> > by the time daemonize() is called, the code is already running as a
> > child.
> > 
> > The thing that I think is missing is that after calling setsid(),
> > fork() needs to be called again. This allows the session leader
> > (the process that called setsid()) to exit, and this its children
> > have no way of regaining the terminal.
> > 
> > Here is a slightly revised patch that puts a second fork() after
> > setsid() (rather than before as the previous incarnation did).
> > If you look at the output of ps you should with and without this
> > patch, and see that the assosiation with the terminal disapears.
> 
> I agree that the second fork is required, so you're on the right track, but
> the rest of the patch seems problematic to me.  Do we really need to have the
> grandchild write to the child, just so that the child can write to the parent
> (hunk 1 of your patch)?  Why not just pass the descriptor from grandparent to
> grandchild directly?  I think that this would mean that the daemonize process
> could keep it's original interface, and just perform the extra fork, with no
> other changes.
> 
> Even if the intermediate pipe is required, closing one descriptor called
> "status" and then opening a new one and assigning that to status just for it
> to be returned from the function is unreasonably complicated.

Sure, I can eliminate the intermediate file descriptor,
I'll send a fresh patch tomorrow. 

On a related issue, can you clarify what the race is that it
is there to avoid? It seems cumbersome as you point out.

-- 
Horms

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel