From 267c1eb9f4c15531f7bf4095ffb56151ad8f9203 Mon Sep 17 00:00:00 2001 From: dgp Date: Tue, 29 Apr 2014 17:40:15 +0000 Subject: Make sure no shared ChannelBuffers get recycled. --- generic/tclIO.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/generic/tclIO.c b/generic/tclIO.c index 08d0d93..6831c47 100644 --- a/generic/tclIO.c +++ b/generic/tclIO.c @@ -164,6 +164,7 @@ typedef struct CloseCallback { static ChannelBuffer * AllocChannelBuffer(int length); static void PreserveChannelBuffer(ChannelBuffer *bufPtr); static void ReleaseChannelBuffer(ChannelBuffer *bufPtr); +static int IsShared(ChannelBuffer *bufPtr); static void ChannelTimerProc(ClientData clientData); static int CheckChannelErrors(ChannelState *statePtr, int direction); @@ -2239,6 +2240,13 @@ ReleaseChannelBuffer( } ckfree((char *) bufPtr); } + +static int +IsShared( + ChannelBuffer *bufPtr) +{ + return bufPtr->refCount > 1; +} /* *---------------------------------------------------------------------- @@ -2269,6 +2277,9 @@ RecycleBuffer( /* * Do we have to free the buffer to the OS? */ + if (IsShared(bufPtr)) { + mustDiscard = 1; + } if (mustDiscard) { ReleaseChannelBuffer(bufPtr); -- cgit v0.12 From 75c22ad9870b02a3a9b9d213a1d38a1e93a7f7e7 Mon Sep 17 00:00:00 2001 From: dgp Date: Wed, 30 Apr 2014 19:12:37 +0000 Subject: Another segfault demo test, this one with [close] during [gets]. --- tests/ioCmd.test | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/ioCmd.test b/tests/ioCmd.test index 8bf32d2..d2c0173 100644 --- a/tests/ioCmd.test +++ b/tests/ioCmd.test @@ -806,6 +806,22 @@ test iocmd-21.22 {[close] in [read] segfaults} -setup { catch {close $ch} rename foo {} } -match glob -result {*invalid argument*} +test iocmd-21.23 {[close] in [gets] segfaults} -setup { + proc foo {method chan args} { + switch -- $method initialize { + return {initialize finalize watch read} + } finalize {} watch {} read { + catch {close $chan} + return \n + } + } + set ch [chan create read foo] +} -body { + gets $ch +} -returnCodes error -cleanup { + catch {close $ch} + rename foo {} +} -match glob -result {*invalid argument*} # --- --- --- --------- --------- --------- # Helper commands to record the arguments to handler methods. -- cgit v0.12 From 1264e873bbaae9d4328826b749e459e88b32e820 Mon Sep 17 00:00:00 2001 From: dgp Date: Wed, 30 Apr 2014 19:33:21 +0000 Subject: Panic message to pinpoint the cause of iocmd-21.23 segfault. --- generic/tclIO.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/generic/tclIO.c b/generic/tclIO.c index 6831c47..0c9db67 100644 --- a/generic/tclIO.c +++ b/generic/tclIO.c @@ -4570,6 +4570,9 @@ FilterInputBytes( } bufPtr = statePtr->inQueueTail; gsPtr->bufPtr = bufPtr; + if (bufPtr == NULL) { + Tcl_Panic("GetInput nuked buffers!"); + } } /* -- cgit v0.12 From 1c909a3352991d577a7164d5bb33dbe8d295ae67 Mon Sep 17 00:00:00 2001 From: dgp Date: Wed, 30 Apr 2014 19:54:51 +0000 Subject: Stop the segfaults in [close] during [gets] tests. Not sure this is the right behavior, but it's better than crashing. --- generic/tclIO.c | 23 ++++++++++++++--------- tests/ioCmd.test | 21 +++++++++++++++++++-- 2 files changed, 33 insertions(+), 11 deletions(-) diff --git a/generic/tclIO.c b/generic/tclIO.c index 0c9db67..cd28a0e 100644 --- a/generic/tclIO.c +++ b/generic/tclIO.c @@ -4163,12 +4163,12 @@ Tcl_GetsObj( restore: bufPtr = statePtr->inQueueHead; - if (bufPtr == NULL) { - Tcl_Panic("Tcl_GetsObj: restore reached with bufPtr==NULL"); + if (bufPtr != NULL) { + bufPtr->nextRemoved = oldRemoved; + bufPtr = bufPtr->nextPtr; } - bufPtr->nextRemoved = oldRemoved; - for (bufPtr = bufPtr->nextPtr; bufPtr != NULL; bufPtr = bufPtr->nextPtr) { + for ( ; bufPtr != NULL; bufPtr = bufPtr->nextPtr) { bufPtr->nextRemoved = BUFFER_PADDING; } CommonGetsCleanup(chanPtr); @@ -4298,6 +4298,9 @@ TclGetsObjBinary( goto restore; } bufPtr = statePtr->inQueueTail; + if (bufPtr == NULL) { + goto restore; + } } dst = (unsigned char *) RemovePoint(bufPtr); @@ -4410,12 +4413,12 @@ TclGetsObjBinary( restore: bufPtr = statePtr->inQueueHead; - if (bufPtr == NULL) { - Tcl_Panic("TclGetsObjBinary: restore reached with bufPtr==NULL"); + if (bufPtr) { + bufPtr->nextRemoved = oldRemoved; + bufPtr = bufPtr->nextPtr; } - bufPtr->nextRemoved = oldRemoved; - for (bufPtr = bufPtr->nextPtr; bufPtr != NULL; bufPtr = bufPtr->nextPtr) { + for ( ; bufPtr != NULL; bufPtr = bufPtr->nextPtr) { bufPtr->nextRemoved = BUFFER_PADDING; } CommonGetsCleanup(chanPtr); @@ -4571,7 +4574,9 @@ FilterInputBytes( bufPtr = statePtr->inQueueTail; gsPtr->bufPtr = bufPtr; if (bufPtr == NULL) { - Tcl_Panic("GetInput nuked buffers!"); + gsPtr->charsWrote = 0; + gsPtr->rawRead = 0; + return -1; } } diff --git a/tests/ioCmd.test b/tests/ioCmd.test index d2c0173..bb133f9 100644 --- a/tests/ioCmd.test +++ b/tests/ioCmd.test @@ -818,10 +818,27 @@ test iocmd-21.23 {[close] in [gets] segfaults} -setup { set ch [chan create read foo] } -body { gets $ch -} -returnCodes error -cleanup { +} -cleanup { catch {close $ch} rename foo {} -} -match glob -result {*invalid argument*} +} -result {} +test iocmd-21.24 {[close] in binary [gets] segfaults} -setup { + proc foo {method chan args} { + switch -- $method initialize { + return {initialize finalize watch read} + } finalize {} watch {} read { + catch {close $chan} + return \n + } + } + set ch [chan create read foo] +} -body { + chan configure $ch -translation binary + gets $ch +} -cleanup { + catch {close $ch} + rename foo {} +} -result {} # --- --- --- --------- --------- --------- # Helper commands to record the arguments to handler methods. -- cgit v0.12 From 7df749abdc0780eb176e6fade94388d60cd8a0ef Mon Sep 17 00:00:00 2001 From: "jan.nijtmans" Date: Wed, 30 Apr 2014 21:59:00 +0000 Subject: Better (safer) fix for [0e92c404f1] --- generic/tclCmdMZ.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/generic/tclCmdMZ.c b/generic/tclCmdMZ.c index 6fd468c..d106f53 100644 --- a/generic/tclCmdMZ.c +++ b/generic/tclCmdMZ.c @@ -2086,7 +2086,7 @@ StringRangeCmd( * Unicode string rep to get the range. */ - if (objv[1]->typePtr == &tclByteArrayType) { + if (objv[1]->typePtr == &tclByteArrayType && (objv[1]->bytes==NULL)) { string = Tcl_GetByteArrayFromObj(objv[1], &length); length--; } else { -- cgit v0.12