Tcl Source Code

OriginalBugID: 5829 Bug
Version: 8.4a1
SubmitDate: '2000-06-06'
LastModified: '2000-09-01'
Severity: LOW
Status: UnAssn
Submitter: techsupp
ChangedBy: davidg
RelatedBugIDs: 5793 5312
OS: Windows 2000
FixedDate: '2000-10-25'
ClosedDate: '2000-10-25'


Name:

David Gravereaux



CVS:

updated saturday



CustomShell:

my tclAsync.c patch



Comments:

This appears to be working with SetConsoleCtrlHandler and the

    CtrlHandler returning TRUE, so Win32 doesn't kill it.  I think

    I was being too aggressive with

    CloseHandle(GetStdHandle(STD_INPUT_HANDLE));

    as Tcl_Gets in Tcl_Main was properly dropping-out.  The

    tclWinNotify.c change is there just for the DLL as that's a

    possible senario with a threaded build.



ObservedBehavior:

Ctrl+C crashes tclsh84.exe rather then a clean exit.



Patch:

*** win/tclAppInit.c1999/12/02 02:03:371.6

--- win/tclAppInit.c2000/06/05 00:50:23

***************

*** 29,35 ****

--- 29,39 ----

  #endif /* TCL_TEST */

  

  static voidsetargv _ANSI_ARGS_((int *argcPtr, char ***argvPtr));

+ static BOOL __stdcallsigHandler _ANSI_ARGS_((DWORD fdwCtrlType));

  

+ Tcl_AsyncHandlerexitToken;

+ Tcl_AsyncProcasyncExit;

+ 

  

  /*

   *----------------------------------------------------------------------

***************

*** 156,161 ****

--- 162,170 ----

              Procbodytest_SafeInit);

  #endif /* TCL_TEST */

  

+     SetConsoleCtrlHandler(sigHandler, TRUE);

+     exitToken = Tcl_AsyncCreate(asyncExit, (ClientData) interp);

+ 

      /*

       * Call the init procedures for included packages.  Each call should

       * look like this:

***************

*** 298,301 ****

--- 307,375 ----

  

      *argcPtr = argc;

      *argvPtr = argv;

+ }

+ 

+ /*

+  *----------------------------------------------------------------------

+  *

+  * asyncExit --

+  *

+  *The AsyncProc for the exitToken.

+  *

+  * Results:

+  *doesn't actually return.

+  *

+  * Side effects:

+  *tclsh cleanly exits.

+  *

+  *----------------------------------------------------------------------

+  */

+ 

+ int asyncExit (ClientData clientData, Tcl_Interp *interp, int code)

+ {

+     char *buffer = "exit -1";

+     Tcl_Eval((Tcl_Interp *) clientData, buffer);

+ 

+     /* NOTREACHED */

+     return code;

+ }

+ 

+ /*

+  *----------------------------------------------------------------------

+  *

+  * sigHandler --

+  *

+  *Signal handler for the Win32 OS.  Catches Ctrl+C, Ctrl+Break and

+  *other exits.  This is needed so tclsh can do it's real clean-up

+  *and not an unclean crash terminate.

+  *

+  * Results:

+  *TRUE.

+  *

+  * Side effects:

+  *effects the way the app exits from a signal.  This is an 

+  *operating system supplied thread and unsafe to call ANY

+  *Tcl commands except for Tcl_AsyncMark.

+  *

+  *----------------------------------------------------------------------

+  */

+ 

+ BOOL __stdcall sigHandler(DWORD fdwCtrlType)

+ {

+     /*

+      *  If Tcl is currently executing some bytecode or in the eventloop,

+      *  this will cause Tcl to enter asyncExit at the next command

+      *  boundry.

+      */

+     Tcl_AsyncMark(exitToken);

+ 

+     /*

+      *  This will cause Tcl_Gets in Tcl_Main() to drop-out with an <EOF>

+      *  should it be blocked on input and our Tcl_AsyncMark didn't grab

+      *  the attention of the interpreter.

+      */

+ /*    CloseHandle(GetStdHandle(STD_INPUT_HANDLE)); */

+ 

+     /* indicate to the OS not to call the default terminator */

+     return TRUE;

  }



