Index: generic/tclIO.c ================================================================== --- generic/tclIO.c +++ generic/tclIO.c @@ -1174,11 +1174,19 @@ Tcl_Panic("Tcl_RegisterChannel: duplicate channel names"); } Tcl_SetHashValue(hPtr, chanPtr); } + + /* + * Increase ref-count of the state and channel (note the transfer-process to + * other thread could temporary share the channel in two threads (especially + * if initiated from event-handler like TclChannelEventScriptInvoker, + * see [815e246806]). + */ statePtr->refCount++; + TclChannelPreserve(chan); } /* *---------------------------------------------------------------------- * @@ -1376,10 +1384,12 @@ */ CleanupChannelHandlers(interp, chanPtr); } + /* Opposite of TclChannelPreserve in Tcl_RegisterChannel */ + TclChannelRelease(chan); statePtr->refCount--; return TCL_OK; } @@ -1963,11 +1973,11 @@ void TclChannelPreserve( Tcl_Channel chan) { - ((Channel *)chan)->refCount++; + TclAtomicFetchInc(&((Channel *)chan)->refCount); } void TclChannelRelease( Tcl_Channel chan) @@ -1975,11 +1985,11 @@ Channel *chanPtr = (Channel *) chan; if (chanPtr->refCount == 0) { Tcl_Panic("Channel released more than preserved"); } - if (--chanPtr->refCount) { + if (TclAtomicFetchDec(&chanPtr->refCount)) { return; } if (chanPtr->typePtr == NULL) { ckfree(chanPtr); } @@ -3197,10 +3207,16 @@ * states - used to splice a channel out of * the list on close. */ ChannelState *statePtr = chanPtr->state; /* State of the channel stack. */ + /* + * Firstly notify any - channel has no owner threads anymore. + */ + + statePtr->managingThread = NULL; + /* * Remove this channel from of the list of all channels (in the current * thread). */ @@ -8703,10 +8719,24 @@ } UpdateInterest(statePtr->topChanPtr); } +static void +SafeFreeScriptRecord( + EventScriptRecord *esPtr) +{ + /* if not executed - remove it right now */ + if (esPtr->execDepth == 0) { + TclDecrRefCount(esPtr->scriptPtr); + ckfree(esPtr); + } else if (esPtr->execDepth > 0) { + /* inverse depth to notify event-handlers, they should remove it hereafter */ + esPtr->execDepth = -esPtr->execDepth; + } + /* execution in-between and already notified (negative). */ +} /* *---------------------------------------------------------------------- * * DeleteScriptRecord -- * @@ -8746,12 +8776,11 @@ } Tcl_DeleteChannelHandler((Tcl_Channel) chanPtr, TclChannelEventScriptInvoker, esPtr); - TclDecrRefCount(esPtr->scriptPtr); - ckfree(esPtr); + SafeFreeScriptRecord(esPtr); break; } } } @@ -8787,12 +8816,11 @@ EventScriptRecord *esPtr; int makeCH; for (esPtr=statePtr->scriptRecordPtr; esPtr!=NULL; esPtr=esPtr->nextPtr) { if ((esPtr->interp == interp) && (esPtr->mask == mask)) { - TclDecrRefCount(esPtr->scriptPtr); - esPtr->scriptPtr = NULL; + SafeFreeScriptRecord(esPtr); break; } } makeCH = (esPtr == NULL); @@ -8812,10 +8840,11 @@ esPtr->chanPtr = chanPtr; esPtr->interp = interp; esPtr->mask = mask; Tcl_IncrRefCount(scriptPtr); esPtr->scriptPtr = scriptPtr; + esPtr->execDepth = 0; if (makeCH) { esPtr->nextPtr = statePtr->scriptRecordPtr; statePtr->scriptRecordPtr = esPtr; @@ -8854,10 +8883,17 @@ * in. */ int result; /* Result of call to eval script. */ esPtr = clientData; chanPtr = esPtr->chanPtr; + + if ( esPtr->scriptPtr == NULL || esPtr->execDepth < 0 + || Tcl_GetCurrentThread() != chanPtr->state->managingThread + ) { + return; + } + mask = esPtr->mask; interp = esPtr->interp; /* * We must preserve the interpreter so we can report errors on it later. @@ -8865,11 +8901,26 @@ * by Tcl_NotifyChannel before calling channel handlers. */ Tcl_Preserve(interp); TclChannelPreserve((Tcl_Channel)chanPtr); + + esPtr->execDepth++; result = Tcl_EvalObjEx(interp, esPtr->scriptPtr, TCL_EVAL_GLOBAL); + + if (esPtr->execDepth > 0) { + --esPtr->execDepth; + } else if (esPtr->execDepth < 0) { + /* + * Negative depth meants - the handler was removed, so we should increase, + * and remove esPtr record by the last handler (if it reached 0). + */ + if (++esPtr->execDepth == 0) { + SafeFreeScriptRecord(esPtr); + /* don't use esPtr at here. */ + } + } /* * On error, cause a background error and remove the channel handler and * the script record. * Index: generic/tclIO.h ================================================================== --- generic/tclIO.h +++ generic/tclIO.h @@ -81,10 +81,12 @@ Tcl_Interp *interp; /* In what interpreter to invoke script? */ int mask; /* Events must overlap current mask for the * stored script to be invoked. */ struct EventScriptRecord *nextPtr; /* Next in chain of records. */ + int execDepth; /* Execution depth inside the event-handlers, + * if negative it should be removed as 0 reached. */ } EventScriptRecord; /* * struct Channel: * @@ -111,11 +113,14 @@ */ ChannelBuffer *inQueueHead; /* Points at first buffer in input queue. */ ChannelBuffer *inQueueTail; /* Points at last buffer in input queue. */ - int refCount; + TclAtomicInt refCount; /* Reference counter, volatile forces atomic + * direct incr/decr during transfer process, + * if temorary shared between threads (avoids + * unexpected compiler optimization) */ } Channel; /* * struct ChannelState: * Index: generic/tclInt.h ================================================================== --- generic/tclInt.h +++ generic/tclInt.h @@ -121,10 +121,38 @@ #if defined(_WIN32) && defined(_MSC_VER) # define vsnprintf _vsnprintf #endif +/* + * Interlocked (atomic fetch) primitives. + * + * Note gcc > 4.1 (also for mingw) has native support for builtin atomic operations. + * ToDo: extend with support for atomic_ops, Darwin atomic, Sun atomics if available. + */ +#if !defined(__WIN32__) || defined(__GNUC__) || defined(__MINGW32__) /* UNIX or GCC */ +# define TclAtomicInt long volatile + +# define TclAtomicFetchAdd(p, add) \ + __sync_fetch_and_add((p), (add)) +# define TclAtomicFetchSub(p, sub) \ + __sync_fetch_and_sub((p), (sub)) +# define TclAtomicFetchInc(p) \ + (__sync_add_and_fetch((p), 1)) +# define TclAtomicFetchDec(p) \ + (__sync_sub_and_fetch((p), 1)) +#else /* defined(__WIN32__) */ +# define TclAtomicInt LONG volatile +# define TclAtomicFetchAdd(p, add) \ + InterlockedExchangeAdd((LONG volatile *)(p), (add)) +# define TclAtomicFetchSub(p, sub) \ + InterlockedExchangeSubtract((LONG volatile *)(p), (sub)) +# define TclAtomicFetchInc(p) \ + InterlockedIncrement((LONG volatile *)(p)) +# define TclAtomicFetchDec(p) \ + InterlockedDecrement((LONG volatile *)(p)) +#endif /* defined(__WIN32__) */ /* * The following procedures allow namespaces to be customized to support * special name resolution rules for commands/variables. */