[cvsnt-dev] tentative bugfix: CVSNT 2.5.01.1898 PuTTY/ssh remote sometimes hangs right at the end

[Bjoren Davis]

Dear CVSNT folks,

I have to admit I don't know much about your project, but I
was using CVSNT and I had the problem that sometimes the
thing would hang at the end of an SSH transfer.

It bugged me, and I happened to have the MS VStudio 2003
Trial Edition and the better part of the day to kill, so
I (think) I fixed it.

The source I worked from was cvsnt-2.5.01.1898, and I
used the Microsoft Development Environment 2003 Version 7.1.3088.

Here are the deltas:

diff -cwr cvsnt-2.5.01.1898/plink/plink_cvsnt.c
cvsnt-2.5.01.1898-bkd_hacked/plink/plink_cvsnt.c
*** cvsnt-2.5.01.1898/plink/plink_cvsnt.c	Fri Oct 22 17:59:13 2004
--- cvsnt-2.5.01.1898-bkd_hacked/plink/plink_cvsnt.c	Sun Mar  6 19:48:48
2005
***************
*** 162,230 ****

  static struct output_data *global_odata;

! int plink_read_data(void *buffer, int max_length)
  {
!     struct output_data *odata = global_odata;
! 	int todo = max_length,l;
! 	char *ptr = buffer;

! 	if(fatal_exit)
  		return -1;
-
- 	if(odata->wrap)
- 	{
- 		l = odata->len - odata->lenwritten;
-
- 		if(l <= todo)
- 		{
- 			todo -= l;
- 			memcpy(ptr,odata->buffer+odata->lenwritten,l);
- 			ptr+=l;
- 			odata->lenwritten=odata->len;
- 			odata->writeret = odata->lenwritten;
- 			odata->wrap=0;
- 			SetEvent(odata->event);
- 			if(!todo)
- 				return 0;
  		}
! 		else
! 		{
! 			/* Actually, I'm not sure that this case ever happens.. */
! 			memcpy(ptr,odata->buffer+l,todo);
! 			ptr+=todo;
! 			odata->lenwritten+=todo;
  			return 0;
  		}
  	}

! 	WaitForSingleObject(odata->eventback, INFINITE);
! 	while(todo)
! 	{
! 		if (odata->done)
! 			break;

! 		if(odata->len <= (size_t)todo)
! 		{
! 			todo -= odata->len;
! 			memcpy(ptr, odata->buffer, odata->len);
! 			ptr+=odata->len;
! 			odata->lenwritten = odata->len;
! 			odata->writeret = odata->len;
  		}
! 		else
! 		{
! 			memcpy(ptr, odata->buffer, todo);
! 			ptr += todo;
! 			odata->lenwritten = todo;
! 			odata->wrap=1;
! 			break;
  		}
! 		SetEvent(odata->event);
! 		if(todo && WaitForSingleObject(odata->eventback, 40)!=WAIT_OBJECT_0)
! 			break;
  	}

!     return ptr - (char*)buffer;
  }

  static DWORD WINAPI stderr_write_thread(void *param)
--- 162,255 ----

  static struct output_data *global_odata;

! /*
!  * Read up to max_length worth of data.  This routine will
!  *  block only if there are no data to read.  If anything is
!  *  available, up to max_length of it will be returned immediately.
!  *
!  * Rewritten March 6, 2005, Bjoren Davis.
!  *
!  * I was having problems with cvsnt hanging at the end of
!  *  some ssh transfers, and I had no idea why.  I looked at the
!  *  old version of this routine, and I really couldn't understand
!  *  what it was trying to do.  It looked as though it was doing
!  *  a read-and-block-only-if-there's-nothing-but-otherwise-gimme
!  *  -what-you-got.  However, it would block and loop if at the
!  *  outset of the routine you didn't have any data.  Strange.
!  *  So I wrote the read-and-block-only-if-there's-nothing-but-otherwise-
!  *  gimme-what-you-got routine you see here.
!  */
! int
! plink_read_data(void *buffer, int max_length)
  {
!     struct output_data		*odata;
!     int				 todo, l;
!     char			*ptr;
!
!     odata = global_odata;
!     todo = max_length;
!     ptr = (char *) buffer;

!     /*
!      * Loop until we can get something into the return buffer.
!      */
!     while (TRUE) {
! 	/*
! 	 * I put the test for fatal_exit here so that it gets checked
! 	 *  everytime after we wait.
! 	 */
! 	if (fatal_exit) {
  	    return -1;
  	}
!
! 	if (odata->done) {  /* !!!this flag doesn't seem to be set anywhere. */
! 	    odata->writeret = 0;
  	    return 0;
  	}
+
+ 	/*
+ 	 * Do we have data, and have we not sent all of it up yet?
+ 	 */
+
+ 	if (odata->len > 0 && (l = odata->len - odata->lenwritten) > 0) {
+ 	    /* Yes, we have some data to return. */
+
+ 	    /*
+ 	     * If we have fewer data than what's being asked for, then
+ 	     *  just settle for what we have.
+ 	     */
+ 	    if (todo > l) {
+ 		/* we can return at most l bytes right now. */
+ 		todo = l;
  	    }

! 	    memcpy(ptr, odata->buffer + odata->lenwritten, todo);
! 	    odata->lenwritten += todo;
! 	    l -= todo;
! 	    odata->writeret = todo;

! 	    /*
! 	     * Have we completely drained our supply?  If so, then
! 	     *  we want to trigger a reload.  If not, then we'll
! 	     *  fetch more the next time around.
! 	     */
!
! 	    if (l == 0) {
! 		/* There's nothing left.  Trigger the reload. */
!
! 		SetEvent(odata->event);
  	    }
!
! 	    return todo;
  	}
!
! 	/*
! 	 * We're here because we have no data.  So wait for some.
! 	 */
! 	WaitForSingleObject(odata->eventback, INFINITE);
      }

!     /* NOTREACHED */
  }

  static DWORD WINAPI stderr_write_thread(void *param)
***************
*** 261,266 ****
--- 286,292 ----
  			&senddata, &sendlen);
  	data->buffer = senddata;
  	data->len = sendlen;
+ 	data->lenwritten = 0;
  	SetEvent(data->eventback);
  	data->busy = 1;
      }
***************
*** 436,442 ****
  	} else if (n == 3) {
  	    edata.busy = 0;
  	    if (!edata.writeret) {
! 		fprintf(stderr, "Unable to write to standard output\n");
  		cleanup_exit(0);
  	    }
  	    bufchain_consume(&stderr_data, edata.lenwritten);
--- 462,468 ----
  	} else if (n == 3) {
  	    edata.busy = 0;
  	    if (!edata.writeret) {
! 		fprintf(stderr, "Unable to write to standard error\n");
  		cleanup_exit(0);
  	    }
  	    bufchain_consume(&stderr_data, edata.lenwritten);
***************
*** 598,603 ****
--- 624,630 ----
      odata.event = stdoutevent;
      odata.eventback = CreateEvent(NULL, FALSE, FALSE, NULL);
      odata.busy = odata.done = 0;
+     odata.len = odata.lenwritten = 0;

  	global_odata = &odata;



This seems to fix my problems.  I hope it doesn't cause any others.
It might be that the entire problem can be fixed by setting lenwritten
to 0 in the try_output routine (lines 261-266).  I dunno.


Thanks for a great product!

--Bjoren Davis