Many hyperlinks are disabled.
Use anonymous login
to enable hyperlinks.
Overview
Comment: | Reverted changes to refchan event generation - details below. Tx to Jan for actual revert work.
See https://core.tcl-lang.org/tcl/wiki?name=Rationale+for+rollback+of+refchan+event+generation+in+core Revert tested on Windows/VC++, Linux w/valgrind, twapi tls, tcllib virtual channels, iocp channels. Revert approved by multiple TCT members. The following bugs impacted: 67a5eabb de232b49 080f846f ac7592e7 |
---|---|
Downloads: | Tarball | ZIP archive |
Timelines: | family | ancestors | descendants | both | trunk | main |
Files: | files | file ages | folders |
SHA3-256: |
1ec9927351b255fb981abbfa7605641d |
User & Date: | apnadkarni 2024-05-29 09:07:30.732 |
Context
2024-05-29
| ||
09:45 | Merge-mark 8.7 check-in: 95f9e8176e user: jan.nijtmans tags: trunk, main | |
09:07 |
Reverted changes to refchan event generation - details below. Tx to Jan for actual revert work.
See... check-in: 1ec9927351 user: apnadkarni tags: trunk, main | |
08:37 | Merge trunk Closed-Leaf check-in: 0258b07434 user: apnadkarni tags: bug-18f4a94d03 | |
2024-05-28
| ||
13:24 | Merge 8.7 check-in: 9fa0318dcd user: jan.nijtmans tags: trunk, main | |
Changes
Changes to generic/tclIO.c.
︙ | ︙ | |||
164 165 166 167 168 169 170 | static int CheckChannelErrors(ChannelState *statePtr, int direction); static int CheckForDeadChannel(Tcl_Interp *interp, ChannelState *statePtr); static void CheckForStdChannelsBeingClosed(Tcl_Channel chan); static void CleanupChannelHandlers(Tcl_Interp *interp, Channel *chanPtr); | < | 164 165 166 167 168 169 170 171 172 173 174 175 176 177 | static int CheckChannelErrors(ChannelState *statePtr, int direction); static int CheckForDeadChannel(Tcl_Interp *interp, ChannelState *statePtr); static void CheckForStdChannelsBeingClosed(Tcl_Channel chan); static void CleanupChannelHandlers(Tcl_Interp *interp, Channel *chanPtr); static int CloseChannel(Tcl_Interp *interp, Channel *chanPtr, int errorCode); static int CloseChannelPart(Tcl_Interp *interp, Channel *chanPtr, int errorCode, int flags); static int CloseWrite(Tcl_Interp *interp, Channel *chanPtr); static void CommonGetsCleanup(Channel *chanPtr); static int CopyData(CopyState *csPtr, int mask); |
︙ | ︙ | |||
3529 3530 3531 3532 3533 3534 3535 | TclDecrRefCount(statePtr->chanMsg); statePtr->chanMsg = NULL; } } Tcl_ClearChannelHandlers(chan); | < < < < < | 3528 3529 3530 3531 3532 3533 3534 3535 3536 3537 3538 3539 3540 3541 | TclDecrRefCount(statePtr->chanMsg); statePtr->chanMsg = NULL; } } Tcl_ClearChannelHandlers(chan); /* * Invoke the registered close callbacks and delete their records. */ while (statePtr->closeCbPtr != NULL) { cbPtr = statePtr->closeCbPtr; statePtr->closeCbPtr = cbPtr->nextPtr; |
︙ | ︙ | |||
8660 8661 8662 8663 8664 8665 8666 | static void UpdateInterest( Channel *chanPtr) /* Channel to update. */ { ChannelState *statePtr = chanPtr->state; /* State info for channel */ | < | 8654 8655 8656 8657 8658 8659 8660 8661 8662 8663 8664 8665 8666 8667 | static void UpdateInterest( Channel *chanPtr) /* Channel to update. */ { ChannelState *statePtr = chanPtr->state; /* State info for channel */ int mask = statePtr->interestMask; if (chanPtr->typePtr == NULL) { /* Do not update interest on a closed channel */ return; } |
︙ | ︙ | |||
8738 8739 8740 8741 8742 8743 8744 | TclChannelPreserve((Tcl_Channel)chanPtr); statePtr->timerChanPtr = chanPtr; statePtr->timer = Tcl_CreateTimerHandler(SYNTHETIC_EVENT_TIME, ChannelTimerProc, chanPtr); } } } | < < < < < < < < < < < < < | 8731 8732 8733 8734 8735 8736 8737 8738 8739 8740 8741 8742 8743 8744 | TclChannelPreserve((Tcl_Channel)chanPtr); statePtr->timerChanPtr = chanPtr; statePtr->timer = Tcl_CreateTimerHandler(SYNTHETIC_EVENT_TIME, ChannelTimerProc, chanPtr); } } } ChanWatch(chanPtr, mask); } /* *---------------------------------------------------------------------- * * ChannelTimerProc -- |
︙ | ︙ | |||
8779 8780 8781 8782 8783 8784 8785 | ChannelTimerProc( void *clientData) { Channel *chanPtr = (Channel *)clientData; /* State info for channel */ ChannelState *statePtr = chanPtr->state; | < < < < < < | > > | < | | | < < < < < < < < < < < < | < < | > | | | < < > | | < < < < < < < | < | < | > | 8759 8760 8761 8762 8763 8764 8765 8766 8767 8768 8769 8770 8771 8772 8773 8774 8775 8776 8777 8778 8779 8780 8781 8782 8783 8784 8785 8786 8787 8788 8789 8790 8791 8792 8793 8794 8795 8796 8797 8798 8799 8800 8801 8802 8803 8804 8805 8806 8807 8808 8809 | ChannelTimerProc( void *clientData) { Channel *chanPtr = (Channel *)clientData; /* State info for channel */ ChannelState *statePtr = chanPtr->state; if (chanPtr->typePtr == NULL) { statePtr->timer = NULL; TclChannelRelease((Tcl_Channel)statePtr->timerChanPtr); statePtr->timerChanPtr = NULL; } else { if (!GotFlag(statePtr, CHANNEL_NEED_MORE_DATA) && (statePtr->interestMask & TCL_READABLE) && (statePtr->inQueueHead != NULL) && IsBufferReady(statePtr->inQueueHead)) { /* * Restart the timer in case a channel handler reenters the event loop * before UpdateInterest gets called by Tcl_NotifyChannel. */ statePtr->timer = Tcl_CreateTimerHandler(SYNTHETIC_EVENT_TIME, ChannelTimerProc,chanPtr); Tcl_Preserve(statePtr); Tcl_NotifyChannel((Tcl_Channel) chanPtr, TCL_READABLE); Tcl_Release(statePtr); } else { statePtr->timer = NULL; UpdateInterest(chanPtr); TclChannelRelease((Tcl_Channel)statePtr->timerChanPtr); statePtr->timerChanPtr = NULL; } } } static void DeleteTimerHandler( ChannelState *statePtr) { if (statePtr->timer != NULL) { Tcl_DeleteTimerHandler(statePtr->timer); statePtr->timer = NULL; TclChannelRelease((Tcl_Channel)statePtr->timerChanPtr); statePtr->timerChanPtr = NULL; } } /* *---------------------------------------------------------------------- * * Tcl_CreateChannelHandler -- * |
︙ | ︙ |
Changes to generic/tclIORChan.c.
︙ | ︙ | |||
50 51 52 53 54 55 56 | Tcl_Interp *interp, const char *optionName, Tcl_DString *dsPtr); static int ReflectSetOption(void *clientData, Tcl_Interp *interp, const char *optionName, const char *newValue); static int ReflectTruncate(void *clientData, long long length); | < < | 50 51 52 53 54 55 56 57 58 59 60 61 62 63 | Tcl_Interp *interp, const char *optionName, Tcl_DString *dsPtr); static int ReflectSetOption(void *clientData, Tcl_Interp *interp, const char *optionName, const char *newValue); static int ReflectTruncate(void *clientData, long long length); /* * The C layer channel type/driver definition used by the reflection. */ static const Tcl_ChannelType tclRChannelType = { "tclrchannel", /* Type name. */ |
︙ | ︙ | |||
108 109 110 111 112 113 114 | int mode; /* Mask of R/W mode */ int interest; /* Mask of events the channel is interested * in. */ int dead; /* Boolean signal that some operations * should no longer be attempted. */ | < < < < < < < > > | > | < | 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 | int mode; /* Mask of R/W mode */ int interest; /* Mask of events the channel is interested * in. */ int dead; /* Boolean signal that some operations * should no longer be attempted. */ /* * Note regarding the usage of timers. * * Most channel implementations need a timer in the C level to ensure that * data in buffers is flushed out through the generation of fake file * events. * * See 'refchan', '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. */ } ReflectedChannel; /* * Structure of the table mapping from channel handles to reflected * channels. Each interpreter which has the handler command for one or more * reflected channels records them in such a table, so that 'chan postevent' |
︙ | ︙ | |||
940 941 942 943 944 945 946 | /* * We have the channel and the events to post. */ #if TCL_THREADS if (rcPtr->owner == rcPtr->thread) { #endif | | < < < < < < < < < < < | 933 934 935 936 937 938 939 940 941 942 943 944 945 946 947 | /* * We have the channel and the events to post. */ #if TCL_THREADS if (rcPtr->owner == rcPtr->thread) { #endif Tcl_NotifyChannel(chan, events); #if TCL_THREADS } else { ReflectEvent *ev = (ReflectEvent *)Tcl_Alloc(sizeof(ReflectEvent)); ev->header.proc = ReflectEventRun; ev->events = events; ev->rcPtr = rcPtr; |
︙ | ︙ | |||
998 999 1000 1001 1002 1003 1004 | Tcl_ResetResult(interp); return TCL_OK; #undef CHAN #undef EVENT } | < < < < < < < < < < < < < < < < < < | 980 981 982 983 984 985 986 987 988 989 990 991 992 993 | Tcl_ResetResult(interp); return TCL_OK; #undef CHAN #undef EVENT } /* * Channel error message marshalling utilities. */ static Tcl_Obj * MarshallError( |
︙ | ︙ | |||
1215 1216 1217 1218 1219 1220 1221 | #endif tctPtr = ((Channel *)rcPtr->chan)->typePtr; if (tctPtr && tctPtr != &tclRChannelType) { Tcl_Free((void *)tctPtr); ((Channel *)rcPtr->chan)->typePtr = NULL; } | < < < < < < | 1179 1180 1181 1182 1183 1184 1185 1186 1187 1188 1189 1190 1191 1192 | #endif tctPtr = ((Channel *)rcPtr->chan)->typePtr; if (tctPtr && tctPtr != &tclRChannelType) { Tcl_Free((void *)tctPtr); ((Channel *)rcPtr->chan)->typePtr = NULL; } Tcl_EventuallyFree(rcPtr, FreeReflectedChannel); return EOK; } /* * Are we in the correct thread? */ |
︙ | ︙ | |||
1290 1291 1292 1293 1294 1295 1296 | } #endif tctPtr = ((Channel *)rcPtr->chan)->typePtr; if (tctPtr && tctPtr != &tclRChannelType) { Tcl_Free((void *)tctPtr); ((Channel *)rcPtr->chan)->typePtr = NULL; } | < < < < < < | 1248 1249 1250 1251 1252 1253 1254 1255 1256 1257 1258 1259 1260 1261 | } #endif tctPtr = ((Channel *)rcPtr->chan)->typePtr; if (tctPtr && tctPtr != &tclRChannelType) { Tcl_Free((void *)tctPtr); ((Channel *)rcPtr->chan)->typePtr = NULL; } Tcl_EventuallyFree(rcPtr, FreeReflectedChannel); return (result == TCL_OK) ? EOK : EINVAL; } /* *---------------------------------------------------------------------- * |
︙ | ︙ | |||
2244 2245 2246 2247 2248 2249 2250 | rcPtr = (ReflectedChannel *)Tcl_Alloc(sizeof(ReflectedChannel)); /* rcPtr->chan: Assigned by caller. Dummy data here. */ rcPtr->chan = NULL; rcPtr->interp = interp; rcPtr->dead = 0; | < < | 2196 2197 2198 2199 2200 2201 2202 2203 2204 2205 2206 2207 2208 2209 | rcPtr = (ReflectedChannel *)Tcl_Alloc(sizeof(ReflectedChannel)); /* rcPtr->chan: Assigned by caller. Dummy data here. */ rcPtr->chan = NULL; rcPtr->interp = interp; rcPtr->dead = 0; #if TCL_THREADS rcPtr->thread = Tcl_GetCurrentThread(); #endif rcPtr->mode = mode; rcPtr->interest = 0; /* Initially no interest registered */ rcPtr->cmd = TclListObjCopy(NULL, cmdpfxObj); |
︙ | ︙ |
Changes to tests/io.test.
︙ | ︙ | |||
2478 2479 2480 2481 2482 2483 2484 2485 2486 2487 2488 2489 2490 2491 | close channel in write event handler Should not produce a segmentation fault in a Tcl built with --enable-symbols and -DPURIFY } -body { variable done variable res after 0 [list coroutine c1 apply [list {} { variable done set chan [chan create w {apply {{cmd chan args} { switch $cmd { blocking - finalize { } watch { | > > | > > > > | > > | 2478 2479 2480 2481 2482 2483 2484 2485 2486 2487 2488 2489 2490 2491 2492 2493 2494 2495 2496 2497 2498 2499 2500 2501 2502 2503 2504 2505 2506 2507 2508 2509 2510 2511 2512 2513 2514 2515 2516 2517 2518 2519 2520 2521 2522 2523 2524 2525 2526 2527 2528 2529 2530 2531 2532 2533 2534 2535 2536 2537 2538 2539 2540 2541 2542 2543 2544 2545 2546 2547 2548 2549 2550 2551 2552 2553 2554 2555 2556 2557 2558 2559 2560 2561 2562 2563 2564 2565 | close channel in write event handler Should not produce a segmentation fault in a Tcl built with --enable-symbols and -DPURIFY } -body { variable done variable res # Not a complete / correct channel implementation. Just enough # to exercise the crash - closing from a write handler after 0 [list coroutine c1 apply [list {} { variable done set chan [chan create w {apply {{cmd chan args} { switch $cmd { blocking - finalize { } watch { lappend ::timers286 [after 0 chan postevent $chan write] } initialize { list initialize finalize watch read write configure blocking } default { error [list {unexpected command} $cmd] } } }}}] chan configure $chan -blocking 0 while 1 { chan event $chan writable [list [info coroutine]] yield close $chan set done 1 return } } [namespace current]]] vwait [namespace current]::done return success } -cleanup { foreach timer $::timers286 {after cancel $timer} } -result success test io-28.7 { close channel in read event handler Should not produce a segmentation fault in a Tcl built with --enable-symbols and -DPURIFY } -body { variable done variable res after 0 [list coroutine c1 apply [list {} { variable done # Not a complete / correct channel implementation. Just enough # to exercise the crash - closing from a read handler set chan [chan create r {apply {{cmd chan args} { switch $cmd { blocking - finalize { } watch { lappend ::timers287 [after 0 chan postevent $chan read] } initialize { list initialize finalize watch read write configure blocking } default { error [list {unexpected command} $cmd] } } }}}] chan configure $chan -blocking 0 while 1 { chan event $chan readable [list [info coroutine]] yield close $chan set done 1 return } } [namespace current]]] vwait [namespace current]::done return success } -cleanup { foreach timer $::timers287 {after cancel $timer} } -result success test io-29.1 {Tcl_WriteChars, channel not writable} { list [catch {puts stdin hello} msg] $msg } {1 {channel "stdin" wasn't opened for writing}} test io-29.2 {Tcl_WriteChars, empty string} { file delete $path(test1) |
︙ | ︙ | |||
6241 6242 6243 6244 6245 6246 6247 6248 | set x } -cleanup { close $f4 } -result {initial foo eof} close $f test io-44.6 {FileEventProc procedure: write-only non-blocking channel} -setup { | > > > | | > > > | < > > > | > > | > > > > > > > > > > > > > > > > | < | > > | > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > | 6249 6250 6251 6252 6253 6254 6255 6256 6257 6258 6259 6260 6261 6262 6263 6264 6265 6266 6267 6268 6269 6270 6271 6272 6273 6274 6275 6276 6277 6278 6279 6280 6281 6282 6283 6284 6285 6286 6287 6288 6289 6290 6291 6292 6293 6294 6295 6296 6297 6298 6299 6300 6301 6302 6303 6304 6305 6306 6307 6308 6309 6310 6311 6312 6313 6314 6315 6316 6317 6318 6319 6320 6321 6322 6323 6324 6325 6326 6327 6328 6329 6330 6331 6332 6333 6334 6335 6336 6337 6338 6339 6340 6341 6342 6343 6344 6345 6346 6347 6348 6349 6350 6351 6352 6353 6354 6355 6356 6357 6358 6359 6360 6361 6362 6363 6364 6365 6366 6367 6368 6369 6370 6371 6372 6373 6374 6375 6376 6377 6378 6379 6380 6381 6382 6383 6384 6385 6386 6387 6388 6389 6390 6391 6392 6393 | set x } -cleanup { close $f4 } -result {initial foo eof} close $f # Bug https://core.tcl-lang.org/tcl/info/de232b49f26da1c1 with a corrected # refchan implementation. refchans should be responsible for their own # event generation and the one in the bug report was not doing so. test io-44.6 {FileEventProc procedure: write-only non-blocking channel} -setup { } -constraints {stdio fileevent} -body { namespace eval refchan { namespace ensemble create namespace export * # Change to taste depending on how much CPU you want to hog variable delay 0 proc finalize {chan args} { namespace upvar c_$chan timer timer catch {after cancel $timer} namespace delete c_$chan } proc initialize {chan args} { namespace eval c_$chan {} namespace upvar c_$chan watching watching timer timer set watching {} list finalize initialize seek watch write } proc watch {chan args} { namespace upvar c_$chan watching watching foreach arg $args { switch $arg { write { if {$arg ni $watching} { lappend watching $arg } } } } update $chan } proc write {chan args} { return 1 } # paraphrased from tcllib proc update {chan} { namespace upvar c_$chan watching watching timer timer variable delay catch {after cancel $timer} if {"write" in $watching} { set timer [after idle after $delay \ [namespace code [list post $chan]]] } } # paraphrased from tcllib proc post {chan} { variable delay namespace upvar c_$chan watching watching timer timer if {"write" in $watching} { set timer [after idle after $delay \ [namespace code [list post $chan]]] chan postevent $chan write } } } set f [chan create w [namespace which refchan]] chan configure $f -blocking 0 set data "some data" set x 0 chan event $f writable [namespace code { puts $f $data incr count [string length $data] if {$count > 262144} { chan event $f writable {} set x done } }] # Note: timeout needs to be very long under valgrind set token [after 240000 [namespace code { set x timeout }]] vwait [namespace which -variable x] return $x } -cleanup { after cancel $token catch {chan close $f} } -result done # Bug https://core.tcl-lang.org/tcl/info/67a5eabbd3d1 with a corrected # refchan implementation. refchans that are not reentrant should use # event loop to post events and the script in the bug report was not # doing so. test io-44.7 {refchan + coroutine yield error } -setup { set bghandler [interp bgerror {}] namespace eval schan { namespace ensemble create namespace export * proc open {} { set chan [chan create read [namespace current]] } proc initialize {chan mode} { return [list initialize finalize read watch] } proc finalize args {} proc read {chan count} {} proc watch {chan eventspec} { foreach event $eventspec { after idle after 0 chan postevent $chan $event } } } } -cleanup { interp bgerror {} $bghandler unset -nocomplain ::io-44.7-result namespace delete schan } -body { interp bgerror {} [list apply {{res opts} { set ::io-44.7-result [dict get $opts -errorinfo] }}] coroutine c1 apply [list {} { set chan [schan::open] chan event $chan readable [list [info coroutine]] yield close $chan set ::io-44.7-result success } [namespace current]] vwait ::io-44.7-result set ::io-44.7-result } -result success makeFile "foo bar" foo test io-45.1 {DeleteFileEvent, cleanup on close} {fileevent} { set f [open $path(foo) r] fileevent $f readable [namespace code { lappend x "binding triggered: \"[gets $f]\"" |
︙ | ︙ |
Changes to tests/ioCmd.test.
︙ | ︙ | |||
967 968 969 970 971 972 973 | return -code return $args } proc onfinal {} { upvar args hargs if {[lindex $hargs 0] ne "finalize"} {return} return -code return "" } | < < < < < < < < < < < | 967 968 969 970 971 972 973 974 975 976 977 978 979 980 | return -code return $args } proc onfinal {} { upvar args hargs if {[lindex $hargs 0] ne "finalize"} {return} return -code return "" } } # Set everything up in the main thread. eval $helperscript # --- --- --- --------- --------- --------- # method finalize |
︙ | ︙ | |||
2059 2060 2061 2062 2063 2064 2065 | set stop [after 15000 {lappend res TIMEOUT; set tock 1}] after 1000 {note [chan postevent $c r]} vwait ::tock catch {after cancel $stop} close $c rename foo {} set res | | | < < < < < < < < < < < < < < < < < < < < < < < < < | 2048 2049 2050 2051 2052 2053 2054 2055 2056 2057 2058 2059 2060 2061 2062 2063 2064 2065 2066 2067 2068 2069 2070 2071 2072 2073 2074 2075 2076 2077 2078 2079 2080 2081 2082 2083 2084 2085 2086 2087 | set stop [after 15000 {lappend res TIMEOUT; set tock 1}] after 1000 {note [chan postevent $c r]} vwait ::tock catch {after cancel $stop} close $c rename foo {} set res } -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 {lappend res TOCK; set tock 1}] set stop [after 15000 {lappend res TIMEOUT; set tock 1}] after 1000 {note [chan postevent $c w]} vwait ::tock catch {after cancel $stop} close $c rename foo {} set res } -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 } set c [chan create {r w} foo] fileevent $c readable dummy } -body { close $c chan postevent $c read } -cleanup { rename foo {} rename dummy {} } -returnCodes error -result {can not find reflected channel named "rc*"} # --- === *** ########################### # 'Pull the rug' tests. Create channel in a interpreter A, move to # other interpreter B, destroy the origin interpreter (A) before or # during access from B. Must not crash, must return proper errors. test iocmd-32.0 {origin interpreter of moved channel gone} -match glob -body { |
︙ | ︙ |