[PATCH] openssh-agent polling

[Alan Jenkins]

Alan Jenkins wrote:
> David Leonard wrote:
>> Damien Miller wrote:
>>> On Wed, 12 Sep 2007, Alan Jenkins wrote:
>>>> This may be a stupid question - but why does ssh-agent fork off
>>>>  a child and then exec the subcommand in the *parent* process? It 
>>>> forces the agent in the child process to poll the parent, so
>>>>  it can exit when the parent finishes.  If the agent was the parent 
>>>> process, it could use wait() instead.  I'm sure there's some reason.
>>>>
>>>> It'd be nice to take ssh-agent off the list of polling processes you 
>>>> get with powertop[1], however far down it is.
>>>>
> 
>> ssh-agent starts its command in the parent process probably so that
>>  you can get something useful from $! returned in your startup script,
> 
> Surely that's fixable - wait() gives you the terminated child's exit code.
> 
>> and to avoid the situation where ssh-agent could get an untrappable
>> signal and leaves the underlying command running.
> 
> Which way round do you mean this?  Are we trying to avoid leaving the 
> underlying command running if ssh-agent is killed, or do we want it to 
> carry on running independently?  It sounds like you meant the subcommand 
> should be killed if ssh-agent is, but my understanding of the current 
> behaviour is that the subcommand keeps on running.
> 
> If the aim is to keep the subcommand running, I guess we're stuck with 
> polling.  If I've got my un*x right, it's not possible to wait() on a 
> process unless it would get killed if you terminate.

I had my un*x wrong.  In general child processes keep on running when
their parent dies.

So you can do:

fork();
    parent: /* foreground process */
      int status = 0; wait(&status); exit(status);

    child: /* agent */
      fork();
        parent:
          signal(SIGCHLD, {
            int status = 0; wait(&status); exit(status);
          });
          detach();
          run_agent(); /* does not return */

        child: /* subcommand */
          exec(subcommand);

I think you can't get rid of the extra fork.  You need it in order for
setsid() to be guaranteed to succeed in the agent process (this is a
required in order to daemonize, which is represented above by the
detach() function).

I has a patch. It makes things a little harder to understand but it
does work.  Anyone like, dislike?

To recap: this avoids polling when a subcommand is started, by waiting
for SIGCHLD instead of waking up every 10 seconds to check the
subcommand is still alive.  An extra wakeup every 10 seconds is not
practically significant, but means that ssh-agent will show up on
profiling tools such as powertop[1] for no good reason.

[1] http://www.linuxpowertop.org

Alan

-------------- next part --------------
A non-text attachment was scrubbed...
Name: ssh-agent.diff
Type: text/x-patch
Size: 4166 bytes
Desc: not available
Url : http://lists.mindrot.org/pipermail/openssh-unix-dev/attachments/20070920/939e075e/attachment.bin