Tcl Source Code

View Ticket
Login
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: