Re: [Patch] channel_get_exit_status always return -1

[Vic Lee <llyzs@xxxxxxx>]

Hi Aris,

Yes you are right, I tested with openssh server and it did close the
channel after exit_status was sent. Thanks for your comments, it really
helped.

Just for curiosity, I also did some testing for the following senario
with the current libssh master. My requirement is just to open a
channel, run a command, get the exit_status, then close. So the
"official" way in sample.c (using select -> is_closed ->
get_exit_status) looks not like the best way for me.

1) exec -> send_eof -> get_exit_status -> close
Result: 100% got the exit_status. This works just fine.

2) exec -> close -> get_exit_status
Result: got exit_status at 50% chance, otherwise got -1.

Looks like 2) will put the server in a race condition: it depends on
whether the cmd exit first, or the channel close request comes first.

Anyway, I think I am done with this research. :) Thanks again!

Vic

On Mon, 2009-08-17 at 16:21 +0200, Aris Adamantiadis wrote:
> Vic,
> 
> You made interesting research. Let me quote the SSH RFC:
> 
>    When the command running at the other end terminates, the following
>    message can be sent to return the exit status of the command.
>    Returning the status is RECOMMENDED.  No acknowledgement is sent for
>    this message.  The channel needs to be closed with
>    SSH_MSG_CHANNEL_CLOSE after this message.
> 
>    The client MAY ignore these messages.
> 
> It means
> 1- The server is going to close the channel when this command is sent
> 2- If you receive this message, the command stops outputing things, so the channel 'should' be EOF
> 3- After you receive this message, the channel is going to be closed which implies EOF.
> 
> I have yet to read why I test local_eof and not remote_eof. Dropping these may be ok if you do it 
> the same way that isclosed.
> 
> By the way, to execute multiple commands, you are forced to use several channels. Sorry
> 
> Regards,
> 
> Aris
> 
> Vic Lee a écrit :
> > Hi Aris,
> >
> > Today I played a little bit more with exit_status and you are right, it
> > won't be able to get it if the channel is closed. If I just try to
> > channel_send_eof then it works just fine.
> >
> > But now I come up with an interesting question: why I have to send eof
> > before I can call channel_get_exit_status? I actually tried to remove
> > the following lines, and I have no problem getting exit_status without
> > sending eof:
> >
> >  int channel_get_exit_status(ssh_channel channel) {
> > -  if (channel->local_eof == 0) {
> > -    return -1;
> > -  }
> >
> > Although right now it's not problem for me because I only need to issue
> > one exec, but let's imagine someone else in the future might need to
> > execute several commands and he needs to get exit_status for each
> > command before he can continue with the next. In this case, does he have
> > to open/close a new channel for each command? I have a strong feeling
> > that we should remove the above three lines; or did I miss something
> > else?
> >
> > Thanks a lot and sorry for my long question. :)
> >
> > Vic
> >
> > On Sun, 2009-08-16 at 13:55 +0200, Aris Adamantiadis wrote:
> >   
> >> Hi Vic,
> >>
> >> Thanks for your patch, there is indeed a problem. But your patch does 
> >> not resolve it the clean way. When a channel has been closed, the 
> >> optional exit_status message can't come anymore, hence the return -1 in 
> >> the code (simply removing the test may block the function forever if no 
> >> exit status is sent). I replaced it by a break; so it returns whatever 
> >> value was saved.
> >>
> >> Regards,
> >> Aris
> >>
> >> Vic Lee a écrit :
> >>     
> >>> Hi,
> >>>
> >>> I just found out that the channel_get_exit_status() function always
> >>> return -1. What I saw in the codes is that, the channel is supposed to
> >>> close first before we can call channel_get_exit_status; which is true
> >>> because it's checking "channel->local_eof == 0" in the first line of
> >>> that function.
> >>>
> >>> But if I have to close the channel, why another checking "channel->open
> >>> == 0" to check whether the channel is open? So I tried to remove these
> >>> three lines, and finally it's working as expected.
> >>>
> >>> Thanks,
> >>>
> >>> Vic
> >>>       
> >
> >
> >
> >   
> 
>