Many hyperlinks are disabled.
Use anonymous login
to enable hyperlinks.
Overview
Comment: | closes [79474c58800cdf94]: fixes 2 segfaults and 2 leaks (common IO handlers and reflected channels) |
---|---|
Downloads: | Tarball | ZIP archive | SQL archive |
Timelines: | family | ancestors | descendants | both | core-8-6-branch |
Files: | files | file ages | folders |
SHA3-256: |
4342b27a4be98a74ac9f1b57a5cb2b79 |
User & Date: | sebres 2024-05-21 09:45:37 |
Context
2024-05-21
| ||
20:19 | "TCL_TOMMATH" is not used anywhere check-in: 4c1393b596 user: jan.nijtmans tags: core-8-6-branch | |
09:48 | merge 8.6 check-in: 5be1c6c979 user: sebres tags: core-8-branch | |
09:45 | closes [79474c58800cdf94]: fixes 2 segfaults and 2 leaks (common IO handlers and reflected channels) check-in: 4342b27a4b user: sebres tags: core-8-6-branch | |
2024-05-20
| ||
15:18 | Fix [7842f33a5c]: Call chain creation could crash in destructors in some tangled cases check-in: 8b4a3295ed user: dkf tags: core-8-6-branch | |
2024-05-17
| ||
13:21 | split iocmd-32.3 in two tests (move cycle outside of the test) Closed-Leaf check-in: b055b41a3b user: sebres tags: fix-79474c58800cdf94 | |
Changes
Changes to generic/tclIO.c.
︙ | ︙ | |||
93 94 95 96 97 98 99 100 101 102 103 104 105 106 | * copy. Note that the data buffer for the copy will be appended to this * structure. */ typedef struct CopyState { struct Channel *readPtr; /* Pointer to input channel. */ struct Channel *writePtr; /* Pointer to output channel. */ int readFlags; /* Original read channel flags. */ int writeFlags; /* Original write channel flags. */ Tcl_WideInt toRead; /* Number of bytes to copy, or -1. */ 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 bufSize; /* Size of appended buffer. */ | > | 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 | * copy. Note that the data buffer for the copy will be appended to this * structure. */ typedef struct CopyState { struct Channel *readPtr; /* Pointer to input channel. */ struct Channel *writePtr; /* Pointer to output channel. */ int refCount; /* Reference counter. */ int readFlags; /* Original read channel flags. */ int writeFlags; /* Original write channel flags. */ Tcl_WideInt toRead; /* Number of bytes to copy, or -1. */ 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 bufSize; /* Size of appended buffer. */ |
︙ | ︙ | |||
215 216 217 218 219 220 221 222 223 224 225 226 227 228 | int charsLeft, int *factorPtr); static void RecycleBuffer(ChannelState *statePtr, ChannelBuffer *bufPtr, int mustDiscard); static int StackSetBlockMode(Channel *chanPtr, int mode); static int SetBlockMode(Tcl_Interp *interp, Channel *chanPtr, int mode); static void StopCopy(CopyState *csPtr); static void TranslateInputEOL(ChannelState *statePtr, char *dst, const char *src, int *dstLenPtr, int *srcLenPtr); static void UpdateInterest(Channel *chanPtr); static int Write(Channel *chanPtr, const char *src, int srcLen, Tcl_Encoding encoding); static Tcl_Obj * FixLevelCode(Tcl_Obj *msg); static void SpliceChannel(Tcl_Channel chan); | > | 216 217 218 219 220 221 222 223 224 225 226 227 228 229 230 | int charsLeft, int *factorPtr); static void RecycleBuffer(ChannelState *statePtr, ChannelBuffer *bufPtr, int mustDiscard); static int StackSetBlockMode(Channel *chanPtr, int mode); static int SetBlockMode(Tcl_Interp *interp, Channel *chanPtr, int mode); static void StopCopy(CopyState *csPtr); static void CopyDecrRefCount(CopyState *csPtr); static void TranslateInputEOL(ChannelState *statePtr, char *dst, const char *src, int *dstLenPtr, int *srcLenPtr); static void UpdateInterest(Channel *chanPtr); static int Write(Channel *chanPtr, const char *src, int srcLen, Tcl_Encoding encoding); static Tcl_Obj * FixLevelCode(Tcl_Obj *msg); static void SpliceChannel(Tcl_Channel chan); |
︙ | ︙ | |||
2077 2078 2079 2080 2081 2082 2083 | Tcl_SetObjResult(interp, Tcl_ObjPrintf( "could not flush channel \"%s\"", Tcl_GetChannelName((Tcl_Channel) chanPtr))); } return TCL_ERROR; } | | | 2079 2080 2081 2082 2083 2084 2085 2086 2087 2088 2089 2090 2091 2092 2093 | Tcl_SetObjResult(interp, Tcl_ObjPrintf( "could not flush channel \"%s\"", Tcl_GetChannelName((Tcl_Channel) chanPtr))); } return TCL_ERROR; } statePtr->csPtrR = csPtrR; statePtr->csPtrW = csPtrW; } /* * Anything in the input queue and the push-back buffers of the * transformation going away is transformed data, but not yet read. As * unstacking means that the caller does not want to see transformed |
︙ | ︙ | |||
2942 2943 2944 2945 2946 2947 2948 2949 2950 2951 2952 2953 2954 2955 | goto done; } done: TclChannelRelease((Tcl_Channel)chanPtr); return errorCode; } /* *---------------------------------------------------------------------- * * CloseChannel -- * * Utility procedure to close a channel and free associated resources. | > > > > > > > > > > > > > > > > > > > > > > > > > > > > | 2944 2945 2946 2947 2948 2949 2950 2951 2952 2953 2954 2955 2956 2957 2958 2959 2960 2961 2962 2963 2964 2965 2966 2967 2968 2969 2970 2971 2972 2973 2974 2975 2976 2977 2978 2979 2980 2981 2982 2983 2984 2985 | goto done; } done: TclChannelRelease((Tcl_Channel)chanPtr); return errorCode; } static void FreeChannelState( char *blockPtr) /* Channel state to free. */ { ChannelState *statePtr = (ChannelState *)blockPtr; /* * Even after close some members can be filled again (in events etc). * Test in bug [79474c588] illustrates one leak (on remaining chanMsg). * Possible other fields need freeing on some constellations. */ DiscardInputQueued(statePtr, 1); if (statePtr->curOutPtr != NULL) { ReleaseChannelBuffer(statePtr->curOutPtr); } DiscardOutputQueued(statePtr); DeleteTimerHandler(statePtr); if (statePtr->chanMsg) { Tcl_DecrRefCount(statePtr->chanMsg); } if (statePtr->unreportedMsg) { Tcl_DecrRefCount(statePtr->unreportedMsg); } ckfree(statePtr); } /* *---------------------------------------------------------------------- * * CloseChannel -- * * Utility procedure to close a channel and free associated resources. |
︙ | ︙ | |||
3119 3120 3121 3122 3123 3124 3125 | * There is only the TOP Channel, so we free the remaining pointers we * have and then ourselves. Since this is the last of the channels in the * stack, make sure to free the ChannelState structure associated with it. */ ChannelFree(chanPtr); | | | 3149 3150 3151 3152 3153 3154 3155 3156 3157 3158 3159 3160 3161 3162 3163 | * There is only the TOP Channel, so we free the remaining pointers we * have and then ourselves. Since this is the last of the channels in the * stack, make sure to free the ChannelState structure associated with it. */ ChannelFree(chanPtr); Tcl_EventuallyFree(statePtr, FreeChannelState); return errorCode; } /* *---------------------------------------------------------------------- * |
︙ | ︙ | |||
3937 3938 3939 3940 3941 3942 3943 | } statePtr->chPtr = NULL; /* * Cancel any pending copy operation. */ | > | > > > | > > | 3967 3968 3969 3970 3971 3972 3973 3974 3975 3976 3977 3978 3979 3980 3981 3982 3983 3984 3985 3986 3987 3988 | } statePtr->chPtr = NULL; /* * Cancel any pending copy operation. */ if (statePtr->csPtrR) { StopCopy(statePtr->csPtrR); statePtr->csPtrR = NULL; } if (statePtr->csPtrW) { StopCopy(statePtr->csPtrW); statePtr->csPtrW = NULL; } /* * Must set the interest mask now to 0, otherwise infinite loops will * occur if Tcl_DoOneEvent is called before the channel is finally deleted * in FlushChannel. This can happen if the channel has a background flush * active. */ |
︙ | ︙ | |||
9244 9245 9246 9247 9248 9249 9250 9251 9252 9253 9254 9255 9256 9257 9258 9259 9260 | * completed. */ csPtr = (CopyState *)ckalloc(TclOffset(CopyState, buffer) + 1U + !moveBytes * inStatePtr->bufSize); csPtr->bufSize = !moveBytes * inStatePtr->bufSize; csPtr->readPtr = inPtr; csPtr->writePtr = outPtr; csPtr->readFlags = readFlags; csPtr->writeFlags = writeFlags; csPtr->toRead = toRead; csPtr->total = (Tcl_WideInt) 0; csPtr->interp = interp; if (cmdPtr) { Tcl_IncrRefCount(cmdPtr); } csPtr->cmdPtr = cmdPtr; | > > > > | | | 9280 9281 9282 9283 9284 9285 9286 9287 9288 9289 9290 9291 9292 9293 9294 9295 9296 9297 9298 9299 9300 9301 9302 9303 9304 9305 9306 9307 9308 9309 9310 9311 9312 9313 9314 9315 9316 9317 9318 9319 9320 9321 9322 | * completed. */ csPtr = (CopyState *)ckalloc(TclOffset(CopyState, buffer) + 1U + !moveBytes * inStatePtr->bufSize); csPtr->bufSize = !moveBytes * inStatePtr->bufSize; csPtr->readPtr = inPtr; csPtr->writePtr = outPtr; csPtr->refCount = 2; /* two references below (inStatePtr, outStatePtr) */ csPtr->readFlags = readFlags; csPtr->writeFlags = writeFlags; csPtr->toRead = toRead; csPtr->total = (Tcl_WideInt) 0; csPtr->interp = interp; if (cmdPtr) { Tcl_IncrRefCount(cmdPtr); } csPtr->cmdPtr = cmdPtr; TclChannelPreserve(inChan); TclChannelPreserve(outChan); inStatePtr->csPtrR = csPtr; outStatePtr->csPtrW = csPtr; if (moveBytes) { return MoveBytes(csPtr); } /* * Special handling of -size 0 async transfers, so that the -command is * still called asynchronously. */ if ((nonBlocking == CHANNEL_NONBLOCKING) && (toRead == 0)) { Tcl_CreateTimerHandler(0, ZeroTransferTimerProc, csPtr); return TCL_OK; } /* * Start copying data between the channels. */ return CopyData(csPtr, 0); |
︙ | ︙ | |||
9543 9544 9545 9546 9547 9548 9549 9550 9551 9552 9553 9554 9555 9556 | Tcl_WideInt total; int size; const char *buffer; int inBinary, outBinary, sameEncoding; /* Encoding control */ int underflow; /* Input underflow */ inChan = (Tcl_Channel) csPtr->readPtr; outChan = (Tcl_Channel) csPtr->writePtr; inStatePtr = csPtr->readPtr->state; outStatePtr = csPtr->writePtr->state; interp = csPtr->interp; cmdPtr = csPtr->cmdPtr; | > > | 9583 9584 9585 9586 9587 9588 9589 9590 9591 9592 9593 9594 9595 9596 9597 9598 | Tcl_WideInt total; int size; const char *buffer; int inBinary, outBinary, sameEncoding; /* Encoding control */ int underflow; /* Input underflow */ csPtr->refCount++; /* avoid freeing during handling */ inChan = (Tcl_Channel) csPtr->readPtr; outChan = (Tcl_Channel) csPtr->writePtr; inStatePtr = csPtr->readPtr->state; outStatePtr = csPtr->writePtr->state; interp = csPtr->interp; cmdPtr = csPtr->cmdPtr; |
︙ | ︙ | |||
9663 9664 9665 9666 9667 9668 9669 | continue; } if (bufObj != NULL) { TclDecrRefCount(bufObj); bufObj = NULL; } | | | 9705 9706 9707 9708 9709 9710 9711 9712 9713 9714 9715 9716 9717 9718 9719 | continue; } if (bufObj != NULL) { TclDecrRefCount(bufObj); bufObj = NULL; } goto done; } } /* * Now write the buffer out. */ |
︙ | ︙ | |||
9754 9755 9756 9757 9758 9759 9760 | Tcl_CreateChannelHandler(outChan, TCL_WRITABLE, CopyEventProc, csPtr); } if (bufObj != NULL) { TclDecrRefCount(bufObj); bufObj = NULL; } | | | 9796 9797 9798 9799 9800 9801 9802 9803 9804 9805 9806 9807 9808 9809 9810 | Tcl_CreateChannelHandler(outChan, TCL_WRITABLE, CopyEventProc, csPtr); } if (bufObj != NULL) { TclDecrRefCount(bufObj); bufObj = NULL; } goto done; } /* * For background copies, we only do one buffer per invocation so we * don't starve the rest of the system. */ |
︙ | ︙ | |||
9776 9777 9778 9779 9780 9781 9782 | Tcl_CreateChannelHandler(outChan, TCL_WRITABLE, CopyEventProc, csPtr); } if (bufObj != NULL) { TclDecrRefCount(bufObj); bufObj = NULL; } | | | 9818 9819 9820 9821 9822 9823 9824 9825 9826 9827 9828 9829 9830 9831 9832 | Tcl_CreateChannelHandler(outChan, TCL_WRITABLE, CopyEventProc, csPtr); } if (bufObj != NULL) { TclDecrRefCount(bufObj); bufObj = NULL; } goto done; } } /* while */ if (bufObj != NULL) { TclDecrRefCount(bufObj); bufObj = NULL; } |
︙ | ︙ | |||
9828 9829 9830 9831 9832 9833 9834 9835 9836 9837 9838 9839 9840 9841 | result = TCL_ERROR; } else { Tcl_ResetResult(interp); Tcl_SetObjResult(interp, Tcl_NewWideIntObj(total)); } } } return result; } /* *---------------------------------------------------------------------- * * DoRead -- | > > > | 9870 9871 9872 9873 9874 9875 9876 9877 9878 9879 9880 9881 9882 9883 9884 9885 9886 | result = TCL_ERROR; } else { Tcl_ResetResult(interp); Tcl_SetObjResult(interp, Tcl_NewWideIntObj(total)); } } } done: CopyDecrRefCount(csPtr); return result; } /* *---------------------------------------------------------------------- * * DoRead -- |
︙ | ︙ | |||
9934 9935 9936 9937 9938 9939 9940 | * to fill the dst */ int code; moreData: code = GetInput(chanPtr); bufPtr = statePtr->inQueueHead; | < < | < | < | < < < | > > > > > > | 9979 9980 9981 9982 9983 9984 9985 9986 9987 9988 9989 9990 9991 9992 9993 9994 9995 9996 9997 9998 9999 10000 10001 10002 10003 10004 10005 10006 10007 10008 10009 10010 10011 10012 10013 10014 10015 | * to fill the dst */ int code; moreData: code = GetInput(chanPtr); bufPtr = statePtr->inQueueHead; if (GotFlag(statePtr, CHANNEL_EOF|CHANNEL_BLOCKED)) { /* * Further reads cannot do any more. */ break; } if (code || !bufPtr) { /* Read error (or channel dead/closed) */ goto readErr; } assert(IsBufferFull(bufPtr)); } if (!bufPtr) { readErr: UpdateInterest(chanPtr); TclChannelRelease((Tcl_Channel)chanPtr); return -1; } bytesRead = BytesLeft(bufPtr); bytesWritten = bytesToRead; TranslateInputEOL(statePtr, p, RemovePoint(bufPtr), &bytesWritten, &bytesRead); bufPtr->nextRemoved += bytesRead; |
︙ | ︙ | |||
10166 10167 10168 10169 10170 10171 10172 10173 | Tcl_DeleteChannelHandler(inChan, CopyEventProc, csPtr); if (inChan != outChan) { Tcl_DeleteChannelHandler(outChan, CopyEventProc, csPtr); } Tcl_DeleteChannelHandler(inChan, MBEvent, csPtr); Tcl_DeleteChannelHandler(outChan, MBEvent, csPtr); TclDecrRefCount(csPtr->cmdPtr); } | > > > > | > > > > | > > > > > > > > > > > > > > > | 10210 10211 10212 10213 10214 10215 10216 10217 10218 10219 10220 10221 10222 10223 10224 10225 10226 10227 10228 10229 10230 10231 10232 10233 10234 10235 10236 10237 10238 10239 10240 10241 10242 10243 10244 10245 10246 10247 10248 10249 | Tcl_DeleteChannelHandler(inChan, CopyEventProc, csPtr); if (inChan != outChan) { Tcl_DeleteChannelHandler(outChan, CopyEventProc, csPtr); } Tcl_DeleteChannelHandler(inChan, MBEvent, csPtr); Tcl_DeleteChannelHandler(outChan, MBEvent, csPtr); TclDecrRefCount(csPtr->cmdPtr); csPtr->cmdPtr = NULL; } if (inStatePtr->csPtrR) { assert(inStatePtr->csPtrR == csPtr); inStatePtr->csPtrR = NULL; CopyDecrRefCount(csPtr); } if (outStatePtr->csPtrW) { assert(outStatePtr->csPtrW == csPtr); outStatePtr->csPtrW = NULL; CopyDecrRefCount(csPtr); } } static void CopyDecrRefCount( CopyState *csPtr) { if (csPtr->refCount-- > 1) { return; } TclChannelRelease((Tcl_Channel)csPtr->readPtr); TclChannelRelease((Tcl_Channel)csPtr->writePtr); ckfree(csPtr); } /* *---------------------------------------------------------------------- * * StackSetBlockMode -- |
︙ | ︙ |
Changes to generic/tclIORChan.c.
︙ | ︙ | |||
2207 2208 2209 2210 2211 2212 2213 2214 2215 2216 2217 2218 2219 2220 2221 | resObj = Tcl_ObjPrintf("rc%lu", rcCounter); rcCounter++; Tcl_MutexUnlock(&rcCounterMutex); return resObj; } static void FreeReflectedChannel( char *blockPtr) { ReflectedChannel *rcPtr = (ReflectedChannel *) blockPtr; Channel *chanPtr = (Channel *) rcPtr->chan; TclChannelRelease((Tcl_Channel)chanPtr); | > > > > > > > > > > > > > > > > > > > > > > | < < < < < < < < | 2207 2208 2209 2210 2211 2212 2213 2214 2215 2216 2217 2218 2219 2220 2221 2222 2223 2224 2225 2226 2227 2228 2229 2230 2231 2232 2233 2234 2235 2236 2237 2238 2239 2240 2241 2242 2243 2244 2245 2246 2247 2248 2249 2250 2251 | resObj = Tcl_ObjPrintf("rc%lu", rcCounter); rcCounter++; Tcl_MutexUnlock(&rcCounterMutex); return resObj; } static inline void CleanRefChannelInstance( ReflectedChannel *rcPtr) { if (rcPtr->name) { /* * Reset obj-type (channel is deleted or dead anyway) to avoid leakage * by cyclic references (see bug [79474c58800cdf94]). */ TclFreeIntRep(rcPtr->name); Tcl_DecrRefCount(rcPtr->name); rcPtr->name = NULL; } if (rcPtr->methods) { Tcl_DecrRefCount(rcPtr->methods); rcPtr->methods = NULL; } if (rcPtr->cmd) { Tcl_DecrRefCount(rcPtr->cmd); rcPtr->cmd = NULL; } } static void FreeReflectedChannel( char *blockPtr) { ReflectedChannel *rcPtr = (ReflectedChannel *) blockPtr; Channel *chanPtr = (Channel *) rcPtr->chan; TclChannelRelease((Tcl_Channel)chanPtr); CleanRefChannelInstance(rcPtr); ckfree(rcPtr); } /* *---------------------------------------------------------------------- * * InvokeTclMethod -- |
︙ | ︙ | |||
2493 2494 2495 2496 2497 2498 2499 | static void MarkDead( ReflectedChannel *rcPtr) { if (rcPtr->dead) { return; } | | < < < < < < < < < < < | 2507 2508 2509 2510 2511 2512 2513 2514 2515 2516 2517 2518 2519 2520 2521 | static void MarkDead( ReflectedChannel *rcPtr) { if (rcPtr->dead) { return; } CleanRefChannelInstance(rcPtr); rcPtr->dead = 1; } static void DeleteReflectedChannelMap( void *clientData, /* The per-interpreter data structure. */ Tcl_Interp *interp) /* The interpreter being deleted. */ |
︙ | ︙ |
Changes to tests/ioCmd.test.
︙ | ︙ | |||
2112 2113 2114 2115 2116 2117 2118 2119 2120 2121 2122 2123 2124 2125 | child eval { proc no-op args {} proc driver {sub args} {return {initialize finalize watch read}} chan event [chan create read driver] readable no-op } interp delete child } {} # ### ### ### ######### ######### ######### ## Same tests as above, but exercising the code forwarding and ## receiving driver operations to the originator thread. # -*- tcl -*- # ### ### ### ######### ######### ######### | > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > | 2112 2113 2114 2115 2116 2117 2118 2119 2120 2121 2122 2123 2124 2125 2126 2127 2128 2129 2130 2131 2132 2133 2134 2135 2136 2137 2138 2139 2140 2141 2142 2143 2144 2145 2146 2147 2148 2149 2150 2151 2152 2153 2154 2155 2156 2157 2158 2159 2160 2161 2162 2163 2164 2165 2166 2167 2168 2169 2170 2171 2172 2173 2174 2175 2176 2177 2178 2179 2180 2181 2182 2183 2184 2185 2186 2187 2188 2189 2190 2191 2192 2193 | child eval { proc no-op args {} proc driver {sub args} {return {initialize finalize watch read}} chan event [chan create read driver] readable no-op } interp delete child } {} # 1st attempt without error in write, another with error in write: foreach ::writeErr {0 1} { test iocmd-32.3.$::writeErr {prevent copy-state against segfault by finalize, bug [79474c58800cdf94]} -setup { proc test_chan {args} { set rest [lassign $args mode chan] lappend ::ret $mode switch -exact $mode { read {puts $chan "Test" ; close $chan} write {if {$::writeErr} {return "boom"}; set data [lindex $rest 0]; string length $data} finalize {after 20 {set ::done done}} initialize {return "initialize watch finalize read write"} } } set clchlst {} set toev [after 5000 {set ::done tout}] } -body { set ::ret {} set ch [chan create "read write" test_chan] lappend clchlst $ch lassign [chan pipe] in1 out1 lappend clchlst $in1 $out1 lassign [chan pipe] in2 out2 lappend clchlst $in2 $out2 lassign [chan pipe] in3 out3 lappend clchlst $in3 $out3 # simulate exec: echo test >@ $out2 2>@ $out3 <@ $in1 &: fileevent $out2 writable [list apply {{cho che} { puts $cho test; close $cho; close $che }} $out2 $out3] # recopy to given chans in handler fileevent $in2 readable [list apply {{in out} { if {[catch { chan copy $in $out } msg]} { #puts err:$msg fileevent $in readable {} } }} $in2 $ch] fileevent $in3 readable [list apply {{in out} { if {[catch { chan copy $in $out } msg]} { #puts err:$msg fileevent $in readable {} } }} $in3 $ch] fileevent $out1 writable [list apply {{in out} { if {[catch { chan copy $in $out } msg]} { #puts err:$msg fileevent $out writable {} } }} $ch $out1] vwait ::done lappend ::ret $::done } -cleanup { foreach ch $clchlst { catch {close $ch} } after cancel $toev unset -nocomplain ::done ::ret ch in1 in2 in3 out1 out2 out3 toev clchlst } -result {initialize read write finalize done} }; unset ::writeErr # ### ### ### ######### ######### ######### ## Same tests as above, but exercising the code forwarding and ## receiving driver operations to the originator thread. # -*- tcl -*- # ### ### ### ######### ######### ######### |
︙ | ︙ |