Hi Catalin, Sorry for taking so long to get back on this. I've committed it now, slightly changed the check_close case since there's no point waiting to flush data to an exited child.
Cheers, Matt On Sun, Aug 25, 2013 at 03:37:38AM -0400, Catalin Patulea wrote: > ping? > > On Fri, Jul 26, 2013 at 9:31 PM, Matt Johnston <m...@ucc.asn.au> wrote: > > Hi Catalin, > > > > Thanks for looking at that - the last patch looks sensible, I'll give it a > > good test. There are a lot of subtle scenarios in channel closing (and > > variations between OSes). > > > > Cheers, > > Matt > > > > Catalin Patulea <c...@vv.carleton.ca> wrote: > >>Hm, that broke channel-close-by-child-exit. One more try, where we > >>check for the child exiting and close writefd as a result. If writefd > >>is the last remaining open pipe to the child, then we also close the > >>channel as a whole. > >> > >>On Wed, Jul 17, 2013 at 3:25 PM, Catalin Patulea <c...@vv.carleton.ca> > >>wrote: > >>> Attached patch should fix both, and use hard tabs so should apply > >>easily. > >>> > >>> Rather than replacing readfd with writefd, *both* are checked for > >>> FD_CLOSED before closing the entire channel. Then each direction can > >>> be initially closed independently. > >>> > >>> On Wed, Jul 17, 2013 at 7:57 PM, Catalin Patulea <c...@vv.carleton.ca> > >>wrote: > >>>> dbclient handling of remote EOF/local not at EOF also appears > >>broken: > >>>> > >>>> openssh: > >>>> $ ssh -v root@1.2.3.4 'exec cat >/dev/null 2>/dev/null' > >>>> [...] > >>>> debug1: Entering interactive session. > >>>> debug1: Sending command: exec cat >/dev/null 2>/dev/null > >>>> # remote has sent EOF by virtue of &>/dev/null, but local->remote > >>>> # direction still active > >>>> foo # local sends > >>>> bar > >>>> ^D # local EOF, channel tears down > >>>> debug1: client_input_channel_req: channel 1 rtype exit-status reply > >>0 > >>>> debug1: channel 1: free: client-session, nchannels 2 > >>>> <exit> > >>>> > >>>> Note 'exec' is required to replace shell and prevent it from holding > >>a > >>>> ref to the stdout pipe. > >>>> > >>>> dropbear: > >>>> $ ./dbclient -v r...@lun.nanobit.org 'exec cat >/dev/null > >>2>/dev/null' > >>>> [...] > >>>> TRACE (31787) 1374079900.330457: process_packet: packet type = 96, > >>len 5 > >>>> TRACE (31787) 1374079900.330499: enter recv_msg_channel_eof > >>>> TRACE (31787) 1374079900.330513: CLOSE some fd 1 > >>>> TRACE (31787) 1374079900.330526: sending close, readfd is closed > >>>> # remote EOF should *not* cause entire channel teardown; > >>>> # local may still have something to write ("foo bar" from openssh > >>>> # example). > >>>> TRACE (31787) 1374079900.330537: enter send_msg_channel_close > >>0x1f78660 > >>>> TRACE (31787) 1374079900.330549: enter cli_tty_cleanup > >>>> TRACE (31787) 1374079900.330560: leave cli_tty_cleanup: not in raw > >>mode > >>>> TRACE (31787) 1374079900.330606: CLOSE some fd 0 > >>>> TRACE (31787) 1374079900.330618: CLOSE some fd 2 > >>>> <exit> > >>>> > >>>> > >>>> On Sat, Jul 13, 2013 at 12:51 PM, Catalin Patulea > >><c...@vv.carleton.ca> wrote: > >>>>> Maybe the check on common-channel.c:338 should be against writefd > >>>>> instead of readfd? This would get set by > >>>>> close_chan_fd(channel->writefd) once recv_eof happens. > >>>>> > >>>>> This patch indeed causes 'foo' to surface after input EOF: > >>>>> > >>>>> diff -r 69cb561cc4c4 common-channel.c > >>>>> --- a/common-channel.c Sat Jul 13 11:53:24 2013 +0300 > >>>>> +++ b/common-channel.c Sat Jul 13 12:50:41 2013 +0300 > >>>>> @@ -335,7 +335,7 @@ > >>>>> } > >>>>> > >>>>> /* And if we can't receive any more data from them either, close > >>up */ > >>>>> - if (channel->readfd == FD_CLOSED > >>>>> + if (channel->writefd == FD_CLOSED > >>>>> && (ERRFD_IS_WRITE(channel) || channel->errfd == FD_CLOSED) > >>>>> && !channel->sent_close > >>>>> && close_allowed > >>>>> > >>>>> On Sat, Jul 13, 2013 at 12:31 PM, Catalin Patulea > >><c...@vv.carleton.ca> wrote: > >>>>>> Hi, > >>>>>> > >>>>>> I'm seeing a difference in how dbclient handles EOF on input > >>compared > >>>>>> to openssh client. openssh client propagates input EOF to the > >>remote > >>>>>> command, but continues pumping command stdout. dbclient seems to > >>abort > >>>>>> before flushing the stdout buffer. > >>>>>> > >>>>>> In the following examples, 1.2.3.4 is an openwrt router running > >>>>>> dropbear server. The remote command is designed to wait for EOF, > >>then > >>>>>> output something to stdout. > >>>>>> > >>>>>> openssh client: > >>>>>> $ ssh root@1.2.3.4 'cat; echo foo' > >>>>>> ^D > >>>>>> foo > >>>>>> > >>>>>> dbclient: > >>>>>> $ ./dbclient root@1.2.3.4 'cat; echo foo' > >>>>>> ^D > >>>>>> <no output> > >>>>>> > >>>>>> I build dropbear with DEBUG_TRACE and these are the last few > >>lines: > >>>>>> TRACE (...): empty queue dequeing > >>>>>> ^D > >>>>>> TRACE (...): send normal readfd > >>>>>> TRACE (...): enter send_msg_channel_data > >>>>>> TRACE (...): enter send_msg_channel_data isextended 0 fd 0 > >>>>>> TRACE (...): maxlen 16375 > >>>>>> TRACE (...): CLOSE some fd 0 > >>>>>> TRACE (...): leave send_msg_channel_data: len 0 read err 17 or EOF > >>for fd 0 > >>>>>> TRACE (...): enter send_msg_channel_eof > >>>>>> TRACE (...): leave send_msg_channel_eof > >>>>>> TRACE (...): sending close, readfd is closed > >>>>>> TRACE (...): enter send_msg_channel_close 0xcbfdc0 > >>>>>> TRACE (...): enter cli_tty_cleanup > >>>>>> TRACE (...): leave cli_tty_cleanup: not in raw mode > >>>>>> TRACE (...): CLOSE some fd -1 > >>>>>> TRACE (...): CLOSE some fd 2 > >>>>>> > >>>>>> I tried reading through the relevant sections of channel-common.c > >>but > >>>>>> I could use some guidance/background. Is this behaviour > >>intentional? > >>>>>> > >>>>>> Catalin