[BUG][PATCH] lash,hush setting childs pgrp

[Rob Landley]

On Sunday 11 June 2006 6:46 am, Bernhard Fischer wrote:
> The tcsetpgrp() here is setting shell's pgrp to that of the first
> process
> of the new job group. But by that time the child process has terminated
> and hence it sets errno to EPERM ( -1 ) .  The child process terminates
> before the parent bcoz I am using vfork() instead of fork() and due to
> BB_FEATURE_SH_STANDALONE_SHELL && BB_FEATURE_SH_APPLETS_ALWAYS_WIN
> option
> the execve() function is not called by the child and hence the parent
> does
> not get scheduled unless the child terminates.

Good analysis.

> So do I need to have tcsetpgrp() at this case? I think the problem can
> be
> solved by calling waitpid() and if it does not return with errno =
> ECHILD
> ( ie Child hasnt terminated yet ) then only calling  tcsetpgrp().
>
> if( (waitpid((newjob->pgrp), &status , WNOHANG) >= 0) && errno !=ECHILD)
> {
>      if (tcsetpgrp(shell_terminal, newjob->pgrp) && errno != ENOTTY)
>               perror_msg("tcsetpgrp");
> }
>
> It works properly with this. I would like to know whether this is
> correct
> solution. A similar prolem also exists in hash.c
> "
>
> AFAICT his proposed solution works and i intend to apply the attached
> patch for lash to repair this bug. The downside is that it adds
> 26B, so perhaps there is an alternative approach to the attached
> sledgehammer..

The patch doesn't eliminate the race, it just moves it.  We either need to 
move the call to tcsetpgrp() so it happens in the child before the exec, or 
we need to to just not consider it failing an error.  (The first would be 
preferable, but I dunno how that interacts with vfork().)

Rob
-- 
Never bet against the cheap plastic solution.