From 1ce57bb924b57df1f03460ba98eaebc476ca6931 Mon Sep 17 00:00:00 2001 From: "jan.nijtmans" Date: Thu, 6 Aug 2015 03:43:43 +0000 Subject: Gustaf Neumann's experimental Unix notifier improvements. --- unix/tclUnixNotfy.c | 124 ++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 120 insertions(+), 4 deletions(-) diff --git a/unix/tclUnixNotfy.c b/unix/tclUnixNotfy.c index 17fdc95..a09899f 100644 --- a/unix/tclUnixNotfy.c +++ b/unix/tclUnixNotfy.c @@ -1,3 +1,6 @@ +#define LAZY_THREAD_CREATE 1 +#define AT_FORK_INIT_VALUE 0 +#define DEACTIVATE_ATFORK_MUTEX 0 /* * tclUnixNotify.c -- * @@ -160,6 +163,18 @@ static int triggerPipe = -1; TCL_DECLARE_MUTEX(notifierMutex) +#if LAZY_THREAD_CREATE == 1 +TCL_DECLARE_MUTEX(notifierInitMutex) + +/* + * The following static indicates if the notifier thread is running. + * + * You must hold the notifierInitLock before accessing this variable. + */ + +static int notifierThreadRunning = 0; +#endif + /* * The notifier thread signals the notifierCV when it has finished * initializing the triggerPipe and right before the notifier thread @@ -195,7 +210,7 @@ static Tcl_ThreadId notifierThread; #ifdef TCL_THREADS static void NotifierThreadProc(ClientData clientData); #if defined(HAVE_PTHREAD_ATFORK) && !defined(__APPLE__) && !defined(__hpux) -static int atForkInit = 0; +static int atForkInit = AT_FORK_INIT_VALUE; static void AtForkPrepare(void); static void AtForkParent(void); static void AtForkChild(void); @@ -257,6 +272,33 @@ extern unsigned char __stdcall TranslateMessage(const MSG *); static DWORD __stdcall NotifierProc(void *hwnd, unsigned int message, void *wParam, void *lParam); #endif /* TCL_THREADS && __CYGWIN__ */ + +#if LAZY_THREAD_CREATE == 1 +static void +StartNotifierThread(void) +{ + fprintf(stderr, "=== StartNotifierThread()\n"); + Tcl_MutexLock(¬ifierInitMutex); + TclpMasterLock(); + TclpMutexLock(); + fprintf(stderr, "=== StartNotifierThread() locked notifierThreadRunning %d notifierCount %d\n", notifierThreadRunning, notifierCount); + if (!notifierCount) { + Tcl_Panic("StartNotifierThread: notifier not initialized"); + } + if (!notifierThreadRunning) { + fprintf(stderr, "=== StartNotifierThread() really start\n"); + if (TclpThreadCreate(¬ifierThread, NotifierThreadProc, NULL, + TCL_THREAD_STACK_DEFAULT, TCL_THREAD_JOINABLE) != TCL_OK) { + Tcl_Panic("Tcl_InitNotifier: unable to start notifier thread"); + } + processIDInitialized = getpid(); + notifierThreadRunning = 1; + } + TclpMutexUnlock(); + TclpMasterUnlock(); + Tcl_MutexUnlock(¬ifierInitMutex); +} +#endif /* *---------------------------------------------------------------------- @@ -277,11 +319,11 @@ static DWORD __stdcall NotifierProc(void *hwnd, unsigned int message, ClientData Tcl_InitNotifier(void) { + //fprintf(stderr, "==== Tcl_InitNotifier atForkInit %d notifierCount %d\n", atForkInit, notifierCount); if (tclNotifierHooks.initNotifierProc) { return tclNotifierHooks.initNotifierProc(); } else { - ThreadSpecificData *tsdPtr = TCL_TSD_INIT(&dataKey); - + ThreadSpecificData *tsdPtr = TCL_TSD_INIT(&dataKey); #ifdef TCL_THREADS tsdPtr->eventReady = 0; @@ -295,9 +337,9 @@ Tcl_InitNotifier(void) * Install pthread_atfork handlers to reinitialize the notifier in the * child of a fork. */ - if (!atForkInit) { int result = pthread_atfork(AtForkPrepare, AtForkParent, AtForkChild); + fprintf(stderr, "==== calling pthread_atfork()\n"); if (result) { Tcl_Panic("Tcl_InitNotifier: pthread_atfork failed"); @@ -310,22 +352,28 @@ Tcl_InitNotifier(void) * In this case, restart the notifier thread and close the * pipe to the original notifier thread */ +#if LAZY_THREAD_CREATE == 0 if (notifierCount > 0 && processIDInitialized != getpid()) { + // fprintf(stderr, "==== reset notifierCount for %d\n", getpid()); Tcl_ConditionFinalize(¬ifierCV); notifierCount = 0; processIDInitialized = 0; close(triggerPipe); triggerPipe = -1; } +#endif if (notifierCount == 0) { +#if LAZY_THREAD_CREATE == 0 if (TclpThreadCreate(¬ifierThread, NotifierThreadProc, NULL, TCL_THREAD_STACK_DEFAULT, TCL_THREAD_JOINABLE) != TCL_OK) { Tcl_Panic("Tcl_InitNotifier: unable to start notifier thread"); } processIDInitialized = getpid(); +#endif } notifierCount++; +#if LAZY_THREAD_CREATE == 0 /* * Wait for the notifier pipe to be created. */ @@ -333,6 +381,7 @@ Tcl_InitNotifier(void) while (triggerPipe < 0) { Tcl_ConditionWait(¬ifierCV, ¬ifierMutex, NULL); } +#endif Tcl_MutexUnlock(¬ifierMutex); #endif /* TCL_THREADS */ @@ -371,6 +420,7 @@ Tcl_FinalizeNotifier( Tcl_MutexLock(¬ifierMutex); notifierCount--; + //fprintf(stderr, "==== Tcl_Tcl_FinalizeNotifier (after decr) atForkInit %d notifierCount %d\n", atForkInit, notifierCount); /* * If this is the last thread to use the notifier, close the notifier @@ -378,8 +428,39 @@ Tcl_FinalizeNotifier( */ if (notifierCount == 0) { +#if LAZY_THREAD_CREATE == 1 + if (triggerPipe != -1) { + if (write(triggerPipe, "q", 1) != 1) { + Tcl_Panic("Tcl_FinalizeNotifier: %s", + "unable to write q to triggerPipe"); + } + close(triggerPipe); + triggerPipe = -1; + + if (notifierThreadRunning) { + int result = Tcl_JoinThread(notifierThread, NULL); + + if (result) { + Tcl_Panic("Tcl_FinalizeNotifier: unable to join notifier " + "thread"); + } + notifierThreadRunning = 0; + } + } +#else int result; +# if AT_FORK_INIT_VALUE == 1 + if (triggerPipe != -1) { + if (write(triggerPipe, "q", 1) != 1) { + Tcl_Panic("Tcl_FinalizeNotifier: %s", + "unable to write q to triggerPipe"); + } + close(triggerPipe); + triggerPipe = -1; + } +# else + if (triggerPipe < 0) { Tcl_Panic("Tcl_FinalizeNotifier: %s", "notifier pipe not initialized"); @@ -410,6 +491,8 @@ Tcl_FinalizeNotifier( Tcl_Panic("Tcl_FinalizeNotifier: %s", "unable to join notifier thread"); } +# endif +#endif } /* @@ -524,11 +607,18 @@ Tcl_ServiceModeHook( int mode) /* Either TCL_SERVICE_ALL, or * TCL_SERVICE_NONE. */ { + fprintf(stderr, "==== Tcl_ServiceModeHook mode %d\n", mode); if (tclNotifierHooks.serviceModeHookProc) { tclNotifierHooks.serviceModeHookProc(mode); return; } else { +#if LAZY_THREAD_CREATE == 1 + if (mode == TCL_SERVICE_ALL) { + StartNotifierThread(); + } +#else /* Does nothing in this implementation. */ +#endif } } @@ -881,6 +971,9 @@ Tcl_WaitForEvent( * Place this thread on the list of interested threads, signal the * notifier thread, and wait for a response or a timeout. */ +#if LAZY_THREAD_CREATE == 1 + StartNotifierThread(); +#endif #ifdef __CYGWIN__ if (!tsdPtr->hwnd) { @@ -1147,6 +1240,7 @@ NotifierThreadProc( "could not make trigger pipe close-on-exec"); } +// fprintf(stderr, "=== Starting Notifier Thread\n"); /* * Install the write end of the pipe into the global variable. */ @@ -1304,6 +1398,7 @@ NotifierThreadProc( * Clean up the read end of the pipe and signal any threads waiting on * termination of the notifier thread. */ +fprintf(stderr, "=== Stopping Notifier Thread\n"); close(receivePipe); Tcl_MutexLock(¬ifierMutex); @@ -1334,9 +1429,11 @@ NotifierThreadProc( static void AtForkPrepare(void) { +#if DEACTIVATE_ATFORK_MUTEX == 0 Tcl_MutexLock(¬ifierMutex); TclpMasterLock(); TclpMutexLock(); +#endif } /* @@ -1358,9 +1455,12 @@ AtForkPrepare(void) static void AtForkParent(void) { +#if DEACTIVATE_ATFORK_MUTEX == 0 TclpMutexUnlock(); TclpMasterUnlock(); Tcl_MutexUnlock(¬ifierMutex); +#endif + //fprintf(stderr, "==== atParent %d notifierCount %d atForkInit %d\n", atForkInit, notifierCount, atForkInit); } /* @@ -1382,9 +1482,25 @@ AtForkParent(void) static void AtForkChild(void) { +#if DEACTIVATE_ATFORK_MUTEX == 0 TclpMutexUnlock(); TclpMasterUnlock(); TclMutexUnlockAndFinalize(¬ifierMutex); +#endif + +#if LAZY_THREAD_CREATE == 1 + //Tcl_MutexLock(¬ifierInitMutex); + notifierThreadRunning = 0; + if (notifierCount > 0) { + Tcl_ConditionFinalize(¬ifierCV); + notifierCount = 0; + processIDInitialized = 0; + close(triggerPipe); + triggerPipe = -1; + } + //Tcl_MutexUnlock(¬ifierInitMutex); +#endif + // fprintf(stderr, "==== AtForkChild() notifierCount %d notifierThreadRunning %d atForkInit %d\n",notifierCount,notifierThreadRunning, atForkInit); Tcl_InitNotifier(); } #endif /* HAVE_PTHREAD_ATFORK */ -- cgit v0.12 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 41107649bba64d4a990d11c0f5c0fa33ed86b66c Mon Sep 17 00:00:00 2001 From: "jan.nijtmans" Date: Mon, 17 Aug 2015 20:35:15 +0000 Subject: remove superfluous fprintf to stderr. --- unix/tclUnixNotfy.c | 1 - 1 file changed, 1 deletion(-) diff --git a/unix/tclUnixNotfy.c b/unix/tclUnixNotfy.c index 398e13a..0493ee4 100644 --- a/unix/tclUnixNotfy.c +++ b/unix/tclUnixNotfy.c @@ -283,7 +283,6 @@ StartNotifierThread(void) pthread_mutex_lock(¬ifierInitMutex); if (!notifierThreadRunning) { - fprintf(stderr, "=== StartNotifierThread()\n"); if (TclpThreadCreate(¬ifierThread, NotifierThreadProc, NULL, TCL_THREAD_STACK_DEFAULT, TCL_THREAD_JOINABLE) != TCL_OK) { Tcl_Panic("Tcl_InitNotifier: unable to start notifier thread"); -- cgit v0.12 From 1be28d5783ffb769754d37da7a745fb737f7f817 Mon Sep 17 00:00:00 2001 From: "jan.nijtmans" Date: Wed, 2 Sep 2015 11:54:58 +0000 Subject: Fix the Cygwin notifier, doing the initialization of the thread-local variables exactly the same as the Unix notifier. --- unix/tclUnixNotfy.c | 74 ++++++++++++++++++++++++----------------------------- 1 file changed, 33 insertions(+), 41 deletions(-) diff --git a/unix/tclUnixNotfy.c b/unix/tclUnixNotfy.c index dc6c6fd..c873774 100644 --- a/unix/tclUnixNotfy.c +++ b/unix/tclUnixNotfy.c @@ -103,8 +103,8 @@ typedef struct ThreadSpecificData { pthread_cond_t waitCV; /* Any other thread alerts a notifier that an * event is ready to be processed by signaling * this condition variable. */ - int waitCVinitialized; /* Variable to flag initialization of the structure */ #endif /* __CYGWIN__ */ + int waitCVinitialized; /* Variable to flag initialization of the structure */ int eventReady; /* True if an event is ready to be processed. * Used as condition flag together with waitCV * above. */ @@ -254,9 +254,10 @@ extern unsigned char __stdcall ResetEvent(void *); extern unsigned char __stdcall TranslateMessage(const MSG *); /* - * Threaded-cygwin specific functions in this file: + * Threaded-cygwin specific constants and functions in this file: */ +static const WCHAR NotfyClassName[] = L"TclNotifier"; static DWORD __stdcall NotifierProc(void *hwnd, unsigned int message, void *wParam, void *lParam); #endif /* TCL_THREADS && __CYGWIN__ */ @@ -331,15 +332,35 @@ Tcl_InitNotifier(void) #ifdef TCL_THREADS tsdPtr->eventReady = 0; -#ifndef __CYGWIN__ /* * Initialize thread specific condition variable for this thread. */ if (tsdPtr->waitCVinitialized == 0) { +#ifdef __CYGWIN__ + WNDCLASS class; + + class.style = 0; + class.cbClsExtra = 0; + class.cbWndExtra = 0; + class.hInstance = TclWinGetTclInstance(); + class.hbrBackground = NULL; + class.lpszMenuName = NULL; + class.lpszClassName = NotfyClassName; + class.lpfnWndProc = NotifierProc; + class.hIcon = NULL; + class.hCursor = NULL; + + RegisterClassW(&class); + 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); +#else pthread_cond_init(&tsdPtr->waitCV, NULL); +#endif /* __CYGWIN */ tsdPtr->waitCVinitialized = 1; } -#endif pthread_mutex_lock(¬ifierInitMutex); #if defined(HAVE_PTHREAD_ATFORK) && !defined(__APPLE__) && !defined(__hpux) @@ -403,9 +424,7 @@ Tcl_FinalizeNotifier( * Check if FinializeNotifier was called without a prior InitNotifier * in this thread. */ -#ifndef __CYGWIN__ assert(tsdPtr->waitCVinitialized == 1); -#endif /* * If this is the last thread to use the notifier, close the notifier @@ -440,10 +459,12 @@ Tcl_FinalizeNotifier( */ #ifdef __CYGWIN__ + DestroyWindow(tsdPtr->hwnd); CloseHandle(tsdPtr->event); #else /* __CYGWIN__ */ pthread_cond_destroy(&tsdPtr->waitCV); #endif /* __CYGWIN__ */ + tsdPtr->waitCVinitialized = 0; pthread_mutex_unlock(¬ifierInitMutex); #endif /* TCL_THREADS */ @@ -595,9 +616,7 @@ Tcl_CreateFileHandler( /* * Check if InitNotifier was called before in this thread */ -#ifndef __CYGWIN__ assert(tsdPtr->waitCVinitialized == 1); -#endif for (filePtr = tsdPtr->firstFileHandlerPtr; filePtr != NULL; filePtr = filePtr->nextPtr) { if (filePtr->fd == fd) { @@ -672,9 +691,7 @@ Tcl_DeleteFileHandler( /* * Check if InitNotifier was called before in this thread */ -#ifndef __CYGWIN__ assert(tsdPtr->waitCVinitialized == 1); -#endif /* * Find the entry for the given file (and return if there isn't one). @@ -784,9 +801,7 @@ FileHandlerEventProc( /* * Check if InitNotifier was called before in this thread */ -#ifndef __CYGWIN__ assert(tsdPtr->waitCVinitialized == 1); -#endif for (filePtr = tsdPtr->firstFileHandlerPtr; filePtr != NULL; filePtr = filePtr->nextPtr) { @@ -889,9 +904,7 @@ Tcl_WaitForEvent( /* * Check if InitNotifier was called before in this thread */ -#ifndef __CYGWIN__ assert(tsdPtr->waitCVinitialized == 1); -#endif /* * Set up the timeout structure. Note that if there are no events to @@ -938,30 +951,6 @@ Tcl_WaitForEvent( */ StartNotifierThread(); -#ifdef __CYGWIN__ - if (!tsdPtr->hwnd) { - WNDCLASS class; - - class.style = 0; - class.cbClsExtra = 0; - class.cbWndExtra = 0; - class.hInstance = TclWinGetTclInstance(); - class.hbrBackground = NULL; - class.lpszMenuName = NULL; - class.lpszClassName = L"TclNotifier"; - class.lpfnWndProc = NotifierProc; - class.hIcon = NULL; - class.hCursor = NULL; - - RegisterClassW(&class); - 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 */ - pthread_mutex_lock(¬ifierMutex); if (timePtr != NULL && timePtr->sec == 0 && (timePtr->usec == 0 @@ -1471,9 +1460,7 @@ AtForkChild(void) notifierCount = 0; if (notifierThreadRunning == 1) { -#ifndef __CYGWIN__ ThreadSpecificData *tsdPtr = TCL_TSD_INIT(&dataKey); -#endif notifierThreadRunning = 0; close(triggerPipe); @@ -1489,8 +1476,13 @@ AtForkChild(void) * The tsdPtr from before the fork is copied as well. But since * we are paranoic, we don't trust its condvar and reset it. */ -#ifndef __CYGWIN__ assert(tsdPtr->waitCVinitialized == 1); +#ifdef __CYGWIN__ + tsdPtr->hwnd = CreateWindowExW(NULL, NotfyClassName, + NotfyClassName, 0, 0, 0, 0, 0, NULL, NULL, + TclWinGetTclInstance(), NULL); + ResetEvent(tsdPtr->event); +#else pthread_cond_destroy(&tsdPtr->waitCV); pthread_cond_init(&tsdPtr->waitCV, NULL); #endif -- cgit v0.12 From 69d6754d432cdf6e463254245e8f10085cd1f70d Mon Sep 17 00:00:00 2001 From: "jan.nijtmans" Date: Thu, 3 Sep 2015 09:39:43 +0000 Subject: In StartNotifierThread() don't lock mutex if thread is already started. Fix panic message if thread cannot be started. Remove asserts used for debugging. --- unix/tclUnixNotfy.c | 72 ++++++++++++++++------------------------------------- 1 file changed, 22 insertions(+), 50 deletions(-) diff --git a/unix/tclUnixNotfy.c b/unix/tclUnixNotfy.c index c873774..2d5a560 100644 --- a/unix/tclUnixNotfy.c +++ b/unix/tclUnixNotfy.c @@ -17,7 +17,6 @@ #ifndef HAVE_COREFOUNDATION /* Darwin/Mac OS X CoreFoundation notifier is * in tclMacOSXNotify.c */ #include -#include /* * This structure is used to keep track of the notifier info for a registered @@ -279,29 +278,30 @@ static DWORD __stdcall NotifierProc(void *hwnd, unsigned int message, *---------------------------------------------------------------------- */ static void -StartNotifierThread(void) +StartNotifierThread(const char *proc) { - - pthread_mutex_lock(¬ifierInitMutex); if (!notifierThreadRunning) { - if (TclpThreadCreate(¬ifierThread, NotifierThreadProc, NULL, - TCL_THREAD_STACK_DEFAULT, TCL_THREAD_JOINABLE) != TCL_OK) { - Tcl_Panic("Tcl_InitNotifier: unable to start notifier thread"); - } + pthread_mutex_lock(¬ifierInitMutex); + if (!notifierThreadRunning) { + if (TclpThreadCreate(¬ifierThread, NotifierThreadProc, NULL, + TCL_THREAD_STACK_DEFAULT, TCL_THREAD_JOINABLE) != TCL_OK) { + Tcl_Panic("%s: unable to start notifier thread", proc); + } - pthread_mutex_lock(¬ifierMutex); - /* - * Wait for the notifier pipe to be created. - */ + pthread_mutex_lock(¬ifierMutex); + /* + * Wait for the notifier pipe to be created. + */ - while (triggerPipe < 0) { - pthread_cond_wait(¬ifierCV, ¬ifierMutex); - } - pthread_mutex_unlock(¬ifierMutex); + while (triggerPipe < 0) { + pthread_cond_wait(¬ifierCV, ¬ifierMutex); + } + pthread_mutex_unlock(¬ifierMutex); - notifierThreadRunning = 1; + notifierThreadRunning = 1; + } + pthread_mutex_unlock(¬ifierInitMutex); } - pthread_mutex_unlock(¬ifierInitMutex); } #endif /* TCL_THREADS */ @@ -358,7 +358,7 @@ Tcl_InitNotifier(void) 0 /* !signaled */, NULL); #else pthread_cond_init(&tsdPtr->waitCV, NULL); -#endif /* __CYGWIN */ +#endif /* __CYGWIN__ */ tsdPtr->waitCVinitialized = 1; } @@ -421,12 +421,6 @@ Tcl_FinalizeNotifier( notifierCount--; /* - * Check if FinializeNotifier was called without a prior InitNotifier - * in this thread. - */ - assert(tsdPtr->waitCVinitialized == 1); - - /* * If this is the last thread to use the notifier, close the notifier * pipe and wait for the background thread to terminate. */ @@ -574,7 +568,7 @@ Tcl_ServiceModeHook( return; } else if (mode == TCL_SERVICE_ALL) { #if TCL_THREADS - StartNotifierThread(); + StartNotifierThread("Tcl_ServiceModeHook"); #endif } } @@ -613,10 +607,6 @@ Tcl_CreateFileHandler( ThreadSpecificData *tsdPtr = TCL_TSD_INIT(&dataKey); FileHandler *filePtr; - /* - * Check if InitNotifier was called before in this thread - */ - assert(tsdPtr->waitCVinitialized == 1); for (filePtr = tsdPtr->firstFileHandlerPtr; filePtr != NULL; filePtr = filePtr->nextPtr) { if (filePtr->fd == fd) { @@ -689,11 +679,6 @@ Tcl_DeleteFileHandler( ThreadSpecificData *tsdPtr = TCL_TSD_INIT(&dataKey); /* - * Check if InitNotifier was called before in this thread - */ - assert(tsdPtr->waitCVinitialized == 1); - - /* * Find the entry for the given file (and return if there isn't one). */ @@ -798,11 +783,6 @@ FileHandlerEventProc( tsdPtr = TCL_TSD_INIT(&dataKey); - /* - * Check if InitNotifier was called before in this thread - */ - assert(tsdPtr->waitCVinitialized == 1); - for (filePtr = tsdPtr->firstFileHandlerPtr; filePtr != NULL; filePtr = filePtr->nextPtr) { if (filePtr->fd != fileEvPtr->fd) { @@ -902,11 +882,6 @@ Tcl_WaitForEvent( ThreadSpecificData *tsdPtr = TCL_TSD_INIT(&dataKey); /* - * Check if InitNotifier was called before in this thread - */ - assert(tsdPtr->waitCVinitialized == 1); - - /* * 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. @@ -949,7 +924,7 @@ Tcl_WaitForEvent( * interested threads, signal the notifier thread, and wait for a * response or a timeout. */ - StartNotifierThread(); + StartNotifierThread("Tcl_WaitForEvent"); pthread_mutex_lock(¬ifierMutex); @@ -1476,8 +1451,8 @@ AtForkChild(void) * The tsdPtr from before the fork is copied as well. But since * we are paranoic, we don't trust its condvar and reset it. */ - assert(tsdPtr->waitCVinitialized == 1); #ifdef __CYGWIN__ + DestroyWindow(tsdPtr->hwnd); tsdPtr->hwnd = CreateWindowExW(NULL, NotfyClassName, NotfyClassName, 0, 0, 0, 0, 0, NULL, NULL, TclWinGetTclInstance(), NULL); @@ -1492,9 +1467,6 @@ AtForkChild(void) */ } } - assert(notifierCount == 0); - assert(triggerPipe == -1); - assert(waitingListPtr == NULL); Tcl_InitNotifier(); } -- cgit v0.12 From 00e36faa85d26704fb289ee399cad3c810c0eaa2 Mon Sep 17 00:00:00 2001 From: "jan.nijtmans" Date: Mon, 7 Sep 2015 08:35:42 +0000 Subject: Fix for [5d170b5ca5] now available for widespread testing (incl. HPUX and OSX) --- unix/tclUnixNotfy.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/unix/tclUnixNotfy.c b/unix/tclUnixNotfy.c index f942329..90f478b 100644 --- a/unix/tclUnixNotfy.c +++ b/unix/tclUnixNotfy.c @@ -196,7 +196,7 @@ static Tcl_ThreadId notifierThread; #ifdef TCL_THREADS static void NotifierThreadProc(ClientData clientData); -#if defined(HAVE_PTHREAD_ATFORK) && !defined(__APPLE__) && !defined(__hpux) +#if defined(HAVE_PTHREAD_ATFORK) static int atForkInit = AT_FORK_INIT_VALUE; static void AtForkPrepare(void); static void AtForkParent(void); @@ -363,7 +363,7 @@ Tcl_InitNotifier(void) } pthread_mutex_lock(¬ifierInitMutex); -#if defined(HAVE_PTHREAD_ATFORK) && !defined(__APPLE__) && !defined(__hpux) +#if defined(HAVE_PTHREAD_ATFORK) /* * Install pthread_atfork handlers to clean up the notifier in the * child of a fork. @@ -1345,7 +1345,7 @@ NotifierThreadProc( TclpThreadExit(0); } -#if defined(HAVE_PTHREAD_ATFORK) && !defined(__APPLE__) && !defined(__hpux) +#if defined(HAVE_PTHREAD_ATFORK) /* *---------------------------------------------------------------------- * -- 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