From 0781259dd17444340c1a926c4cd2b5ade72bfebe Mon Sep 17 00:00:00 2001 From: dgp Date: Fri, 25 Apr 2014 17:34:11 +0000 Subject: Test iortrans-4.8.2 demos an infinite loop. Possible trouble with pushback buffers. --- generic/tclIO.c | 5 +++++ tests/ioTrans.test | 20 ++++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/generic/tclIO.c b/generic/tclIO.c index e6439ef..41ac1e1 100644 --- a/generic/tclIO.c +++ b/generic/tclIO.c @@ -5611,6 +5611,11 @@ DoReadChars( ResetFlag(statePtr, CHANNEL_BLOCKED); } result = GetInput(chanPtr); +if (chanPtr != statePtr->topChanPtr) { +Tcl_Release(chanPtr); +chanPtr = statePtr->topChanPtr; +Tcl_Preserve(chanPtr); +} if (result != 0) { if (result == EAGAIN) { break; diff --git a/tests/ioTrans.test b/tests/ioTrans.test index b21d894..3bbd170 100644 --- a/tests/ioTrans.test +++ b/tests/ioTrans.test @@ -559,6 +559,26 @@ test iortrans-4.8.1 {chan read, bug 721ec69271} -setup { rename foo {} } -result {{read rt* {test data }} file*} +test iortrans-4.8.2 {chan read, bug 721ec69271} -setup { + set res {} +} -match glob -body { + proc foo {fd args} { + handle.initialize + handle.finalize + lappend ::res $args + # Kill and recreate transform while it is operating + chan pop $fd + chan push $fd [list foo $fd] + return x + } + set c [chan push [set c [tempchan]] [list foo $c]] + chan configure $c -buffersize 1 + lappend res [read $c] +} -cleanup { + tempdone + rename foo {} +} -result {{read rt* {test data +}} file*} test iortrans-4.9 {chan read, gets, bug 2921116} -setup { set res {} } -match glob -body { -- cgit v0.12 From 4119a864755c221944bcd1967b8243a2acc3d9aa Mon Sep 17 00:00:00 2001 From: dgp Date: Fri, 25 Apr 2014 19:51:36 +0000 Subject: Disable buffer recycling, which creates mysteries. --- generic/tclIO.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/generic/tclIO.c b/generic/tclIO.c index 41ac1e1..df863cc 100644 --- a/generic/tclIO.c +++ b/generic/tclIO.c @@ -2342,7 +2342,7 @@ RecycleBuffer( * Do we have to free the buffer to the OS? */ - if (mustDiscard) { + if (1 || mustDiscard) { ReleaseChannelBuffer(bufPtr); return; } -- cgit v0.12 From b18161555e63f857014d2306adcb9fbcad3c6144 Mon Sep 17 00:00:00 2001 From: dgp Date: Thu, 1 May 2014 16:33:39 +0000 Subject: Stop the segfault in iogt-2.4. First by changing the UpdateInterest() call that triggers it. "downChanPtr" may no longer be the right argument at that point. Second, after ending the segfault, the test became an infinite loop (nested unstacking?! whoa.), so revised the test to one that terminates (and passes). Left behind a comment that the recursive unstacking case may require more examination. --- generic/tclIO.c | 9 ++++++++- tests/iogt.test | 2 +- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/generic/tclIO.c b/generic/tclIO.c index 776ff12..a83cdcd 100644 --- a/generic/tclIO.c +++ b/generic/tclIO.c @@ -1876,6 +1876,13 @@ Tcl_UnstackChannel( * into the old structure. */ + /* + * TODO: Figure out how to handle the situation where the chan + * operations called below by this unstacking operation cause + * another unstacking recursively. In that case the downChanPtr + * value we're holding on to will not be the right thing. + */ + Channel *downChanPtr = chanPtr->downChanPtr; /* @@ -1980,7 +1987,7 @@ Tcl_UnstackChannel( */ Tcl_EventuallyFree(chanPtr, TCL_DYNAMIC); - UpdateInterest(downChanPtr); + UpdateInterest(statePtr->topChanPtr); if (result != 0) { Tcl_SetErrno(result); diff --git a/tests/iogt.test b/tests/iogt.test index bd3c67b..ded8bb9 100644 --- a/tests/iogt.test +++ b/tests/iogt.test @@ -228,7 +228,7 @@ proc id_torture {chan op data} { delete/read - clear_read {;#ignore} flush/write - - flush/read - + flush/read {} write - read { testchannel unstack $chan -- cgit v0.12 From 4f1714013f16d9993d2d68175d81fdb91ffc8190 Mon Sep 17 00:00:00 2001 From: dgp Date: Fri, 2 May 2014 12:39:14 +0000 Subject: Re-enable buffer recycling. --- generic/tclIO.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/generic/tclIO.c b/generic/tclIO.c index a83cdcd..8ae2fd2 100644 --- a/generic/tclIO.c +++ b/generic/tclIO.c @@ -2360,7 +2360,7 @@ RecycleBuffer( mustDiscard = 1; } - if (1 || mustDiscard) { + if (mustDiscard) { ReleaseChannelBuffer(bufPtr); return; } -- cgit v0.12 From 3452d681c93ec5cab5edc2d45bbd0d02f9beadb1 Mon Sep 17 00:00:00 2001 From: dgp Date: Fri, 2 May 2014 13:02:48 +0000 Subject: Fully restore topChan resetting to accommodate self-restacking channels. --- generic/tclIO.c | 43 ++++++++++++++++++++++++++----------------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/generic/tclIO.c b/generic/tclIO.c index 8ae2fd2..adea32e 100644 --- a/generic/tclIO.c +++ b/generic/tclIO.c @@ -4554,9 +4554,12 @@ Tcl_GetsObj( * Regenerate the top channel, in case it was changed due to * self-modifying reflected transforms. */ - /* - chanPtr = statePtr->topChanPtr; - */ + + if (chanPtr != statePtr->topChanPtr) { + Tcl_Release(chanPtr); + chanPtr = statePtr->topChanPtr; + Tcl_Preserve(chanPtr); + } bufPtr = gs.bufPtr; if (bufPtr == NULL) { @@ -4590,9 +4593,11 @@ Tcl_GetsObj( * Regenerate the top channel, in case it was changed due to * self-modifying reflected transforms. */ - /* - chanPtr = statePtr->topChanPtr; - */ + if (chanPtr != statePtr->topChanPtr) { + Tcl_Release(chanPtr); + chanPtr = statePtr->topChanPtr; + Tcl_Preserve(chanPtr); + } bufPtr = statePtr->inQueueHead; if (bufPtr != NULL) { bufPtr->nextRemoved = oldRemoved; @@ -4632,9 +4637,11 @@ Tcl_GetsObj( * Regenerate the top channel, in case it was changed due to * self-modifying reflected transforms. */ - /* - chanPtr = statePtr->topChanPtr; - */ + if (chanPtr != statePtr->topChanPtr) { + Tcl_Release(chanPtr); + chanPtr = statePtr->topChanPtr; + Tcl_Preserve(chanPtr); + } UpdateInterest(chanPtr); Tcl_Release(chanPtr); return copiedTotal; @@ -5638,11 +5645,11 @@ DoReadChars( ResetFlag(statePtr, CHANNEL_BLOCKED); } result = GetInput(chanPtr); -if (chanPtr != statePtr->topChanPtr) { -Tcl_Release(chanPtr); -chanPtr = statePtr->topChanPtr; -Tcl_Preserve(chanPtr); -} + if (chanPtr != statePtr->topChanPtr) { + Tcl_Release(chanPtr); + chanPtr = statePtr->topChanPtr; + Tcl_Preserve(chanPtr); + } if (result != 0) { if (result == EAGAIN) { break; @@ -5673,9 +5680,11 @@ Tcl_Preserve(chanPtr); * Regenerate the top channel, in case it was changed due to * self-modifying reflected transforms. */ - /* - chanPtr = statePtr->topChanPtr; - */ + if (chanPtr != statePtr->topChanPtr) { + Tcl_Release(chanPtr); + chanPtr = statePtr->topChanPtr; + Tcl_Preserve(chanPtr); + } UpdateInterest(chanPtr); Tcl_Release(chanPtr); return copied; -- cgit v0.12 From ab2c4a52f1dbcc67939ad86233d21cf7fc38a5cd Mon Sep 17 00:00:00 2001 From: dgp Date: Fri, 2 May 2014 14:45:03 +0000 Subject: Add some comments about possible other self-restacking troubles. --- generic/tclIO.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/generic/tclIO.c b/generic/tclIO.c index adea32e..58c7b3c 100644 --- a/generic/tclIO.c +++ b/generic/tclIO.c @@ -1747,6 +1747,10 @@ Tcl_StackChannel( statePtr->csPtrR = NULL; statePtr->csPtrW = NULL; + /* + * TODO: Examine what can go wrong if Tcl_Flush() call disturbs + * the stacking state of this channel during its operations. + */ if (Tcl_Flush((Tcl_Channel) prevChanPtr) != TCL_OK) { statePtr->csPtrR = csPtrR; statePtr->csPtrW = csPtrW; @@ -9786,12 +9790,15 @@ StackSetBlockMode( { int result = 0; Tcl_DriverBlockModeProc *blockModeProc; + ChannelState *statePtr = chanPtr->state; /* * Start at the top of the channel stack + * TODO: Examine what can go wrong when blockModeProc calls + * disturb the stacking state of the channel. */ - chanPtr = chanPtr->state->topChanPtr; + chanPtr = statePtr->topChanPtr; while (chanPtr != NULL) { blockModeProc = Tcl_ChannelBlockModeProc(chanPtr->typePtr); if (blockModeProc != NULL) { -- cgit v0.12 From 60ed1a9051fbb233b229facc524d7a2aed49a18b Mon Sep 17 00:00:00 2001 From: dgp Date: Fri, 2 May 2014 15:19:57 +0000 Subject: Fixup restacking tests to expect the right results. --- tests/ioTrans.test | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/ioTrans.test b/tests/ioTrans.test index 3bbd170..7f4f7f0 100644 --- a/tests/ioTrans.test +++ b/tests/ioTrans.test @@ -539,7 +539,7 @@ test iortrans-4.8 {chan read, read, bug 2921116} -setup { tempdone rename foo {} } -result {{read rt* {test data -}} file*} +}} {}} test iortrans-4.8.1 {chan read, bug 721ec69271} -setup { set res {} } -match glob -body { @@ -557,8 +557,8 @@ test iortrans-4.8.1 {chan read, bug 721ec69271} -setup { } -cleanup { tempdone rename foo {} -} -result {{read rt* {test data -}} file*} +} -result {{read rt* te} {read rt* st} {read rt* { d}} {read rt* at} {read rt* {a +}} {}} test iortrans-4.8.2 {chan read, bug 721ec69271} -setup { set res {} } -match glob -body { @@ -577,8 +577,8 @@ test iortrans-4.8.2 {chan read, bug 721ec69271} -setup { } -cleanup { tempdone rename foo {} -} -result {{read rt* {test data -}} file*} +} -result {{read rt* t} {read rt* e} {read rt* s} {read rt* t} {read rt* { }} {read rt* d} {read rt* a} {read rt* t} {read rt* a} {read rt* { +}} {}} test iortrans-4.9 {chan read, gets, bug 2921116} -setup { set res {} } -match glob -body { @@ -596,7 +596,7 @@ test iortrans-4.9 {chan read, gets, bug 2921116} -setup { tempdone rename foo {} } -result {{read rt* {test data -}} file*} +}} {}} # --- === *** ########################### # method write (via puts) -- cgit v0.12