From 70d68cb355c39d317d984605a623b9d242d0fdc7 Mon Sep 17 00:00:00 2001 From: pooryorick Date: Tue, 23 Apr 2019 11:29:16 +0000 Subject: Fix for [67a5eabbd3d1], refchan, coroutine, and postevent from the "watch" proc. --- generic/tclIORChan.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++------ tests/ioCmd.test | 53 +++++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 95 insertions(+), 14 deletions(-) diff --git a/generic/tclIORChan.c b/generic/tclIORChan.c index be82b48..8a7a16a 100644 --- a/generic/tclIORChan.c +++ b/generic/tclIORChan.c @@ -54,6 +54,8 @@ static int ReflectGetOption(ClientData clientData, static int ReflectSetOption(ClientData clientData, Tcl_Interp *interp, const char *optionName, const char *newValue); +static void TimerRunRead(ClientData clientData); +static void TimerRunWrite(ClientData clientData); /* * The C layer channel type/driver definition used by the reflection. This is @@ -112,6 +114,17 @@ typedef struct { int dead; /* Boolean signal that some operations * should no longer be attempted. */ + Tcl_TimerToken readTimer; /* + A token for the timer that is scheduled in + order to call Tcl_NotifyChannel when the + channel is readable + */ + Tcl_TimerToken writeTimer; /* + A token for the timer that is scheduled in + order to call Tcl_NotifyChannel when the + channel is writable + */ + /* * Note regarding the usage of timers. * @@ -121,11 +134,9 @@ typedef struct { * * See 'rechan', 'memchan', etc. * - * Here this is _not_ required. Interest in events is posted to the Tcl - * level via 'watch'. And posting of events is possible from the Tcl level - * as well, via 'chan postevent'. This means that the generation of all - * events, fake or not, timer based or not, is completely in the hands of - * the Tcl level. Therefore no timer here. + * A timer is used here as well in order to ensure at least on pass through + * the event loop when a channel becomes ready. See issues 67a5eabbd3d1 and + * ef28eb1f1516. */ } ReflectedChannel; @@ -920,7 +931,14 @@ TclChanPostEventObjCmd( #if TCL_THREADS if (rcPtr->owner == rcPtr->thread) { #endif - Tcl_NotifyChannel(chan, events); + if (events & TCL_READABLE) { + rcPtr->readTimer = Tcl_CreateTimerHandler(SYNTHETIC_EVENT_TIME, + TimerRunRead, rcPtr); + } + if (events & TCL_WRITABLE) { + rcPtr->writeTimer = Tcl_CreateTimerHandler(SYNTHETIC_EVENT_TIME, + TimerRunWrite, rcPtr); + } #if TCL_THREADS } else { ReflectEvent *ev = ckalloc(sizeof(ReflectEvent)); @@ -968,6 +986,24 @@ TclChanPostEventObjCmd( #undef EVENT } +static void +TimerRunRead( + ClientData clientData) +{ + ReflectedChannel *rcPtr = clientData; + rcPtr->readTimer = 0; + Tcl_NotifyChannel(rcPtr->chan, TCL_READABLE); +} + +static void +TimerRunWrite( + ClientData clientData) +{ + ReflectedChannel *rcPtr = clientData; + rcPtr->writeTimer = 0; + Tcl_NotifyChannel(rcPtr->chan, TCL_WRITABLE); +} + /* * Channel error message marshalling utilities. */ @@ -1161,6 +1197,12 @@ ReflectClose( ckfree(tctPtr); ((Channel *)rcPtr->chan)->typePtr = NULL; } + if (rcPtr->readTimer != NULL) { + Tcl_DeleteTimerHandler(rcPtr->readTimer); + } + if (rcPtr->writeTimer != NULL) { + Tcl_DeleteTimerHandler(rcPtr->writeTimer); + } Tcl_EventuallyFree(rcPtr, (Tcl_FreeProc *) FreeReflectedChannel); return EOK; } @@ -2131,6 +2173,8 @@ NewReflectedChannel( rcPtr->chan = NULL; rcPtr->interp = interp; rcPtr->dead = 0; + rcPtr->readTimer = 0; + rcPtr->writeTimer = 0; #if TCL_THREADS rcPtr->thread = Tcl_GetCurrentThread(); #endif diff --git a/tests/ioCmd.test b/tests/ioCmd.test index 9c93102..b15be21 100644 --- a/tests/ioCmd.test +++ b/tests/ioCmd.test @@ -930,6 +930,17 @@ proc onfinal {} { if {[lindex $hargs 0] ne "finalize"} {return} return -code return "" } + +proc onwatch {} { + upvar args hargs + lassign $hargs watch chan eventspec + if {$watch ne "watch"} return + foreach spec $eventspec { + chan postevent $chan $spec + } + return +} + } # Set everything up in the main thread. @@ -2002,28 +2013,29 @@ test iocmd-31.6 {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 readable {note TOCK}] - set stop [after 10000 {note TIMEOUT}] + set tock {} + note [fileevent $c readable {lappend res TOCK; set tock 1}] + set stop [after 10000 {lappend res TIMEOUT; set tock 1}] after 1000 {note [chan postevent $c r]} - vwait ::res + vwait ::tock 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}] + note [fileevent $c writable {lappend res TOCK; set tock 1}] + set stop [after 10000 {lappend res TIMEOUT; set tock 1}] after 1000 {note [chan postevent $c w]} - vwait ::res + vwait ::tock 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 } @@ -2036,6 +2048,31 @@ test iocmd-31.8 {chan postevent after close throws error} -match glob -setup { rename foo {} rename dummy {} } -returnCodes error -result {can not find reflected channel named "rc*"} +test iocmd-31.9 { + chan postevent + + call to current coroutine + + see 67a5eabbd3d1 +} -match glob -body { + set res {} + proc foo {args} {oninit; onwatch; onfinal; track; return} + set c [chan create {r w} foo] + after 0 [list ::apply [list c { + coroutine c1 ::apply [list c { + chan event $c readable [list [info coroutine]] + yield + set ::done READING + } [namespace current]] $c + } [namespace current]] $c] + set stop [after 10000 {set done TIMEOUT}] + vwait ::done + catch {after cancel $stop} + lappend res $done + close $c + rename foo {} + set res +} -result {{watch rc* read} READING {watch rc* {}}} # --- === *** ########################### # 'Pull the rug' tests. Create channel in a interpreter A, move to -- cgit v0.12 From 2a04ff4dd5c087cfb03656d828ed02be8ddac3d8 Mon Sep 17 00:00:00 2001 From: pooryorick Date: Tue, 23 Apr 2019 12:59:20 +0000 Subject: Ensure that Tcl_CreateTimerHandler is not called if there is an existing timer already scheduled. --- generic/tclIORChan.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/generic/tclIORChan.c b/generic/tclIORChan.c index 8a7a16a..477452b 100644 --- a/generic/tclIORChan.c +++ b/generic/tclIORChan.c @@ -932,12 +932,16 @@ TclChanPostEventObjCmd( if (rcPtr->owner == rcPtr->thread) { #endif if (events & TCL_READABLE) { + if (rcPtr->readTimer == NULL) { rcPtr->readTimer = Tcl_CreateTimerHandler(SYNTHETIC_EVENT_TIME, TimerRunRead, rcPtr); + } } if (events & TCL_WRITABLE) { + if (rcPtr->writeTimer == NULL) { rcPtr->writeTimer = Tcl_CreateTimerHandler(SYNTHETIC_EVENT_TIME, TimerRunWrite, rcPtr); + } } #if TCL_THREADS } else { @@ -991,7 +995,7 @@ TimerRunRead( ClientData clientData) { ReflectedChannel *rcPtr = clientData; - rcPtr->readTimer = 0; + rcPtr->readTimer = NULL; Tcl_NotifyChannel(rcPtr->chan, TCL_READABLE); } @@ -1000,7 +1004,7 @@ TimerRunWrite( ClientData clientData) { ReflectedChannel *rcPtr = clientData; - rcPtr->writeTimer = 0; + rcPtr->writeTimer = NULL; Tcl_NotifyChannel(rcPtr->chan, TCL_WRITABLE); } -- cgit v0.12 From a0465a4be2fa1aa32512bfe1671d7bd50754031a Mon Sep 17 00:00:00 2001 From: pooryorick Date: Wed, 24 Apr 2019 04:04:46 +0000 Subject: Add missed timer cleanup in tclIORChan.c/ReflectClose. --- generic/tclIORChan.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/generic/tclIORChan.c b/generic/tclIORChan.c index 477452b..cebc33f 100644 --- a/generic/tclIORChan.c +++ b/generic/tclIORChan.c @@ -1276,6 +1276,12 @@ ReflectClose( ckfree(tctPtr); ((Channel *)rcPtr->chan)->typePtr = NULL; } + if (rcPtr->readTimer != NULL) { + Tcl_DeleteTimerHandler(rcPtr->readTimer); + } + if (rcPtr->writeTimer != NULL) { + Tcl_DeleteTimerHandler(rcPtr->writeTimer); + } Tcl_EventuallyFree(rcPtr, (Tcl_FreeProc *) FreeReflectedChannel); return (result == TCL_OK) ? EOK : EINVAL; } -- cgit v0.12