Ticket UUID: | 770053 | |||
Title: | Tcl_CreateThread breaks notifier thread refcount | |||
Type: | Bug | Version: | obsolete: 8.5a2 | |
Submitter: | vasiljevic | Created on: | 2003-07-12 08:30:50 | |
Subsystem: | 49. Threading | Assigned To: | vasiljevic | |
Priority: | 6 | Severity: | ||
Status: | Closed | Last Modified: | 2004-07-16 04:25:11 | |
Resolution: | Fixed | Closed By: | vasiljevic | |
Closed on: | 2004-07-15 21:25:11 | |||
Description: |
Tcl API is quite unsymetrical when creating threads without embedded interpreter. What hapenns is that when one uses Tcl_CreateThread API call to create new thread and Tcl_ExitThread to cleanup after itself when the thread exits, the notifier thread reference counter is decremented one time too much. This eventually tears-down the notifier thread and the inter-thread communication betwenn threads becomes impossible. If, however, a newly created thread calls the Tcl_CreateInterp, the reference count is ok. The problem lies in the fact that Tcl_CreateInterp calls, as part of its processing, Tcl_InitNotifier. This call bumps the refcount of the notifier thread. The Tcl_ExitThread calls Tcl_FinalizeNotifier and this decrements the refcount. In this case, all is ok. But, the Tcl_CreateThread *does not* call the Tcl_InitNotifier, which then results in wrong refcount of the notifier thread when Tcl_ExitThread is called. I'm assigning this to Jeff since I do not know who is maintaining the thread API subsytem. | |||
User Comments: |
vasiljevic added on 2004-07-16 04:25:11:
Logged In: YES user_id=95086 The notifier ref-counting problem is now fixed by conditionaly finalizing the notifier. If the notifier for the thread has never been initialized by a call to TclInitNotifier, it will not be finalized by the call to TclFinalizeNotifier. Simple as that. For this to work, an already existing threadId element of the private TSD key from generic/tclNotify.c is used. This is now fixed in core-8-4-branch and in cvs head. Lets see whose toes we're trampling on now... This one corrects the ref-counting problem but all API users should be aware of the fact that inter-thread communication using Tcl_ThreadAlert() and bros is *not* possible for threads created with Tcl_CreateThread which have no Tcl interpreter assigned. vasiljevic added on 2004-07-15 16:33:28: Logged In: YES user_id=95086 I'm backing out the call to TclInitNotifier() from NewThreadProc for the time being, effectively reopening the bug again. This will allow Mac Tk project to proceed but will leave the bug exposed to all other Tcl API users involved with creating threads using the Tcl_CreateThread API call. The backout affects both core-8-4-branch and the current head. vasiljevic added on 2004-07-12 23:27:23: Logged In: YES user_id=95086 Can please somebody verify that this is now working OK? I cannot do it here. Just go to generic/tclEvent.c and replace the NewThreadProc() call with the one below: static Tcl_ThreadCreateType NewThreadProc(ClientData clientData) { ThreadClientData *cdPtr; ClientData threadClientData; Tcl_ThreadCreateProc *threadProc; ThreadSpecificData *tsdPtr; tsdPtr = (ThreadSpecificData *)TclThreadDataKeyGet(&dataKey); cdPtr = (ThreadClientData *)clientData; threadProc = cdPtr->proc; threadClientData = cdPtr->clientData; Tcl_Free((char*)clientData); /* Allocated in Tcl_CreateThread() */ if (tsdPtr == NULL) { tsdPtr = TCL_TSD_INIT(&dataKey); TclInitNotifier(); } (*threadProc)(threadClientData); } Please tell me what you get. vasiljevic added on 2004-07-12 23:06:54: Logged In: YES user_id=95086 Urks... this I haven't thought about, sorry. Of course, if you declare a custom init by calling Tcl_SetNotifier and you use Tcl_CreateThread() from the tclStubs.tcl_InitNotifier which now points to your custom one, you get the (endless) recursion. Well, this is bad and should be fixed. I will have to see how I can avoid this. Thanks for the hint. I will put this open again and correct the issue. tallniel added on 2004-07-12 22:34:42: Logged In: YES user_id=102050 This bug fix may have broken the Tk build on Mac OS X. Wish hangs on startup, and gdb seems to show deadlock. The following summary from Jim Ingham: ----- The problem is that the fix to Tcl bug 770053 causes TclInitNotifier to get called recursively which stalls. The short term fix is to comment out the call to TclInitNotifier in tcl/generic/tclEvent.c. The core-8-4 patch is: Index: tclEvent.c ============================================== ===================== RCS file: /cvsroot/tcl/tcl/generic/tclEvent.c,v retrieving revision 1.28.2.4 diff -p -p -r1.28.2.4 tclEvent.c *** tclEvent.c 22 Jun 2004 11:55:35 -0000 1.28.2.4 --- tclEvent.c 11 Jul 2004 19:44:18 -0000 *************** NewThreadProc(ClientData clientData) *** 1192,1198 **** threadClientData = cdPtr->clientData; Tcl_Free((char*)clientData); /* Allocated in Tcl_CreateThread() */ ! TclInitNotifier(); (*threadProc)(threadClientData); } --- 1192,1198 ---- threadClientData = cdPtr->clientData; Tcl_Free((char*)clientData); /* Allocated in Tcl_CreateThread() */ ! // TclInitNotifier(); (*threadProc)(threadClientData); } The same patch should apply to TOT. I don't think the fix to 770053 is correct, it means that you can't call Tcl_CreateThread inside a call to TclInitNotifier, which doesn't seem right to me. I will send this on to the Tcl Core list. ---- vasiljevic added on 2004-06-22 20:09:37: Logged In: YES user_id=95086 Applied to head. Closing this bug report. vasiljevic added on 2004-06-22 18:56:06: Logged In: YES user_id=95086 Applied to core-8-4-branch. vasiljevic added on 2004-06-21 18:46:47: File Added - 91347: threadCreate.diff Logged In: YES user_id=95086 Forgot the patch file... vasiljevic added on 2004-06-21 18:29:23: Logged In: YES user_id=95086 There is a patch which rectifies this problem. The patch has been taken against the core-8-4-branch. Summary: * renamed Tcl_CreateThread in to platfrom-specific TclpThreadCreate (both win and unix) * added new Tcl_CreateThread in tclEvent.c * added new NewThreadProc call which is used as the bootstrap function for every new thread created by the Tcl_CreateThread. Purpose of this function is to perform the Tcl notifier init for a thread and then jump into the user-supplied thread-start function. If nobody objects, I will commit this to both core-8-4-branch and current head. vasiljevic added on 2003-10-16 14:34:33: Logged In: YES user_id=95086 Not yet, unfortunately, but I can make some. If you like, I can assign this bug to myself and take care about it as soon as I get some releif from the daily work; perpahs in a week or so. hobbs added on 2003-10-13 06:43:00: Logged In: YES user_id=72656 Zoran, do you have any good patch resolutions for this? |
Attachments:
- threadCreate.diff [download] added by vasiljevic on 2004-06-21 18:46:47. [details]