[Tcl-bugs] [ tcl-Bugs-2001201 ] Tcl_Exit() shouldn't call the full Tcl_Finalize() path

[SourceForge.net <no...@so...>]

Bugs item #2001201, was opened at 2008-06-23 22:53
Message generated for change (Comment added) made by ferrieux
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=110894&aid=2001201&group_id=10894

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: 49. Threading
Group: obsolete: 8.6a0
Status: Open
Resolution: None
Priority: 7
Private: No
Submitted By: George Peter Staplin (georgeps)
Assigned to: Kevin B KENNY (kennykb)
Summary: Tcl_Exit() shouldn't call the full Tcl_Finalize() path

Initial Comment:
Tcl_Exit() often results in segfaults on exit, when more than 1 thread is busy at the time of a Tcl_Exit().  The problem is that Tcl_Finalize cleans up the state used by active threads.  This problem has existed for a long time.  The comments in Tcl_Finalize indicate the expectation of 1 thread "there really should only be one thread alive at this moment."

I propose this possible solution:
1. move the invoking in Tcl_Finalize of Tcl exit handlers into TclInvokeExitHandlers();  

2. Tcl_Finalize can call TclInvokeExitHandlers()  

3. Tcl_Exit calls TclInvokeExitHandlers() instead of Tcl_Finalize().


Tcl_Finalize will still work for those that wish to use it, and Tcl_Exit will work in the multiple active threads case.

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

>Comment By: Alexandre Ferrieux (ferrieux)
Date: 2009-01-27 08:41

Message:
George, now Jeff has okayed my patch and it's been committed. So you can
update yours I guess, and ping Jeff.

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

Comment By: Alexandre Ferrieux (ferrieux)
Date: 2009-01-20 23:50

Message:
After a chat with George I believe this is a good cleanup, by analogy with
the accepted wisdom regarding the interaction between threads and fork():
in all cases it is impractical to organize an orderly cleanup of an
arbitrary graph of mutexed threads *before* the "singularity" (exit/fork).
So better just let it blow the can of worms away (either by exit() or by
exec() after fork()).

Who's giving the green light for George to commit ?

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

Comment By: George Peter Staplin (georgeps)
Date: 2008-09-18 23:06

Message:
das, I'm not sure if the Tk deadlock is related.  Can you test rev 5 of the
patch with Tk to see if it fixes the deadlock with a threaded build?

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

Comment By: George Peter Staplin (georgeps)
Date: 2008-09-18 23:03

Message:
Reassigning to kbk, because he has some time/interest in looking into
this.

I'm not ready yet to commit the change, because it seems too simple, and
other developers haven't known the area as well.

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

Comment By: Daniel A. Steffen (das)
Date: 2008-08-16 01:40

Message:
Logged In: YES 
user_id=90580
Originator: NO

is the window-2.9 deadlock in threaded Tk relevant here? (Tk bugs 1715716 
& 740570)

I wrote there:

> The problem is definitely caused by nested invocation of [exit] from
the
> <Destroy> handlers; not sure how to go about a fix in Tk, DestroyNotify
> events explicitly ignore TK_ALREADY_DEAD...
> 
> The easiest option might be to check inFinalize before setting it in
> Tcl_Finalize(), or equivalently, to check TclInExit() in Tcl_Exit()
before
> calling Tcl_Finalize(), but I don't understand tcl finalization well
enough
> to know if that would have undesireable effects.



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

Comment By: George Peter Staplin (georgeps)
Date: 2008-08-15 19:25

Message:
Logged In: YES 
user_id=585068
Originator: YES

Below is another test case that sometimes segfaults before the patch (the
number of runs before a fault varies, but generally once every 5).  After
the patch I haven't been able to duplicate the problem.

package require Thread

thread::create {
   set i 0
   while 1 {
       puts $i
       incr i
   }
}
#now for the Tcl_Finalize race

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

Comment By: George Peter Staplin (georgeps)
Date: 2008-08-15 19:19

Message:
Logged In: YES 
user_id=585068
Originator: YES

Here is a simple Thread test case that faults before the patch is applied,
due to the exit of the main thread causing the other threads to have
invalid state during Tcl_Finalize, but before TclpExit.

package require Thread

set script {
  puts [time {
    set total 0 ; for {set i 0} {$i < 10000000} {incr i} {incr total $i} ;
set total
  }]
}


set t1 [thread::create]
set t2 [thread::create]

thread::send -async $t1 $script
thread::send -async $t2 $script

----
$ tclsh8.6 tcl_thread_speed_test.tcl 
Segmentation fault (core dumped)

(gdb) bt
#0  0xb7e99a04 in __pthread_mutex_unlock_usercnt ()
   from /lib/tls/i686/cmov/libpthread.so.0
#1  0xb7f9b74f in Tcl_MutexUnlock () from /usr/local/lib/libtcl8.6.so
#2  0xb7f9c55f in Tcl_WaitForEvent () from /usr/local/lib/libtcl8.6.so
#3  0xb7f64726 in Tcl_DoOneEvent () from /usr/local/lib/libtcl8.6.so
#4  0xb7485027 in ThreadWaitObjCmd ()
   from /usr/local/lib/thread2.6.5/libthread2.6.5.so
#5  0xfffffffd in ?? ()
#6  0xb7fbf324 in ?? () from /usr/local/lib/libtcl8.6.so
#7  0xb7fc65ec in tclTomMathConstStubsPtr () from
/usr/local/lib/libtcl8.6.so
#8  0xb7f6f4c0 in ?? () from /usr/local/lib/libtcl8.6.so
Cannot access memory at address 0xfca3bad3


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

Comment By: George Peter Staplin (georgeps)
Date: 2008-08-15 05:01

Message:
Logged In: YES 
user_id=585068
Originator: YES

I updated the patch for the HEAD, and now we have revision 5.  The patch
for some reason is more readable than the last, most likely due to
whitespace changes.

It's unclear to me whether or not we should have an inFinalize variable
and use it within TclInitSubsystems for use with the previous Tcl_Panic
pattern, that now only panics when inExit is true.

File Added: tclEvent_ExitChange-5.patch

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

Comment By: George Peter Staplin (georgeps)
Date: 2008-08-01 19:19

Message:
Logged In: YES 
user_id=585068
Originator: YES

I solved the problem in almost the way stated in this report.  I had to
call the channel finalization function, so that the Tcl_Channels get
flushed in Tcl_Exit.   It passes all tests now.
File Added: tclEvent_ExitChange-4.patch

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

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=110894&aid=2001201&group_id=10894