From e3220aa63174d2d49368a02ce956eb6a0b7e8ae0 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 e024894c53ffc2dec708a84210d4000e6a9c7209 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 8e5e88f4dbbd30500ce43e26c083e19cd44c217b 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 c09e2ded082244a2dd009a6f46fdfc75158e555d 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 fd83709b456fbbfab52d30daa8dc327491ac4cca 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 322ea4d71ddf82436ab84f2b3d24c4c75444257c 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 9bed70dea46200cece274e91624f42d97feb95ba 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