Added a refcounting mechanism to ChannelBuffers. Other edits to

stop segfaults in tests iocmd-21.2[12].

4 files changed, 64 insertions, 10 deletions

diff --git a/generic/tclIO.c b/generic/tclIO.c
index 9e675c6..a60c97d 100644
--- a/generic/tclIO.c
+++ b/generic/tclIO.c
@@ -162,6 +162,8 @@ typedef struct CloseCallback {
  */
 
 static ChannelBuffer *	AllocChannelBuffer(int length);
+static void		PreserveChannelBuffer(ChannelBuffer *bufPtr);
+static void		ReleaseChannelBuffer(ChannelBuffer *bufPtr);
 static void		ChannelTimerProc(ClientData clientData);
 static int		CheckChannelErrors(ChannelState *statePtr,
 			    int direction);
@@ -352,6 +354,7 @@ ChanRead(
     int *errnoPtr)
 {
     if (WillRead(chanPtr) < 0) {
+	*errnoPtr = Tcl_GetErrno();
         return -1;
     }
 
@@ -2216,8 +2219,26 @@ AllocChannelBuffer(
     bufPtr->nextRemoved	= BUFFER_PADDING;
     bufPtr->bufLength	= length + BUFFER_PADDING;
     bufPtr->nextPtr	= NULL;
+    bufPtr->refCount	= 1;
     return bufPtr;
 }
+
+static void
+PreserveChannelBuffer(
+    ChannelBuffer *bufPtr)
+{
+    bufPtr->refCount++;
+}
+
+static void
+ReleaseChannelBuffer(
+    ChannelBuffer *bufPtr)
+{
+    if (--bufPtr->refCount) {
+	return;
+    }
+    ckfree((char *) bufPtr);
+}
 
 /*
  *----------------------------------------------------------------------
@@ -2250,7 +2271,7 @@ RecycleBuffer(
      */
 
     if (mustDiscard) {
-	ckfree((char *) bufPtr);
+	ReleaseChannelBuffer(bufPtr);
 	return;
     }
 
@@ -2261,7 +2282,7 @@ RecycleBuffer(
      */
 
     if ((bufPtr->bufLength - BUFFER_PADDING) < statePtr->bufSize) {
-	ckfree((char *) bufPtr);
+	ReleaseChannelBuffer(bufPtr);
 	return;
     }
 
@@ -2296,7 +2317,7 @@ RecycleBuffer(
      * If we reached this code we return the buffer to the OS.
      */
 
-    ckfree((char *) bufPtr);
+    ReleaseChannelBuffer(bufPtr);
     return;
 
   keepBuffer:
@@ -2470,6 +2491,7 @@ FlushChannel(
 	 * Produce the output on the channel.
 	 */
 
+	PreserveChannelBuffer(bufPtr);
 	toWrite = BytesLeft(bufPtr);
 	if (toWrite == 0) {
 	    written = 0;
@@ -2597,6 +2619,7 @@ FlushChannel(
 	    }
 	    RecycleBuffer(statePtr, bufPtr, 0);
 	}
+	ReleaseChannelBuffer(bufPtr);
     }	/* Closes "while (1)". */
 
     /*
@@ -2681,7 +2704,7 @@ CloseChannel(
      */
 
     if (statePtr->curOutPtr != NULL) {
-	ckfree((char *) statePtr->curOutPtr);
+	ReleaseChannelBuffer(statePtr->curOutPtr);
 	statePtr->curOutPtr = NULL;
     }
 
@@ -3566,6 +3589,11 @@ static void WillWrite(Channel *chanPtr)
 
 static int WillRead(Channel *chanPtr)
 {
+    if (chanPtr->typePtr == NULL) {
+	/* Prevent read attempts on a closed channel */
+	Tcl_SetErrno(EINVAL);
+	return -1;
+    }
     if ((chanPtr->typePtr->seekProc != NULL)
         && (Tcl_OutputBuffered((Tcl_Channel) chanPtr) > 0)) {
         if ((chanPtr->state->curOutPtr != NULL)
@@ -3652,6 +3680,7 @@ Write(
 	    bufPtr->nextAdded += saved;
 	    saved = 0;
 	}
+	PreserveChannelBuffer(bufPtr);
 	dst = InsertPoint(bufPtr);
 	dstLen = SpaceLeft(bufPtr);
 
@@ -3665,6 +3694,7 @@ Write(
 	
 	if ((result != TCL_OK) && (srcRead + dstWrote == 0)) {
 	    /* We're reading from invalid/incomplete UTF-8 */
+	    ReleaseChannelBuffer(bufPtr);
 	    if (total == 0) {
 		Tcl_SetErrno(EINVAL);
 		return -1;
@@ -3748,6 +3778,7 @@ Write(
 		needNlFlush = 0;
 	    }
 	}
+	ReleaseChannelBuffer(bufPtr);
     }
     if ((flushed < total) && (statePtr->flags & CHANNEL_UNBUFFERED ||
 	    (needNlFlush && statePtr->flags & CHANNEL_LINEBUFFERED))) {
@@ -5935,7 +5966,7 @@ DiscardInputQueued(
      */
 
     if (discardSavedBuffers && statePtr->saveInBufPtr != NULL) {
-	ckfree((char *) statePtr->saveInBufPtr);
+	ReleaseChannelBuffer(statePtr->saveInBufPtr);
 	statePtr->saveInBufPtr = NULL;
     }
 }
@@ -6028,7 +6059,7 @@ GetInput(
 
 	if ((bufPtr != NULL)
 		&& (bufPtr->bufLength - BUFFER_PADDING < statePtr->bufSize)) {
-	    ckfree((char *) bufPtr);
+	    ReleaseChannelBuffer(bufPtr);
 	    bufPtr = NULL;
 	}
 
@@ -6090,6 +6121,7 @@ GetInput(
     } else {
 #endif /* TCL_IO_TRACK_OS_FOR_DRIVER_WITH_BAD_BLOCKING */
 
+	PreserveChannelBuffer(bufPtr);
 	nread = ChanRead(chanPtr, InsertPoint(bufPtr), toRead, &result);
 
 #ifdef TCL_IO_TRACK_OS_FOR_DRIVER_WITH_BAD_BLOCKING
@@ -6097,6 +6129,7 @@ GetInput(
 #endif /* TCL_IO_TRACK_OS_FOR_DRIVER_WITH_BAD_BLOCKING */
 
     if (nread > 0) {
+	result = 0;
 	bufPtr->nextAdded += nread;
 
 	/*
@@ -6122,6 +6155,7 @@ GetInput(
 #endif /* TCL_IO_TRACK_OS_FOR_DRIVER_WITH_BAD_BLOCKING */
 
     } else if (nread == 0) {
+	result = 0;
 	SetFlag(statePtr, CHANNEL_EOF);
 	statePtr->inputEncodingFlags |= TCL_ENCODING_END;
     } else if (nread < 0) {
@@ -6130,9 +6164,9 @@ GetInput(
 	    result = EAGAIN;
 	}
 	Tcl_SetErrno(result);
-	return result;
     }
-    return 0;
+    ReleaseChannelBuffer(bufPtr);
+    return result;
 }
 
 /*
diff --git a/generic/tclIO.h b/generic/tclIO.h
index ebf2ef7..0ea7d1c 100644
--- a/generic/tclIO.h
+++ b/generic/tclIO.h
@@ -36,6 +36,7 @@
  */
 
 typedef struct ChannelBuffer {
+    int refCount;		/* Current uses count */
     int nextAdded;		/* The next position into which a character
 				 * will be put in the buffer. */
     int nextRemoved;		/* Position of next byte to be removed from
diff --git a/generic/tclIOCmd.c b/generic/tclIOCmd.c
index 2958bc8..7e8e91a 100644
--- a/generic/tclIOCmd.c
+++ b/generic/tclIOCmd.c
@@ -447,6 +447,7 @@ Tcl_ReadObjCmd(
 
     resultPtr = Tcl_NewObj();
     Tcl_IncrRefCount(resultPtr);
+    Tcl_Preserve(chan);
     charactersRead = Tcl_ReadChars(chan, resultPtr, toRead, 0);
     if (charactersRead < 0) {
 	/*
@@ -462,6 +463,7 @@ Tcl_ReadObjCmd(
 		    TclGetString(chanObjPtr), "\": ",
 		    Tcl_PosixError(interp), NULL);
 	}
+	Tcl_Release(chan);
 	Tcl_DecrRefCount(resultPtr);
 	return TCL_ERROR;
     }
@@ -480,6 +482,7 @@ Tcl_ReadObjCmd(
 	}
     }
     Tcl_SetObjResult(interp, resultPtr);
+    Tcl_Release(chan);
     Tcl_DecrRefCount(resultPtr);
     return TCL_OK;
 }
diff --git a/tests/ioCmd.test b/tests/ioCmd.test
index 184f773..8bf32d2 100644
--- a/tests/ioCmd.test
+++ b/tests/ioCmd.test
@@ -785,11 +785,27 @@ test iocmd-21.21 {[close] in [read] segfaults} -setup {
     }
     set ch [chan create read foo]
 } -body {
-    read $ch 1
+    read $ch 0
 } -cleanup {
     close $ch
     rename foo {}
-} -result a
+} -result {}
+test iocmd-21.22 {[close] in [read] segfaults} -setup {
+    proc foo {method chan args} {
+	switch -- $method initialize {
+	    return {initialize finalize watch read}
+	} finalize {} watch {} read {
+	    catch {close $chan}
+	    return a
+	}
+    }
+    set ch [chan create read foo]
+} -body {
+    read $ch 1
+} -returnCodes error -cleanup {
+    catch {close $ch}
+    rename foo {}
+} -match glob -result {*invalid argument*}
 
 # --- --- --- --------- --------- ---------
 # Helper commands to record the arguments to handler methods.