*** win/tclWinNotify.c1999/07/02 22:08:281.5

--- win/tclWinNotify.c2000/06/05 00:50:37

***************

*** 150,155 ****

--- 150,163 ----

  {

      ThreadSpecificData *tsdPtr = (ThreadSpecificData *) clientData;

  

+     /*

+      *  Should the operating system call DllMain with

+      *  DLL_PROCESS_DETACH from any alternate thread path, such

+      *  as Task Manager terminating the process, tsdPtr will

+      *  be NULL and invalid.

+      */

+     if (tsdPtr == NULL) return;

+ 

      DeleteCriticalSection(&tsdPtr->crit);

      CloseHandle(tsdPtr->event);



PatchFiles:

win/tclAppInit.c

    win/tclWinNotify.c

Logged In: YES 
user_id=7549

To me, I look at this as a pure bug fix.  As wish gets going 
down notices because the system closes the UI windows, 
tclsh gets no notices.

A Ctrl+C brings down tclsh improperly from an unknown 
native thread, but with this patch the main thread is used 
which to me, seems like the proper solution then living with 
checks in TclFinalizeNotifier that aviod cleanup due to 
missing thread-specific data.

Logged In: YES 
user_id=80530


Haven't followed this one closely; just curious about the
recent commit.  Are we sure it's OK to go in between
an 8.4.0 release and an 8.4.1 release?  Or should it wait
for a real "alpha" development branch to open on the HEAD?

I've got nothing against the patch technically -- haven't
examined it.  Just want to be careful about large-ish,
potentially interface-changing changes during point releases.

Logged In: YES 
user_id=7549

Fix committed to HEAD for -r1.9

File Deleted - 31994: 



File Added - 31995: patch.txt

File Added - 31994: patch.txt

Logged In: YES 
user_id=7549

Adding updated patch file.

Logged In: YES 
user_id=7549

Jeff,

How do you feel about this?:

>The scary part is that the simplicity of tclAppInit.c will get 
>destroyed and start taking on windows specifics.  It'll be a 
>first to #include <windows.h>.
>
>If it does, a real shell with colors and mouse support 
>accessing the console API couldn't be far off as the door
>would be open...

Just wondering.  The possibility for nice interactive tclsh 
behavior on Win9x would be a plus: up-arrow for history and 
in-place editing.  Just NT does this by default.

Logged In: YES 
user_id=7549

Yes, half this problem still occurs in 8.4a4.  Just build for 
threads, start tclsh, then exit by way of the 'X' upper right 
button.  It doesn't go "boom" anymore because the 
tclWinNotify.c change is there, but no exit handlers get run.  
At least, it doesn't look this way...  I could be wrong.  
The "exit request" should get "posted" in some manner to the 
main thread.

Logged In: YES 
user_id=79902

ISTR doing something to do with threads that Tcl didn't
control doing things with shutting down Tcl fairly recently
(I didn't *understand* it, but nonetheless...)  Does the
problem still occur in 8.4a4 (I think it made the cut)?

Logged In: YES 
user_id=7549

>  Instead, my problem could be fixed cleanly *along* with
>bug 5312
>    by changing Tcl_Finalize to correctly finalize Tcl even
>    if called on a virgin, never-called-Tcl-before thread.

I like that solution the best.  How would the code look?  
Should we track all threads where a notifier is 
initialized?  Should a Tcl_Finalize() also hard close all 
running threads, too?

Logged In: YES 
user_id=80530

>From "Egon Pasztor" <[email protected]>


 Hi, I'm using Tcl 8.3.1 on a Windows 2000 Dual-Pentium III PC;
     exploring with Microsoft Visual C++ 6.0.

 This is regarding bug TR#5312 as listed on the scriptics
tcl bug 
database
    -- "Application Error" when closing tclsh83.exe
        (tcl8.3.1 with threads) Tcl Core"

     >> From www.scriptics.com, bug database, TR#5312:
     >
> ObservedBehavior: Tclsh83.exe throws an "Application Error"
     > when terminated with "Close" from the window's menu.
     > It does that ONLY with activated thread support.


 I ran into .. sortof a more severe version of this bug,
     and ended up tracking it here.   I notice additional
     information has been posted:


     >> From www.scriptics.com, bug database, TR#5312:
     >
> It throws the exception on line 151 of tclWinNotify.c in
     > Tcl_FinalizeNotifier().  It looks like the notifier
     > data coming in to the function is invalid.


 Invalid indeed -- on my computer, zero, uninitialized:
 I think the problem is that TclFinalizeNotifier (tclNotify.c)
   is being called from a thread in which TclInitNotifier
was never 
called.

     > This needs a more thorough investigation.

  .... read on ...  :)



 
===================================================================
  ====================         1         
===========================
 
===================================================================


 I see that Tcl_FinalizeNotifier (tclWinNotify.c)
    is actually called by TclFinalizeNotifier (tclNotify.c),
    which contains the lines


     >> ThreadSpecificData *tsdPtr = TCL_TSD_INIT(&dataKey);
     >> ....
     >> Tcl_FinalizeNotifier(tsdPtr->clientData);


 This macro, "TCL_TSD_INIT",
    gets a pointer to thread-local-storage if some has been
allocated,
    or allocates some new if none has.

 So ** if TclFinalizeNotifier were called
       from a thread that had no thread-local-storage
constructed **

   then TCL_TSD_INIT would allocate fresh space, filled with
zeros;
      and pass it into Tcl_FinalizeNotifier, causing the crash.


                     -- -- -- -- -- -- -- --

 This is what I think is happening --
 I tried to trace the antics of the various threads
   started and stopped while executing the steps needed
   to reproduce the crash
                            -- (ie, starting tclsh83,
                                then selecting "CLOSE" from
                                the console's pull-down menu) --

   the "CLOSE" signal seems to cause Tcl_Finalize
(tclEvent.c) to be 
called
     (from DllMain (tclWin32Dll.c)) from within a new,
virgin thread --
     one which has never yet called any Tcl function at all.


 Tcl_Finalize calls Tcl_FinalizeThread (tclEvent.c),
    which calls TclFinalizeNotifier.



 
===================================================================
  ====================         2         
===========================
 
===================================================================



  That is, *Suppose* that Tcl_Finalize were called on such a
virgin 
thread:

    A:  I see that one of the first lines in Tcl_Finalize
(tclEvent.c) 
is

          >> tsdPtr = TCL_TSD_INIT(&dataKey);

        if this were a "virgin" thread, this line would *create*
          new thread-local storage, associated with the
"dataKey"
          in that file (tclEvent.c).



    B:  The next block of code is protected by an "if" statement

          >> if (subsystemsInitialized != 0) {

        presumably to keep Tcl_Finalize from doing anything
if no Tcl
        functions have ever run.  But I'm supposing that we
are at the
        end of a successful Tcl program; Tcl functions
*have* been 
called,
        and all subsystems initialized; just nothing from
*this* thread.

        So control proceeds into the main body of
Tcl_Finalize ...



    C:  ... down to the call to Tcl_FinalizeThread, about
halfway
        through the body of Tcl_Finalize.

        Tcl_FinalizeThread (tclEvent.c) begins with

          >> ThreadSpecificData *tsdPtr =
          >> (ThreadSpecificData
*)TclThreadDataKeyGet(&dataKey);
          >>
>> if (tsdPtr != NULL) { ....

        which gets a pointer to thread-local-storage in a
manner that
          doesn't create the storage if it's not already
present --
          and checks it for NULL.

        However, thread-local-storage for this thread *was*
created
          in Tcl_Finalize (see point A, above) -- and with
the same
          dataKey, since control is still in (tclEvent.c).

        So control proceeds past the if-statement's guard
into the
          main body of Tcl_FinalizeThread ...


    D:  Most of what follows in Tcl_FinalizeThread is in a
loop that
          will be skipped if "tsdPtr->firstExitPtr" is 0.

        Since the thread-local-storage was just allocated,
          it is likely to be 0,
          so control will make it down to
          the call to "TclFinalizeNotifier".


    E:  Which brings us back to the scenario posited above --
        TclFinalizeNotifier (tclNotify.c)
        begins with

          >> ThreadSpecificData *tsdPtr =
TCL_TSD_INIT(&dataKey);

        which allocates another new block of
thread-local-storage
          (since this is a different "dataKey", in
tclNotify.c now)
        and proceeds to call Tcl_FinalizeNotifier
        with its uninitialized contents, causing the crash.

 )



 
===================================================================
  ====================         3         
===========================
 
===================================================================


  So it looks like Tcl_Finalize would crash if called from a
virgin 
thread.
  I think this is what actually happens:


  According to the Microsoft documentation, a DLL's DllMain
function
    is called off and on for one of four reasons,
      "DLL_PROCESS_ATTACH",   "DLL_PROCESS_DETACH"
      "DLL_THREAD_ATTACH", & "DLL_THREAD_DETACH"

  So I put calls to my own "printf-like" external
message-logging 
function
    inside "DllMain" (tclWin32Dll.c) to watch the threads
startup
    and shutdown.


          >> From www.scriptics.com, bug database, TR#5312:
          >
> So it looks like the OS might be terminating
          > running threads for us in reverse order.


  Well.. not "reverse order", exactly.
  It seems that when the user selects "CLOSE" from the
console's menu,
    Windows actually spawns a *brand new thread* from which
to call
    "ExitProcess"  -- calling DllMain (DLL_PROCESS_DETACH))
                          -- calling Tcl_Finalize
                          -- all from within this virgin thread.


                     -- -- -- -- -- -- -- --

  That is, a dump of all the calls to DllMain during a run
of tclsh83
  looks like this:

   <1>: DllMain (DLL_PROCESS_ATTACH) is called on thread A
         (The main thread starting up; "main" (tclAppInit.c)
is called 
next
          .. this calls Tcl_Main, which inits lots of things
.. and 
soon:

   <2>: DllMain (DLL_THREAD_ATTACH) is called on thread B
         (Evidently running "ConsoleReaderThread" in
tclWinConsole.c;
          a thread "managing" STDIN in some way; spawned by
          CreateThread called in TclWinOpenConsoleChannel 
(tclWinConsole.c))

   <3>: DllMain (DLL_THREAD_ATTACH) is called on thread C
         (Same thing, this time "ConsoleWriterThread",
managing STDOUT)
         ... now the console appears on screen and cursor's
blinking ...
         ... and after the first command is interpreted, we
get one 
more:

   <4>: DllMain (DLL_THREAD_ATTACH) is called on thread D
         ... to represent STDERR.

         *Now* .. Take your mouse to the console menu and
select CLOSE 
...
                  and we get ...

   <5>: DllMain (DLL_THREAD_ATTACH) is called on thread E
         ..  ?? ?? ?? Evidently started by Windows ?? ?? ?? ..
         followed by

   <6>: DllMain (DLL_PROCESS_DETACH) is called on thread E
         -- which calls Tcl_Finalize on thread E,
            causing the crash as traced above, since thread E
            has no Tcl initializations.


                     -- -- -- -- -- -- -- --

  If instead, after Event 4, instead of selecting CLOSE you
    simply type "exit", and execute it,
    it gets interpreted (in thread A of course)
    calling Tcl_Exit,
       calling Tcl_Finalize (in thread A, the Tcl thread)
       and everything is fine.

    Tcl_Exit next calls TclpExit, which is #defined to the
       native OS "exit" (tclWinPort.h) --
         (all this still on thread A)
       -- which finally kills the process.

  So in this case, instead of Events 5 and 6,
    there is simply

    <5>: DllMain (DLL_PROCESS_DETACH) is called on thread A

    .. (which, of course, calls Tcl_Finalize a second time
        (still on thread A) .. but Tcl_Finalize checks for
this.)


 
===================================================================


  When I first saw this, I was initially surprised that
there weren't
    any "DLL_THREAD_DETACH"'es.  Oddly, I'd thought that
they would
    come in predictable pairs.   Nope.

  I found confirmation of this in Microsoft Docs on "DllMain"

   >>
http://msdn.microsoft.com/library/psdk/winbase/dll_8asu.htm

which contains the text:


      > When a DLL is unloaded from a process as a result of
      > ... termination of the process ...
      > the system does not call the DLL's entry-point function
      > with the DLL_THREAD_DETACH value for the individual
threads
      > of the process. The DLL is only sent a
DLL_PROCESS_DETACH
      > notification.


 
===================================================================


  I was less successful in finding official explanatation of
this
    random extra "thread E" started when the user selects
"CLOSE"
    in the console's menu.

  The best documentation I could find was noting that the
Microsoft
    Docs for Win32's "SetConsoleCtrlHandler" -- the API for
setting
    a CLOSE or Control-C handler --

    >>
http://msdn.microsoft.com/library/psdk/winbase/conchar_599u.htm

-- continually refer to these things -- Control-C,
Control-BREAK,
     perhaps CLOSE too -- as "signals", reminiscent of Unix
signals,
     and the Microsoft Doc for "signals" --

    >>
http://msdn.microsoft.com/library/devprods/vs6/visualc/vccore/_crt_signa
l.htm

  -- contains the .. umm.. ominous warning:

      > When a CTRL+C interrupt occurs, Win32 operating systems
      > generate a new thread to specifically handle that
interrupt.
      > This can cause a single-thread application such as UNIX,
      > to become multithreaded, resulting in unexpected
behavior.


  (This makes me wonder about bug TR#4132 as well --
("Control-BREAK"
   causing a hang in tclsh) -- deadlock from unexpected
multithreading?)


  But the reality of this "thread E" created just to handle
the CLOSE
  is confirmed:  If you actually register a "CLOSE handler"
function,
  using "SetConsoleCtrlHandler" -- it is called on "thread E".




 
===================================================================
  ====================         4         
===========================
 
===================================================================


  On the scriptics page, it is suggested:


          >> From www.scriptics.com, bug database, TR#5312:
          >
> I'll assume right now that tclAppInit.c (the app,
          > not the dll) might need a control-c handler
provided by
          > Win32's SetConsoleCtrlHandler() for a controlled
shut-down

  and of course, the problem *can* be overcome this way.

  The simplest thing I can think of is
    .. "well, if Tcl_Finalize is going to get called on
thread E,
        let's make sure Tcl is initialized on it."

  By adding the function:


    >> BOOL WINAPI HandlerRoutine (DWORD dwCtrlType)
    >> {
    >> Tcl_DeleteInterp(Tcl_CreateInterp());
    >> return FALSE;
    >> }

  to (tclAppInit.c), along with a call to

    >> SetConsoleCtrlHandler(HandlerRoutine, TRUE);

  inside "main", the problem goes away,
  and you can CLOSE tclsh83 with ease.


                        BUT


  (and this brings us to the point of my long email)

  I'd like to suggest a deeper solution --
  one that would also fix * my * problem.


 
===================================================================
 
===================================================================


  Because the "fundamental" problem seems to be that
    it looks like Tcl_Finalize wasn't designed to be called
    from a non-tcl thread;
      And I ran into this bug from a completely different angle.


  * Me *, I'm writing a C++ program,
      with a Tcl/Tk interpreter embedded in it.


  Mine is a multithreaded Win32 application, not console at all,
     but most of the time it works invisibly, (no windows at
all)
     and only with native Win32 calls.

     Only some of the time (ie, not every time the program runs)
       do I need to pop up a GUI --- for which I init and
use Tk.

  So basically:

    Most of the time:

        My program starts (Thread A)
         .. runs for a while ..
            and then exits and (Thread A) dies.

    However, sometimes:

        My program starts (Thread A)
         .. runs for a while ..
         .. something happens requiring a GUI

              .. so I spawn (Thread B),
                 which runs code cut-and-pasted from (tkMain.c)
                 to start a tk-based GUI
              .. which runs for a while ..
              .. and then is stopped,
                 interpreter deleted, thread finalized,
                 (Thread B) dies.

         .. and then some time later,
            my program exits and (Thread A) dies.


  But every time my program uses Tk, when my program
subsequently
     exits, I get the same Tcl_FinalizeNotifier crash --
     the cause being the same thing:

        When thread A exits,
           Tcl/Tk DllMain's are called with DLL_PROCESS_DETACH
             which calls Tcl_Finalize ---- from thread A

         But my main thread -- thread A -- is not a tcl thread,
             and so none of its "Notifiers" have been
initialized.




        ###############################################
        ##  Hey, has this bug been reported before?  ##
        ###############################################

  I've seen oodles of examples of embedding Tk
         into a single-threaded C++ application,
      and many examples of embedding Tk into many threads
      of a multi-threaded C++ application, always invariably
      including the main thread.

  But had anyone tried to embed Tk into *only subordinate
threads*
      of a C++ application, and seen this problem?

        ###############################################
        ###############################################



  Of course, there are tons of workarounds --
      it's easy to initialize Tcl on the main thread;
      a simple Tcl_DeleteInterp(Tcl_CreateInterp()) does it,
      and then the implicit Tcl_Finalize caused by DLL unloading
      would work.   But this.. feels hacky -- having to
initialize
                    stuff just so the finalizer doesn't crash.

  Or I could abandon automatic DLL loading
      and switch to explicity loading and unloading the Tcl
      library in thread B -- this would prevent Tcl_Finalize
      from being called on thread A at all -- but this is a
pain.

  Instead, my problem could be fixed cleanly *along* with
bug 5312
    by changing Tcl_Finalize to correctly finalize Tcl even
    if called on a virgin, never-called-Tcl-before thread.




 
===================================================================
  ====================         5         
===========================
 
===================================================================


  Naively, this doesn't seem hard --

     1:  Tcl_Finalize (tclEvent.c)
         starts with the line

           >> tsdPtr = TCL_TSD_INIT(&dataKey);

         which runs the risk of inadvertently *creating*
         thread-local-storage if none exists.  Replacing this
         with

          >> tsdPtr = (ThreadSpecificData 
*)TclThreadDataKeyGet(&dataKey);

         doesn't, and prevents Tcl_FinalizeThread (called lower
         in Tcl_Finalize) from doing anything, since
Tcl_FinalizeThread
         uses identical code to *check* if
thread-local-storage exists,
         and does nothing if it hasn't.


    2:   When Tcl_Finalize is called on a virgin thread,
           "tsdPtr" will now be zero, which wasn't possible
before.

         This requires one more change -- lower in Tcl_Finalize
           is the code

          >> if (tsdPtr->tclLibraryPath != NULL) {
          >> Tcl_DecrRefCount(tsdPtr->tclLibraryPath);
          >> tsdPtr->tclLibraryPath = NULL;
          >> }

         which dereferences "tsdPtr" without checking it.
         Adding an additional

          >> if (tsdPtr)

         in front of this, protects against the "tsdPtr==0"
case.


  As far as I can tell, that's enough.   ??
  .. and my C++ program
     (linked with the Tcl/Tk Dlls) no longer crashes on exit;

  .. and tclsh83, (also linked with this Dll)
     can be CLOSE'd fine.

                     -- -- -- -- -- -- -- --

  I hesitate to go further .. because I must admit I don't
understand
    all the stuff in Tcl_Finalize ..

     those calls to TclFinalizeCompExecEnv()
                and TclFinalizeSynchronization(),
                just to name two .. are a mystery to me
                and I've grown too tired to track them,

     so I can't tell if there's any other reason that I'm not
     currently understanding -- why Tcl_Finalize should need
     to be called on a tcl thread.

  But I've made the above changes in my own copy, and it
seems better...


 
===================================================================
 
===================================================================

Logged In: YES 
user_id=7549

I guess so.  The scary part is that the simplicity of 
tclAppInit.c will get destroyed and start taking on windows 
specifics.  It'll be a first to #include <windows.h>.

If it does, a real shell with colors and mouse support 
accessing the console API couldn't be far off...

Sorry for the delay with a reply.

Logged In: YES 
user_id=72656

is this still valid with the latest 8.4 cvs?

Logged In: YES 
user_id=7549

no time to do this.

File Added - 6398: noBoom.c

File Added - 6397: noBoom.msg

Logged In: YES 
user_id=7549

here's another follow-up describing a users difficulty with 
this issue and editted code to show that this method works.

Logged In: YES 
user_id=7549

Same as https://sourceforge.net/tracker/index.php?
func=detail&aid=219245&group_id=10894&atid=110894

Logged In: YES 
user_id=7549

This bug only happens with a threaded build of tclsh