From 4e12c83774ba3d73e4970089e9c04a886bdbae94 Mon Sep 17 00:00:00 2001 From: dgp Date: Mon, 10 Aug 2015 19:44:56 +0000 Subject: Add the critical missing UpdateInterest() call at the exit of DoRead(). (Compare with same approach in DoReadChars()). This involves removing some other calls that are now replaced by the new one. Also marked several UpdateInterest() calls throughout tclIO.c with comments raising the suspicion that they serve no function. --- generic/tclIO.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/generic/tclIO.c b/generic/tclIO.c index 564df27..9a4735f 100644 --- a/generic/tclIO.c +++ b/generic/tclIO.c @@ -5317,6 +5317,7 @@ DoReadChars( assert( statePtr->inputEncodingFlags & TCL_ENCODING_END ); assert( !GotFlag(statePtr, CHANNEL_BLOCKED|INPUT_SAW_CR) ); + /* TODO: We don't need this call? */ UpdateInterest(chanPtr); return 0; } @@ -5328,6 +5329,7 @@ DoReadChars( } ResetFlag(statePtr, CHANNEL_BLOCKED|CHANNEL_EOF); statePtr->inputEncodingFlags &= ~TCL_ENCODING_END; + /* TODO: We don't need this call? */ UpdateInterest(chanPtr); return 0; } @@ -7910,6 +7912,11 @@ Tcl_NotifyChannel( */ if (chanPtr->typePtr != NULL) { + /* + * TODO: This call may not be needed. If a handler induced a + * change in interest, that handler should have made its own + * UpdateInterest() call, one would think. + */ UpdateInterest(chanPtr); } @@ -9032,6 +9039,7 @@ DoRead( assert( statePtr->inputEncodingFlags & TCL_ENCODING_END ); assert( !GotFlag(statePtr, CHANNEL_BLOCKED|INPUT_SAW_CR) ); + /* TODO: Don't need this call */ UpdateInterest(chanPtr); return 0; } @@ -9043,6 +9051,7 @@ DoRead( } ResetFlag(statePtr, CHANNEL_BLOCKED|CHANNEL_EOF); statePtr->inputEncodingFlags &= ~TCL_ENCODING_END; + /* TODO: Don't need this call */ UpdateInterest(chanPtr); return 0; } @@ -9109,7 +9118,6 @@ DoRead( */ if (bytesToRead == 0) { - UpdateInterest(chanPtr); break; } @@ -9118,7 +9126,6 @@ DoRead( */ if (GotFlag(statePtr, CHANNEL_STICKY_EOF)) { - UpdateInterest(chanPtr); break; } @@ -9143,7 +9150,6 @@ DoRead( } else if (GotFlag(statePtr, CHANNEL_BLOCKED)) { /* ...and we cannot get more now. */ SetFlag(statePtr, CHANNEL_NEED_MORE_DATA); - UpdateInterest(chanPtr); break; } else { /* ... so we need to get some. */ @@ -9195,6 +9201,7 @@ DoRead( || Tcl_InputBuffered((Tcl_Channel)chanPtr) == 0); assert( !(GotFlag(statePtr, CHANNEL_EOF|CHANNEL_BLOCKED) == (CHANNEL_EOF|CHANNEL_BLOCKED)) ); + UpdateInterest(chanPtr); TclChannelRelease((Tcl_Channel)chanPtr); return (int)(p - dst); } -- cgit v0.12 From 2c66fc643c1e662526ad4cd59b0b1836b171a35c Mon Sep 17 00:00:00 2001 From: dgp Date: Wed, 12 Aug 2015 16:37:28 +0000 Subject: New test io-53.18 adapted from demo script in [32ae34e63a]. This test segfaults without changes to source code. This checkin also contains a revised implementationf of [chan postevent] that stops calling Tcl_NotifyChannel() directly, and queues an event to do it instead. This stops the segfault, but causes tests iocmd-31.[67] to fail. Need advice on whether that matters. --- generic/tclIORChan.c | 26 +++++++++++++++++++++- tests/io.test | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 1 deletion(-) diff --git a/generic/tclIORChan.c b/generic/tclIORChan.c index bbb5b88..6592f9e 100644 --- a/generic/tclIORChan.c +++ b/generic/tclIORChan.c @@ -741,6 +741,24 @@ TclChanCreateObjCmd( *---------------------------------------------------------------------- */ +typedef struct PostEvent { + Tcl_Event event; /* Basic event data, has to be first item */ + Tcl_Channel chan; + int events; +} PostEvent; + +static int +CallNotify( + Tcl_Event *evPtr, + int flags) +{ + PostEvent *pevPtr = (PostEvent *)evPtr; + + Tcl_NotifyChannel(pevPtr->chan, pevPtr->events); + TclChannelRelease(pevPtr->chan); + return 1; +} + int TclChanPostEventObjCmd( ClientData clientData, @@ -769,6 +787,7 @@ TclChanPostEventObjCmd( int events; /* Mask of events to post */ ReflectedChannelMap* rcmPtr; /* Map of reflected channels with handlers in this interp */ Tcl_HashEntry* hPtr; /* Entry in the above map */ + PostEvent *pevPtr; /* * Number of arguments... @@ -857,7 +876,12 @@ TclChanPostEventObjCmd( * We have the channel and the events to post. */ - Tcl_NotifyChannel(chan, events); + pevPtr = (PostEvent *)ckalloc(sizeof(PostEvent)); + pevPtr->event.proc = CallNotify; + pevPtr->chan = chan; + pevPtr->events = events; + TclChannelPreserve(chan); + Tcl_QueueEvent((Tcl_Event *)pevPtr, TCL_QUEUE_HEAD); /* * Squash interp results left by the event script. diff --git a/tests/io.test b/tests/io.test index 50c5808..46e3f05 100644 --- a/tests/io.test +++ b/tests/io.test @@ -7886,6 +7886,68 @@ test io-53.15 {[ed29c4da21] DoRead: fblocked seen as error} -setup { removeFile out } -result 100 +test io-53.18 {[32ae34e63a] recursize CopyData} -setup { + proc driver {cmd args} { + variable buffer + variable index + set chan [lindex $args 0] + switch -- $cmd { + initialize { + set index($chan) 0 + set buffer($chan) [encoding convertto utf-8 \ + [string repeat a 100]] + return {initialize finalize watch read} + } + finalize { + unset index($chan) buffer($chan) + return + } + watch { + if {"read" in [lindex $args 1]} { + chan postevent $chan read + } + return + } + read { + set n [lindex $args 1] + set new [expr {$index($chan) + $n}] + set result [string range $buffer($chan) $index($chan) $new-1] + set index($chan) $new + return $result + } + } + } + proc more {c outChan bytes args} { + if {[eof $c]} { + set ::done eof + catch {close $c} + return + } + if {[llength $args]} { + set ::done error + } else { + chan copy $c $outChan -command [list [namespace which more] $c $outChan] + } + } + set c [chan create read [namespace which driver]] + chan configure $c -encoding utf-8 + set out [makeFile {} out] + set outChan [open $out w] + # Different encoding to force use of DoReadChars() + chan configure $outChan -encoding iso8859-1 +} -body { + after 2000 {set ::done timeout} + chan copy $c $outChan -size 99 -command [list [namespace which more] $c $outChan] + vwait ::done + set ::done +} -cleanup { + close $outChan + removeFile out + rename driver {} + rename more {} + unset ::done +} -result eof + test io-54.1 {Recursive channel events} {socket fileevent} { # This test checks to see if file events are delivered during recursive # event loops when there is buffered data on the channel. -- cgit v0.12 From 1db16b915a2fc6e315586502121d1ab7c9b39a59 Mon Sep 17 00:00:00 2001 From: dgp Date: Wed, 12 Aug 2015 17:49:42 +0000 Subject: Adjustments to failing tests to account for changed [chan postevent]. --- tests/ioCmd.test | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/ioCmd.test b/tests/ioCmd.test index 5a76d48..c706e50 100644 --- a/tests/ioCmd.test +++ b/tests/ioCmd.test @@ -1949,26 +1949,26 @@ test iocmd-31.6 {chan postevent, posted events do happen} -match glob -body { set c [chan create {r w} foo] note [fileevent $c readable {note TOCK}] set stop [after 10000 {note TIMEOUT}] - after 1000 {note [chan postevent $c r]} + after 1000 {chan postevent $c r} vwait ::res catch {after cancel $stop} close $c rename foo {} set res -} -result {{watch rc* read} {} TOCK {} {watch rc* {}}} +} -result {{watch rc* read} {} TOCK {watch rc* {}}} test iocmd-31.7 {chan postevent, posted events do happen} -match glob -body { set res {} proc foo {args} {oninit; onfinal; track; return} set c [chan create {r w} foo] note [fileevent $c writable {note TOCK}] set stop [after 10000 {note TIMEOUT}] - after 1000 {note [chan postevent $c w]} + after 1000 {chan postevent $c w} vwait ::res catch {after cancel $stop} close $c rename foo {} set res -} -result {{watch rc* write} {} TOCK {} {watch rc* {}}} +} -result {{watch rc* write} {} TOCK {watch rc* {}}} test iocmd-31.8 {chan postevent after close throws error} -match glob -setup { proc foo {args} {oninit; onfinal; track; return} proc dummy args { return } -- cgit v0.12 From 13850d4952b34d30e3e2ae68c3cb2c10c0019737 Mon Sep 17 00:00:00 2001 From: dgp Date: Thu, 13 Aug 2015 16:45:03 +0000 Subject: A bit of code safety, and then a test demonstrating next issue. --- generic/tclIORChan.c | 5 +++- tests/io.test | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 67 insertions(+), 2 deletions(-) diff --git a/generic/tclIORChan.c b/generic/tclIORChan.c index 6592f9e..85a35b8 100644 --- a/generic/tclIORChan.c +++ b/generic/tclIORChan.c @@ -753,8 +753,11 @@ CallNotify( int flags) { PostEvent *pevPtr = (PostEvent *)evPtr; + Channel *chanPtr = (Channel *)pevPtr->chan; - Tcl_NotifyChannel(pevPtr->chan, pevPtr->events); + if (chanPtr->typePtr != NULL) { + Tcl_NotifyChannel(pevPtr->chan, pevPtr->events); + } TclChannelRelease(pevPtr->chan); return 1; } diff --git a/tests/io.test b/tests/io.test index 46e3f05..840274d 100644 --- a/tests/io.test +++ b/tests/io.test @@ -7936,7 +7936,7 @@ test io-53.18 {[32ae34e63a] recursize CopyData} -setup { # Different encoding to force use of DoReadChars() chan configure $outChan -encoding iso8859-1 } -body { - after 2000 {set ::done timeout} + after 100 {set ::done timeout} chan copy $c $outChan -size 99 -command [list [namespace which more] $c $outChan] vwait ::done set ::done @@ -7948,6 +7948,68 @@ test io-53.18 {[32ae34e63a] recursize CopyData} -setup { unset ::done } -result eof +test io-53.19 {[e0a7b3e5f8] DoRead calls to UpdateInterest} -setup { + proc driver {cmd args} { + variable buffer + variable index + set chan [lindex $args 0] + switch -- $cmd { + initialize { + set index($chan) 0 + set buffer($chan) [encoding convertto utf-8 \ + [string repeat a 100]] + return {initialize finalize watch read} + } + finalize { + unset index($chan) buffer($chan) + return + } + watch { + if {"read" in [lindex $args 1]} { + chan postevent $chan read + } + return + } + read { + set n [lindex $args 1] + set new [expr {$index($chan) + $n}] + set result [string range $buffer($chan) $index($chan) $new-1] + set index($chan) $new + return $result + } + } + } + proc more {c outChan bytes args} { + if {[eof $c]} { + set ::done eof + catch {close $c} + return + } + if {[llength $args]} { + set ::done error + } else { + chan copy $c $outChan -size 30 -command [list [namespace which more] $c $outChan] + } + } + set c [chan create read [namespace which driver]] + chan configure $c -encoding utf-8 -buffersize 20 + set out [makeFile {} out] + set outChan [open $out w] + # Different encoding to force use of DoReadChar() + chan configure $outChan -encoding iso8859-1 +} -body { + after 100 {set ::done timeout} + chan copy $c $outChan -size 30 -command [list [namespace which more] $c $outChan] + vwait ::done + set ::done +} -cleanup { + catch {close $outChan} + removeFile out + rename driver {} + rename more {} + unset ::done +} -result eof + test io-54.1 {Recursive channel events} {socket fileevent} { # This test checks to see if file events are delivered during recursive # event loops when there is buffered data on the channel. -- cgit v0.12 From bf4a5c995fc82d4d7841bdf0970e027623904529 Mon Sep 17 00:00:00 2001 From: dgp Date: Thu, 13 Aug 2015 17:02:04 +0000 Subject: Disable the filtering of ReflectWatch so that every UpdateInterest() call flows through to become a [driver watch] evaluation. This makes new test io-53.19 pass. It also makes a collection of 10 test in ioCmd.test start failing, all of which are recording detailed reflected channel driver command evaluation. The now unfiltered [driver watch] change this record without (at least apparently) changing any behavior. Need review. --- generic/tclIORChan.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/generic/tclIORChan.c b/generic/tclIORChan.c index 85a35b8..b059c79 100644 --- a/generic/tclIORChan.c +++ b/generic/tclIORChan.c @@ -1535,7 +1535,7 @@ ReflectWatch( mask &= rcPtr->mode; - if (mask == rcPtr->interest) { + if (0 && mask == rcPtr->interest) { /* * Same old, same old, why should we do something? */ -- cgit v0.12 From 65644eeb87cb55253126bba237afb569d3fe6f4a Mon Sep 17 00:00:00 2001 From: dgp Date: Thu, 13 Aug 2015 19:28:26 +0000 Subject: New test attempting to demo Bug [e0a7b3e5f8]. Doesn't work yet. --- tests/io.test | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 65 insertions(+), 3 deletions(-) diff --git a/tests/io.test b/tests/io.test index 840274d..914cbca 100644 --- a/tests/io.test +++ b/tests/io.test @@ -7886,7 +7886,7 @@ test io-53.15 {[ed29c4da21] DoRead: fblocked seen as error} -setup { removeFile out } -result 100 -test io-53.18 {[32ae34e63a] recursize CopyData} -setup { +test io-53.18 {[32ae34e63a] recursive CopyData} -setup { proc driver {cmd args} { variable buffer variable index @@ -7948,7 +7948,7 @@ test io-53.18 {[32ae34e63a] recursize CopyData} -setup { unset ::done } -result eof -test io-53.19 {[e0a7b3e5f8] DoRead calls to UpdateInterest} -setup { +test io-53.19 {[32ae34e63a] stop ReflectWatch filtering} -setup { proc driver {cmd args} { variable buffer variable index @@ -7995,7 +7995,7 @@ test io-53.19 {[e0a7b3e5f8] DoRead calls to UpdateInterest} -setup { chan configure $c -encoding utf-8 -buffersize 20 set out [makeFile {} out] set outChan [open $out w] - # Different encoding to force use of DoReadChar() + # Different encoding to force use of DoReadChars() chan configure $outChan -encoding iso8859-1 } -body { after 100 {set ::done timeout} @@ -8010,6 +8010,68 @@ test io-53.19 {[e0a7b3e5f8] DoRead calls to UpdateInterest} -setup { unset ::done } -result eof +test io-53.20 {[e0a7b3e5f8] DoRead calls to UpdateInterest} -setup { + proc driver {cmd args} { + variable buffer + variable index + set chan [lindex $args 0] + switch -- $cmd { + initialize { + set index($chan) 0 + set buffer($chan) [encoding convertto utf-8 \ + [string repeat a 100]] + return {initialize finalize watch read} + } + finalize { + unset index($chan) buffer($chan) + return + } + watch { + if {"read" in [lindex $args 1]} { + chan postevent $chan read + } + return + } + read { + set n [lindex $args 1] + set new [expr {$index($chan) + $n}] + set result [string range $buffer($chan) $index($chan) $new-1] + set index($chan) $new + return $result + } + } + } + proc more {c outChan bytes args} { + if {[eof $c]} { + set ::done eof + catch {close $c} + return + } + if {[llength $args]} { + set ::done error + } else { + chan copy $c $outChan -size 10 -command [list [namespace which more] $c $outChan] + } + } + set c [chan create read [namespace which driver]] + chan configure $c -encoding utf-8 -buffersize 20 + set out [makeFile {} out] + set outChan [open $out w] + # Same encoding to force use of DoRead() + chan configure $outChan -encoding utf-8 +} -body { + after 100 {set ::done timeout} + chan copy $c $outChan -size 10 -command [list [namespace which more] $c $outChan] + vwait ::done + set ::done +} -cleanup { + catch {close $outChan} + removeFile out + rename driver {} + rename more {} + unset ::done +} -result eof + test io-54.1 {Recursive channel events} {socket fileevent} { # This test checks to see if file events are delivered during recursive # event loops when there is buffered data on the channel. -- cgit v0.12 From 58cebf8eebe511c29bb83d3d64ee229d2e72b594 Mon Sep 17 00:00:00 2001 From: dgp Date: Wed, 23 Sep 2015 09:40:46 +0000 Subject: WIP --- generic/tclIO.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 47 insertions(+), 3 deletions(-) diff --git a/generic/tclIO.c b/generic/tclIO.c index 564df27..d4db7ec 100644 --- a/generic/tclIO.c +++ b/generic/tclIO.c @@ -113,11 +113,32 @@ typedef struct CopyState { Tcl_WideInt total; /* Total bytes transferred (written). */ Tcl_Interp *interp; /* Interp that started the copy. */ Tcl_Obj *cmdPtr; /* Command to be invoked at completion. */ + int refCount; /* Claim count on the struct */ int bufSize; /* Size of appended buffer. */ char buffer[1]; /* Copy buffer, this must be the last * field. */ } CopyState; +static void +PreserveCopyState( + CopyState *csPtr) +{ + csPtr->refCount++; +fprintf(stdout, "PRESERVE -> %d %p\n", csPtr->refCount, csPtr); fflush(stdout); +} + +static void +ReleaseCopyState( + CopyState *csPtr) +{ +fprintf(stdout, "%d -> RELEASE %p\n", csPtr->refCount, csPtr); fflush(stdout); + if (--csPtr->refCount) { + return; + } +fprintf(stdout, "FREE %p\n", csPtr); fflush(stdout); + ckfree((char *) csPtr); +} + /* * All static variables used in this file are collected into a single instance * of the following structure. For multi-threaded implementations, there is @@ -8642,9 +8663,12 @@ TclCopyChannel( Tcl_IncrRefCount(cmdPtr); } csPtr->cmdPtr = cmdPtr; + csPtr->refCount = 1; inStatePtr->csPtrR = csPtr; + PreserveCopyState(csPtr); outStatePtr->csPtrW = csPtr; + PreserveCopyState(csPtr); /* * Special handling of -size 0 async transfers, so that the -command is @@ -8696,6 +8720,9 @@ CopyData( /* Encoding control */ int underflow; /* Input underflow */ +fprintf(stdout, "CD: %p\n", csPtr); fflush(stdout); + PreserveCopyState(csPtr); + inChan = (Tcl_Channel) csPtr->readPtr; outChan = (Tcl_Channel) csPtr->writePtr; inStatePtr = csPtr->readPtr->state; @@ -8807,6 +8834,8 @@ CopyData( TclDecrRefCount(bufObj); bufObj = NULL; } +fprintf(stdout, "ECD 1: %p\n", csPtr); fflush(stdout); + ReleaseCopyState(csPtr); return TCL_OK; } } @@ -8898,6 +8927,8 @@ CopyData( TclDecrRefCount(bufObj); bufObj = NULL; } +fprintf(stdout, "ECD 2: %p\n", csPtr); fflush(stdout); + ReleaseCopyState(csPtr); return TCL_OK; } @@ -8920,6 +8951,8 @@ CopyData( TclDecrRefCount(bufObj); bufObj = NULL; } +fprintf(stdout, "ECD 3: %p\n", csPtr); fflush(stdout); + ReleaseCopyState(csPtr); return TCL_OK; } } /* while */ @@ -8971,6 +9004,8 @@ CopyData( } } } +fprintf(stdout, "ECD 4: %p\n", csPtr); fflush(stdout); + ReleaseCopyState(csPtr); return result; } @@ -9253,6 +9288,7 @@ StopCopy( return; } +fprintf(stdout, "SC: %p\n", csPtr); fflush(stdout); inStatePtr = csPtr->readPtr->state; outStatePtr = csPtr->writePtr->state; @@ -9285,9 +9321,17 @@ StopCopy( } TclDecrRefCount(csPtr->cmdPtr); } - inStatePtr->csPtrR = NULL; - outStatePtr->csPtrW = NULL; - ckfree((char *) csPtr); + if (inStatePtr->csPtrR) { + ReleaseCopyState(inStatePtr->csPtrR); + inStatePtr->csPtrR = NULL; + } + if (outStatePtr->csPtrW) { + ReleaseCopyState(outStatePtr->csPtrW); + outStatePtr->csPtrW = NULL; + } + ReleaseCopyState(csPtr); +// ckfree((char *) csPtr); +fprintf(stdout, "ESC: %p\n", csPtr); fflush(stdout); } /* -- cgit v0.12 From 55cb367ecb24a49e0fe76b104f6fe709672b066b Mon Sep 17 00:00:00 2001 From: dgp Date: Wed, 23 Sep 2015 13:18:32 +0000 Subject: CopyData() now tolerates recursion enough so that io-53.18 does not segfault. --- generic/tclIO.c | 30 +++++++++--------------------- 1 file changed, 9 insertions(+), 21 deletions(-) diff --git a/generic/tclIO.c b/generic/tclIO.c index d4db7ec..27eee5a 100644 --- a/generic/tclIO.c +++ b/generic/tclIO.c @@ -124,18 +124,15 @@ PreserveCopyState( CopyState *csPtr) { csPtr->refCount++; -fprintf(stdout, "PRESERVE -> %d %p\n", csPtr->refCount, csPtr); fflush(stdout); } static void ReleaseCopyState( CopyState *csPtr) { -fprintf(stdout, "%d -> RELEASE %p\n", csPtr->refCount, csPtr); fflush(stdout); if (--csPtr->refCount) { return; } -fprintf(stdout, "FREE %p\n", csPtr); fflush(stdout); ckfree((char *) csPtr); } @@ -8720,7 +8717,6 @@ CopyData( /* Encoding control */ int underflow; /* Input underflow */ -fprintf(stdout, "CD: %p\n", csPtr); fflush(stdout); PreserveCopyState(csPtr); inChan = (Tcl_Channel) csPtr->readPtr; @@ -8834,7 +8830,6 @@ fprintf(stdout, "CD: %p\n", csPtr); fflush(stdout); TclDecrRefCount(bufObj); bufObj = NULL; } -fprintf(stdout, "ECD 1: %p\n", csPtr); fflush(stdout); ReleaseCopyState(csPtr); return TCL_OK; } @@ -8927,7 +8922,6 @@ fprintf(stdout, "ECD 1: %p\n", csPtr); fflush(stdout); TclDecrRefCount(bufObj); bufObj = NULL; } -fprintf(stdout, "ECD 2: %p\n", csPtr); fflush(stdout); ReleaseCopyState(csPtr); return TCL_OK; } @@ -8951,7 +8945,6 @@ fprintf(stdout, "ECD 2: %p\n", csPtr); fflush(stdout); TclDecrRefCount(bufObj); bufObj = NULL; } -fprintf(stdout, "ECD 3: %p\n", csPtr); fflush(stdout); ReleaseCopyState(csPtr); return TCL_OK; } @@ -8968,15 +8961,14 @@ fprintf(stdout, "ECD 3: %p\n", csPtr); fflush(stdout); */ total = csPtr->total; - if (cmdPtr && interp) { + if (cmdPtr && interp && csPtr->cmdPtr) { int code; /* * Get a private copy of the command so we can mutate it by adding * arguments. Note that StopCopy frees our saved reference to the * original command obj. */ - - cmdPtr = Tcl_DuplicateObj(cmdPtr); + cmdPtr = Tcl_DuplicateObj(csPtr->cmdPtr); Tcl_IncrRefCount(cmdPtr); StopCopy(csPtr); Tcl_Preserve(interp); @@ -9004,7 +8996,6 @@ fprintf(stdout, "ECD 3: %p\n", csPtr); fflush(stdout); } } } -fprintf(stdout, "ECD 4: %p\n", csPtr); fflush(stdout); ReleaseCopyState(csPtr); return result; } @@ -9288,7 +9279,6 @@ StopCopy( return; } -fprintf(stdout, "SC: %p\n", csPtr); fflush(stdout); inStatePtr = csPtr->readPtr->state; outStatePtr = csPtr->writePtr->state; @@ -9320,18 +9310,16 @@ fprintf(stdout, "SC: %p\n", csPtr); fflush(stdout); CopyEventProc, csPtr); } TclDecrRefCount(csPtr->cmdPtr); + csPtr->cmdPtr = NULL; } - if (inStatePtr->csPtrR) { - ReleaseCopyState(inStatePtr->csPtrR); - inStatePtr->csPtrR = NULL; - } - if (outStatePtr->csPtrW) { - ReleaseCopyState(outStatePtr->csPtrW); - outStatePtr->csPtrW = NULL; + if (inStatePtr->csPtrR == NULL) { + return; } + ReleaseCopyState(inStatePtr->csPtrR); + inStatePtr->csPtrR = NULL; + ReleaseCopyState(outStatePtr->csPtrW); + outStatePtr->csPtrW = NULL; ReleaseCopyState(csPtr); -// ckfree((char *) csPtr); -fprintf(stdout, "ESC: %p\n", csPtr); fflush(stdout); } /* -- cgit v0.12 From 4fb1c76abea23369b4d961e2b20f900dd0294e82 Mon Sep 17 00:00:00 2001 From: dgp Date: Wed, 23 Sep 2015 16:00:57 +0000 Subject: Protect CopyState buffer from conflicting uses when CopyData() is called recursively. Also, have ReflectWatch() always give driver a chance to act. --- generic/tclIO.c | 7 +++++++ generic/tclIORChan.c | 8 -------- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/generic/tclIO.c b/generic/tclIO.c index 27eee5a..09b2537 100644 --- a/generic/tclIO.c +++ b/generic/tclIO.c @@ -114,6 +114,7 @@ typedef struct CopyState { Tcl_Interp *interp; /* Interp that started the copy. */ Tcl_Obj *cmdPtr; /* Command to be invoked at completion. */ int refCount; /* Claim count on the struct */ + int bufInUse; /* Flag to govern access to buffer */ int bufSize; /* Size of appended buffer. */ char buffer[1]; /* Copy buffer, this must be the last * field. */ @@ -8661,6 +8662,7 @@ TclCopyChannel( } csPtr->cmdPtr = cmdPtr; csPtr->refCount = 1; + csPtr->bufInUse = 0; inStatePtr->csPtrR = csPtr; PreserveCopyState(csPtr); @@ -8717,6 +8719,9 @@ CopyData( /* Encoding control */ int underflow; /* Input underflow */ + if (csPtr->bufInUse) { + return TCL_OK; + } PreserveCopyState(csPtr); inChan = (Tcl_Channel) csPtr->readPtr; @@ -8780,6 +8785,7 @@ CopyData( sizeb = csPtr->toRead; } + csPtr->bufInUse = 1; if (inBinary || sameEncoding) { size = DoRead(inStatePtr->topChanPtr, csPtr->buffer, sizeb); } else { @@ -8851,6 +8857,7 @@ CopyData( } else { sizeb = WriteChars(outStatePtr->topChanPtr, buffer, sizeb); } + csPtr->bufInUse = 0; /* * [Bug 2895565]. At this point 'size' still contains the number of diff --git a/generic/tclIORChan.c b/generic/tclIORChan.c index bbb5b88..2e5fa45 100644 --- a/generic/tclIORChan.c +++ b/generic/tclIORChan.c @@ -1508,14 +1508,6 @@ ReflectWatch( mask &= rcPtr->mode; - if (mask == rcPtr->interest) { - /* - * Same old, same old, why should we do something? - */ - - return; - } - rcPtr->interest = mask; /* -- cgit v0.12 From 261ff342b1e18433e104577ad6f8dc7c50f9292a Mon Sep 17 00:00:00 2001 From: dgp Date: Wed, 23 Sep 2015 16:08:30 +0000 Subject: Update tests to account for changed ReflectWatch behavior. --- tests/ioCmd.test | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/tests/ioCmd.test b/tests/ioCmd.test index 5a76d48..e2a6d84 100644 --- a/tests/ioCmd.test +++ b/tests/ioCmd.test @@ -981,7 +981,7 @@ test iocmd-23.1 {chan read, regular data return} -match glob -body { close $c rename foo {} set res -} -result {{read rc* 4096} {read rc* 4096} snarfsnarf} +} -result {{read rc* 4096} {read rc* 4096} {watch rc* {}} snarfsnarf} test iocmd-23.2 {chan read, bad data return, to much} -match glob -body { set res {} proc foo {args} { @@ -993,7 +993,7 @@ test iocmd-23.2 {chan read, bad data return, to much} -match glob -body { close $c rename foo {} set res -} -result {{read rc* 4096} 1 {read delivered more than requested}} +} -result {{read rc* 4096} {watch rc* {}} 1 {read delivered more than requested}} test iocmd-23.3 {chan read, for non-readable channel} -match glob -body { set res {} proc foo {args} { @@ -1016,7 +1016,7 @@ test iocmd-23.4 {chan read, error return} -match glob -body { close $c rename foo {} set res -} -result {{read rc* 4096} 1 BOOM!} +} -result {{read rc* 4096} {watch rc* {}} 1 BOOM!} test iocmd-23.5 {chan read, break return is error} -match glob -body { set res {} proc foo {args} { @@ -1028,7 +1028,7 @@ test iocmd-23.5 {chan read, break return is error} -match glob -body { close $c rename foo {} set res -} -result {{read rc* 4096} 1 *bad code*} +} -result {{read rc* 4096} {watch rc* {}} 1 *bad code*} test iocmd-23.6 {chan read, continue return is error} -match glob -body { set res {} proc foo {args} { @@ -1040,7 +1040,7 @@ test iocmd-23.6 {chan read, continue return is error} -match glob -body { close $c rename foo {} set res -} -result {{read rc* 4096} 1 *bad code*} +} -result {{read rc* 4096} {watch rc* {}} 1 *bad code*} test iocmd-23.7 {chan read, custom return is error} -match glob -body { set res {} proc foo {args} { @@ -1052,7 +1052,7 @@ test iocmd-23.7 {chan read, custom return is error} -match glob -body { close $c rename foo {} set res -} -result {{read rc* 4096} 1 *bad code*} +} -result {{read rc* 4096} {watch rc* {}} 1 *bad code*} test iocmd-23.8 {chan read, level is squashed} -match glob -body { set res {} proc foo {args} { @@ -1064,7 +1064,7 @@ test iocmd-23.8 {chan read, level is squashed} -match glob -body { close $c rename foo {} set res -} -result {{read rc* 4096} 1 *bad code* {-code 1 -level 0 -errorcode NONE -errorline 1 -errorinfo *bad code*subcommand "read"*}} +} -result {{read rc* 4096} {watch rc* {}} 1 *bad code* {-code 1 -level 0 -errorcode NONE -errorline 1 -errorinfo *bad code*subcommand "read"*}} test iocmd-23.9 {chan read, no data means eof} -match glob -setup { set res {} proc foo {args} { @@ -1080,7 +1080,7 @@ test iocmd-23.9 {chan read, no data means eof} -match glob -setup { close $c rename foo {} unset res -} -result {{read rc* 4096} {} 1} +} -result {{read rc* 4096} {watch rc* {}} {} 1} test iocmd-23.10 {chan read, EAGAIN means no data, yet no eof either} -match glob -setup { set res {} proc foo {args} { @@ -1096,7 +1096,7 @@ test iocmd-23.10 {chan read, EAGAIN means no data, yet no eof either} -match glo close $c rename foo {} unset res -} -result {{read rc* 4096} {} 0} +} -result {{read rc* 4096} {watch rc* {}} {} 0} test iocmd-23.11 {chan read, close pulls the rug out} -match glob -body { set res {} proc foo {args} { @@ -1410,14 +1410,14 @@ test iocmd-25.10 {chan configure, cgetall, level is ignored} -match glob -body { test iocmd-26.1 {chan configure, set standard option} -match glob -body { set res {} proc foo {args} { - oninit configure; onfinal; track; note MUST_NOT_HAPPEN; return + oninit configure; onfinal; track; return } set c [chan create {r w} foo] note [fconfigure $c -translation lf] close $c rename foo {} set res -} -result {{}} +} -result {{watch rc* {}} {}} test iocmd-26.2 {chan configure, set option, error return} -match glob -body { set res {} proc foo {args} { @@ -1955,7 +1955,7 @@ test iocmd-31.6 {chan postevent, posted events do happen} -match glob -body { close $c rename foo {} set res -} -result {{watch rc* read} {} TOCK {} {watch rc* {}}} +} -result {{watch rc* read} {} TOCK {watch rc* read} {} {watch rc* {}}} test iocmd-31.7 {chan postevent, posted events do happen} -match glob -body { set res {} proc foo {args} {oninit; onfinal; track; return} @@ -1968,7 +1968,7 @@ test iocmd-31.7 {chan postevent, posted events do happen} -match glob -body { close $c rename foo {} set res -} -result {{watch rc* write} {} TOCK {} {watch rc* {}}} +} -result {{watch rc* write} {} TOCK {watch rc* write} {} {watch rc* {}}} test iocmd-31.8 {chan postevent after close throws error} -match glob -setup { proc foo {args} {oninit; onfinal; track; return} proc dummy args { return } -- cgit v0.12 From 2b9f58aaac9d943888960daa1394c1abeae2e374 Mon Sep 17 00:00:00 2001 From: dgp Date: Wed, 23 Sep 2015 17:29:15 +0000 Subject: Update tests to new ReflectWatch() behavior. --- tests/ioCmd.test | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/tests/ioCmd.test b/tests/ioCmd.test index a0caab2..f66480a 100644 --- a/tests/ioCmd.test +++ b/tests/ioCmd.test @@ -2300,7 +2300,7 @@ test iocmd.tf-23.1 {chan read, regular data return} -match glob -body { } c] rename foo {} set res -} -constraints {testchannel thread} -result {{read rc* 4096} {read rc* 4096} snarfsnarf} +} -constraints {testchannel thread} -result {{read rc* 4096} {read rc* 4096} {watch rc* {}} snarfsnarf} test iocmd.tf-23.2 {chan read, bad data return, to much} -match glob -body { set res {} proc foo {args} { @@ -2315,7 +2315,7 @@ test iocmd.tf-23.2 {chan read, bad data return, to much} -match glob -body { } c] rename foo {} set res -} -constraints {testchannel thread} -result {{read rc* 4096} 1 {read delivered more than requested}} +} -constraints {testchannel thread} -result {{read rc* 4096} {watch rc* {}} 1 {read delivered more than requested}} test iocmd.tf-23.3 {chan read, for non-readable channel} -match glob -body { set res {} proc foo {args} { @@ -2344,7 +2344,7 @@ test iocmd.tf-23.4 {chan read, error return} -match glob -body { } c] rename foo {} set res -} -result {{read rc* 4096} 1 BOOM!} \ +} -result {{read rc* 4096} {watch rc* {}} 1 BOOM!} \ -constraints {testchannel thread} test iocmd.tf-23.5 {chan read, break return is error} -match glob -body { set res {} @@ -2360,7 +2360,7 @@ test iocmd.tf-23.5 {chan read, break return is error} -match glob -body { } c] rename foo {} set res -} -result {{read rc* 4096} 1 *bad code*} \ +} -result {{read rc* 4096} {watch rc* {}} 1 *bad code*} \ -constraints {testchannel thread} test iocmd.tf-23.6 {chan read, continue return is error} -match glob -body { set res {} @@ -2376,7 +2376,7 @@ test iocmd.tf-23.6 {chan read, continue return is error} -match glob -body { } c] rename foo {} set res -} -result {{read rc* 4096} 1 *bad code*} \ +} -result {{read rc* 4096} {watch rc* {}} 1 *bad code*} \ -constraints {testchannel thread} test iocmd.tf-23.7 {chan read, custom return is error} -match glob -body { set res {} @@ -2392,7 +2392,7 @@ test iocmd.tf-23.7 {chan read, custom return is error} -match glob -body { } c] rename foo {} set res -} -result {{read rc* 4096} 1 *bad code*} \ +} -result {{read rc* 4096} {watch rc* {}} 1 *bad code*} \ -constraints {testchannel thread} test iocmd.tf-23.8 {chan read, level is squashed} -match glob -body { set res {} @@ -2408,7 +2408,7 @@ test iocmd.tf-23.8 {chan read, level is squashed} -match glob -body { } c] rename foo {} set res -} -result {{read rc* 4096} 1 *bad code* {-code 1 -level 0 -errorcode NONE -errorline 1 -errorinfo *bad code*subcommand "read"*}} \ +} -result {{read rc* 4096} {watch rc* {}} 1 *bad code* {-code 1 -level 0 -errorcode NONE -errorline 1 -errorinfo *bad code*subcommand "read"*}} \ -constraints {testchannel thread} test iocmd.tf-23.9 {chan read, no data means eof} -match glob -setup { set res {} @@ -2428,7 +2428,7 @@ test iocmd.tf-23.9 {chan read, no data means eof} -match glob -setup { } -cleanup { rename foo {} unset res -} -result {{read rc* 4096} {} 1} \ +} -result {{read rc* 4096} {watch rc* {}} {} 1} \ -constraints {testchannel thread} test iocmd.tf-23.10 {chan read, EAGAIN means no data, yet no eof either} -match glob -setup { set res {} @@ -2448,7 +2448,7 @@ test iocmd.tf-23.10 {chan read, EAGAIN means no data, yet no eof either} -match } -cleanup { rename foo {} unset res -} -result {{read rc* 4096} {} 0} \ +} -result {{read rc* 4096} {watch rc* {}} {} 0} \ -constraints {testchannel thread} # --- === *** ########################### @@ -3001,7 +3001,7 @@ test iocmd.tf-25.10 {chan configure, cgetall, level is ignored} -match glob -bod test iocmd.tf-26.1 {chan configure, set standard option} -match glob -body { set res {} proc foo {args} { - oninit configure; onfinal; track; note MUST_NOT_HAPPEN; return + oninit configure; onfinal; track; return } set c [chan create {r w} foo] notes [inthread $c { @@ -3011,7 +3011,7 @@ test iocmd.tf-26.1 {chan configure, set standard option} -match glob -body { } c] rename foo {} set res -} -constraints {testchannel thread} -result {{}} +} -constraints {testchannel thread} -result {{watch rc* {}} {}} test iocmd.tf-26.2 {chan configure, set option, error return} -match glob -body { set res {} proc foo {args} { -- cgit v0.12 From 61ad28e5b697aee8b22a823efd9333905ce1e787 Mon Sep 17 00:00:00 2001 From: "jan.nijtmans" Date: Thu, 24 Sep 2015 09:55:14 +0000 Subject: Clean-up tclUnixNotfy.c, restructure it the same as "trunk" version. No functional changes. This will make upcoming merging work easier. --- unix/tclUnixNotfy.c | 386 ++++++++++++++++++++++++++++------------------------ 1 file changed, 207 insertions(+), 179 deletions(-) diff --git a/unix/tclUnixNotfy.c b/unix/tclUnixNotfy.c index 52f6b55..e21e744 100644 --- a/unix/tclUnixNotfy.c +++ b/unix/tclUnixNotfy.c @@ -7,8 +7,8 @@ * * Copyright (c) 1995-1997 Sun Microsystems, Inc. * - * See the file "license.terms" for information on usage and redistribution of - * this file, and for a DISCLAIMER OF ALL WARRANTIES. + * See the file "license.terms" for information on usage and redistribution + * of this file, and for a DISCLAIMER OF ALL WARRANTIES. */ #include "tclInt.h" @@ -59,13 +59,13 @@ typedef struct FileHandlerEvent { /* * The following structure contains a set of select() masks to track readable, - * writable, and exceptional conditions. + * writable, and exception conditions. */ typedef struct SelectMasks { fd_set readable; fd_set writable; - fd_set exceptional; + fd_set exception; } SelectMasks; /* @@ -212,23 +212,10 @@ static void AtForkChild(void); static int FileHandlerEventProc(Tcl_Event *evPtr, int flags); /* - *---------------------------------------------------------------------- - * - * Tcl_InitNotifier -- - * - * Initializes the platform specific notifier state. - * - * Results: - * Returns a handle to the notifier state for this thread. - * - * Side effects: - * None. - * - *---------------------------------------------------------------------- + * Import of Windows API when building threaded with Cygwin. */ #if defined(TCL_THREADS) && defined(__CYGWIN__) - typedef struct { void *hwnd; unsigned int *message; @@ -240,34 +227,60 @@ typedef struct { } MSG; typedef struct { - unsigned int style; - void *lpfnWndProc; - int cbClsExtra; - int cbWndExtra; - void *hInstance; - void *hIcon; - void *hCursor; - void *hbrBackground; - void *lpszMenuName; - void *lpszClassName; + unsigned int style; + void *lpfnWndProc; + int cbClsExtra; + int cbWndExtra; + void *hInstance; + void *hIcon; + void *hCursor; + void *hbrBackground; + void *lpszMenuName; + void *lpszClassName; } WNDCLASS; -extern unsigned char __stdcall PeekMessageW(MSG *, void *, int, int, int); -extern unsigned char __stdcall GetMessageW(MSG *, void *, int, int); -extern unsigned char __stdcall TranslateMessage(const MSG *); -extern int __stdcall DispatchMessageW(const MSG *); -extern void __stdcall PostQuitMessage(int); -extern void * __stdcall CreateWindowExW(void *, void *, void *, DWORD, int, int, int, int, void *, void *, void *, void *); -extern unsigned char __stdcall DestroyWindow(void *); -extern unsigned char __stdcall PostMessageW(void *, unsigned int, void *, void *); -extern void *__stdcall RegisterClassW(const WNDCLASS *); -extern DWORD __stdcall DefWindowProcW(void *, int, void *, void *); -extern void *__stdcall CreateEventW(void *, unsigned char, unsigned char, void *); -extern void __stdcall CloseHandle(void *); -extern void __stdcall MsgWaitForMultipleObjects(DWORD, void *, unsigned char, DWORD, DWORD); -extern unsigned char __stdcall ResetEvent(void *); - -#endif +extern void __stdcall CloseHandle(void *); +extern void *__stdcall CreateEventW(void *, unsigned char, unsigned char, + void *); +extern void * __stdcall CreateWindowExW(void *, const void *, const void *, + DWORD, int, int, int, int, void *, void *, void *, void *); +extern DWORD __stdcall DefWindowProcW(void *, int, void *, void *); +extern unsigned char __stdcall DestroyWindow(void *); +extern int __stdcall DispatchMessageW(const MSG *); +extern unsigned char __stdcall GetMessageW(MSG *, void *, int, int); +extern void __stdcall MsgWaitForMultipleObjects(DWORD, void *, + unsigned char, DWORD, DWORD); +extern unsigned char __stdcall PeekMessageW(MSG *, void *, int, int, int); +extern unsigned char __stdcall PostMessageW(void *, unsigned int, void *, + void *); +extern void __stdcall PostQuitMessage(int); +extern void *__stdcall RegisterClassW(const WNDCLASS *); +extern unsigned char __stdcall ResetEvent(void *); +extern unsigned char __stdcall TranslateMessage(const MSG *); + +/* + * Threaded-cygwin specific functions in this file: + */ + +static DWORD __stdcall NotifierProc(void *hwnd, unsigned int message, + void *wParam, void *lParam); +#endif /* TCL_THREADS && __CYGWIN__ */ + +/* + *---------------------------------------------------------------------- + * + * Tcl_InitNotifier -- + * + * Initializes the platform specific notifier state. + * + * Results: + * Returns a handle to the notifier state for this thread. + * + * Side effects: + * None. + * + *---------------------------------------------------------------------- + */ ClientData Tcl_InitNotifier(void) @@ -314,7 +327,7 @@ Tcl_InitNotifier(void) TCL_THREAD_STACK_DEFAULT, TCL_THREAD_JOINABLE) != TCL_OK) { Tcl_Panic("Tcl_InitNotifier: unable to start notifier thread"); } - processIDInitialized = getpid(); + processIDInitialized = getpid(); } notifierCount++; @@ -360,30 +373,32 @@ Tcl_FinalizeNotifier( notifierCount--; /* - * If this is the last thread to use the notifier, close the notifier pipe - * and wait for the background thread to terminate. + * If this is the last thread to use the notifier, close the notifier + * pipe and wait for the background thread to terminate. */ if (notifierCount == 0) { int result; if (triggerPipe < 0) { - Tcl_Panic("Tcl_FinalizeNotifier: notifier pipe not initialized"); + Tcl_Panic("Tcl_FinalizeNotifier: %s", + "notifier pipe not initialized"); } /* - * Send "q" message to the notifier thread so that it will terminate. - * The notifier will return from its call to select() and notice that - * a "q" message has arrived, it will then close its side of the pipe - * and terminate its thread. Note the we can not just close the pipe - * and check for EOF in the notifier thread because if a background - * child process was created with exec, select() would not register - * the EOF on the pipe until the child processes had terminated. [Bug: - * 4139] [Bug: 1222872] + * Send "q" message to the notifier thread so that it will + * terminate. The notifier will return from its call to select() + * and notice that a "q" message has arrived, it will then close + * its side of the pipe and terminate its thread. Note the we can + * not just close the pipe and check for EOF in the notifier thread + * because if a background child process was created with exec, + * select() would not register the EOF on the pipe until the child + * processes had terminated. [Bug: 4139] [Bug: 1222872] */ if (write(triggerPipe, "q", 1) != 1) { - Tcl_Panic("Tcl_FinalizeNotifier: unable to write q to triggerPipe"); + Tcl_Panic("Tcl_FinalizeNotifier: %s", + "unable to write q to triggerPipe"); } close(triggerPipe); while(triggerPipe >= 0) { @@ -392,7 +407,8 @@ Tcl_FinalizeNotifier( result = Tcl_JoinThread(notifierThread, NULL); if (result) { - Tcl_Panic("Tcl_FinalizeNotifier: unable to join notifier thread"); + Tcl_Panic("Tcl_FinalizeNotifier: %s", + "unable to join notifier thread"); } } @@ -407,7 +423,7 @@ Tcl_FinalizeNotifier( #endif /* __CYGWIN__ */ Tcl_MutexUnlock(¬ifierMutex); -#endif +#endif /* TCL_THREADS */ } /* @@ -435,15 +451,16 @@ Tcl_AlertNotifier( { #ifdef TCL_THREADS ThreadSpecificData *tsdPtr = (ThreadSpecificData *) clientData; + Tcl_MutexLock(¬ifierMutex); tsdPtr->eventReady = 1; #ifdef __CYGWIN__ PostMessageW(tsdPtr->hwnd, 1024, 0, 0); -#else +#else /* __CYGWIN__ */ Tcl_ConditionNotify(&tsdPtr->waitCV); -#endif +#endif /* __CYGWIN__ */ Tcl_MutexUnlock(¬ifierMutex); -#endif +#endif /* TCL_THREADS */ } /* @@ -469,9 +486,9 @@ Tcl_SetTimer( Tcl_Time *timePtr) /* Timeout value, may be NULL. */ { /* - * The interval timer doesn't do anything in this implementation, because - * the only event loop is via Tcl_DoOneEvent, which passes timeout values - * to Tcl_WaitForEvent. + * The interval timer doesn't do anything in this implementation, + * because the only event loop is via Tcl_DoOneEvent, which passes + * timeout values to Tcl_WaitForEvent. */ if (tclStubs.tcl_SetTimer != tclOriginalNotifier.setTimerProc) { @@ -560,19 +577,19 @@ Tcl_CreateFileHandler( */ if (mask & TCL_READABLE) { - FD_SET(fd, &(tsdPtr->checkMasks.readable)); + FD_SET(fd, &tsdPtr->checkMasks.readable); } else { - FD_CLR(fd, &(tsdPtr->checkMasks.readable)); + FD_CLR(fd, &tsdPtr->checkMasks.readable); } if (mask & TCL_WRITABLE) { - FD_SET(fd, &(tsdPtr->checkMasks.writable)); + FD_SET(fd, &tsdPtr->checkMasks.writable); } else { - FD_CLR(fd, &(tsdPtr->checkMasks.writable)); + FD_CLR(fd, &tsdPtr->checkMasks.writable); } if (mask & TCL_EXCEPTION) { - FD_SET(fd, &(tsdPtr->checkMasks.exceptional)); + FD_SET(fd, &tsdPtr->checkMasks.exception); } else { - FD_CLR(fd, &(tsdPtr->checkMasks.exceptional)); + FD_CLR(fd, &tsdPtr->checkMasks.exception); } if (tsdPtr->numFdBits <= fd) { tsdPtr->numFdBits = fd+1; @@ -615,7 +632,7 @@ Tcl_DeleteFileHandler( */ for (prevPtr = NULL, filePtr = tsdPtr->firstFileHandlerPtr; ; - prevPtr = filePtr, filePtr = filePtr->nextPtr) { + prevPtr = filePtr, filePtr = filePtr->nextPtr) { if (filePtr == NULL) { return; } @@ -629,13 +646,13 @@ Tcl_DeleteFileHandler( */ if (filePtr->mask & TCL_READABLE) { - FD_CLR(fd, &(tsdPtr->checkMasks.readable)); + FD_CLR(fd, &tsdPtr->checkMasks.readable); } if (filePtr->mask & TCL_WRITABLE) { - FD_CLR(fd, &(tsdPtr->checkMasks.writable)); + FD_CLR(fd, &tsdPtr->checkMasks.writable); } if (filePtr->mask & TCL_EXCEPTION) { - FD_CLR(fd, &(tsdPtr->checkMasks.exceptional)); + FD_CLR(fd, &tsdPtr->checkMasks.exception); } /* @@ -646,9 +663,9 @@ Tcl_DeleteFileHandler( int numFdBits = 0; for (i = fd-1; i >= 0; i--) { - if (FD_ISSET(i, &(tsdPtr->checkMasks.readable)) - || FD_ISSET(i, &(tsdPtr->checkMasks.writable)) - || FD_ISSET(i, &(tsdPtr->checkMasks.exceptional))) { + if (FD_ISSET(i, &tsdPtr->checkMasks.readable) + || FD_ISSET(i, &tsdPtr->checkMasks.writable) + || FD_ISSET(i, &tsdPtr->checkMasks.exception)) { numFdBits = i+1; break; } @@ -734,7 +751,7 @@ FileHandlerEventProc( mask = filePtr->readyMask & filePtr->mask; filePtr->readyMask = 0; if (mask != 0) { - (*filePtr->proc)(filePtr->clientData, mask); + filePtr->proc(filePtr->clientData, mask); } break; } @@ -760,12 +777,12 @@ NotifierProc( * Process all of the runnable events. */ - tsdPtr->eventReady = 1; + tsdPtr->eventReady = 1; Tcl_ServiceAll(); return 0; } -#endif /* __CYGWIN__ */ - +#endif /* TCL_THREADS && __CYGWIN__ */ + /* *---------------------------------------------------------------------- * @@ -789,19 +806,18 @@ Tcl_WaitForEvent( Tcl_Time *timePtr) /* Maximum block time, or NULL. */ { FileHandler *filePtr; - FileHandlerEvent *fileEvPtr; int mask; Tcl_Time vTime; #ifdef TCL_THREADS int waitForFiles; -# ifdef __CYGWIN__ +#ifdef __CYGWIN__ MSG msg; -# endif +#endif /* __CYGWIN__ */ #else /* - * Impl. notes: timeout & timeoutPtr are used if, and only if threads are - * not enabled. They are the arguments for the regular select() used when - * the core is not thread-enabled. + * Impl. notes: timeout & timeoutPtr are used if, and only if threads + * are not enabled. They are the arguments for the regular select() + * used when the core is not thread-enabled. */ struct timeval timeout, *timeoutPtr; @@ -814,20 +830,21 @@ Tcl_WaitForEvent( } /* - * Set up the timeout structure. Note that if there are no events to check - * for, we return with a negative result rather than blocking forever. + * Set up the timeout structure. Note that if there are no events to + * check for, we return with a negative result rather than blocking + * forever. */ if (timePtr != NULL) { /* - * TIP #233 (Virtualized Time). Is virtual time in effect? And do we - * actually have something to scale? If yes to both then we call the - * handler to do this scaling. + * TIP #233 (Virtualized Time). Is virtual time in effect? And do + * we actually have something to scale? If yes to both then we + * call the handler to do this scaling. */ if (timePtr->sec != 0 || timePtr->usec != 0) { vTime = *timePtr; - (*tclScaleTimeProcPtr) (&vTime, tclTimeClientData); + tclScaleTimeProcPtr(&vTime, tclTimeClientData); timePtr = &vTime; } #ifndef TCL_THREADS @@ -836,17 +853,17 @@ Tcl_WaitForEvent( timeoutPtr = &timeout; } else if (tsdPtr->numFdBits == 0) { /* - * If there are no threads, no timeout, and no fds registered, then - * there are no events possible and we must avoid deadlock. Note that - * this is not entirely correct because there might be a signal that - * could interrupt the select call, but we don't handle that case if - * we aren't using threads. + * If there are no threads, no timeout, and no fds registered, + * then there are no events possible and we must avoid deadlock. + * Note that this is not entirely correct because there might be a + * signal that could interrupt the select call, but we don't + * handle that case if we aren't using threads. */ return -1; } else { timeoutPtr = NULL; -#endif /* TCL_THREADS */ +#endif /* !TCL_THREADS */ } #ifdef TCL_THREADS @@ -855,38 +872,9 @@ Tcl_WaitForEvent( * notifier thread, and wait for a response or a timeout. */ - Tcl_MutexLock(¬ifierMutex); - - if (timePtr != NULL && timePtr->sec == 0 && (timePtr->usec == 0 -#if defined(__APPLE__) && defined(__LP64__) - /* - * On 64-bit Darwin, pthread_cond_timedwait() appears to have a bug - * that causes it to wait forever when passed an absolute time which - * has already been exceeded by the system time; as a workaround, - * when given a very brief timeout, just do a poll. [Bug 1457797] - */ - || timePtr->usec < 10 -#endif - )) { - /* - * Cannot emulate a polling select with a polling condition variable. - * Instead, pretend to wait for files and tell the notifier thread - * what we are doing. The notifier thread makes sure it goes through - * select with its select mask in the same state as ours currently is. - * We block until that happens. - */ - - waitForFiles = 1; - tsdPtr->pollState = POLL_WANT; - timePtr = NULL; - } else { - waitForFiles = (tsdPtr->numFdBits > 0); - tsdPtr->pollState = 0; - } - #ifdef __CYGWIN__ if (!tsdPtr->hwnd) { - WNDCLASS class; + WNDCLASS class; class.style = 0; class.cbClsExtra = 0; @@ -900,18 +888,49 @@ Tcl_WaitForEvent( class.hCursor = NULL; RegisterClassW(&class); - tsdPtr->hwnd = CreateWindowExW(NULL, class.lpszClassName, class.lpszClassName, - 0, 0, 0, 0, 0, NULL, NULL, TclWinGetTclInstance(), NULL); + tsdPtr->hwnd = CreateWindowExW(NULL, class.lpszClassName, + class.lpszClassName, 0, 0, 0, 0, 0, NULL, NULL, + TclWinGetTclInstance(), NULL); tsdPtr->event = CreateEventW(NULL, 1 /* manual */, 0 /* !signaled */, NULL); + } +#endif /* __CYGWIN__ */ + + Tcl_MutexLock(¬ifierMutex); + + if (timePtr != NULL && timePtr->sec == 0 && (timePtr->usec == 0 +#if defined(__APPLE__) && defined(__LP64__) + /* + * On 64-bit Darwin, pthread_cond_timedwait() appears to have + * a bug that causes it to wait forever when passed an + * absolute time which has already been exceeded by the system + * time; as a workaround, when given a very brief timeout, + * just do a poll. [Bug 1457797] + */ + || timePtr->usec < 10 +#endif /* __APPLE__ && __LP64__ */ + )) { + /* + * Cannot emulate a polling select with a polling condition + * variable. Instead, pretend to wait for files and tell the + * notifier thread what we are doing. The notifier thread makes + * sure it goes through select with its select mask in the same + * state as ours currently is. We block until that happens. + */ + + waitForFiles = 1; + tsdPtr->pollState = POLL_WANT; + timePtr = NULL; + } else { + waitForFiles = (tsdPtr->numFdBits > 0); + tsdPtr->pollState = 0; } -#endif if (waitForFiles) { /* - * Add the ThreadSpecificData structure of this thread to the list of - * ThreadSpecificData structures of all threads that are waiting on - * file events. + * Add the ThreadSpecificData structure of this thread to the list + * of ThreadSpecificData structures of all threads that are + * waiting on file events. */ tsdPtr->nextPtr = waitingListPtr; @@ -923,18 +942,20 @@ Tcl_WaitForEvent( tsdPtr->onList = 1; if ((write(triggerPipe, "", 1) == -1) && (errno != EAGAIN)) { - Tcl_Panic("Tcl_WaitForEvent: unable to write to triggerPipe"); + Tcl_Panic("Tcl_WaitForEvent: %s", + "unable to write to triggerPipe"); } } - FD_ZERO(&(tsdPtr->readyMasks.readable)); - FD_ZERO(&(tsdPtr->readyMasks.writable)); - FD_ZERO(&(tsdPtr->readyMasks.exceptional)); + FD_ZERO(&tsdPtr->readyMasks.readable); + FD_ZERO(&tsdPtr->readyMasks.writable); + FD_ZERO(&tsdPtr->readyMasks.exception); if (!tsdPtr->eventReady) { #ifdef __CYGWIN__ if (!PeekMessageW(&msg, NULL, 0, 0, 0)) { DWORD timeout; + if (timePtr) { timeout = timePtr->sec * 1000 + timePtr->usec / 1000; } else { @@ -946,7 +967,7 @@ Tcl_WaitForEvent( } #else Tcl_ConditionWait(&tsdPtr->waitCV, ¬ifierMutex, timePtr); -#endif +#endif /* __CYGWIN__ */ } tsdPtr->eventReady = 0; @@ -955,17 +976,19 @@ Tcl_WaitForEvent( /* * Retrieve and dispatch the message. */ + DWORD result = GetMessageW(&msg, NULL, 0, 0); + if (result == 0) { PostQuitMessage(msg.wParam); /* What to do here? */ - } else if (result != (DWORD)-1) { + } else if (result != (DWORD) -1) { TranslateMessage(&msg); DispatchMessageW(&msg); } } ResetEvent(tsdPtr->event); -#endif +#endif /* __CYGWIN__ */ if (waitForFiles && tsdPtr->onList) { /* @@ -986,25 +1009,26 @@ Tcl_WaitForEvent( tsdPtr->nextPtr = tsdPtr->prevPtr = NULL; tsdPtr->onList = 0; if ((write(triggerPipe, "", 1) == -1) && (errno != EAGAIN)) { - Tcl_Panic("Tcl_WaitForEvent: unable to write to triggerPipe"); + Tcl_Panic("Tcl_WaitForEvent: %s", + "unable to write to triggerPipe"); } } #else tsdPtr->readyMasks = tsdPtr->checkMasks; - numFound = select(tsdPtr->numFdBits, &(tsdPtr->readyMasks.readable), - &(tsdPtr->readyMasks.writable), &(tsdPtr->readyMasks.exceptional), + numFound = select(tsdPtr->numFdBits, &tsdPtr->readyMasks.readable, + &tsdPtr->readyMasks.writable, &tsdPtr->readyMasks.exception, timeoutPtr); /* - * Some systems don't clear the masks after an error, so we have to do it - * here. + * Some systems don't clear the masks after an error, so we have to do + * it here. */ if (numFound == -1) { - FD_ZERO(&(tsdPtr->readyMasks.readable)); - FD_ZERO(&(tsdPtr->readyMasks.writable)); - FD_ZERO(&(tsdPtr->readyMasks.exceptional)); + FD_ZERO(&tsdPtr->readyMasks.readable); + FD_ZERO(&tsdPtr->readyMasks.writable); + FD_ZERO(&tsdPtr->readyMasks.exception); } #endif /* TCL_THREADS */ @@ -1014,15 +1038,14 @@ Tcl_WaitForEvent( for (filePtr = tsdPtr->firstFileHandlerPtr; (filePtr != NULL); filePtr = filePtr->nextPtr) { - mask = 0; - if (FD_ISSET(filePtr->fd, &(tsdPtr->readyMasks.readable))) { + if (FD_ISSET(filePtr->fd, &tsdPtr->readyMasks.readable)) { mask |= TCL_READABLE; } - if (FD_ISSET(filePtr->fd, &(tsdPtr->readyMasks.writable))) { + if (FD_ISSET(filePtr->fd, &tsdPtr->readyMasks.writable)) { mask |= TCL_WRITABLE; } - if (FD_ISSET(filePtr->fd, &(tsdPtr->readyMasks.exceptional))) { + if (FD_ISSET(filePtr->fd, &tsdPtr->readyMasks.exception)) { mask |= TCL_EXCEPTION; } @@ -1031,12 +1054,13 @@ Tcl_WaitForEvent( } /* - * Don't bother to queue an event if the mask was previously non-zero - * since an event must still be on the queue. + * Don't bother to queue an event if the mask was previously + * non-zero since an event must still be on the queue. */ if (filePtr->readyMask == 0) { - fileEvPtr = (FileHandlerEvent *) ckalloc(sizeof(FileHandlerEvent)); + FileHandlerEvent *fileEvPtr = + (FileHandlerEvent *) ckalloc(sizeof(FileHandlerEvent)); fileEvPtr->header.proc = FileHandlerEventProc; fileEvPtr->fd = filePtr->fd; Tcl_QueueEvent((Tcl_Event *) fileEvPtr, TCL_QUEUE_TAIL); @@ -1081,7 +1105,7 @@ NotifierThreadProc( ThreadSpecificData *tsdPtr; fd_set readableMask; fd_set writableMask; - fd_set exceptionalMask; + fd_set exceptionMask; int fds[2]; int i, numFdBits = 0, receivePipe; long found; @@ -1089,22 +1113,26 @@ NotifierThreadProc( char buf[2]; if (pipe(fds) != 0) { - Tcl_Panic("NotifierThreadProc: could not create trigger pipe"); + Tcl_Panic("NotifierThreadProc: %s", "could not create trigger pipe"); } receivePipe = fds[0]; if (TclUnixSetBlockingMode(receivePipe, TCL_MODE_NONBLOCKING) < 0) { - Tcl_Panic("NotifierThreadProc: could not make receive pipe non blocking"); + Tcl_Panic("NotifierThreadProc: %s", + "could not make receive pipe non blocking"); } if (TclUnixSetBlockingMode(fds[1], TCL_MODE_NONBLOCKING) < 0) { - Tcl_Panic("NotifierThreadProc: could not make trigger pipe non blocking"); + Tcl_Panic("NotifierThreadProc: %s", + "could not make trigger pipe non blocking"); } if (fcntl(receivePipe, F_SETFD, FD_CLOEXEC) < 0) { - Tcl_Panic("NotifierThreadProc: could not make receive pipe close-on-exec"); + Tcl_Panic("NotifierThreadProc: %s", + "could not make receive pipe close-on-exec"); } if (fcntl(fds[1], F_SETFD, FD_CLOEXEC) < 0) { - Tcl_Panic("NotifierThreadProc: could not make trigger pipe close-on-exec"); + Tcl_Panic("NotifierThreadProc: %s", + "could not make trigger pipe close-on-exec"); } /* @@ -1128,7 +1156,7 @@ NotifierThreadProc( while (1) { FD_ZERO(&readableMask); FD_ZERO(&writableMask); - FD_ZERO(&exceptionalMask); + FD_ZERO(&exceptionMask); /* * Compute the logical OR of the select masks from all the waiting @@ -1139,14 +1167,14 @@ NotifierThreadProc( timePtr = NULL; for (tsdPtr = waitingListPtr; tsdPtr; tsdPtr = tsdPtr->nextPtr) { for (i = tsdPtr->numFdBits-1; i >= 0; --i) { - if (FD_ISSET(i, &(tsdPtr->checkMasks.readable))) { + if (FD_ISSET(i, &tsdPtr->checkMasks.readable)) { FD_SET(i, &readableMask); } - if (FD_ISSET(i, &(tsdPtr->checkMasks.writable))) { + if (FD_ISSET(i, &tsdPtr->checkMasks.writable)) { FD_SET(i, &writableMask); } - if (FD_ISSET(i, &(tsdPtr->checkMasks.exceptional))) { - FD_SET(i, &exceptionalMask); + if (FD_ISSET(i, &tsdPtr->checkMasks.exception)) { + FD_SET(i, &exceptionMask); } } if (tsdPtr->numFdBits > numFdBits) { @@ -1173,7 +1201,7 @@ NotifierThreadProc( } FD_SET(receivePipe, &readableMask); - if (select(numFdBits, &readableMask, &writableMask, &exceptionalMask, + if (select(numFdBits, &readableMask, &writableMask, &exceptionMask, timePtr) == -1) { /* * Try again immediately on an error. @@ -1191,19 +1219,19 @@ NotifierThreadProc( found = 0; for (i = tsdPtr->numFdBits-1; i >= 0; --i) { - if (FD_ISSET(i, &(tsdPtr->checkMasks.readable)) + if (FD_ISSET(i, &tsdPtr->checkMasks.readable) && FD_ISSET(i, &readableMask)) { - FD_SET(i, &(tsdPtr->readyMasks.readable)); + FD_SET(i, &tsdPtr->readyMasks.readable); found = 1; } - if (FD_ISSET(i, &(tsdPtr->checkMasks.writable)) + if (FD_ISSET(i, &tsdPtr->checkMasks.writable) && FD_ISSET(i, &writableMask)) { - FD_SET(i, &(tsdPtr->readyMasks.writable)); + FD_SET(i, &tsdPtr->readyMasks.writable); found = 1; } - if (FD_ISSET(i, &(tsdPtr->checkMasks.exceptional)) - && FD_ISSET(i, &exceptionalMask)) { - FD_SET(i, &(tsdPtr->readyMasks.exceptional)); + if (FD_ISSET(i, &tsdPtr->checkMasks.exception) + && FD_ISSET(i, &exceptionMask)) { + FD_SET(i, &tsdPtr->readyMasks.exception); found = 1; } } @@ -1231,9 +1259,9 @@ NotifierThreadProc( tsdPtr->pollState = 0; } #ifdef __CYGWIN__ - PostMessageW(tsdPtr->hwnd, 1024, 0, 0); + PostMessageW(tsdPtr->hwnd, 1024, 0, 0); #else /* __CYGWIN__ */ - Tcl_ConditionNotify(&tsdPtr->waitCV); + Tcl_ConditionNotify(&tsdPtr->waitCV); #endif /* __CYGWIN__ */ } } -- cgit v0.12