# ticketchng

login: ferrieux
username: ferrieux
mimetype: text/plain
icomment:
  I don't know if this is relevant but... in the tpool.tcl code, in tpool::create, when minworkers is nonzero, the following loop is done outside any lock:

      for {set ii 0} {$ii < [tsv::set $tpid -minworkers]} {incr ii} {
          Worker $tpid
      }

  But the comment for Worker says

      This procedure must be executed under the tsv lock.

  Maybe we could just run this loop inside a tsv::lock block ?


login: ferrieux
username: ferrieux
mimetype: text/plain
icomment:
  Hmm, just realized that in the gdb output the TpoolWorker  function shows that this is the C version of [tpool], not the script version. Forget about my comment (unless the C follows the same logic).


login: ferrieux
username: ferrieux
mimetype: text/plain
icomment:
  Hey, after checking the C code in threadPoolCmd.c, I can witness the logic is exactly the same. And the same flaw is there:

      for (ii = 0; ii < tpoolPtr->minWorkers; ii++) {
          if (CreateWorker(interp, tpoolPtr) != TCL_OK) {
   
  outside any mutex, but CreateWorker is documented as:

        Assumes the caller ho(l)ds the pool mutex.

  So it seems it lacks a pair of 

        Tcl_MutexLock/Unlock(&tpoolPtr->mutex);

  around the loop (but possibly around the whole creation routine. I need Zoran's help here ;-)


login: nathanjett
username: nathanjett
mimetype: text/plain
icomment:
  Alex,

  It appears your suggestion fixes the flaw.  I added a Tcl_MutexLock/Unlock(&tpoolPtr->mutex); around the loop you indicated and recompiled.  I now have no problem spinning off 100+ worker threads in a tpool and my larger application works fine now.  Thanks again for your help!

  Nathan


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  There you go. There was a race condition at that place.
  The protecting mutex pair needs to be put arround the
  while look that creates pool workers, as its done elsewhere. 
  Also if one of the workers fails to initialize, the entire partially
  created pool needs to be destroyed. I commited the fix in HEAD.
  Thanks for looking into that.

  Cheers
  Zoran


login: ferrieux
username: ferrieux
mimetype: text/plain
icomment:
  Zoran, I think the same fix should be applied to tpool.tcl. Just add a tsv::lock around the minworkers loop. You could also do the same cleanup-on-error as you added on the C side.


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  I haven't look at the Tcl implementation. This was just an exercise to 
  show what you can build from pieces provided by the toolbox. The C
  version is really the one people should use.

  But if you insist... I will look into it.
  Cheers
  Zoran


login: ferrieux
username: ferrieux
mimetype: text/plain
icomment:
  I do :-)
  The reason is that the Tcl version serves as a tutorial to understand the C code. Leaving it buggy would be misleading. *And* I doubt you need anybody's help to fix it :-)


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  Yes. This is still wrong in the 2.5.1 
   
  I'm trying to catch up with 2.6 which corrects this problem. 
  I still need to cleanup the docs and write test-suite for 
  tpool and tsv commands.  
  As the matter of fact, I can already check-in the changes  
  in the head branch so you can test it out. 
   
  Given the high-load I have at work, I cannot promise to 
  release 2.6 in next 3-4 weeks. But, the cvs head should be stable 
  enough to use. Please do checkout the head in 2-3 days again 
  and give it a try. 
   
  I will leave this bug report open until I release the 2.6 version.  


login: tsanders
username: tsanders
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=1087228

  A quick test against HEAD using my testcase (and 
  application) seems to be working ok.  I'll let you know if any 
  issues pop up.

  I'll also watch for any further checkin's and track the changes.

  Thanks for the quick response and all the work on the thread 
  library too.


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  I have updated the cvs head with my latest changes. 
  Please try it again. Also, read the ChangeLog to see 
  what is new.

  This bug-report will be left open until I really release 
  the 2.6. On todo list are just tpool.test, tsv.test and
  ttrace.test
  files which need to be written new. 


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  This is a Tcl8.3.5 issue. The 8.4+ does not core 
  so I'm closing this. 
  However, this bug exposed another one in the threading 
  extension and I have corrected that one in the thread 
  tree. The thing was that we were wrongly triggering  
  error at executing scripts which contained [return] 
  command as the last command in the script. This is not 
  compatible to the way Tcl is doing things so I've  
  changed the behaviour to always return the result code 
  as-is to caller and trigger error only for TCL_ERROR  
  return code. 


login: arius-marius
username: arius-marius
mimetype: text/plain
icomment:
  Not reproducible with...

  Tcl Version: 8.6b2
  OS Platform and Version: Windows 7 (64 bit)
  Thread Package: 2.6.7 (Windows)


login: arius-marius
username: arius-marius
mimetype: text/plain
icomment:
  Set the following environment variable on the command line at Windows...

  set TCL_FINALIZE_ON_EXIT=1

  ...and executed the script again with Tcl version 8.6b2.

  => No output on the command line appeared pointing to finalization problems.


login: dgp
username: dgp
mimetype: text/plain
icomment:
  Note that due to variations in thread scheduling, this
  demo script is not deterministic.  The fact that it runs
  once without failing doesn't confirm correct operations.

  Making several runs with combinations of the 8.5 and
  trunk branch tips as well as Thread 2.6.7 and 2.7b1
  I am able to see either panics or segfaults in all combinations
  when TCL_FINALIZE_ON_EXIT is set.

  Using Tcl 8.6's default setting where thread finalization is skipped
  does appear to avoid the problem.  So in some sense, this isn't 
  really fixed, but 8.6 works around it.


login: ferrieux
username: ferrieux
mimetype: text/plain
icomment:
  Yes. This is, as you remember, the very motivation for the 8.6 move (bug 2001201).
  Bottom line: the OS's exit() is the fastest and most rational way of exiting, especially in a multithreaded setup since the ordered untangling of a locking graph is in general intractable.

  Note that the existence of bugs in thread finalization, when forced, is an orthogonal matter; we didn't even need them to prefer quick-exits !


login: anonymous
username: Adrián Medraño Calvo <support@prs.de>
mimetype: text/plain
icomment:
  This seems to still occur on 8.6.3.  The following script:

  ~~~
  package require Thread;
  set delay 5;
  set t1 [::thread::create ::thread::wait]
  set t2 [::thread::create ::thread::wait]
  ::thread::send $t1 [list ::after $delay {::tsv::set sv k "somevalue"}];
  ::thread::send $t2 [list ::nonexistingcommand];
  ~~~

  SEGFAULTs fairly consistently (for what it seems to be a race-condition) when running it under Wish (had no luck trying to reproduce it under Tclsh).  You might need to tune the delay.

  Below is a backtrace:

  ~~~
  #0  0x00007fdb2ef061b9 in Sp_RecursiveMutexLock (muxPtr=0x600) at /prod/g463/src/tcl8.6.3/pkgs/thread2.7.1/generic/threadSpCmd.c:1529
  #1  0x00007fdb2ef00cce in LockArray (interp=0x7fdb28000cd0, array=0x7fdb28037830 "sv", flags=0)
      at /prod/g463/src/tcl8.6.3/pkgs/thread2.7.1/generic/threadSvCmd.c:721
  #2  0x00007fdb2ef002f7 in Sv_GetContainer (interp=0x7fdb28000cd0, objc=4, objv=0x7fdb280051a0, retObj=0x7fdb2eef7b58, offset=0x7fdb2eef7b48, flags=0)
      at /prod/g463/src/tcl8.6.3/pkgs/thread2.7.1/generic/threadSvCmd.c:329
  #3  0x00007fdb2ef02bde in SvSetObjCmd (arg=0x0, interp=0x7fdb28000cd0, objc=4, objv=0x7fdb280051a0)
      at /prod/g463/src/tcl8.6.3/pkgs/thread2.7.1/generic/threadSvCmd.c:1669
  #4  0x00007fdb3204fce7 in TclNRRunCallbacks (interp=interp@entry=0x7fdb28000cd0, result=0, rootPtr=0x7fdb280224b0)
      at /prod/g463/src/tcl8.6.3/generic/tclBasic.c:4392
  #5  0x00007fdb3205269c in TclEvalObjEx (interp=interp@entry=0x7fdb28000cd0, objPtr=<optimized out>, flags=flags@entry=131072, invoker=invoker@entry=0x0, 
      word=word@entry=0) at /prod/g463/src/tcl8.6.3/generic/tclBasic.c:5958
  #6  0x00007fdb320526aa in Tcl_EvalObjEx (interp=interp@entry=0x7fdb28000cd0, objPtr=<optimized out>, flags=flags@entry=131072)
      at /prod/g463/src/tcl8.6.3/generic/tclBasic.c:5939
  #7  0x00007fdb321317e0 in AfterProc (clientData=0x7fdb28039b10) at /prod/g463/src/tcl8.6.3/generic/tclTimer.c:1191
  #8  0x00007fdb32131add in TimerHandlerEventProc (evPtr=evPtr@entry=0x7fdb2802eb30, flags=flags@entry=-3) at /prod/g463/src/tcl8.6.3/generic/tclTimer.c:593
  #9  0x00007fdb32112cc7 in Tcl_ServiceEvent (flags=flags@entry=-3) at /prod/g463/src/tcl8.6.3/generic/tclNotify.c:670
  #10 0x00007fdb32112fc9 in Tcl_DoOneEvent (flags=-3) at /prod/g463/src/tcl8.6.3/generic/tclNotify.c:967
  #11 0x00007fdb2eefe267 in ThreadWait (interp=0x7fdb28000cd0) at /prod/g463/src/tcl8.6.3/pkgs/thread2.7.1/generic/threadCmd.c:2853
  #12 0x00007fdb2eefb558 in ThreadWaitObjCmd (dummy=0x0, interp=0x7fdb28000cd0, objc=1, objv=0x7fdb28004fc0)
      at /prod/g463/src/tcl8.6.3/pkgs/thread2.7.1/generic/threadCmd.c:1142
  #13 0x00007fdb3204fce7 in TclNRRunCallbacks (interp=interp@entry=0x7fdb28000cd0, result=0, rootPtr=0x0) at /prod/g463/src/tcl8.6.3/generic/tclBasic.c:4392
  #14 0x00007fdb3204fd7f in Tcl_EvalObjv (interp=interp@entry=0x7fdb28000cd0, objc=objc@entry=1, objv=objv@entry=0x7fdb28004fc0, flags=flags@entry=2097168)
      at /prod/g463/src/tcl8.6.3/generic/tclBasic.c:4123
  #15 0x00007fdb32051e5f in TclEvalEx (interp=0x7fdb28000cd0, script=0x7fdb28037c70 "::thread::wait", numBytes=<optimized out>, flags=<optimized out>, 
      line=line@entry=1, clNextOuter=clNextOuter@entry=0x0, outerScript=0x7fdb28037c70 "::thread::wait") at /prod/g463/src/tcl8.6.3/generic/tclBasic.c:5261
  #16 0x00007fdb32052176 in Tcl_EvalEx (interp=<optimized out>, script=<optimized out>, numBytes=<optimized out>, flags=<optimized out>)
      at /prod/g463/src/tcl8.6.3/generic/tclBasic.c:4926
  #17 0x00007fdb2eefc87e in NewThread (clientData=0x7fffd35d3d40) at /prod/g463/src/tcl8.6.3/pkgs/thread2.7.1/generic/threadCmd.c:1833
  #18 0x00007fdb316fd182 in start_thread (arg=0x7fdb2eef8700) at pthread_create.c:312
  #19 0x00007fdb31d4247d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:111
  ~~~


login: anonymous
username: Adrián Medraño Calvo <support@prs.de>
mimetype: text/plain
icomment:
  Thank you for your response.

  I must say that I don't see the relation to the pointed out issue, but admittedly I don't know enough about the internals.

  I've been investigating a little bit.  The issue seems to be caused by Sv_Finalize being invoked prior to setting the tsv::array's value.  That is, when `Sv_GetContainer` is invoked the `static Bucket* buckets` is already NULL, and no checks are performed on LockArray.  This seems to be the source of the SEGFAULT.

  Something worth pointing out is that SV_FINALIZE is unconditionally defined in the module, despite some comments warning against:

  ~~~
  #define SV_FINALIZE
  #ifdef SV_FINALIZE
  static void SvFinalizeContainers(Bucket*);
  static void SvFinalize(ClientData);
  #endif /* SV_FINALIZE */

  [...]
  #ifdef SV_FINALIZE
  /*
   * Left for reference, but unused since multithreaded finalization is
   * unsolvable in the general case. Brave souls can revive this by
   * installing a late exit handler on Thread's behalf, bringing the
   * function back onto the Tcl_Finalize (but not Tcl_Exit) path.
   */
  ~~~
  [...]
  static void
  SvFinalize (ClientData clientData)
  {
  [...]
  #endif

  Additionally, want to mention that I've been able to reproduce using Tclsh as well: one needs to prepend 'package require Tk` to the script.  This hints at some bad interaction between Tk and thread finalization...

  Again, thank you for your help.

  Best regards,
  Adrián.


login: anonymous
username: Adrián Medraño Calvo <support@prs.de>
mimetype: text/plain
icomment:
  The patch below modifies LockArray to ensure that `buckets`
  has not been cleaned up, returning an error if it has.
  Additionally, the exit handler is added conditional on
  `SV_FINALIZE` being defined, for consistency.


  diff --git i/src/tcl8.6.3/pkgs/thread2.7.1/generic/threadSvCmd.c w/src/tcl8.6.3/pkgs/thread2.7.1/generic/threadSvCmd.c
  index b8d90c0..1bccca7 100644
  --- i/src/tcl8.6.3/pkgs/thread2.7.1/generic/threadSvCmd.c
  +++ w/src/tcl8.6.3/pkgs/thread2.7.1/generic/threadSvCmd.c
  @@ -698,40 +698,47 @@ LockArray(
       register unsigned int result;
       register int i;
       Bucket *bucketPtr;
  -    Array *arrayPtr;
  +    Array *arrayPtr = NULL;
   
       /*
        * Compute a hash to map an array to a bucket.
        */
   
  -    p = array;
  -    result = 0;
  -    while (*p++) {
  -        i = *p;
  -        result += (result << 3) + i;
  -    }
  -    i = result % NUMBUCKETS;
  -    bucketPtr = &buckets[i];
  -
  -    /*
  -     * Lock the bucket and find the array, or create a new one.
  -     * The bucket will be left locked on success.
  -     */
  -
  -    LOCK_BUCKET(bucketPtr); /* Note: no matching unlock below ! */
  -    if (flags & FLAGS_CREATEARRAY) {
  -        arrayPtr = CreateArray(bucketPtr, array);
  -    } else {
  -        Tcl_HashEntry *hPtr = Tcl_FindHashEntry(&bucketPtr->arrays, array);
  -        if (hPtr == NULL) {
  -            UNLOCK_BUCKET(bucketPtr);
  -            if (!(flags & FLAGS_NOERRMSG)) {
  -                Tcl_AppendResult(interp, "\"", array,
  -                                 "\" is not a thread shared array", NULL);
  -            }
  -            return NULL;
  +    if (buckets != NULL) {
  +        p = array;
  +        result = 0;
  +        while (*p++) {
  +            i = *p;
  +            result += (result << 3) + i;
  +        }
  +        i = result % NUMBUCKETS;
  +        bucketPtr = &buckets[i];
  +
  +        /*
  +         * Lock the bucket and find the array, or create a new one.
  +         * The bucket will be left locked on success.
  +         */
  +
  +        LOCK_BUCKET(bucketPtr); /* Note: no matching unlock below ! */
  +        if (flags & FLAGS_CREATEARRAY) {
  +            arrayPtr = CreateArray(bucketPtr, array);
  +        } else {
  +            Tcl_HashEntry *hPtr = Tcl_FindHashEntry(&bucketPtr->arrays, array);
  +            if (hPtr != NULL) {
  +                arrayPtr = (Array*)Tcl_GetHashValue(hPtr);
  +            } else {
  +                UNLOCK_BUCKET(bucketPtr);
  +                if (!(flags & FLAGS_NOERRMSG)) {
  +                    Tcl_AppendResult(interp, "\"", array,
  +                                     "\" is not a thread shared array", NULL);
  +                }
  +            }
  +        }
  +    } else {
  +        if (!(flags & FLAGS_NOERRMSG)) {
  +            Tcl_AppendResult(interp, "\"", array,
  +                             "\" has been finalized", NULL);
           }
  -        arrayPtr = (Array*)Tcl_GetHashValue(hPtr);
       }
   
       return arrayPtr;
  @@ -2184,7 +2191,9 @@ Sv_Init (interp)
           Tcl_MutexLock(&bucketsMutex);
           if (buckets == NULL) {
               buckets = (Bucket *)ckalloc(sizeof(Bucket) * NUMBUCKETS);
  +#if defined(SV_FINALIZE)
   	    Tcl_CreateExitHandler(SvFinalize, NULL);
  +#endif /* SV_FINALIZE */
   
               for (i = 0; i < NUMBUCKETS; ++i) {
                   bucketPtr = &buckets[i];


login: anonymous
username: Adrián Medraño Calvo <support@prs.de>
mimetype: text/plain
icomment:
  Hello, any thoughts on this?  The patch is quite simple, but wraps most of the function in a conditional causing many white-space changes.  Follows a version of the patch without white-space changes for easier review:

  --- i/src/tcl8.6.3/pkgs/thread2.7.1/generic/threadSvCmd.c
  +++ w/src/tcl8.6.3/pkgs/thread2.7.1/generic/threadSvCmd.c
  @@ -700,3 +700,3 @@
       Bucket *bucketPtr;
  -    Array *arrayPtr;
  +    Array *arrayPtr = NULL;
   
  @@ -706,2 +706,3 @@
   
  +    if (buckets != NULL) {
       p = array;
  @@ -725,3 +726,5 @@
           Tcl_HashEntry *hPtr = Tcl_FindHashEntry(&bucketPtr->arrays, array);
  -        if (hPtr == NULL) {
  +            if (hPtr != NULL) {
  +                arrayPtr = (Array*)Tcl_GetHashValue(hPtr);
  +            } else {
               UNLOCK_BUCKET(bucketPtr);
  @@ -731,5 +734,9 @@
               }
  -            return NULL;
           }
  -        arrayPtr = (Array*)Tcl_GetHashValue(hPtr);
  +        }
  +    } else {
  +        if (!(flags & FLAGS_NOERRMSG)) {
  +            Tcl_AppendResult(interp, "\"", array,
  +                             "\" has been finalized", NULL);
  +        }
       }
  @@ -2186,3 +2193,5 @@
               buckets = (Bucket *)ckalloc(sizeof(Bucket) * NUMBUCKETS);
  +#if defined(SV_FINALIZE)
   	    Tcl_CreateExitHandler(SvFinalize, NULL);
  +#endif /* SV_FINALIZE */


login: gahr
username: gahr
mimetype: text/plain
icomment:
  Hi, I can't reproduce with Thread 2.7.3 running with Tcl 8.6.5 and Tcl 8.5.19. Can you try to upgrade?


login: adrianmedranocalvo
username: Adrián Medraño Calvo <support@prs.de>
mimetype: text/plain
icomment:
  Hmm, the bug is certainly slippy, but I am able to reproduce it with Tcl 8.6.5 and Thread 2.7.3.  Please, store the following version of the previous testcase in some file (the only change w.r.t. previous version is that it accepts a delay as command line argument):

  ~~~~~ bug.tcl
  package require Thread;
  set delay [lindex $::argv 0];
  set t1 [::thread::create ::thread::wait]
  set t2 [::thread::create ::thread::wait]
  ::thread::send $t1 [list ::after $delay {::tsv::set sv k "somevalue"}];
  ::thread::send $t2 [list ::nonexistingcommand];
  ~~~~~~

  Then run the following (quite convoluted) pipeline that invokes the script with a random delay.  The `parallel' utility seems to help reproducing it, perhaps due to increased system load.  catchsegv makes sure we get some output mentioning the SEGFAULT.

  ~~~
  tr -dc '0-9' </dev/urandom | fold -w1 | parallel catchsegv tclsh bug.tcl 2>&1 | grep Segment
  ~~~

  You should start seeing some lines such as the following:

  ~~~
  *** Segmentation fault
  ~~

  Thank you for having a look.

  Best regards,
  Adrián.


login: gahr
username: gahr
mimetype: text/html
icomment:
  Thank you, I was able to replicate running your script through this driver:

  <pre>
  while {1} {
      set delay [expr {int(rand() * 10)}]
      catch {exec [info nameofexecutable] test3.tcl $delay} err
  }
  </pre>

  and I was able to get a core dump. I'll look into it!


login: gahr
username: gahr
mimetype: text/plain
icomment:
  I added a patch that solves the problem using a different approach, that is, keeping count of how many threads have registered and avoiding removing shared data until the last one unregisters. This seems to solve this specific problem (I can't reproduce the segfault) and frees us from having to put checks for buckets == NULL at all entry points.


login: adrianmedranocalvo
username: Adrián Medraño Calvo <support@prs.de>
mimetype: text/plain
icomment:
  The approach is definitely cleaner, thank you.

  It seems to me that exit handlers created with Tcl_CreateExitHandler are only ever invoked once, upon Tcl_Finalize (or Tcl_Exit).  Shouldn't we instead register with Tcl_CreateThreadExitHandler?  Not that it affects this particular issue, but otherwise the following script would not invoke the finalization:

  ~~~ (run with tclsh)
  package require Thread
  thread::release [thread::create];
  while {[incr i] < 10000} {}; # needed for some reason
  ~~~

  I suggest the following changes on top of yours:

  ~~~
  diff -u generic/threadSvCmd.c generic/threadSvCmd.c
  --- generic/threadSvCmd.c
  +++ generic/threadSvCmd.c
  @@ -21,6 +21,8 @@
   #include "psGdbm.h"             /* The gdbm persistent store implementation */
   #include "psLmdb.h"             /* The lmdb persistent store implementation */
   
  +#define SV_FINALIZE
  +
   /*
    * Number of buckets to spread shared arrays into. Each bucket is
    * associated with one mutex so locking a bucket locks all arrays
  @@ -59,8 +61,10 @@
    * Global variables used within this file.
    */
   
  +#ifdef SV_FINALIZE
   static size_t     nofThreads;      /* Number of initialized threads */
   static Tcl_Mutex  nofThreadsMutex; /* Protects the nofThreads variable */
  +#endif
   
   static Bucket*    buckets;      /* Array of buckets. */
   static Tcl_Mutex  bucketsMutex; /* Protects the array of buckets */
  @@ -116,7 +120,6 @@
   static void SvAllocateContainers(Bucket*);
   static void SvRegisterStdCommands(void);
   
  -#define SV_FINALIZE
   #ifdef SV_FINALIZE
   static void SvFinalizeContainers(Bucket*);
   static void SvFinalize(ClientData);
  @@ -2133,12 +2136,14 @@
       const Tcl_UniChar no[3] = {'n', 'o', 0} ;
       Tcl_Obj *obj;
   
  +#ifdef SV_FINALIZE
       /*
        * Increment number of threads
        */
       Tcl_MutexLock(&nofThreadsMutex);
       ++nofThreads;
       Tcl_MutexUnlock(&nofThreadsMutex);
  +#endif
   
       /*
        * Add keyed-list datatype
  @@ -2199,9 +2204,6 @@
           Tcl_MutexLock(&bucketsMutex);
           if (buckets == NULL) {
               buckets = (Bucket *)ckalloc(sizeof(Bucket) * NUMBUCKETS);
  -#ifdef SV_FINALIZE
  -            Tcl_CreateExitHandler(SvFinalize, NULL);
  -#endif
   
               for (i = 0; i < NUMBUCKETS; ++i) {
                   bucketPtr = &buckets[i];
  @@ -2234,6 +2236,10 @@
           Tcl_MutexUnlock(&bucketsMutex);
       }
   
  +#ifdef SV_FINALIZE
  +    Tcl_CreateThreadExitHandler(SvFinalize, NULL);
  +#endif
  +
       return TCL_OK;
   }
  ~~~

  What do you think?

  ---

  Relatedly, there is this disconcerting comment in that file, just above SvFinalize:

  ~~~
  #ifdef SV_FINALIZE
  /*
   * Left for reference, but unused since multithreaded finalization is
   * unsolvable in the general case. Brave souls can revive this by
   * installing a late exit handler on Thread's behalf, bringing the
   * function back onto the Tcl_Finalize (but not Tcl_Exit) path.
   */
  ~~~

  ... which contradicts the "#define SV_FINALIZE" being unconditionally defined at the top of the file.  Should it actually be disabled!?

  Best regards,
  Adrián.


login: gahr
username: gahr
mimetype: text/plain
icomment:
  You are right, the cleanup routine must definitely be installed in the thread's exit handler, not the process'. However, I can't get the routine to be reliably called at every thread's exit, which is basically equivalent to not calling it at all. At this point, I'm more towards disabling it altogether. Thoughts?


login: adrianmedranocalvo
username: Adrián Medraño Calvo <support@prs.de>
mimetype: text/x-fossil-wiki
icomment:
  In my opinion different aspects should be handled separately.

  This issue was about a SEGFAULT under rare circunstances (threads, errors, exit, tsv access).  As far as I can see the changes discussed in previous messages prevent the SEGFAULT.

  If thread exit handlers do not run reliably a new ticket should be opened with some details.  Using the following script with printfs in SvFinalize, it seems to run every time:

  ~~~
  package require Thread
  set tid [thread::create -joinable];
  ::thread::release $tid;
  ::thread::join $tid;
  ::exit
  ~~~

  With regards to enabling or disabling thread finalization altogether, I would suggest contacting the authors the reasons for the comment and whether it has been considered before enabling the feature.  See [fd09ee731a3707e2d89414e1b04483abcbbb3291] and [072cffeecf3af65d6d60149c3e0fca67d6292c83], where it is disabled; and later  [606750884097194eb1de16fc7d59ad8975105974], where it is enabled.  It would seem to me that the comment is obsolete.

  Best regards,
  Adrián.


login: gahr
username: gahr
mimetype: text/plain
icomment:
  I 100% agree with you. Thanks for bringing me back on track :)

  I have attached a revised version of the patch (3532972fff-2.diff.txt). I kindly ask for a final ack from your part before I commit this.

  Thanks!


login: adrianmedranocalvo
username: Adrián Medraño Calvo <support@prs.de>
mimetype: text/plain
icomment:
  I would suggest wrapping all changes in:

  #ifdef SV_FINALIZE
  ...
  #endif

  as the thread counter is only decreased in SvFinalize, which is itself wrapped.  Other than that, it looks fine.  I have attached your patch with these changes, in case it helps.

  Best regards,
  Adrián.


login: nobody
username: nobody
mimetype: text/plain
icomment:
  Logged In: NO 

  this is the exception reported by windows:
  <?xml version="1.0" encoding="UTF-16"?>
  <DATABASE>
  <EXE NAME="tclsh84t.exe" 
  FILTER="GRABMI_FILTER_PRIVACY">
      <MATCHING_FILE NAME="iconv.dll" SIZE="876544" 
  CHECKSUM="0xF934495E" MODULE_TYPE="WIN32" 
  PE_CHECKSUM="0x0" LINKER_VERSION="0x0" 
  LINK_DATE="11/24/2003 20:23:05" 
  UPTO_LINK_DATE="11/24/2003 20:23:05" />
      <MATCHING_FILE NAME="libexslt.dll" SIZE="38912" 
  CHECKSUM="0xC0C3719C" MODULE_TYPE="WIN32" 
  PE_CHECKSUM="0x0" LINKER_VERSION="0x7" 
  LINK_DATE="11/24/2003 20:23:41" 
  UPTO_LINK_DATE="11/24/2003 20:23:41" />
      <MATCHING_FILE NAME="libxml2.dll" SIZE="618496" 
  CHECKSUM="0x888DD4B5" MODULE_TYPE="WIN32" 
  PE_CHECKSUM="0x0" LINKER_VERSION="0x20004" 
  LINK_DATE="11/24/2003 20:23:19" 
  UPTO_LINK_DATE="11/24/2003 20:23:19" />
      <MATCHING_FILE NAME="libxslt.dll" SIZE="128000" 
  CHECKSUM="0xE921AAD6" MODULE_TYPE="WIN32" 
  PE_CHECKSUM="0x0" LINKER_VERSION="0x10000" 
  LINK_DATE="11/24/2003 20:23:39" 
  UPTO_LINK_DATE="11/24/2003 20:23:39" />
      <MATCHING_FILE NAME="tcl84.dll" SIZE="573952" 
  CHECKSUM="0x2A02D8D6" BIN_FILE_VERSION="8.4.2.6" 
  BIN_PRODUCT_VERSION="8.4.2.6" 
  PRODUCT_VERSION="8.4.6" FILE_DESCRIPTION="Tcl DLL" 
  COMPANY_NAME="ActiveState Corporation" 
  PRODUCT_NAME="Tcl 8.4 for Windows" 
  FILE_VERSION="8.4.6" ORIGINAL_FILENAME="tcl84.dll" 
  LEGAL_COPYRIGHT="Copyright © 2001 by ActiveState 
  Corporation, et al" VERFILEDATEHI="0x0" 
  VERFILEDATELO="0x0" VERFILEOS="0x4" 
  VERFILETYPE="0x2" MODULE_TYPE="WIN32" 
  PE_CHECKSUM="0x94FF2" LINKER_VERSION="0x0" 
  UPTO_BIN_FILE_VERSION="8.4.2.6" 
  UPTO_BIN_PRODUCT_VERSION="8.4.2.6" 
  LINK_DATE="06/15/2004 10:16:19" 
  UPTO_LINK_DATE="06/15/2004 10:16:19" 
  VER_LANGUAGE="English (United States) [0x409]" />
      <MATCHING_FILE NAME="tcl84t.dll" SIZE="583680" 
  CHECKSUM="0x990ECE0F" BIN_FILE_VERSION="8.4.2.7" 
  BIN_PRODUCT_VERSION="8.4.2.7" 
  PRODUCT_VERSION="8.4.7" FILE_DESCRIPTION="Tcl DLL" 
  COMPANY_NAME="ActiveState Corporation" 
  PRODUCT_NAME="Tcl 8.4 for Windows" 
  FILE_VERSION="8.4.7" ORIGINAL_FILENAME="tcl84t.dll" 
  LEGAL_COPYRIGHT="Copyright © 2001 by ActiveState 
  Corporation, et al" VERFILEDATEHI="0x0" 
  VERFILEDATELO="0x0" VERFILEOS="0x4" 
  VERFILETYPE="0x2" MODULE_TYPE="WIN32" 
  PE_CHECKSUM="0x8FDE2" LINKER_VERSION="0x0" 
  UPTO_BIN_FILE_VERSION="8.4.2.7" 
  UPTO_BIN_PRODUCT_VERSION="8.4.2.7" 
  LINK_DATE="09/02/2004 10:52:23" 
  UPTO_LINK_DATE="09/02/2004 10:52:23" 
  VER_LANGUAGE="English (United States) [0x409]" />
      <MATCHING_FILE NAME="tclpip84.dll" SIZE="16384" 
  CHECKSUM="0xCC55750C" MODULE_TYPE="WIN32" 
  PE_CHECKSUM="0x0" LINKER_VERSION="0x0" 
  LINK_DATE="11/24/2003 20:11:14" 
  UPTO_LINK_DATE="11/24/2003 20:11:14" />
      <MATCHING_FILE NAME="tclsh.exe" SIZE="20480" 
  CHECKSUM="0x2B653635" BIN_FILE_VERSION="8.4.2.5" 
  BIN_PRODUCT_VERSION="8.4.2.5" 
  PRODUCT_VERSION="8.4.5" FILE_DESCRIPTION="Tclsh 
  Application" COMPANY_NAME="ActiveState Corporation" 
  PRODUCT_NAME="Tcl 8.4 for Windows" 
  FILE_VERSION="8.4.5" ORIGINAL_FILENAME="tclsh84.exe" 
  LEGAL_COPYRIGHT="Copyright © 2000 by ActiveState 
  Corporation, et al" VERFILEDATEHI="0x0" 
  VERFILEDATELO="0x0" VERFILEOS="0x4" 
  VERFILETYPE="0x1" MODULE_TYPE="WIN32" 
  PE_CHECKSUM="0x0" LINKER_VERSION="0x0" 
  UPTO_BIN_FILE_VERSION="8.4.2.5" 
  UPTO_BIN_PRODUCT_VERSION="8.4.2.5" 
  LINK_DATE="11/24/2003 20:11:15" 
  UPTO_LINK_DATE="11/24/2003 20:11:15" 
  VER_LANGUAGE="English (United States) [0x409]" />
      <MATCHING_FILE NAME="tclsh84.exe" SIZE="9216" 
  CHECKSUM="0x1AF9DEFB" BIN_FILE_VERSION="8.4.2.6" 
  BIN_PRODUCT_VERSION="8.4.2.6" 
  PRODUCT_VERSION="8.4.6" FILE_DESCRIPTION="Tclsh 
  Application" COMPANY_NAME="ActiveState Corporation" 
  PRODUCT_NAME="Tcl 8.4 for Windows" 
  FILE_VERSION="8.4.6" ORIGINAL_FILENAME="tclsh84.exe" 
  LEGAL_COPYRIGHT="Copyright © 2000 by ActiveState 
  Corporation, et al" VERFILEDATEHI="0x0" 
  VERFILEDATELO="0x0" VERFILEOS="0x4" 
  VERFILETYPE="0x1" MODULE_TYPE="WIN32" 
  PE_CHECKSUM="0xCC0A" LINKER_VERSION="0x0" 
  UPTO_BIN_FILE_VERSION="8.4.2.6" 
  UPTO_BIN_PRODUCT_VERSION="8.4.2.6" 
  LINK_DATE="06/15/2004 10:16:20" 
  UPTO_LINK_DATE="06/15/2004 10:16:20" 
  VER_LANGUAGE="English (United States) [0x409]" />
      <MATCHING_FILE NAME="tclsh84t.exe" SIZE="9216" 
  CHECKSUM="0xF9ABC310" BIN_FILE_VERSION="8.4.2.7" 
  BIN_PRODUCT_VERSION="8.4.2.7" 
  PRODUCT_VERSION="8.4.7" FILE_DESCRIPTION="Tclsh 
  Application" COMPANY_NAME="ActiveState Corporation" 
  PRODUCT_NAME="Tcl 8.4 for Windows" 
  FILE_VERSION="8.4.7" 
  ORIGINAL_FILENAME="tclsh84t.exe" 
  LEGAL_COPYRIGHT="Copyright © 2000 by ActiveState 
  Corporation, et al" VERFILEDATEHI="0x0" 
  VERFILEDATELO="0x0" VERFILEOS="0x4" 
  VERFILETYPE="0x1" MODULE_TYPE="WIN32" 
  PE_CHECKSUM="0xC546" LINKER_VERSION="0x0" 
  UPTO_BIN_FILE_VERSION="8.4.2.7" 
  UPTO_BIN_PRODUCT_VERSION="8.4.2.7" 
  LINK_DATE="09/02/2004 10:52:23" 
  UPTO_LINK_DATE="09/02/2004 10:52:23" 
  VER_LANGUAGE="English (United States) [0x409]" />
      <MATCHING_FILE NAME="tclsh84t_good.exe" 
  SIZE="9216" CHECKSUM="0x38B614E" 
  BIN_FILE_VERSION="8.4.2.6" 
  BIN_PRODUCT_VERSION="8.4.2.6" 
  PRODUCT_VERSION="8.4.6" FILE_DESCRIPTION="Tclsh 
  Application" COMPANY_NAME="ActiveState Corporation" 
  PRODUCT_NAME="Tcl 8.4 for Windows" 
  FILE_VERSION="8.4.6" 
  ORIGINAL_FILENAME="tclsh84t.exe" 
  LEGAL_COPYRIGHT="Copyright © 2000 by ActiveState 
  Corporation, et al" VERFILEDATEHI="0x0" 
  VERFILEDATELO="0x0" VERFILEOS="0x4" 
  VERFILETYPE="0x1" MODULE_TYPE="WIN32" 
  PE_CHECKSUM="0xADF1" LINKER_VERSION="0x0" 
  UPTO_BIN_FILE_VERSION="8.4.2.6" 
  UPTO_BIN_PRODUCT_VERSION="8.4.2.6" 
  LINK_DATE="07/13/2004 09:51:12" 
  UPTO_LINK_DATE="07/13/2004 09:51:12" 
  VER_LANGUAGE="English (United States) [0x409]" />
      <MATCHING_FILE NAME="tk84.dll" SIZE="1032192" 
  CHECKSUM="0xB499DFC9" BIN_FILE_VERSION="8.4.2.5" 
  BIN_PRODUCT_VERSION="8.4.2.5" 
  PRODUCT_VERSION="8.4.5" FILE_DESCRIPTION="Tk DLL" 
  COMPANY_NAME="ActiveState Corporation" 
  PRODUCT_NAME="Tk 8.4 for Windows" 
  FILE_VERSION="8.4.5" ORIGINAL_FILENAME="tk84.dll" 
  LEGAL_COPYRIGHT="Copyright © 2001 by ActiveState 
  Corporation, et al" VERFILEDATEHI="0x0" 
  VERFILEDATELO="0x0" VERFILEOS="0x4" 
  VERFILETYPE="0x2" MODULE_TYPE="WIN32" 
  PE_CHECKSUM="0x108836" LINKER_VERSION="0x0" 
  UPTO_BIN_FILE_VERSION="8.4.2.5" 
  UPTO_BIN_PRODUCT_VERSION="8.4.2.5" 
  LINK_DATE="11/24/2003 20:13:21" 
  UPTO_LINK_DATE="11/24/2003 20:13:21" 
  VER_LANGUAGE="English (United States) [0x409]" />
      <MATCHING_FILE NAME="wish.exe" SIZE="24576" 
  CHECKSUM="0x643137A7" BIN_FILE_VERSION="8.4.2.5" 
  BIN_PRODUCT_VERSION="8.4.2.5" 
  PRODUCT_VERSION="8.4.5" FILE_DESCRIPTION="Wish 
  Application" COMPANY_NAME="ActiveState Corporation" 
  PRODUCT_NAME="Tk 8.4 for Windows" 
  FILE_VERSION="8.4.5" ORIGINAL_FILENAME="wish84.exe" 
  LEGAL_COPYRIGHT="Copyright © 2000 by ActiveState 
  Corporation, et al" VERFILEDATEHI="0x0" 
  VERFILEDATELO="0x0" VERFILEOS="0x4" 
  VERFILETYPE="0x1" MODULE_TYPE="WIN32" 
  PE_CHECKSUM="0x0" LINKER_VERSION="0x0" 
  UPTO_BIN_FILE_VERSION="8.4.2.5" 
  UPTO_BIN_PRODUCT_VERSION="8.4.2.5" 
  LINK_DATE="11/24/2003 20:13:22" 
  UPTO_LINK_DATE="11/24/2003 20:13:22" 
  VER_LANGUAGE="English (United States) [0x409]" />
      <MATCHING_FILE NAME="wish84.exe" SIZE="24576" 
  CHECKSUM="0x643137A7" BIN_FILE_VERSION="8.4.2.5" 
  BIN_PRODUCT_VERSION="8.4.2.5" 
  PRODUCT_VERSION="8.4.5" FILE_DESCRIPTION="Wish 
  Application" COMPANY_NAME="ActiveState Corporation" 
  PRODUCT_NAME="Tk 8.4 for Windows" 
  FILE_VERSION="8.4.5" ORIGINAL_FILENAME="wish84.exe" 
  LEGAL_COPYRIGHT="Copyright © 2000 by ActiveState 
  Corporation, et al" VERFILEDATEHI="0x0" 
  VERFILEDATELO="0x0" VERFILEOS="0x4" 
  VERFILETYPE="0x1" MODULE_TYPE="WIN32" 
  PE_CHECKSUM="0x0" LINKER_VERSION="0x0" 
  UPTO_BIN_FILE_VERSION="8.4.2.5" 
  UPTO_BIN_PRODUCT_VERSION="8.4.2.5" 
  LINK_DATE="11/24/2003 20:13:22" 
  UPTO_LINK_DATE="11/24/2003 20:13:22" 
  VER_LANGUAGE="English (United States) [0x409]" />
      <MATCHING_FILE NAME="zlib.dll" SIZE="44544" 
  CHECKSUM="0x8F24B8EA" MODULE_TYPE="WIN32" 
  PE_CHECKSUM="0x0" LINKER_VERSION="0x10000" 
  LINK_DATE="08/13/2000 16:24:16" 
  UPTO_LINK_DATE="08/13/2000 16:24:16" />
  </EXE>
  <EXE NAME="tcl84t.dll" 
  FILTER="GRABMI_FILTER_THISFILEONLY">
      <MATCHING_FILE NAME="tcl84t.dll" SIZE="583680" 
  CHECKSUM="0x990ECE0F" BIN_FILE_VERSION="8.4.2.7" 
  BIN_PRODUCT_VERSION="8.4.2.7" 
  PRODUCT_VERSION="8.4.7" FILE_DESCRIPTION="Tcl DLL" 
  COMPANY_NAME="ActiveState Corporation" 
  PRODUCT_NAME="Tcl 8.4 for Windows" 
  FILE_VERSION="8.4.7" ORIGINAL_FILENAME="tcl84t.dll" 
  LEGAL_COPYRIGHT="Copyright © 2001 by ActiveState 
  Corporation, et al" VERFILEDATEHI="0x0" 
  VERFILEDATELO="0x0" VERFILEOS="0x4" 
  VERFILETYPE="0x2" MODULE_TYPE="WIN32" 
  PE_CHECKSUM="0x8FDE2" LINKER_VERSION="0x0" 
  UPTO_BIN_FILE_VERSION="8.4.2.7" 
  UPTO_BIN_PRODUCT_VERSION="8.4.2.7" 
  LINK_DATE="09/02/2004 10:52:23" 
  UPTO_LINK_DATE="09/02/2004 10:52:23" 
  VER_LANGUAGE="English (United States) [0x409]" />
  </EXE>
  <EXE NAME="kernel32.dll" 
  FILTER="GRABMI_FILTER_THISFILEONLY">
      <MATCHING_FILE NAME="kernel32.dll" SIZE="930304" 
  CHECKSUM="0xCBCCF8A9" 
  BIN_FILE_VERSION="5.1.2600.1106" 
  BIN_PRODUCT_VERSION="5.1.2600.1106" 
  PRODUCT_VERSION="5.1.2600.1106" 
  FILE_DESCRIPTION="Windows NT BASE API Client DLL" 
  COMPANY_NAME="Microsoft Corporation" 
  PRODUCT_NAME="Microsoft® Windows® Operating System" 
  FILE_VERSION="5.1.2600.1106 (xpsp1.020828-1920)" 
  ORIGINAL_FILENAME="kernel32" 
  INTERNAL_NAME="kernel32" LEGAL_COPYRIGHT="© 
  Microsoft Corporation. All rights reserved." 
  VERFILEDATEHI="0x0" VERFILEDATELO="0x0" 
  VERFILEOS="0x40004" VERFILETYPE="0x2" 
  MODULE_TYPE="WIN32" PE_CHECKSUM="0xE7ED3" 
  LINKER_VERSION="0x50001" 
  UPTO_BIN_FILE_VERSION="5.1.2600.1106" 
  UPTO_BIN_PRODUCT_VERSION="5.1.2600.1106" 
  LINK_DATE="08/29/2002 10:40:40" 
  UPTO_LINK_DATE="08/29/2002 10:40:40" 
  VER_LANGUAGE="English (United States) [0x409]" />
  </EXE>
  </DATABASE>


login: nobody
username: nobody
mimetype: text/plain
icomment:
  Logged In: NO 

  microsoft report:
  <?xml version="1.0" encoding="UTF-16"?>
  <DATABASE>
  <EXE NAME="tclsh84t.exe" 
  FILTER="GRABMI_FILTER_PRIVACY">
      <MATCHING_FILE NAME="iconv.dll" SIZE="876544" 
  CHECKSUM="0xF934495E" MODULE_TYPE="WIN32" 
  PE_CHECKSUM="0x0" LINKER_VERSION="0x0" 
  LINK_DATE="11/24/2003 20:23:05" 
  UPTO_LINK_DATE="11/24/2003 20:23:05" />
      <MATCHING_FILE NAME="libexslt.dll" SIZE="38912" 
  CHECKSUM="0xC0C3719C" MODULE_TYPE="WIN32" 
  PE_CHECKSUM="0x0" LINKER_VERSION="0x7" 
  LINK_DATE="11/24/2003 20:23:41" 
  UPTO_LINK_DATE="11/24/2003 20:23:41" />
      <MATCHING_FILE NAME="libxml2.dll" SIZE="618496" 
  CHECKSUM="0x888DD4B5" MODULE_TYPE="WIN32" 
  PE_CHECKSUM="0x0" LINKER_VERSION="0x20004" 
  LINK_DATE="11/24/2003 20:23:19" 
  UPTO_LINK_DATE="11/24/2003 20:23:19" />
      <MATCHING_FILE NAME="libxslt.dll" SIZE="128000" 
  CHECKSUM="0xE921AAD6" MODULE_TYPE="WIN32" 
  PE_CHECKSUM="0x0" LINKER_VERSION="0x10000" 
  LINK_DATE="11/24/2003 20:23:39" 
  UPTO_LINK_DATE="11/24/2003 20:23:39" />
      <MATCHING_FILE NAME="tcl84.dll" SIZE="573952" 
  CHECKSUM="0x2A02D8D6" BIN_FILE_VERSION="8.4.2.6" 
  BIN_PRODUCT_VERSION="8.4.2.6" 
  PRODUCT_VERSION="8.4.6" FILE_DESCRIPTION="Tcl DLL" 
  COMPANY_NAME="ActiveState Corporation" 
  PRODUCT_NAME="Tcl 8.4 for Windows" 
  FILE_VERSION="8.4.6" ORIGINAL_FILENAME="tcl84.dll" 
  LEGAL_COPYRIGHT="Copyright © 2001 by ActiveState 
  Corporation, et al" VERFILEDATEHI="0x0" 
  VERFILEDATELO="0x0" VERFILEOS="0x4" 
  VERFILETYPE="0x2" MODULE_TYPE="WIN32" 
  PE_CHECKSUM="0x94FF2" LINKER_VERSION="0x0" 
  UPTO_BIN_FILE_VERSION="8.4.2.6" 
  UPTO_BIN_PRODUCT_VERSION="8.4.2.6" 
  LINK_DATE="06/15/2004 10:16:19" 
  UPTO_LINK_DATE="06/15/2004 10:16:19" 
  VER_LANGUAGE="English (United States) [0x409]" />
      <MATCHING_FILE NAME="tcl84t.dll" SIZE="583680" 
  CHECKSUM="0x990ECE0F" BIN_FILE_VERSION="8.4.2.7" 
  BIN_PRODUCT_VERSION="8.4.2.7" 
  PRODUCT_VERSION="8.4.7" FILE_DESCRIPTION="Tcl DLL" 
  COMPANY_NAME="ActiveState Corporation" 
  PRODUCT_NAME="Tcl 8.4 for Windows" 
  FILE_VERSION="8.4.7" ORIGINAL_FILENAME="tcl84t.dll" 
  LEGAL_COPYRIGHT="Copyright © 2001 by ActiveState 
  Corporation, et al" VERFILEDATEHI="0x0" 
  VERFILEDATELO="0x0" VERFILEOS="0x4" 
  VERFILETYPE="0x2" MODULE_TYPE="WIN32" 
  PE_CHECKSUM="0x8FDE2" LINKER_VERSION="0x0" 
  UPTO_BIN_FILE_VERSION="8.4.2.7" 
  UPTO_BIN_PRODUCT_VERSION="8.4.2.7" 
  LINK_DATE="09/02/2004 10:52:23" 
  UPTO_LINK_DATE="09/02/2004 10:52:23" 
  VER_LANGUAGE="English (United States) [0x409]" />
      <MATCHING_FILE NAME="tclpip84.dll" SIZE="16384" 
  CHECKSUM="0xCC55750C" MODULE_TYPE="WIN32" 
  PE_CHECKSUM="0x0" LINKER_VERSION="0x0" 
  LINK_DATE="11/24/2003 20:11:14" 
  UPTO_LINK_DATE="11/24/2003 20:11:14" />
      <MATCHING_FILE NAME="tclsh.exe" SIZE="20480" 
  CHECKSUM="0x2B653635" BIN_FILE_VERSION="8.4.2.5" 
  BIN_PRODUCT_VERSION="8.4.2.5" 
  PRODUCT_VERSION="8.4.5" FILE_DESCRIPTION="Tclsh 
  Application" COMPANY_NAME="ActiveState Corporation" 
  PRODUCT_NAME="Tcl 8.4 for Windows" 
  FILE_VERSION="8.4.5" ORIGINAL_FILENAME="tclsh84.exe" 
  LEGAL_COPYRIGHT="Copyright © 2000 by ActiveState 
  Corporation, et al" VERFILEDATEHI="0x0" 
  VERFILEDATELO="0x0" VERFILEOS="0x4" 
  VERFILETYPE="0x1" MODULE_TYPE="WIN32" 
  PE_CHECKSUM="0x0" LINKER_VERSION="0x0" 
  UPTO_BIN_FILE_VERSION="8.4.2.5" 
  UPTO_BIN_PRODUCT_VERSION="8.4.2.5" 
  LINK_DATE="11/24/2003 20:11:15" 
  UPTO_LINK_DATE="11/24/2003 20:11:15" 
  VER_LANGUAGE="English (United States) [0x409]" />
      <MATCHING_FILE NAME="tclsh84.exe" SIZE="9216" 
  CHECKSUM="0x1AF9DEFB" BIN_FILE_VERSION="8.4.2.6" 
  BIN_PRODUCT_VERSION="8.4.2.6" 
  PRODUCT_VERSION="8.4.6" FILE_DESCRIPTION="Tclsh 
  Application" COMPANY_NAME="ActiveState Corporation" 
  PRODUCT_NAME="Tcl 8.4 for Windows" 
  FILE_VERSION="8.4.6" ORIGINAL_FILENAME="tclsh84.exe" 
  LEGAL_COPYRIGHT="Copyright © 2000 by ActiveState 
  Corporation, et al" VERFILEDATEHI="0x0" 
  VERFILEDATELO="0x0" VERFILEOS="0x4" 
  VERFILETYPE="0x1" MODULE_TYPE="WIN32" 
  PE_CHECKSUM="0xCC0A" LINKER_VERSION="0x0" 
  UPTO_BIN_FILE_VERSION="8.4.2.6" 
  UPTO_BIN_PRODUCT_VERSION="8.4.2.6" 
  LINK_DATE="06/15/2004 10:16:20" 
  UPTO_LINK_DATE="06/15/2004 10:16:20" 
  VER_LANGUAGE="English (United States) [0x409]" />
      <MATCHING_FILE NAME="tclsh84t.exe" SIZE="9216" 
  CHECKSUM="0xF9ABC310" BIN_FILE_VERSION="8.4.2.7" 
  BIN_PRODUCT_VERSION="8.4.2.7" 
  PRODUCT_VERSION="8.4.7" FILE_DESCRIPTION="Tclsh 
  Application" COMPANY_NAME="ActiveState Corporation" 
  PRODUCT_NAME="Tcl 8.4 for Windows" 
  FILE_VERSION="8.4.7" 
  ORIGINAL_FILENAME="tclsh84t.exe" 
  LEGAL_COPYRIGHT="Copyright © 2000 by ActiveState 
  Corporation, et al" VERFILEDATEHI="0x0" 
  VERFILEDATELO="0x0" VERFILEOS="0x4" 
  VERFILETYPE="0x1" MODULE_TYPE="WIN32" 
  PE_CHECKSUM="0xC546" LINKER_VERSION="0x0" 
  UPTO_BIN_FILE_VERSION="8.4.2.7" 
  UPTO_BIN_PRODUCT_VERSION="8.4.2.7" 
  LINK_DATE="09/02/2004 10:52:23" 
  UPTO_LINK_DATE="09/02/2004 10:52:23" 
  VER_LANGUAGE="English (United States) [0x409]" />
      <MATCHING_FILE NAME="tclsh84t_good.exe" 
  SIZE="9216" CHECKSUM="0x38B614E" 
  BIN_FILE_VERSION="8.4.2.6" 
  BIN_PRODUCT_VERSION="8.4.2.6" 
  PRODUCT_VERSION="8.4.6" FILE_DESCRIPTION="Tclsh 
  Application" COMPANY_NAME="ActiveState Corporation" 
  PRODUCT_NAME="Tcl 8.4 for Windows" 
  FILE_VERSION="8.4.6" 
  ORIGINAL_FILENAME="tclsh84t.exe" 
  LEGAL_COPYRIGHT="Copyright © 2000 by ActiveState 
  Corporation, et al" VERFILEDATEHI="0x0" 
  VERFILEDATELO="0x0" VERFILEOS="0x4" 
  VERFILETYPE="0x1" MODULE_TYPE="WIN32" 
  PE_CHECKSUM="0xADF1" LINKER_VERSION="0x0" 
  UPTO_BIN_FILE_VERSION="8.4.2.6" 
  UPTO_BIN_PRODUCT_VERSION="8.4.2.6" 
  LINK_DATE="07/13/2004 09:51:12" 
  UPTO_LINK_DATE="07/13/2004 09:51:12" 
  VER_LANGUAGE="English (United States) [0x409]" />
      <MATCHING_FILE NAME="tk84.dll" SIZE="1032192" 
  CHECKSUM="0xB499DFC9" BIN_FILE_VERSION="8.4.2.5" 
  BIN_PRODUCT_VERSION="8.4.2.5" 
  PRODUCT_VERSION="8.4.5" FILE_DESCRIPTION="Tk DLL" 
  COMPANY_NAME="ActiveState Corporation" 
  PRODUCT_NAME="Tk 8.4 for Windows" 
  FILE_VERSION="8.4.5" ORIGINAL_FILENAME="tk84.dll" 
  LEGAL_COPYRIGHT="Copyright © 2001 by ActiveState 
  Corporation, et al" VERFILEDATEHI="0x0" 
  VERFILEDATELO="0x0" VERFILEOS="0x4" 
  VERFILETYPE="0x2" MODULE_TYPE="WIN32" 
  PE_CHECKSUM="0x108836" LINKER_VERSION="0x0" 
  UPTO_BIN_FILE_VERSION="8.4.2.5" 
  UPTO_BIN_PRODUCT_VERSION="8.4.2.5" 
  LINK_DATE="11/24/2003 20:13:21" 
  UPTO_LINK_DATE="11/24/2003 20:13:21" 
  VER_LANGUAGE="English (United States) [0x409]" />
      <MATCHING_FILE NAME="wish.exe" SIZE="24576" 
  CHECKSUM="0x643137A7" BIN_FILE_VERSION="8.4.2.5" 
  BIN_PRODUCT_VERSION="8.4.2.5" 
  PRODUCT_VERSION="8.4.5" FILE_DESCRIPTION="Wish 
  Application" COMPANY_NAME="ActiveState Corporation" 
  PRODUCT_NAME="Tk 8.4 for Windows" 
  FILE_VERSION="8.4.5" ORIGINAL_FILENAME="wish84.exe" 
  LEGAL_COPYRIGHT="Copyright © 2000 by ActiveState 
  Corporation, et al" VERFILEDATEHI="0x0" 
  VERFILEDATELO="0x0" VERFILEOS="0x4" 
  VERFILETYPE="0x1" MODULE_TYPE="WIN32" 
  PE_CHECKSUM="0x0" LINKER_VERSION="0x0" 
  UPTO_BIN_FILE_VERSION="8.4.2.5" 
  UPTO_BIN_PRODUCT_VERSION="8.4.2.5" 
  LINK_DATE="11/24/2003 20:13:22" 
  UPTO_LINK_DATE="11/24/2003 20:13:22" 
  VER_LANGUAGE="English (United States) [0x409]" />
      <MATCHING_FILE NAME="wish84.exe" SIZE="24576" 
  CHECKSUM="0x643137A7" BIN_FILE_VERSION="8.4.2.5" 
  BIN_PRODUCT_VERSION="8.4.2.5" 
  PRODUCT_VERSION="8.4.5" FILE_DESCRIPTION="Wish 
  Application" COMPANY_NAME="ActiveState Corporation" 
  PRODUCT_NAME="Tk 8.4 for Windows" 
  FILE_VERSION="8.4.5" ORIGINAL_FILENAME="wish84.exe" 
  LEGAL_COPYRIGHT="Copyright © 2000 by ActiveState 
  Corporation, et al" VERFILEDATEHI="0x0" 
  VERFILEDATELO="0x0" VERFILEOS="0x4" 
  VERFILETYPE="0x1" MODULE_TYPE="WIN32" 
  PE_CHECKSUM="0x0" LINKER_VERSION="0x0" 
  UPTO_BIN_FILE_VERSION="8.4.2.5" 
  UPTO_BIN_PRODUCT_VERSION="8.4.2.5" 
  LINK_DATE="11/24/2003 20:13:22" 
  UPTO_LINK_DATE="11/24/2003 20:13:22" 
  VER_LANGUAGE="English (United States) [0x409]" />
      <MATCHING_FILE NAME="zlib.dll" SIZE="44544" 
  CHECKSUM="0x8F24B8EA" MODULE_TYPE="WIN32" 
  PE_CHECKSUM="0x0" LINKER_VERSION="0x10000" 
  LINK_DATE="08/13/2000 16:24:16" 
  UPTO_LINK_DATE="08/13/2000 16:24:16" />
  </EXE>
  <EXE NAME="tcl84t.dll" 
  FILTER="GRABMI_FILTER_THISFILEONLY">
      <MATCHING_FILE NAME="tcl84t.dll" SIZE="583680" 
  CHECKSUM="0x990ECE0F" BIN_FILE_VERSION="8.4.2.7" 
  BIN_PRODUCT_VERSION="8.4.2.7" 
  PRODUCT_VERSION="8.4.7" FILE_DESCRIPTION="Tcl DLL" 
  COMPANY_NAME="ActiveState Corporation" 
  PRODUCT_NAME="Tcl 8.4 for Windows" 
  FILE_VERSION="8.4.7" ORIGINAL_FILENAME="tcl84t.dll" 
  LEGAL_COPYRIGHT="Copyright © 2001 by ActiveState 
  Corporation, et al" VERFILEDATEHI="0x0" 
  VERFILEDATELO="0x0" VERFILEOS="0x4" 
  VERFILETYPE="0x2" MODULE_TYPE="WIN32" 
  PE_CHECKSUM="0x8FDE2" LINKER_VERSION="0x0" 
  UPTO_BIN_FILE_VERSION="8.4.2.7" 
  UPTO_BIN_PRODUCT_VERSION="8.4.2.7" 
  LINK_DATE="09/02/2004 10:52:23" 
  UPTO_LINK_DATE="09/02/2004 10:52:23" 
  VER_LANGUAGE="English (United States) [0x409]" />
  </EXE>
  <EXE NAME="kernel32.dll" 
  FILTER="GRABMI_FILTER_THISFILEONLY">
      <MATCHING_FILE NAME="kernel32.dll" SIZE="930304" 
  CHECKSUM="0xCBCCF8A9" 
  BIN_FILE_VERSION="5.1.2600.1106" 
  BIN_PRODUCT_VERSION="5.1.2600.1106" 
  PRODUCT_VERSION="5.1.2600.1106" 
  FILE_DESCRIPTION="Windows NT BASE API Client DLL" 
  COMPANY_NAME="Microsoft Corporation" 
  PRODUCT_NAME="Microsoft® Windows® Operating System" 
  FILE_VERSION="5.1.2600.1106 (xpsp1.020828-1920)" 
  ORIGINAL_FILENAME="kernel32" 
  INTERNAL_NAME="kernel32" LEGAL_COPYRIGHT="© 
  Microsoft Corporation. All rights reserved." 
  VERFILEDATEHI="0x0" VERFILEDATELO="0x0" 
  VERFILEOS="0x40004" VERFILETYPE="0x2" 
  MODULE_TYPE="WIN32" PE_CHECKSUM="0xE7ED3" 
  LINKER_VERSION="0x50001" 
  UPTO_BIN_FILE_VERSION="5.1.2600.1106" 
  UPTO_BIN_PRODUCT_VERSION="5.1.2600.1106" 
  LINK_DATE="08/29/2002 10:40:40" 
  UPTO_LINK_DATE="08/29/2002 10:40:40" 
  VER_LANGUAGE="English (United States) [0x409]" />
  </EXE>
  </DATABASE>


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  This is known behavior on Windows because 
  of the Tcl core issues when unsplicing and splicing 
  channels between threads and interpreters. 
   
  The issue in the core has not been resolved yet.  
  AFAIK, it needs some changes of the channel  
  layer. I will pass this to Andreas. I think he has 
  the most experience with the channel system. 


login: andreas_kupries
username: andreas_kupries
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=75003

  This was fixed in an adhoc manner in Tcl 8.5, but the fix
  was never backported to the 8.4 series AFAIK. ... Checking,
  I find that the file splicing stuff is in 8.4, either done
  or backported, but not the socksplice work.

  I guess that should be done as well.


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  You know who did the work on the 8.5?  
  Maybe this guy can backport the changes 
  into core-8.4.branch.  
  Or is it worth the effort at all? 
   


login: davygrvy
username: davygrvy
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=7549

  The Tcl_ChannelType struct needs a new version to add the
  cut and splice vectors that are being backwardsly ad-hoc'd
  in 8.5, then this'll be fixed the right way.  I can't
  emphasize that strongly enough.

  To fix this requires a deep redesign, not a simple edit of a
  dangling pointer.


login: davygrvy
username: davygrvy
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=7549

  andreas,

  If you just add win/tclWinSock.c:TclpCutSockChannel,
  win/tclWinSock.c:TclpSpliceSockChannel, and their calls in
  generic/tclIO.c:Tcl_CutChannel and
  generic/tclIO.c:Tcl_SpliceChannel,  I'll become rather
  unhappy as the bug is still there and becomes much deeper.

  The bug is still there for channels that aren't part of the
  core.  That "ad-hoc fix" has insider knowledge and goes
  around how the Tcl_ChannelType is publicized.  When
  extensions that create channels (ie. Expect for one) can
  support cut/splice, the bug is fixed for good.


login: davygrvy
username: davygrvy
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=7549

  Someone asked me to add thread::transfer support to my
  iocpsock extension, but I can't because the core is screwed:

  http://sourceforge.net/tracker/index.php?func=detail&aid=952332&group_id=73356&atid=537538

  I started on the work, but it's unfinished:

  http://sourceforge.net/tracker/index.php?func=detail&aid=875701&group_id=10894&atid=310894


login: dkf
username: dkf
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=79902

  Apparently this issue will not be properly fixable in 8.4
  nor fixable in time for 8.5a2 (requires a TIP), so scaling
  back the priority.


login: andreas_kupries
username: andreas_kupries
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=75003

  Ok, this seems to be TIP #218, which has been put into both
  8.4 and 8.5 .


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  I cannot confirm this on Max OSX using Tcl8.4

  % array get tcl_platform
  osVersion 10.7.0 byteOrder littleEndian tip,268 1 threaded 1 machine i386 platform unix os Darwin tip,280 1 user zoran wordSize 4
  % set tcl_version
  8.4

  When I start your script it ocmplains about math max function.
  I remove that out of the code (it is not really related to anything
  thread-wise) and run the script. It allocates some 30MB virtual
  memory and stays there no matter howmany times i run your
  leak proc.

  I gues this has something to do with new(er) Tcl cores.  I have
  not tested anything later then 8.4.


login: auriocus
username: auriocus
mimetype: text/plain
icomment:
  I've updated the script for 8.4 without max. If I run it with
  tclsh8.4 threadpool_leakdemo84.tcl
  then the value in activity monitor "physical memory" winds up, every ten seconds +0.1MB. It starts from 3.0 and goes up to 6MB. You are right that "virtual memory" does not change; I wonder, why.


login: auriocus
username: auriocus
mimetype: text/plain
icomment:
  My platform:

  91-65-210-59-dynip:Programmieren chris$ tclsh8.4
  % parray tcl_platform
  tcl_platform(byteOrder) = littleEndian
  tcl_platform(machine)   = i386
  tcl_platform(os)        = Darwin
  tcl_platform(osVersion) = 10.8.0
  tcl_platform(platform)  = unix
  tcl_platform(threaded)  = 1
  tcl_platform(tip,268)   = 1
  tcl_platform(tip,280)   = 1
  tcl_platform(user)      = chris
  tcl_platform(wordSize)  = 4
  % 

  PS: Funny version. My OS is Snow Leopard 10.6.8, why does it say 10.8.0?


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  The virtual memory is what counts. The real memory is
  not important here. If virtual memory does not increase
  you do not have leak.

  The OS versioning is weird by Mac, yes.
  I also have SnowLeopard but 10.6.7. This is no problem. 


login: auriocus
username: auriocus
mimetype: text/plain
icomment:
  OK, so try the third version. I shortened the inner loop and made more iterations. Now you'll see, that also virtual memory winds up.  


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  I think I would need to release a new version. My own private sandbox
  seems to work fine. The 2.6.5 that gets distributed with Mac OSX seems
  to leak. But I need to figure out where are the sources! I heard SF is not
  hosting the sources any more...


login: andreas_kupries
username: andreas_kupries
mimetype: text/plain
icomment:
  Thread sources.

  See http://core.tcl.tk/ in general,
  and http://core.tcl.tk/thread/timeline?y=ci
  specifically.

  Further http://wiki.tcl.tk/28127
  and  http://wiki.tcl.tk/28126


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Hey, thanks Andreas! I think I will need to do some reading and learning
  new stuff...

  Apropos reported leak... did you try 2.6.6 release? I saw I have already
  corrected some things related to tpool already...


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  yes. there are no significant core-code related changes there.
  if 2.6.6 does not leak, the 2.6.7 will not either. 


login: hobbs
username: hobbs
mimetype: text/plain
icomment:
  I am able to repro the leak on Windows with ActiveTcl 8.5.9.2 and 8.4.19.5 using Thread 2.6.7 (not final).


login: auriocus
username: auriocus
mimetype: text/plain
icomment:
  I foun dthe leak originally witjin ActiveState 8.5.10.1 on Windows. I can reproduce it using Tcl8.6 HEAD:
  bin/tclsh8.6
  % info patchlevel
  8.6b1.2
  % package require Thread
  2.6.7
  % 

  This Tcl8.6 is 64bit, the leakage seems to be twice as fast as with the 32bit version. 


login: auriocus
username: auriocus
mimetype: text/plain
icomment:
  If I change the tpool::wait command, which is supposed to give back the still running processes in the list, the leakage becomes a bit slower (I checked after a fixed number of iterations). See the attached demo script. Therefore the joblist leaks, but it's not the only leak in here.  


login: techentin
username: techentin
mimetype: text/plain
icomment:
  I see the same memory leak with Tcl 8.5.13 and Thread 2.6.7 on Linux.

  It looks to me like the resource leak is related to the thread pool maintaining information on completed jobs.  

  I inserted periodic calls to tpool::wait and tpool::get inside the loop that posts work to the thread pool, and observed reduced memory growth.  (About half)  If I call tpool::get twice on the same job, I get "no such job," so I presume that it cleans up some resources.  But I don't see anything explicit in the API to discard results from completed jobs. 

  When I changed the post option from -nowait to -detached, the memory leak went away.


  Bob


login: sebres
mimetype: text/plain
icomment:
  Fixed within 3493120 in tcl-core (http://core.tcl.tk/tcl/tktview?name=3493120).
  Also can't reproduce more with attached scripts.


login: nobody
username: nobody
mimetype: text/plain
icomment:
  Logged In: NO 

  This is a regular behaviour for when loading in Tcl < 8.4 
  The latest 2.4 reports 2.1.4 when loaded in Tcl8.4- and  
  2.4 when loaded in Tcl8.4+ 
  This is because some of the channel transfer options need 
  support from the latest Tcl core.  
   


login: andreas_kupries
username: andreas_kupries
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=75003

  I tried this using the newest sources from the
  "core-8-3-1-branch" branch from the CVS at
  SourceForge. This is tcl 8.3.3 + a number of
  bugfixes. I used the head revision of Thread
  (2.1.1) from the CVS at SourceForge. The
  machine was a Sun running Solaris 2.6.

  The bug did not happen. Please retest on your
  system using the sources of Tcl and Thread
  described above to see if it the change in the
  OS or in Tcl/Thread which made the bug go away.

  Please configure and compile with --enable-symbols
  so that symbol information is available. If the
  bug still happens a stack trace would extremely
  valuable in diagnosing the problem.


login: andreas_kupries
username: andreas_kupries
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=75003

  Priority down as I get no response after two weeks since 
  the last comment.


login: andreas_kupries
username: andreas_kupries
mimetype: text/plain
icomment:
  With Thread 2.6.5 and Tcl 8.5.7 the tclsh simply hangs on exist instead of seg.faulting. (I assume crash meant seg.fault and core dump).

  I can also make the script shorter, using only

   package require Thread
   thread::release [thread::id]
   thread::names
   exit

  There is likely something wrong in the thread reference counter management, with us trying to release a thread which was never preserved. Plus the fact that we are releasing the 'main' thread of the system. My guess is that this is also a factor, the main thread is special, being the whole system.

  Re-assigning to Zoran, our thread maintainer.


login: ferrieux
username: ferrieux
mimetype: text/plain
icomment:
  Ok got it: missing Tcl_MutexUnlock(&threadMutex) at line 1954 of threadCmd.c in the "early return" case of function ThreadList().
  Please commit the fix, I am beyond CVS reach for now ;-) 


login: andreas_kupries
username: andreas_kupries
mimetype: text/plain
icomment:
  That was me and Zoran working at the same thing at the same time.
  And while Zoran's change went in, it was identical to mine, so no harm done by this x-ing.


login: ferrieux
username: ferrieux
mimetype: text/plain
icomment:
  allow_comments - 0

  Oh, so the three of us were working on it at the same minute.... Nice coincidence!!!


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  I can confirm this. Not only that... I can confirm a *huge* memory leak in the
  current HEAD. This is very very odd as when I last touched the code it
  was leak-free. I need to debug this further in order to (re)find the culprit.


login: sebres
mimetype: text/plain
icomment:
  Fixed for >= 8.5 within 3493120 in tcl-core (http://core.tcl.tk/tcl/tktview?name=3493120)
  Can be merged in 8.4, because obsolete;


login: hobbs
username: hobbs
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=72656
  Originator: NO

  Can you provide a small script that reproduces the issue on your system?


login: mistachkin
username: mistachkin
mimetype: text/plain
icomment:
  After careful review of the thread package code, I agree that this command should be changed in the way described by dgp.


login: gahr
username: gahr
mimetype: text/plain
icomment:
  Attached is a patch to do this, including test cases and documentation update. I will commit it to trunk if I don't hear any complains soonish. Thanks!


login: dgp
mimetype: text/plain
icomment:
  Please be sure the docs are updated to
  match the change ( I didn't check).

  We should consider whether this is reason
  to move to a new minor version, so that
  we move to Thread 2.8.0


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  This is nice. Thanks for the input. I will review this 
  and put it in the standard thread extension. 
  No, Unix does not have an equivalent of this so I assume 
  that we'd always return empty string on priority inquiry. 
  The only question is: what to do for priority setting?  
  Throw error? Eat every string silently?  


login: dkf
username: dkf
mimetype: text/plain
icomment:
  The other possibility is to say that all Unix threads are priority 0, and that that's both minimum and maximum.


login: dgp
username: dgp
mimetype: text/plain
icomment:
  This was corrected before in checkin 6593e21b9c.

  It got broken again in checkin 51cf545767.

  There's a natural tug-o-war going on between making
  builds work in a fossil checkout and making them work
  in the released distribution.  Unless there's some value
  in having those two keep different directory layouts, I
  recommend getting rid of the vc subdir from the very beginning.

  Otherwise we'll just keep fixing this again and again.


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Don, I have, as already stated, no experience with Windows build.
  I must say I did not understand your answer :-(
  Do I need to do anything? I normally fresh-clone the distro, make 
  changes, commit and push. 


login: decosterjos
username: decosterjos
mimetype: text/plain
icomment:
  To build Thread in a 'Betteries' included' Tcl, under the pkgs directories, one of the following needs to be done:

  - put all files of tcl/pkgs/thread/win/vc in thread/win and adapt the ROOT in the thread makefile.vc 

  or

  - modify tcl/win/makefile.vc to also look in pkgs/???/win/vc directories

  I prefer to remove the thread/win/vc directory.


login: decosterjos
username: decosterjos
mimetype: text/plain
icomment:
  The attached file 'vc.diff' is the 'fossil diff' output after moving the files from the win.vc to the win directory and updating the README file.


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  You are right. There is a bug there. 
  Currently. if -nowait option to thread::post is specified AND 
  there are no workers for the pool configured then we 
  spin-off a new worker and wait for it to catch-up. 
  BUT if there ARE some workers yet they are all busy then 
  we wait for first to become free. 
  Instead we should simply spin-off new worker if none 
  configured OR just post the job ticket and return w/o 
  waiting for any worker to become free.


login: dkf
username: dkf
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=79902
  Originator: NO

  I prefer this:
  Tcl_SetObjResult(interp, Tcl_NewStringObj(resultPtr->result, -1));
  though I'd even more strongly prefer it if the resultPtr pointed to a structure that knew the length of the string being transferred.


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086
  Originator: NO

  I personally do not care as long as it works. I believe this pattern is left
  in the code by mistake as I tried to change that behaviour long time ago.

  And, I haven't sucked it out of my finger. This stood (perhaps still stays)
  in the Tcl docs as a preffered way how to handle result object. Do not ask
  me where and when I read that. I can't remember. I just now I read that
  and wrote it accodring to the recomendation, just to find out that this
  brought lots of problems down the road. So I wholesale changed all
  my code to scrap those out. Obviously in the threading extension there
  are places where this is still used. I will go thru and remove all those
  parts. 

  Instead of 
  Tcl_SetStringObj(Tcl_GetObjResult(interp), resultPtr->result, -1);

  it will read
  Tcl_SetObjResult(interp, Tcl_NewStringObj(resultPtr->result, -1));

  The original "idea" was that the former should save us the need to
  create new Tcl_Obj as Tcl result should already point to a good one.
  And that one should not be shared. This is sometimes not true for
  whatever reasons, I do not know.

  Thanks for the report. 
  Zoran


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  I have cleared the tsdPtr->interp on thread exit so I hope this should fix the problem. I used the sample script above and it did not crash my interp. The fix will appear in 2.6.7 patch release. Please update your sandbox and run the example again and tell me if this solves the problem for you as well. I will keep the ticket open until I hear from you (at least couple of days).


login: sf-robot
username: sf-robot
mimetype: text/plain
icomment:
  This Tracker item was closed automatically by the system. It was
  previously set to a Pending status, and the original submitter
  did not respond within 14 days (the time period specified by
  the administrator of this Tracker).


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  Is this Windows or Unix?
  Can you get some stack-trace of the crashing thread
  using the GDB if on Unix or VC++ debugger (or 
  alternative) if on Windows?

   


login: ctasada
username: ctasada
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=340696

  It happens in both systems. Saddly I cannot provide you with
  a stacktrace because for some reason I cannot compile the
  TclKit in debug mode :(

  The error that I get is:

  Error from thread tid0xb63f5ba0
  couldn't read file
  "/home/carlos/fw5/Bin/fw5-base/lib/tcl8.5/clock.tcl": not a
  irectory
      while executing
  "source -encoding utf-8 [file join $TclLibDir clock.tcl]"
      (procedure "::tcl::clock::format" line 3)
      invoked from within
  "clock format 1154936894 -format {%Y/%m/%d %H:%M:%S}"
      ("uplevel" body line 1)
      invoked from within
  "uplevel 1 [info level 0]"
      (procedure "clock" line 16)
      invoked from within
  "clock format 1154936894 -format "%Y/%m/%d %H:%M:%S""
  called Tcl_FindHashEntry on deleted tableSegmentation fault

  Looking in the code where was generated this error I found
  the TclDbInitNewObj method in the TclObj.c file. There I see
  that the error seems to be produced in the call to
  Tcl_CreateHashEntry where I find that the newPtr looks like
  is always  set to 0 ????. Probably I'm missing something,
  but looks like there are tons of problems to run the TclKit
  in debug mode.

  If I can do something else to find the problem, please ask me.

  Thanks a lot.


login: ctasada
username: ctasada
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=340696
  Originator: YES

  Hi,

  I've been trying this script using ActiveTcl8.5.0.0b5.261540-win32-ix86-threaded.exe base threaded versions with Thread 2.6.5, in Windows and Linux and I get a crash in both cases:
  - Windows: called Tcl_Close on channel with refcount > 0
  - Linux: clock command not found

  Retesting with 8.4.13 (Thread 2.6.3) it works, as I already said in the first post.

  Is there any test that I can do to help you finding the problem? I cannot obtain the stacktrace, or I don't known how to do it :(

  Thanks again


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086
  Originator: NO

  Obviously it has nothing to do with the thread package
  but it has everything to do with the Tcl version. 
  I'm no windows specialist so I cannot tell you how to 
  obtain the stack trace there. On Linux it is simpler: just
  compile the Tcl (and all packages you load) with 
  --enable-symbols. Then run the tclsh under debugger
  (gdb) and run you test. When it crashes, you can use
  "backtrace" (bt) gdb command to show the stack of the
  thread that did cause the exception.


login: ctasada
username: ctasada
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=340696
  Originator: YES

  This problem only happens using a tclkit or an ActiveState base package. Using the standard Tcl/Tk versions it works fine. I've tried to compile TclKit in debug mode, without luck, and I don't have the source to compile an ActiveState base package :(


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  Yes. This has never been tested. The reason is that 
  thread package would work first at 8.3 and later. 
  I will however fix the pkgIndex.tcl accordingly. 
  In the new upcoming 2.6 version. 
   
   


login: dgp
username: dgp
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=80530

  Looks like a bad call to Tcl_Release in the thread 
  package.  Is davygrvy still the maintainer of that
  package, or are his days full with the new Expect
  port these days?


login: andreas_kupries
username: andreas_kupries
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=75003

  David is more concerned with expect I believe.
  I am giving this to Zoran now. Zoran, if you
  don't have time assign it back to me.


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  Andreas,

  thanks for offering help. At the moment I'm *realy* busy
  so if you can jump in on this, it would be great.
  BTW, I expect to close most of the open stuff, related to
  docs, etc towards end of the year. I can also attend this
  problem in the same go, but if you can do it earlier,
  it would be better. 


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  Andreas,

  I've invested couple of minutes in this issue. 
  It is pretty simple. The main application (tclsh) thread
  exits before one of the worker threads gets the
  chance to exit. The main app thread calls PreserveExitProc
  which finalizes the preserved array and the exiting thread
  bombs since its preserved data chunk is not found.

  This is a Tcl core problem, not the thread extension
  problem.

  Whole scenario can be avoided by simply putting the 
  "after 1000" at the end of the application to allow worker 
  threads to properly clean-up and terminate. 

  The question is: how can such scenario be avoided ?
  Is there something like Tcl_InExit which could be
  checked by *any* thread and just abort any processing
  if the application is about to (or in the middle of) exit? 

  Zoran


login: andreas_kupries
username: andreas_kupries
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=75003

  Zoran,

  There is an internal function "TclInExit" (tclEvent.c, line 
  939). I don't know if we can use this one. Looking at its 
  code I see that it looks for thread-specific data first 
  before returning global information.


login: andreas_kupries
username: andreas_kupries
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=75003

  The main thread breaks out of its event loop only after all 
  worker threads have signaled that they are done. As they 
  still have to talk to the logger thread this means that the 
  main thread can exit while one worker thread is in the 
  later stages of cleanup. Even more so the logger thread is 
  told to terminate just before the main thread truly exits. 
  I guess that it is the logger thread which tries to release 
  his interp after PreserveExitProc was called.


login: andreas_kupries
username: andreas_kupries
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=75003

  I am not satisified with the proposed solution of using 
  TclInExit (as public API) to avoid the problem. Threads are 
  usual unwound (thread::unwind) from nested event loops and 
  can process arbitrary commands before actually exiting. 
  This means that this code may use any global subsystem of 
  the tcl-library, including the Preserve-subsystem, and not 
  only by calling Tcl_Release, but Tcl_Preserve as well. As 
  all these global susbsystems are effectively already 
  shutdown anything may happen, most likely crashes in other 
  places. And TclInExit will tide us over these only so far.

  The main problem I see exposed through this bug is IMHO 
  that the main thread is special. Shutting down a thread 
  created by the Thread package finalizes this thread and all 
  data pertaining to it. But shutting down the main thread 
  additionally finalizes all global information shared across 
  all the existing threads, thus rendering all other threads 
  unable to run.

  So, what possible solutions do we have to this ?

  1) Have the thread package register a global exit handler. 
  As these are called before everything else this will allow 
  the thread package to simply kill all threads still running 
  (This would also be the approved and public method of 
  getting the information that we are in exiting the 
  process). The downside would be that the killed threads may 
  not have completed their actions. In the specific case of 
  this bug it would be most likely that the logger thread is 
  killed and unable to print the log messages of the last-
  closing worker-thread. But this is something which can be 
  worked around.

  2) Find a way to block the finalization of the main thread 
  and global information until after all the other threads 
  are finalized. The downside is that an improperly coded 
  application may simply hang on exit because of one or more 
  threads not shutting down, or not shut down.

  Of the solutions above I prefer the first one as its 
  downside is less troubling than the downside of the second 
  to me. If someone else sees other solutions I would be 
  happy to hear about them.


login: andreas_kupries
username: andreas_kupries
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=75003

  See also
  * [ 597997 ] async+thread panic
  * [ 597575 ] [exit] in sub-thread may crash.


login: dgp
username: dgp
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=80530


  Is this still valid?

  Much revision of finalization
  has been done since this
  was reported.


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  Yes. This is still valid.

  The problem is that by the concept of the Tcl lib, there is a
  implicit distinction between the main thread (the startup 
  thread) doing the Tcl_Exit and other threads doing Tcl_ThreadExit. 

  Actaully the entire cleanup/teardown is not thread-friendly and 
  relies on the fact that startup thread must exit last, which is 
  not always true. Rather, the thread which exits AND it is the
  LAST thread in the process must initiate teardown. This requires
  quite a lot of plumbing here and there and it is questionable if
  it is worth the effort in the 8.x branch.

  I could imagine closing this bug, yet opening another RFE to
  make the Tcl finalization more thread-compatible.


login: msofer
username: msofer
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=148712

  Bug #597997 (closed as a dup of this one) has a small test
  script to trigger the panic:

      package require Thread
      set i 0
      while {[incr i] <= 2} {
          thread::create "puts $i"
      }

  Avoid the panic by adding the following at the end:

      # make sure to wait until all threads have returned
      while {[llength [thread::names]] > 1} {
          after 100 set x 1
          vwait x
      }


login: ferrieux
username: ferrieux
mimetype: text/plain
icomment:
  Upping the prio because it aborts on very simple scripts even on Windows (where threading is not so alien ;-). Will look a this shortly in the light of recent work on exit handlers.
  The current abort message on Win32/mingw is:

   exit handlers were created during Tcl_Finalizecalled Tcl_FindHashEntry on deleted table

   This application has requested the Runtime to terminate it in an unusual way.
   Please contact the application's support team for more information.


login: ferrieux
username: ferrieux
mimetype: text/plain
icomment:
  Here's a copy of a message I sent to tclcore regarding these issues:

  fromAlexandre Ferrieux <alexandre.ferrieux@gmail.com>
  toTclCore <tcl-core@lists.sourceforge.net>
  dateWed, May 6, 2009 at 12:42 AM
  subjectFinalization vs. Exit
  mailed-bygmail.com
  hide details 12:42 AM (21 hours ago)

  Reply

  Hi,

  As exhibited e.g. in
  https://sourceforge.net/tracker/?func=detail&aid=486399&group_id=10894&atid=110894
  , the Tcl finalization sequence is a complex thing.

  In this specific bug, the problem involves avoiding bad interaction of
  second-class threads with the main (first-class) one during the very
  last steps of teardown, when the ground starts to dissolve under
  everybody's feet.
  In other instances, it can be argued that we're doing way too much
  "administrative" cleanup (like freeing memory) just before exiting.

  Now a simple approach seems to be viable: make a clear distinction
  between the [exit] path and full finalization (eg in embedded
  scenarios, since Donal dislikes scenarii ;-).

  The idea, then, is to make clearer in the code the dichotomy between
  the exit handlers that are "administrative" (meaning they have no
  effect outside the dying process) and can thus be skipped in the
  [exit] case, and those that are really compulsory in all cases because
  they have side-effects on the outside world, these side-effects being
  part of a documented or implicit contract that we simply cannot break.
  Then, for the [exit] path, just do the compulsory ones, and call the
  OS's exit() function.

  Question: could you help me draw this dichotomy ?

  Here is what I have spotted so far:

    - process-wide exit handlers registered with Tcl_CreateExitHandler,
  aka "early exit handlers" --> compulsory

    - per-thread exit handlers (freeing memory) --> skippable for [exit]

  Still in the gray zone, is FinalizeIOSubsystem. I know of cases where
  not calling it might have long-ranging effects (like RST sent on all
  non-closed sockets), but since it deals with per-thread/interp
  structures (like channel lists), it should either be entirely skipped
  or done for all threads... which is problematic at exit time if some
  threads are blocked or in an uncontrolled state.

  Thanks in advance for any insight on this,

  -Alex


login: ferrieux
username: ferrieux
mimetype: text/plain
icomment:
  Note that GPS's patch in 2001201 applies exactly this strategy. But I would still like to get answers to the questions raised below.


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Hi!

  I am afraid you will not be able to "rectify" this w/o rewriting the 
  complete finalization logic introducing ref-counts and similar
  tricks. Furthermore, no thread in the application is a "main" thread.
  Every thread is equal. So long ANY thread is using Tcl datastructures
  they cannot/shouldnot be freed, regardles if this is the "first" thread
  started in the process or any other. 

  I tried to do that (not only myself, but also some AOLserver developers)
  just to find out that it is a huge and complex task, perhaps too complex
  to justify the benefits. Anyway, by adhering to the one simple rule:
  never exit while there are other threads in the process.
  one can live with this pretty fine. If you want to make it "by the book"
  you are welcome to try, but be prepared to sweat hard.

  Cheers
  Zoran


login: ferrieux
username: ferrieux
mimetype: text/plain
icomment:
  Well, the idea as explained below, is to let [exit] share only 10% of the full-finalization path.Hence the remaining 90% don't need any work. The question is rather what to let into those 10%. I believe the early exit handlers should be let in, because they allow e.g. a DB extension to close transactions etc. For the IO subsystem I'm starting to think it should be left out because if a thread/interp owns a channel and is blocked on something else, we cannot safely close it from a central point. And anyway exit() closes fd's.

  So, do you see a problem if we restrict Tcl_Exit's operation to just the early exit handler loop (beginning of Finalize) ?


login: ferrieux
username: ferrieux
mimetype: text/plain
icomment:
  Could this just be history building up, in an interactive shell ?
  If not, try increasing the [time] itercount and checking the effect on the slope.


login: msofer
username: msofer
mimetype: text/plain
icomment:
  To remove history problems do one of:
    - run the thing in an infinite loop (with an [after 10] in there), watch what top has to say about mem
    - do [proc history args {}] before the test


mimetype: text/plain
icomment:
  Ran a new test, non-interactive, wrapped in a proc, 100000 iterations. The results are:

              VIRT    RES  SHR
  START  32476 4192 1960
  END      36284 7088 1960


login: ferrieux
username: ferrieux
mimetype: text/plain
icomment:
  OK thanks. Now what about 1*100000 vs. 10*10000 ? This to decide whether it leaks rather "per [times]" or "per [thread::*]" invocation.


mimetype: text/plain
icomment:
  Here are the 10*10000 results:

  32872 4652 1960
  34120 4964 1960
  34384 5244 1960
  34664 5504 1960
  34888 5732 1960
  35116 5956 1960
  35380 6204 1960
  35584 6428 1960
  35868 6672 1960
  36092 6916 1960


mimetype: text/plain
icomment:
  Results for:
   time {set ::t [thread::create -joinable]; thread::release $t; thread::join $t} 10000

  ran ten times --- 

  24392 4284 1968
  24616 4528 1968
  24844 4752 1968
  25068 4996 1968
  25292 5220 1968
  25516 5464 1968
  25744 5692 1968
  25968 5936 1968
  26192 6160 1968
  26416 6404 1968


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Not yet. I do not think we can do anything here fast, as most probably
  this is some deep finalization issue in Tcl library. The extension itself
  does not leak at this point but the core could. I do not have resources at
  the moment for deeper analysis but will throw a valgrind againt it just to
  be 100% sure. If you aim at speedy releasing 2.6.7 I think you should not
  wait.

   


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  This is not the issue of the threading extension. I have tried the Tcl 8.4 core
  with 2.6.6 thr-extension on Linux and Mac and double-checked with valgrind.
  There are no leaks there at all. The Linux top shows pretty much same virtual
  process size no matter now often I repeat the following command;

  time {set ::t [thread::create -joinable]; thread::release $t; thread::join $t} 

  I run if 1000000 times and then check vith valgrind and top and there is no
  leak. I guess later (8.5/8.6) cores might not be leak-free; i haven't tested those releases as we are still using 8.4 core.

  Unfortunately I missed the new move-fro-sf-cvs so I do not yet) khow where
  are the new sources and I need to catch-up with that.


login: twylite
username: twylite
mimetype: text/plain
icomment:
  Using 8.6b2rc1 I can reproduce a leak on WIN32 using the time{} loop recommended by Zoran.  Only the Windows build leaks about 135,000 bytes per iteration.  Task Manager's thread count confirms that all additional threads have stopped.

  For reference, a simple interp create/delete loop doesn't seem to leak memory, implying that this isn't an interp leak.


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  I think the problem is in the Tcl finalization area. 
  The interp itself is clean. The finalization isn't :-( 
  Some years ago I contemplated to rewrite the 
  finalization code using refcounts but this was a 
  major undertaking and I never found time for it. 
  The 8.4 seems to be more stable in this respect
  which is why we still stick to it.  

  In the meantime, creating a set of worker threads
  upfront (or using tpool with fixed number of workers)
  should keep the memory in the "reasonable" area.
  Also, turning off the greedy threading allocator that
  never releases memory to the system and using the
  regular system malloc instead could also be a good 
  move. I recall we turned that off some years ago and
  I cannot recall our app becomming slower (yet it 
  became a fifth of its original memory footprint!).

  I think this could be a hard nut to crack. In some days
  I will get more spare time to checkout the latest head
  and see what all happened there since the last time 
  I looked at it. 


login: dgp
username: dgp
mimetype: text/plain
icomment:
  Using the usual tcltest harness for memory leak testing,
  I tried this:

  testConstraint memory [llength [info commands memory]]
  test thread-8.1 {Bug 3381371} -constraints memory -setup {
      proc getbytes {} {
          set lines [split [memory info] "\n"]
          lindex $lines 3 3
      }
      package require Thread
  } -body {
      set end [getbytes]
      for {set i 0} {$i < 5} {incr i} {
          set t [thread::create -joinable]
          thread::release $t
          thread::join $t
          set start $end
          set end [getbytes]
      }
      expr {$end - $start}
  } -cleanup {
      rename getbytes {}
      unset -nocomplain i start end t
  } -result 0

  It shows no leak, using the trunk of Tcl & Thread.

  What is it that's claimed to be leaking?  Apparently
  it's not Tcl_Alloc'ed  memory?  Or is this a leak I 
  should not be able to see testing on Linux?


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  This is true. However I do not have a slightest idea where to fix this as the
  whole Make machinery is provided by TEA package. 
  Can you tell me what should I change and where?


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  allow_comments - 1

  AHA! The TEA sets our prefix/exec_prefix to what Tcl is using. This should all be handled by the TEA_PREFIX directive that we call. So I guess there is nothing I could do here. I will close this ticket consequently.


login: hobbs
username: hobbs
mimetype: text/plain
icomment:
  TEA actually supports user setting --prefix/--exec-prefix, if --with-tcl is also used to find the right Tcl.  I don't think there is any build system issue here.


login: elfring
username: elfring
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=572001

  Values are only available globally with TCL threads if the data is stored in 
  thread shared variables. (http://cvs.sourceforge.net/cgi-
  bin/viewcvs.cgi/*checkout*/tcl/thread/doc/html/tsv.html).

  So I 
  adjust my suggestion for the function interface design. The identifier for the 
  mutex object with the global scope must be accessed by a thread shared 
  variable.

  1. thread::mutex::create ?tsvname 
  ...?

  2.
  thread::safecall id tsvname script ?varname?


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  Mutexes created with "thread::mutex create" are  
  already in the global scope. 
  They can be used from any thread. A mutex tied to a thread 
  has no real benefit. So you're perfectly ok with that. 
  The mutex indentifier returned by "thread::mutex create" can 
  be passed to another thread or stored in the thread shared 
  variable (array) and used from other threads as well. 
   
  Also, thread::eval is a nice shortcut of 
   
    thread::mutex lock $mutex 
    # do some Tcl work 
    thread::mutex unlock $mutex 
   
  so you can just say: 
   
    thread::eval -mutex $mutex { 
      # do some Tcl work 
    } 
   
  The benefit is: you can never forget to unlock the 
  mutex. Also, In case of script error, the implementation 
  guarantees that the mutex will be unlocked. Without this 
  you must take care yoursefl about unlocking the mutex 
  which may prove complicated in some cases. 
  A very simplified use of thread eval is: 
    thread.:eval { 
      # do some Tcl wor 
    } 
   
  where the mutex is implicit and this way you can be sure 
  that only *one* thread at a time in the entire process can 
  run the Tcl code. 
   
   


login: elfring
username: elfring
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=572001

  1. Thanks for your answer.
      Please add to the documentation that the TCL 
  mutexes have got global scope.
     (The mutex identifiers should not be 
  stored in local variables. They must be stored in thread shared variables to be 
  useable by all threads.)

  2. But I think that something is still missing for 
  the suggested function "thread::safecall id tsvname script 
  ?varname?".
  I want that a different thread does something with 
  "thread::eval -mutex". How will the calling thread get the return values from the 
  called thread?

  3. Will the (missing) sychronization primitives that 
  are described in the chapter "2.3. Interprocess Communication" of the 
  book "Modern Opreation Systems" 
  (http://www.cs.vu.nl/~ast/books/mos2/) be added to the thread package?


login: elfring
username: elfring
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=572001

  1. And your explanation "... this way you can be sure
  that only *one* 
  thread at a time in the entire process can
  run ..." is easier to understand 
  than the current description "If no mutex is specified ... an internal static 
  mutex is used". (http://cvs.sourceforge.net/cgi-
  bin/viewcvs.cgi/*checkout*/tcl/thread/doc/html/thread.html)


login: elfring
username: elfring
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=572001

  2. Or will the following code example do what I want?

  set $TID 
  [thread::create -preserved]
  tsv::set X myMutex [thread::mutex 
  create]
  thread::send $TID \
    {
     thread::eval -lock [tsv::get X 
  myMutex] {open Test.txt}
    } File
  puts $File Hallo
  close 
  $File
  thread::mutex destroy [tsv::get X myMutex]
  thread::release 
  $TID


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  This example will instruct the $TID thread to open the Test.tst 
  file under lock. If you however want to both open and write 
  to the file in atomic way, you should move the "puts $File" 
  in the thread::eval block. 
   
  Normally, your application will create some mutexes up front 
  and store them in the shared variable. Those live a long as 
  the app lives and are generally never destroyed. During 
  runtime, threads come and go and they use the precreated 
  mutexes for various sync tasks. 
   
   


login: elfring
username: elfring
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=572001

  1. Please add your better explanation to the documentation.

  2. My intention is just to execute a single function calll under the 
  lock protection.
  But I think that I will not get the return code of the "unsafe" 
  function "open" in my first version of the example. I will get the 
  return value of the function call "thread::eval" instead. How do 
  you think about it?

  set $TID [thread::create -preserved]
  tsv::set X myMutex [thread::mutex create]
  thread::send $TID \
    {
     thread::eval -lock [tsv::get X myMutex] {catch {open Test.txt w} 
  Test}
     return $Test
    } File
  puts $File Hallo
  close $File
  thread::mutex destroy [tsv::get X myMutex]
  thread::release $TID

  Can the retrieval for return values from other threads made 
  easier?
  Does the second example show a use case that can be 
  supported by a single function in the TCL threads API?


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  Well, returning of values from threads, using the thread::send 
  is already damn-simple. I do not know how to make it simpler. 
  It is either: 
   
    set result [thread::send $tid "set somevar 12"] 
  or 
    thread::send $tid "set somevar 12" result.  
   
  What can be simpler than this? 
   
  If you want to eval the script in an atomic fashion 
  in the other thread, one can perhaps say: 
   
    thread::send -lock myMutex $tid {open file.txt} result 
   
  Would this satisfy your requirement? 
   


login: elfring
username: elfring
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=572001

  Well, your last command seems to be very close to my wish.
  1. I just do not read an option "-lock" for the 
  function "thread::send" in the HTML documentation at the 
  moment.

  2. What will happen if the script that is executed this way by the 
  other thread will "throw" an error?
     Can the error be caught?


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  1. There is none yet. It was just an idea to see if it would fit. 
  2. Of course, Just make the catch 
    catch {thread::send $tid $script} 
  or examine return value of: 
    thread::send $tid $script result 
  to be 1 
   
  Note that "thread::send $tid $script result" is the same as  
  you would do with: 
      catch {thread::send $tid $script} result 
   
  BTW, do you mind going to the mail-list for some discussions 
  as Donal suggested? When we come to some nice conclusion 
  we (or you) might post the feature request/bug etc so we do 
  not clutter the SF. It is much more easier to me to answer 
  your questions over email. 
   
   


login: elfring
username: elfring
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=572001

  1. Please add it. I like because it seems to be exatly the enhancement I 
  want.

  2. Please add this to the documentation.

  3. Yes. Let us 
  continue the discussion on the new mailing list "tcl-threads".


login: elfring
username: elfring
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=572001

  Where and when will our discussion be continued?
  Here or at the mailing 
  list (more synchronization primitives for the threads package - 
  https://sourceforge.net/mailarchive/forum.php?thread_id=1585197&forum_id=24972)?


login: elfring
username: elfring
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=572001

  I assume that the requested function can be implemented 
  with condition variables.
  But I see an important detail. - The called thread should 
  perform a "thread::cond wait" for the calling thread.
  The caller will be woken up after the sent script was executed.

  The scheduling strategy for the calling threads will be an 
  interesting part in the solution.


login: elfring
username: elfring
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=572001

  I hope that my requests 
  "https://sourceforge.net/tracker/?func=detail&aid=770193&group_id=7586&atid=357586" 
  (Boost C++) and 
  "https://sourceforge.net/tracker/?func=detail&aid=712663&group_id=7232&atid=357232" 
  (ZThread) will help for this topic.


login: elfring
username: elfring
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=572001

  Example:
  It is described on the page 
  "http://tcljava.sourceforge.net/docs/website/getstart.html" how 
  the function "eval" must be protected against multithreaded calls.


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  Your assumption about mutexes is wrong. Mutex handles 
  can be shared among threads since very nature of the 
  mutex is to synchronize access from many threads. 
  So, handles to mutexes created with tnread::mutex create 
  can be left in thread shared arrays and accesed from  
  many threads simultaneously. The thread package contains 
  examples of Tcl code (web server and command server) where 
  sync techniques with mutexes and condition variables are 
  employed. 
   
  Zoran 
   


login: elfring
username: elfring
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=572001

  1. Are you going to add this description to the 
  documentation?

  2. I am looking for a monitor implementation 
  that has got the scope of a library or package and not a single 
  synchronized method or block like it is an Java.
  A function call will 
  get this monitor as a parameter.


login: elfring
username: elfring
mimetype: text/plain
icomment:
  File Added - 66690: MonitoredExecutor.zip

  Logged In: YES 
  user_id=572001

  I have published the uploaded file also in the forum "class for function calls that are not thread safe" (http://groups.google.de/groups?th=ee30cb7159ff86eb&group=comp.programming.threads).
  Can it improve TCL source code?


login: elfring
username: elfring
mimetype: text/plain
icomment:
  File Added - 66887: MonitoredExecutor.zip

  Logged In: YES 
  user_id=572001

  I have added a TCL file.
  I hope that this source code variant can help to express my ideas and concepts on this topic.


login: elfring
username: elfring
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=572001

  # Would this implementation have the same effect?

  namespace eval monitored_executor \
  {
  namespace export do new delete

  proc do {id work} \
   {
    return [thread::send $id $work]
   }

  proc new {} \
   {
    return [thread::create -preserved]
   }

  proc delete (id) \
   {
    thread::release $id
   }
  }

  # Did my last point of view from C/C++ make it harder to see the TCL functionality?
  # Can the requested option "-lock" help if the function "send" is synchronously and completely processed?


login: dgp
username: dgp
mimetype: text/plain
icomment:
  Any chance someone can look into
  these?  

  I'd like to consider Thread for
  bundling with Tcl 8.6, but failures
  like this make me think it's not ready.


login: ferrieux
username: ferrieux
mimetype: text/plain
icomment:
  Given this, could we do a dichotomy with (1) The high level synchronization model (synchronous thread::send and eventloops) and (2) the trickier, low-level set of sync primitives (mutexes, r/w locks, condition vars) ? We could #ifdef out the script level exposition of (2), and bundle it ("Threads Lite" ?).

  The idea is that (1) suffices for ~95% of uses of the Threads extension (otherwise the current bug would have been detected earlier), and as a consequence is much more "polished" than the lower half.

  Just my $0.02...


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  Uuuups... this is right. I have fixed it in the CVS head branch.
  Thanks for the report.


login: mdejong
username: mdejong
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=90858

  Humm, you did not use the ChangeLog entry
  I sent in. Since mine was more descriptive,
  I went ahead and put it into the ChangeLog.


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  Oh, sorry, I somehow missed that. 
  But where did you leave the ChangeLog message?
  I was not able to find it. Or am I really blind?


login: mdejong
username: mdejong
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=90858

  ChangeLog entries are typically included in the
  patch file, right at the top.


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Hm.... this new interface isn't really intuitive.... Did you get my comment/question
  about the platform where you compile?


login: joergwendt
username: joergwendt
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=1079782

  can that reproduce somebody -- I have a big problem with a
  server, after 30h the process-memory is full (2 GByte).

  does someone have any note, workaround or tip for me ?   

  ~joerg


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  Let us first clarify this: 
   
     thread::send $thid -async "thread::release" 
   
  is wrong, hence you never release the thread actually. 
  You should have written: 
     
    thread::send -async $thid  "thread::release" 
   
  But, it is perfectly sufficient to call: 
   
     thread::release  $thid 
   
  from the main thread, i.e. w/o posting an async event 
  to the worker thread. 
   
  Now, if you modify your test-bed and run again, the 
  memory should be released. Each thread cleans-up 
  its unserviced events on exit so I should not expect 
  any problems there. Otherwise we'll have to dig into 
  more deeply. 
   
  Tell me what u get. 


login: joergwendt
username: joergwendt
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=1079782

  sorry for my inadvertence, I work on this problem the last
  weeks ... 

  I repeated the test (and go back to 8.4.6/2.5.2)

  1. download the tar-files from sourceforge
  2. compile with threads (testrun with & without
  USE_THREAD_ALLOC) 
  3. I have eliminated the bug in the script :-(
  4. testing with
  thread::send -async $thid "thread::release"
  &
  thread::release $thid

  hm, the same result, the thread is down but the memory will
  not be freed ! the program starts with 1.5MB and ends with
  ~11 MB.
  that the resources of the threads with thread::release to be
  released is ok.  but why not after the idel-script was finish ?

  the testrun with USE_THREAD_ALLOC need more memory - I think
  that am normal !

  ~joerg 


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  OK. All is clear now. You try this and watch the proc virtual size 
  with top utility: 
   
  time { 
    set thid [thread::create "thread::wait"]; 
    for {set ni 0} {$ni < 25000} {incr ni} { 
      thread::send -async $thid$::Thread 
    } 
    thread::send -async  $thid thread::release 
  } 
   
  Hit this command once, twice and so on and the process 
  memory will always stay at about 15 MB (on Linux).  
  DO NOT try "time {...} 100" for example because this will 
  fill the async event loop queue with tons of events. This  
  is not what we're testing. Just repeat the above time  
  command *by hand* several times in a row. 
   
  The thing with USE_THREAD_ALLOC is that allocated Tcl 
  memory is never freed. It is kept for process reuse in several 
  per-thread and global buckets. The idea is to put less strain 
  on the malloc's mutex, thus speeding up the whole thing. 
  So, technically speaking, there is no leak. 
   
  Now this little example does not reveal any problem actually. 
  We have yet to find out what is the real problem you're hitting. 
  It might be that your app loads some non-thread-safe and/or 
  memory leaking extensions. What extensions do you load,  
  if any? What is the pattern of your server app, i.e. what it does? 
  Can you sketch this in couple of lines? 
   
  The bottom line is that using USE_THREAD_ALLOC, your process 
  may grow over time, but should eventually settle down on some 
  pretty-much constant value. This value might be high, but it  
  should not break your VM. If this is not what you can afford,  
  try compiling without USE_THREAD_ALLOC. This will slow-down  
  the app for sure but will avoid the above. The thing is: you may 
  not even see that your app is slowed down. This depends entirely 
  on what your app is actually doing. It is worth a try. 
   
   


login: joergwendt
username: joergwendt
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=1079782

  for the moment I still work on the tests (munich summer flu
  & and a typical monday :-( )

  your example shows a confusing result.

  ~ 5%   1.5MB starts and ends with 1.5MB
  ~ 90% 1.5MB start and ends with 22MB
  ~ 5 % the rest ends between 3~80MB

  loop with hand-test & for loop with after

  to yours ask for the function of the application. 
  it is a batch server ! the test-enviroment don't use any 
  tcl-extention (only tcl & libthread). I removed all C-code
  for my tests und disable TCL_THREAD_ALLOC.

  principleal function:
  Main-Thread : scan a batch-queue
  Main-Thread : send batch-exeute cmd to Batch-Thread
  Batch-Thread: execute Batch-Job (Tcl-Script)
  Batch-Thread: send finish-sig to Main-Thread
  Main-Thread : scan batch-queue
  .....

  I began to test systematically all used Tcl instructions ..
  for the moment file mtime and glob are in the my focus.

  ~joerg
     


login: nobody
username: nobody
mimetype: text/plain
icomment:
  Logged In: NO 

  Just an observation... 385 bytes in $::Thread and 25,000 of 
  them on the async queue for that thread that's 9.2MB of 
  data sitting in the queue for your thread.  That's not TCL 
  leaking memory.  Tcl threads are cooperative and not 
  preemptive so none of those async requests are executed 
  until your [vwait].

  I tried two variations of the testcase and didn't observe any 
  memory leaking... Here they are.

  #1: throw a bunch of -async's and then release the thread 
  over and over.

  for {set x 0} {$x < 2500} {incr x} {
      set thid [thread::create {thread::wait}]
      for {set ni 0} {$ni < 100} {incr ni} {
          thread::send -async $thid $::Thread
      }
      thread::release $thid
  }
  puts "DONE"

  ---------------------------------------------------
  #2: throw -async's at the thread every 10 milliseconds so it 
  can execute as we go.

  set ::thid [thread::create {thread::wait}]
  set ::requests 25000
  proc issue {} {
      if {[incr ::requests -1] != 0} {
          if {($::requests % 1000) == 0} {
              puts $::requests
          }
          thread::send -async $::thid $::Thread
          after 10 [namespace code issue]
      } else {
          puts "DONE"
      }
  }
  issue
  thread::wait


login: tombert
username: tombert
mimetype: text/plain
icomment:
  I took it from http://core.tcl.tk/thread and simply tried the latest 2-6-branch. The version tagged 2-6-6 compiles fine - so I thought I'll write a note here.


login: andreas_kupries
username: andreas_kupries
mimetype: text/plain
icomment:
  Looking at this I see no branch named '2-6-branch' either, only 'trunk'.
  Do you have the revision id, by chance ?
  (The hex number of your revision you would see in http://core.tcl.tk/thread/timeline?n=20&y=ci)
  The tag "thread-2-6-6" is a bit over a year old.


login: tombert
username: tombert
mimetype: text/plain
icomment:
  Again sorry - the branch full name of course is thread-2-6-branch:

  D:\TCL8.5\sources\thread>fossil status
  repository:   D:/TCL8.5/sources/thread/thread.fossil
  local-root:   D:/TCL8.5/sources/thread/
  server-code:  4a1051044a6b047539b22dff4e449aca600a2b03
  checkout:     60da38f803b49b8d9246ba1f55878264861b9fd2 2010-03-31 17:17:51 UTC
  parent:       ca047fa38443b91c8af5128af3259a18e155d161 2010-03-31 17:17:50 UTC
  tags:         rel-2-6-6-branch, thread-2-6-6
  comment:      Creating branch rel-2-6-6-branch (user: cvs)

  D:\TCL8.5\sources\thread>fossil timeline
  === 2011-06-27 ===
  17:33:12 [3cd44c9189] Restore compatibility with the latest TEA 3.9 revisions.
           (user: dgp tags: trunk)
  === 2011-04-27 ===
  02:09:13 [513462d6be] New ticket [d2df28c065] <i>repo missing two files</i>.
           (user: anonymous)
  === 2011-04-07 ===
  20:09:28 [46301b0bbb] Changes to wiki page [Tracking Thread Sources] (user:
           dgp)
  20:06:30 [2a50bdef79] Changes to wiki page [Tracking Thread Sources] (user:
           dgp)
  20:04:54 [cf50ec816c] Changes to wiki page [Tracking Thread Sources] (user:
           dgp)
  20:04:39 [3190817ee3] Changes to wiki page [Tracking Thread Sources] (user:
           dgp)
  19:45:12 [8abd2e0920] Changes to wiki page [Tracking Thread Sources] (user:
           dgp)
  17:13:55 [2ba78427ab] Changes to wiki page [Thread Bug Reports and Patch
           Submissions] (user: dgp)
  17:08:37 [505370c46f] Changes to wiki page [Tcl package Thread source code]
           (user: dgp)




  D:\TCL8.5\sources\thread>fossi
  rel-2-6-5-branch
  rel-2-6-6-branch
  thread-1-0
  thread-2-0
  thread-2-1
  thread-2-2
  thread-2-4
  thread-2-5
  thread-2-5-1
  thread-2-5-2
  thread-2-6
  thread-2-6-1
  thread-2-6-1-branch
  thread-2-6-2
  thread-2-6-2-branch
  thread-2-6-3
  thread-2-6-3-branch
  thread-2-6-4
  thread-2-6-4-branch
  thread-2-6-5
  thread-2-6-5-branch
  thread-2-6-6
  thread-2-6-branch
  thread-vendor-branch
  trunk


login: andreas_kupries
username: andreas_kupries
mimetype: text/plain
icomment:
  Note this:

  checkout: 60da38f803b49b8d9246ba1f55878264861b9fd2
  ==>           2010-03-31 17:17:51

  This is a checkout over a year old as well.

  ...

  Try fossil update trunk to get the latest.

  All the tags we have, even those named ...-branch' are plain tags from the conversion of CVS to fossil. Not a true branch. Closed as well, for the most part.


login: tombert
username: tombert
mimetype: text/plain
icomment:
  allow_comments - 1

  thx - I didn't realize that ... I'am new to fossil as well ...
  Did an update on 'trunk' and this compiles fine now.

  greets


login: hobbs
username: hobbs
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=72656
  Originator: NO

  I am not able to repro this bug using valgrind 3.2.1 on Linux x64.  Perhaps it is because it is being properly freed at exit, just not in the interim use?


login: nobody
username: nobody
mimetype: text/plain
icomment:
  Logged In: NO 

  So "current bytes allocated" output by [memory info] doesn't increase after each new connection in your tests ?


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086
  Originator: NO

  What you experience might be a problem in Tcl core
  that badly handles std channels. I'm loking at that
  at this moment.

  It appears to leak std channels on every thread 
  creation and exit. I haven't examined your code
  but will do that later on when I find out what the
  damn is happening with those channels and why do
  they leak.

  This is considered to be a very problematic point
  in Tcl design (handling of std channels when 
  building thread-enabled Tcl library).


login: oleg_derevenetz
username: oleg_derevenetz
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=1654299
  Originator: NO

  vasiljevic, if this issue will be resolved some day - it will be good. Are there any solutions or recommendations for this case now ?


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  I'm aware of that. I'll sync this one plus the tmml part
  as soon as I get the 2.4 version ready (working on it).
  Zoran


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  Now ready and checked in. See doc/man and doc/html
  directories. You must checkout the extension again, not
  just update your local copy, since some new files appeared.

  Zoran


login: davygrvy
username: davygrvy
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=7549

  ThreadKill() can't work on windows anyways.  Tcl_ThreadCreate doesn't save the new thread's handle.  Without it, we can't TerminateThread() with it.  Some people made strong requests for a way to kill a thread and there it is.  As far as can be done showing the limits of Tcl's API waiting for someone else to come along and finish it.

  suggest:  Se how the core save's handles associated to process IDs with the [pid] command.  Copy the idea for threads to allow the ability to get a handle from a global thread ID.

  Zoran is the owner of this project, not me.


login: tombert
username: tombert
mimetype: text/plain
icomment:
  Stripped down my application to reproduce the crash.
  The configured default path in the files point to d:/crash/...

  To reproduce call: wish crash.tcl
  If one removes a single procedure from the util.tcl the crash won't happen no more. Or you can remove a few method declarations from the socket_crash class, this will lead to an allocation error.
  Doing only "exit" without release works fine.

  thx for support


login: tombert
username: tombert
mimetype: text/plain
icomment:
  Another interesting behaviour is calling (in a virgin wish):
  > package re Thread
  > thread::release [thread::id]

  which does nothing in the first attempt, but calling it a second time it reports that the thread does not exists.
  What is the expected result when releasing the main thread itself?

  thx


login: andreas_kupries
username: andreas_kupries
mimetype: text/plain
icomment:
  Huh. Destruction of the main thread I would suspect to be a case of "pulling the rug out from under yourself".


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Releasing the main thread is syntactically possible
  and wrong. Actually releasing the 'current' thread
  ie. one calling the thread::release should be disabled
  or just a no-op'ed. I never thought of that as this is 
  rarely practically done.

  Releasing (i.e. letting it die away) any thread is always
  connected with various cleanups that may go astray
  in certain situations. For example, you lock a mutex in
  a thread and then release the thread. Moreover, the 
  Tcl lib was initially not desiged for thread safety. It is
  added some years later. So the entire cleanup/finalize
  machinery is not very thread-friendly and still assumes
  a "main" thread (the one that started the program) is
  always arround. 

  So.,, you never exit a "main" thread (whichever that is)
  and I will add a block that will effectively no-op the
  release of the 'self' i.e current thread (as explained above).

  Cheers
  Zoran 


login: tombert
username: tombert
mimetype: text/plain
icomment:
  ok - thx for the explanation, I'll take precaution not to release "myself" ...
  ... but this still leaves the crash open, I'll post a quick look into the code here:

  package require Thread
  proc crash {} {
      set ::tid [thread::create {thread::wait}]
      puts $::tid
      thread::send $::tid {
  package require Itcl
  namespace import itcl::*
  source d:/crash/socket_crash.itcl
  source d:/crash/util.tcl
  set obj [socket_crash "#auto"]
      }
      thread::release $::tid
  }
  after idle crash


login: sebres
username: sebres
mimetype: text/plain
icomment:
  possible duplicate to 3494470?
  https://sourceforge.net/tracker/?func=detail&aid=3494470&group_id=10894&atid=110894


login: dkf
username: dkf
mimetype: text/plain
icomment:
  Technically, since neither is actually particularly clearer as to the deep nature of the bug, we'd roll that one into this.

  My main concern is that we have code that can crash at all. That's Bad.


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  Corrected starting from 1.8 of doc/thread.n.
  The *.html and *.tmml docfiles are still not updated.


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  Fixed in current CVS HEAD and replicated into thread.n
  and thread.html


login: juliannoble
username: juliannoble
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=598324
  Originator: YES

  Checked with Tcl 8.5a6 & Thread 2.6.5 (2006-dec-04 cvs) on windows.
  Above script causes Tclsh to crash.


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086
  Originator: NO

  Confirmed. Fixed in current HEAD branch. Will appear
  first in 2.6.6 patch release.

  Thanks for reporting.


login: dgp
username: dgp
mimetype: text/plain
icomment:
  committed a possible patch to fix this
  to the "finalize" branch.  If you agree
  feel free to cherry pick it onto the trunk.


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086
  Originator: NO

  I think we can close this one as it is related
  to Bug# 1716587. Lets not keep both open
  at the same time.


login: taylor_venable
username: taylor_venable
mimetype: text/plain
icomment:
  Sorry for the late reply; I just tried the version from CVS and it works as expected.  Thanks!  I'm going to close this ticket since it doesn't really apply to the development version of the code.


login: tombert
username: tombert
mimetype: text/plain
icomment:
  A bit more on the tcl what I'am doing:
  It's a rather simple procedure that calls itself after a random timeout, the first line is accessing the tsv "host" variable which you can see in the segmentation fault. So I've got the feeling that the "after" is called during or after the thread has been destroyed. Hope this helps a bit.
  Thx for your support.


  proc pingCheckCycle {tid cbObj} {
      set newState [isping [tsv::get host [thread::id]]]
      if {$newState != [tsv::get pingStatus [thread::id]]} {
          tsv::set pingStatus [thread::id] $newState
          thread::send -async $tid [list $cbObj callCallback -type onpingcb -map [list %ping $newState]]
      }
      after [expr {entier(3000 + rand()*3000)}] [list pingCheckCycle $tid $cbObj]
  }


login: dgp
username: dgp
mimetype: text/plain
icomment:
  The Thread 2.7 branch includes some experimental
  efforts to attend to finalization matters.  You may be
  demonstrating to use that's a fool's errand.

  If you can try the same testing using the "trunk" of
  the Thread package to check whether it's any different,
  that would be helpful.  If it's more convenient, testing
  release Thread 2.6.7 should be good enough.


login: tombert
username: tombert
mimetype: text/plain
icomment:
  Head gives me a compile error, but I will try with 2.6.7 as you suggested.

  config.status: creating pkgIndex.tcl
  gcc -DPACKAGE_NAME=\"thread\" -DPACKAGE_TARNAME=\"thread\" -DPACKAGE_VERSION=\"2.7b1\" -DPACKAGE_STRI
  NG=\"thread\ 2.7b1\" -DPACKAGE_BUGREPORT=\"\" -DBUILD_thread= -DSTDC_HEADERS=1 -DHAVE_SYS_TYPES_H=1 -
  DHAVE_SYS_STAT_H=1 -DHAVE_STDLIB_H=1 -DHAVE_STRING_H=1 -DHAVE_MEMORY_H=1 -DHAVE_STRINGS_H=1 -DHAVE_IN
  TTYPES_H=1 -DHAVE_STDINT_H=1 -DHAVE_UNISTD_H=1 -DTCL_THREADS=1 -DMODULE_SCOPE=extern -DNO_VIZ= -DTCL_
  WIDE_INT_TYPE=long\ long -DHAVE_LSEEK64=1 -DUSE_TCL_STUBS=1   -I"./win" -I"/d/tcltk85/release/include
  "    -pipe -O2 -fomit-frame-pointer -Wall   -c `echo ./generic/threadCmd.c` -o threadCmd.o
  ./generic/threadCmd.c: In function 'ThreadInit':
  ./generic/threadCmd.c:466:13: error: 'threadHaveInterpCancel' undeclared (first use in this function)

  ./generic/threadCmd.c:466:13: note: each undeclared identifier is reported only once for each functio
  n it appears in
  ./generic/threadCmd.c:471:13: error: 'ThreadCancelObjCmd' undeclared (first use in this function)
  ./generic/threadCmd.c: In function 'ThreadWait':
  ./generic/threadCmd.c:2797:9: warning: unused variable 'haveInterpCancel' [-Wunused-variable]
  make: *** [threadCmd.o] Error 1


login: tombert
username: tombert
mimetype: text/plain
icomment:
  2.6.7 crashes with two times:
  1)
  called Tcl_FindHashEntry on deleted table
  [Inferior 1 (process 4528) exited with code 03]

  2)
  #0  CreateHashEntry (tablePtr=0x5ba1a8, key=0x64ea3404 "", newPtr=0x0) at ./../generic/tclHash.c:338
  338                 if (hash != PTR2UINT(hPtr->hash)) {
  (gdb) up
  #1  0x64df8baf in FindHashEntry (tablePtr=0x5ba1a8, key=0x64ea3404 "") at ./../generic/tclHash.c:242
  242         return CreateHashEntry(tablePtr, key, NULL);
  (gdb)
  #2  0x64e46f4e in TclpThreadDataKeyGet (keyPtr=0x64ea3404) at ./../generic/tclThreadStorage.c:359
  359         Tcl_HashEntry *hPtr = Tcl_FindHashEntry(hashTablePtr, (char *) keyPtr);
  (gdb)
  #3  0x64e4542f in Tcl_GetThreadData (keyPtr=0x64ea3404, size=8) at ./../generic/tclThread.c:85
  85          result = TclpThreadDataKeyGet(keyPtr);
  (gdb)
  #4  0x64e25ebb in TclFreeObj (objPtr=0x376a140) at ./../generic/tclObj.c:1410
  1410            ObjInitDeletionContext(context);
  (gdb)
  #5  0x64e2386d in NsEnsembleImplementationCmd (clientData=0x37637e8, interp=0x3761d68, objc=4, objv=0x376b010) at ./../g
  eneric/tclNamesp.c:6242
  6242            Tcl_DecrRefCount(copyObj);
  (gdb)
  #6  0x64d92aa9 in TclEvalObjvInternal (interp=0x3761d68, objc=4, objv=0x376b010, command=0xffffffff <Address 0xffffffff
  out of bounds>, length=-1, flags=0) at ./../generic/tclBasic.c:3703
  3703            code = (*cmdPtr->objProc)(cmdPtr->objClientData, interp, objc, objv);
  (gdb)
  #7  0x64de4ba5 in TclExecuteByteCode (interp=0x3761d68, codePtr=0x3b0a118) at ./../generic/tclExecute.c:2416
  2416                result = TclEvalObjvInternal(interp, objc, objv,
  (gdb)
  #8  0x64e3624a in TclObjInterpProcCore (interp=0x3761d68, procNameObj=0x3a449f0, skip=1, errorProc=0x64e3675d <MakeProcE
  rror>) at ./../generic/tclProc.c:1763
  1763            result = TclExecuteByteCode(interp, codePtr);
  (gdb)
  #9  0x64e361bd in TclObjInterpProc (clientData=0x3a5fd18, interp=0x3761d68, objc=7, objv=0x376af30) at ./../generic/tclP
  roc.c:1653
  1653            return TclObjInterpProcCore(interp, objv[0], 1, &MakeProcError);
  (gdb)
  #10 0x64d92aa9 in TclEvalObjvInternal (interp=0x3761d68, objc=7, objv=0x376af30, command=0xffffffff <Address 0xffffffff
  out of bounds>, length=-1, flags=0) at ./../generic/tclBasic.c:3703
  3703            code = (*cmdPtr->objProc)(cmdPtr->objClientData, interp, objc, objv);
  (gdb)
  #11 0x64de4ba5 in TclExecuteByteCode (interp=0x3761d68, codePtr=0x3aa33b0) at ./../generic/tclExecute.c:2416
  2416                result = TclEvalObjvInternal(interp, objc, objv,
  (gdb)
  #12 0x64de2d81 in TclCompEvalObj (interp=0x3761d68, objPtr=0x376a0f8, invoker=0x0, word=0) at ./../generic/tclExecute.c:
  1538
  1538            result = TclExecuteByteCode(interp, codePtr);
  (gdb)
  #13 0x64d947f4 in TclEvalObjEx (interp=0x3761d68, objPtr=0x376a0f8, flags=0, invoker=0x0, word=0) at ./../generic/tclBas
  ic.c:5272
  5272            result = TclCompEvalObj(interp, objPtr, invoker, word);
  (gdb)
  #14 0x64d94414 in Tcl_EvalObjEx (interp=0x3761d68, objPtr=0x10, flags=0) at ./../generic/tclBasic.c:5054
  5054        return TclEvalObjEx(interp, objPtr, flags, NULL, 0);


login: tombert
username: tombert
mimetype: text/plain
icomment:
  2.7 head still crashes:

  Program received signal SIGSEGV, Segmentation fault.
  [Switching to Thread 6480.0x1a7c]

  #0  0xfeeefeee in ?? ()
  (gdb) up
  #1  0x64e46f4e in TclpThreadDataKeyGet (keyPtr=0x64ea3404) at ./../generic/tclThreadStorage.c:359
  359         Tcl_HashEntry *hPtr = Tcl_FindHashEntry(hashTablePtr, (char *) keyPtr);
  (gdb)
  #2  0x64e4542f in Tcl_GetThreadData (keyPtr=0x64ea3404, size=8) at ./../generic/tclThread.c:85
  85          result = TclpThreadDataKeyGet(keyPtr);
  (gdb)
  #3  0x64e25ebb in TclFreeObj (objPtr=0x4fa6240) at ./../generic/tclObj.c:1410
  1410            ObjInitDeletionContext(context);
  (gdb)
  #4  0x64e55a3f in TclArraySet (interp=0x38c35a0, arrayNameObj=0x4f3beb0, arrayElemObj=0x5033128) at ./../generic/tclVar.
  c:3437
  3437            Tcl_DecrRefCount(copyListObj);
  (gdb)
  #5  0x64e5528b in Tcl_ArrayObjCmd (dummy=0x0, interp=0x38c35a0, objc=4, objv=0x38b7790) at ./../generic/tclVar.c:3162
  3162            return TclArraySet(interp, objv[2], objv[3]);
  (gdb)
  #6  0x64d92aa9 in TclEvalObjvInternal (interp=0x38c35a0, objc=4, objv=0x38b7790, command=0xffffffff <Address 0xffffffff
  out of bounds>, length=-1, flags=0) at ./../generic/tclBasic.c:3703
  3703            code = (*cmdPtr->objProc)(cmdPtr->objClientData, interp, objc, objv);
  (gdb)
  #7  0x64de4ba5 in TclExecuteByteCode (interp=0x38c35a0, codePtr=0x5070398) at ./../generic/tclExecute.c:2416
  2416                result = TclEvalObjvInternal(interp, objc, objv,
  (gdb)
  #8  0x64de2d81 in TclCompEvalObj (interp=0x38c35a0, objPtr=0x38cb8d0, invoker=0x0, word=0) at ./../generic/tclExecute.c:
  1538
  1538            result = TclExecuteByteCode(interp, codePtr);
  (gdb)
  #9  0x64d947f4 in TclEvalObjEx (interp=0x38c35a0, objPtr=0x38cb8d0, flags=0, invoker=0x0, word=0) at ./../generic/tclBas
  ic.c:5272
  5272            result = TclCompEvalObj(interp, objPtr, invoker, word);
  (gdb)
  #10 0x64d94414 in Tcl_EvalObjEx (interp=0x38c35a0, objPtr=0xfeeefeee, flags=0) at ./../generic/tclBasic.c:5054
  5054        return TclEvalObjEx(interp, objPtr, flags, NULL, 0);
  (gdb)
  #11 0x710ccd20 in Itcl_EvalMemberCode (interp=0x38c35a0, mfunc=0x506e3c0, member=0x5038138, contextObj=0x5037eb8, objc=9
  , objv=0x38b773c) at ./generic/itcl_methods.c:993
  993             result = Tcl_EvalObj(interp, mcode->procPtr->bodyPtr);
  (gdb)
  #12 0x710cd9b8 in Itcl_ExecMethod (clientData=0x506e3c0, interp=0x38c35a0, objc=9, objv=0x38b773c) at ./generic/itcl_met
  hods.c:1504
  1504        result = Itcl_EvalMemberCode(interp, mfunc, member, contextObj,
  (gdb)
  #13 0x710d3559 in Itcl_EvalArgs (interp=0x38c35a0, objc=9, objv=0x38b773c) at ./generic/itcl_util.c:1389
  1389        result = (*cmdPtr->objProc)(cmdPtr->objClientData, interp,
  (gdb)
  #14 0x710d0062 in Itcl_HandleInstance (clientData=0x5037eb8, interp=0x38c35a0, objc=10, objv=0x38b7738) at ./generic/itc
  l_objects.c:682
  682         result = Itcl_EvalArgs(interp, objc-1, objv+1);
  (gdb)
  #15 0x64d92aa9 in TclEvalObjvInternal (interp=0x38c35a0, objc=10, objv=0x38b7738, command=0xffffffff <Address 0xffffffff
   out of bounds>, length=-1, flags=0) at ./../generic/tclBasic.c:3703
  3703            code = (*cmdPtr->objProc)(cmdPtr->objClientData, interp, objc, objv);
  (gdb)
  #16 0x64de4ba5 in TclExecuteByteCode (interp=0x38c35a0, codePtr=0x1a843e30) at ./../generic/tclExecute.c:2416
  2416                result = TclEvalObjvInternal(interp, objc, objv,
  (gdb)


login: tombert
username: tombert
mimetype: text/plain
icomment:
  Just a question: would a crashing sample be more of help than a backtrace? Because I already tried and failed ... but I could try harder if you like me to.
  thx


login: tombert
username: tombert
mimetype: text/plain
icomment:
  Are there any differences between Win7 and XP? because yesterday on Win7 I have been able to reproduce the crashes on nearly every attempt. Today working on XP (same binary same source) I'am unable to let it crash.

  I also tried to write a tcl script resembling my threading architecture but havn't been able to let it crash.


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Somebody/something is either trampling over the memory
  OR 
  there is some race-condition on cleanup/finalization if you
  regularily exit threads. 

  The latter can be circumvented by never exiting threads
  (I know it sounds weird).

  The former may cost lots of resources and hair pulling to
  find out (as any other memory corruption does). In this
  case you need to build a minimalistic system that still 
  exposes the error so other people may throw some mem
  analysers on that. The threading extension is completely
  platform neutral. If there is a problem there is must hit
  you on any platfrom and valgrind et.al. should be able
  to point to the problem...


   


login: tombert
username: tombert
mimetype: text/plain
icomment:
  Well ... good news (depending on the viewpoint) - I have been able to track down the cause ... it consists of two lines:

  console show
  package require Pgtcl

  Then an exit and it crashes.
  The Pgtcl I'am using is 1.9.0 and I'am going to report this immediately to the developer:
  https://sourceforge.net/tracker/?func=detail&aid=3454495&group_id=516797&atid=2100300

  Thx for your support, sorry for reporting a "wrong" error ...
  Thomas


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  allow_comments - 1

  OK. Exiting an Tcl shell with any user-threads still active
  is a bad idea. Because the Tcl library is NOT safe w.r.t.
  finalisation. If you (or the other package) has created
  some threads that use Tcl library you need to be sure
  they are all gone before exiting the main thread (the one
  that started-up the application).

  As I see you are able to "kill" the app entirely w/o the
  Thread extension involved which means there is some
  memory problem *elsewhere*. This is good for us but bad
  for you. Well... that's life. I hope you will be able to track
  this problem and I wish you good luck with that.

  I am going to close this ticker.


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  Hi!

  This is I believe the side-effect of the thread memory allocator
  which is known to be memory greedy as it was optimized on
  speed and not memory usage. I have seen this behaviour on 
  all Unices as well, not only Windows. Although it seems that on
  Unices is far less visible. I'm able to create and terminate about
  30.000 threads and the VM usage increases about 1MB.


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086
  Originator: NO

  I will still leave this open but will try to replace
  the greedy Tcl memory allocator with just calls
  to regular system allocator and see if this helps. 
  If yes then we can think of making an alternate 
  allocator for Tcl OR even just simple use the 
  system-level one as those beasts are more
  thread-aware today as they have been 5 year ago.


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Perhaps this is a problem related to memory leak in the teardown logic.
  I will need to investigate the memory leak problem first.


login: dgp
username: dgp
mimetype: text/plain
icomment:
  Testing the trunk (2.6.8) and thread-2-7-branch (2.7b1)
  on Linux with the submitted test scripts I see no leaks
  with the Tcl trunk.

  Would be good for someone to confirm on Windows.


login: sebres
username: sebres
mimetype: text/plain
icomment:
  I could also not reproduce it on linux, but in windows.
  see also bug 3493120: https://sourceforge.net/tracker/?func=detail&aid=3493120&group_id=10894&atid=110894
  Possible the same bug?
  Have a linux kernel any "garbage collector" for therminated threads, its shared memory or something else?


login: dgp
username: dgp
mimetype: text/plain
icomment:
  Joe, did we decide this one is out of date?

  That is, the leak problems on Windows are
  already fixed?  Thanks for closing this ticket
  if you confirm.


login: mistachkin
username: mistachkin
mimetype: text/plain
icomment:
  Used the provided test script on Windows 8 and created 16K threads.  Total memory used by the process was around 2MB.  Therefore, no leak.


login: elfring
username: elfring
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=572001

  How are "cancellation requests" treated?
  - 
  http://docs.sun.com/db/doc/806-6867/6jfpgdcmh?l=de&a=view#tlib-
  82704, http://docs.sun.com/db/doc/806-
  6867/6jfpgdcmd?q=pthread&a=view
  - 
  http://www.gnu.org/software/pth/pth-manual.html#cancellation 
  management


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  Very simple. Thread cancellation is not part of the Tcl API. 
  There are no hooks for that. Also, I have a very bad feeling 
  about that and I do not think that cancellation is a good thing. 
  Threads shound not be cancelled. They should be marked 
  for exit and then they should exit gracefully. Everything else 
  is looking for trouble, 


login: elfring
username: elfring
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=572001

  The technique "interruption" is used as a solution to avoid cancellation by the 
  library "ZThreads". 
  (http://zthread.sourceforge.net/overview.html)
  Can this design 
  pattern be used by your implementation, too?


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  I find this very interesting. I will have a look on Zthreads. 
  Thanks for providing the link. 


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  I'm closing this once since there is nothing really we could do 
  to influence the thread cancellation, which, in itself,  is a bad thing 
  anyway. 


login: zoro2
username: zoro2
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=191529

  Did some more testing. This seems to only crash if thread::
  release is also called. Tested to work on Linux/x86 and 
  Tcl8.4.13+Thread2.6.3 and Tcl8.4.5+Thread2.5.

  Also, adding -wait to thread::release does not help. But, 
  calling:

  package require Thread; set t [thread::create]; thread::
  release $t; after 25; exit

  Did not throw any exception. So it seems that the thread 
  itself needs some time to clean up.


login: zoro2
username: zoro2
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=191529

  Did some more research on Solaris. It seems to also exit 
  with an error there, but doing some wait also helps.


login: dgp
username: dgp
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=80530


  Huh?  The same combination both
  works and fails?  Care to start
  over?


login: zoro2
username: zoro2
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=191529

  Sorry, forgot to attach the gdb dump:

  Starting program: /home/users/tcldev/8413/bin/tclsh8.4 /tmp/
  x.tcl
  [Thread debugging using libthread_db enabled]
  [New Thread 16384 (LWP 631)]
  [New Thread 32769 (LWP 632)]
  [New Thread 16386 (LWP 633)]
  [New Thread 32771 (LWP 634)]
  Tcl_Release couldn't find reference for 0x80e0450

  Program received signal SIGABRT, Aborted.
  [Switching to Thread 32771 (LWP 634)]
  0x401ad7c1 in kill () from /lib/libc.so.6
  (gdb) bt
  #0  0x401ad7c1 in kill () from /lib/libc.so.6
  #1  0x40119771 in pthread_kill () from /lib/libpthread.so.0
  #2  0x40119a7b in raise () from /lib/libpthread.so.0
  #3  0x401ad554 in raise () from /lib/libc.so.6
  #4  0x401aea88 in abort () from /lib/libc.so.6
  #5  0x400a104a in Tcl_PanicVA (format=0x400d6640 
  "Tcl_Release couldn't find reference for 0x%x",
      argList=0xbf5ffa78 
  "(D\020@0Ă\f\b\222Ł+@P\004\016\b\230Î\016\b\f") at /home/
  users/tcldev/dqkit/src/tcl-8.4.13/generic/tclPanic.c:106
  #6  0x400a1080 in Tcl_Panic (arg1=0x400d6640 "Tcl_Release 
  couldn't find reference for 0x%x")
      at /home/users/tcldev/dqkit/src/tcl-8.4.13/generic/
  tclPanic.c:134
  #7  0x400a99cb in Tcl_Release (clientData=0x80e0450) at /
  home/users/tcldev/dqkit/src/tcl-8.4.13/generic/tclPreserve.
  c:245
  #8  0x402ba392 in NewThread () from /home/users/tcldev/8413/
  lib/thread2.6.3/libthread2.6.3.so
  #9  0x080ece98 in ?? ()
  #10 0x0000000c in ?? ()
  #11 0x00020000 in ?? ()
  #12 0x4010302c in __JCR_LIST__ () from /home/users/tcldev/
  8413/lib/libtcl8.4g.so
  #13 0x0000000c in ?? ()
  #14 0x402c9274 in threadList () from /home/users/tcldev/
  8413/lib/thread2.6.3/libthread2.6.3.so
  #15 0x4010302c in __JCR_LIST__ () from /home/users/tcldev/
  8413/lib/libtcl8.4g.so
  #16 0xbf5ffbe0 in ?? ()
  #17 0x00000000 in ?? ()
  #18 0xbf5ffacc in ?? ()
  #19 0x4006e11d in NewThreadProc (clientData=0x1) at /home/
  users/tcldev/dqkit/src/tcl-8.4.13/generic/tclEvent.c:1229
  Previous frame inner to this frame (corrupt stack?)


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  Ah this again....

  This is a LONG known problem of the Tcl library 
  It is inherently UNSAFE to exit a process while
  still having some other threads arround using
  the Tcl library. The problem is that invoking 
  the exit, the Tcl library will be finalized 
  WHILE some other thread potentially still 
  makes use of it. 

  This cannot be solved without extensive modifications
  on how Tcl library is handling teardowns. This is NOT
  trivial and I will NOT fix that way until 9.0 Tcl as
  this will require numerous changes at various points
  and I can't justify that for the 8.x code.


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086
  Originator: NO

  If this is the only one, then I did an extremely good job ;-)
  Anyways, thanks for spotting this...


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  I'm unable to get this reproduced on and latest 
  CVS state of Tcl as of Sun Aug 06 11:28:15 CEST 2006
  With or without symbols enabled.
  I'm running tests on the following machines:
  Linux devlinux 2.6.4-52-default #1 Wed Apr 7 02:08:30 UTC 2004 i686 
  athlon i386 GNU/Linux
  Darwin zvpb.local 8.7.0 Darwin Kernel Version 8.7.0: Fri May 26 15:20:53 
  PDT 2006; root:xnu-792.6.76.obj~1/RELEASE_PPC Power Macintosh powerpc


login: mic42
username: mic42
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=302287

  The crashing machine has this for uname:
  Linux chronos 2.6.16.21-0.13-smp #1 SMP Mon Jul 17 17:22:44
  UTC 2006 x86_64 x86_64 x86_64 GNU/Linux

  With this glibc:
  GNU C Library development release version 2.4 (20060526), by
  Roland McGrath et al.
  Copyright (C) 2006 Free Software Foundation, Inc.
  This is free software; see the source for copying conditions.
  There is NO warranty; not even for MERCHANTABILITY or
  FITNESS FOR A
  PARTICULAR PURPOSE.
  Configured for i686-suse-linux.
  Compiled by GNU CC version 4.1.0 (SUSE Linux).
  Compiled on a Linux 2.6.16 system on 2006-05-26.
  Available extensions:
          crypt add-on version 2.1 by Michael Glad and others
          GNU Libidn by Simon Josefsson
          GNU libio by Per Bothner
          NIS(YP)/NIS+ NSS modules 0.19 by Thorsten Kukuk
          NoVersion patch for broken glibc 2.0 binaries
          Native POSIX Threads Library by Ulrich Drepper et al
          BIND-8.2.3-T5B
  Thread-local storage support included.

  So maybe this is an SMP issue?

  Any advice what i could do to get more info about this issue?


login: hobbs
username: hobbs
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=72656
  Originator: NO

  Is this possibly related to 1654104?  It requires using unlimited stack size in your shell settings.  Fixed 2007-02-08.


login: hobbs
username: hobbs
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=72656
  Originator: NO

  I believe this fix covered it:

  2007-05-29  Jeff Hobbs  <jeffh@ActiveState.com>

  * unix/tclUnixThrd.c (Tcl_JoinThread): fix for 64-bit handling of
  pthread_join exit return code storage. [Bug 1712723]


login: ferrieux
username: ferrieux
mimetype: text/plain
icomment:
  Can you retry with 8.6 HEAD ? It seems careful steps are taken to avoid destroying locked things now.


login: dgp
username: dgp
mimetype: text/plain
icomment:
  A partial remedy has been committed
  (thanks aku!) using [::apply] when it
  is available to avoid the pollution.

  Still would like to see the Thread and Ttrace
  packages disentangled, but getting Thread
  bundled with Tcl is more important than that.


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Somebody already added this in the pkgIndex.tcl.in in HEAD (wasn't me). 
  Was it yourself? 
  Can I close this one?


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  Depends. I'm on the business-trip around Europe until 
  end of this week. At the same time, our internal release 
  schedule is over, leaving me more time beginning of the 
  next week to close open issues.  
  So, if the 8.4.8 is hitting this week, no. Otherwise I see 
  a great chance to fix this before 8.4.8 release. 
   


login: hobbs
username: hobbs
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=72656

  Please look into this ASAP.  We have an 8.4.9 scheduled for
  early Dec.


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  Fixed in the CVS head. The threading extension is still not at 
  2.6 release although I see no big problem why not releasing 
  it now. Note that I disabled all channel-transfer tests for  
  Windows ports since core is still not operating correctly there. 


login: davygrvy
username: davygrvy
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=7549

  All fixed.  Thanks for the new feature.

  NOTICE: this has been adopted into the thread extension 
  rather than the testthread extension of the test suite.


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  This has been fixed in CVS head and the 2.6 release  
  of the threading extension should be out together with 
  the 8.4.9 core. Therefore, I will close this item and mark 
  it "Wont Fix". 


login: twylite
username: twylite
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=91629
  Originator: YES

  On this patch is applied the following bugs/patches can also be closed:

  [ 1716587 ] Thread tests can't run (bad args from makefile)
  [ 1689375 ] Update VC makefile for Threads (Fix for bug 1643367)
  [ 1643367 ] Thread extension cannot compile with VC8


login: patthoyts
username: patthoyts
mimetype: text/plain
icomment:
  Update the msvc build files for the more recent versions that support the newer compilers and platforms. All tests pass ok on 32 and 64 bit versions.


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  OK. I am planning to release 2.7.0 until end of October containing one 
  important thread::cancel command implementation. Along that I will
  look into adding this (for me still unknown) feature of Tcl 8.5.


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  Generally speaking, all Tcl code should be thread-safe. 
  It is the C-code which makes (might make) problems. 
  Also very important to note is the Tcl "cd" command, which 
  although thread-safe per-se, changes the process current 
  directory for every thread in the process and is, thus, source 
  of potential problems.  
   


login: elfring
username: elfring
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=572001

  1. Is the library "Tcllib" written in "pure Tcl code"?

  2. Please add your 
  answers to the documentation.
     I would like to see there a general 
  statement about reentrant, thread-safe and asynchronous-safe 
  functions.
  - http://www.gnu.org/software/pth/pth-manual.html#the 
  world of threading
  - 
  http://www.tru64unix.compaq.com/docs/base_doc/DOCUMENTATION/V51_HTML/ARH9RBTE/DOCU0008.HTM#nonthreaded


login: elfring
username: elfring
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=572001

  1. Is there a source file that might be not thread-safe and not 
  asynchronous-safe because it was written in a programming 
  language like C oder C++?

  3. Are the source files that are not written in TCL checked with 
  tools like the following?
  - http://splint.org/
  - http://www.garret.ru/~knizhnik/jlint/ReadMe.htm#introduction
  - http://www.cs.utexas.edu/users/emery/#broadway


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  I do not understand... 
   
  The thread-extension itself is thread-safe. 
  All Tcl code is already thread-safe because of the  
  thread-safe Tcl core. 
  Arbitrary exensions loaded into the Tcl shell are out 
  of the scope of my work. Normally, the package author 
  should make his/her code thread-safe if you want to use 
  his/her code in the MT-enabled Tcl shell.  
   
  I normally run the Purify checker on all of my C-code. 
  I do not exercise the code wth any other MT-tool. 
  I never had any need for that, yet :-) 
   
   


login: dkf
username: dkf
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=79902

  Markus: Tcllib does not contain any non-Tcl source files, by
  explicit policy (though not for thread-safety but rather
  app-developer accessibility reasons.)  Consequently, they
  are guaranteed to be at least as thread-safe as the Tcl core
  itself.  However, there are some operations that are part of
  the core which are best not used from a multi-threaded
  context, though the only one I can think of right now is
  [cd] (because it affects a per-process resource.)  However,
  it is quite possible (and in-fact easy, especially with the
  right options to [glob] etc.) to write Tcl scripts that
  never rely on the current directory after a start-up period,
  and it is poor policy indeed to write a library which
  depends on the current directory!  Ultimately, it is best to
  keep the current directory as a user-concept, coupling each
  [cd] to a request to change the current directory by the
  user.

  Closing this FRQ since it is categorically satisfied to the
  extent that the Tcl project at SF is interested in it. 
  Further questions should be raised with the tcllib project
  itself, or in the tcl-threads mailing list.


login: elfring
username: elfring
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=572001

  Thank you for your answers.
  It was that kind of information that I was looking for.

  I hope that all Tcllib users know about these thread-safety issues 
  with Tcl code.


login: twylite
username: twylite
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=91629
  Originator: NO

  And that's only the start of your problems ...

  I have just got the latest thread module from CVS, and am getting errors building it with VC6 or VC8 (on Windows 2000).

    c:\build\thread\win\vc>nmake /f makefile.vc all OPTS=threads TCLDIR=..\..\..\tcl
    ...<snip>
    LINK : fatal error LNK1181: cannot open input file "..\..\..\tcl\win\Release\tclstub84.lib"
    NMAKE : fatal error U1077: 'link' : return code '0x49d'
    Stop.

  I am, of course, building this extension for use with Tcl 8.5.  The problem can be solved by adding a TCL_DOTVERSION argument to nmake, but this should be detected via tcl.h (and TCLDIR).

    nmake /f makefile.vc all OPTS=threads TCLDIR=..\..\..\tcl TCL_DOTVERSION=8.5
    
  --

  Next, the tests fail to run.

    c:\build\thread\win\vc>nmake /f makefile.vc test OPTS=threads TCLDIR=..\..\..\tcl TCL_DOTVERSION=8.5
    ...<snip>
    thread.test
    can't read "::tcl_PatchLevel": no such variable
    ...<snip>
    Tests ended at Tue Mar 27 17:16:53 +0200 2007
    all.tcl:        Total   0       Passed  0       Skipped 0       Failed  0

  Capatalisation problem with patchLevel.  Easily solved -- but it's been that way since January 2005?!  

  --

  Third, "make install" only copies the DLL; it doesn't create pkgIndex.tcl.

  This can be solved by manually creating a pkgIndex.tcl based on pkgIndex.tcl.in, or modifying the makefile.vc to create the pkgIndex.tcl using its variables.

  The install also doesn't name the lib folder correctly (thread instead of thread2.6), and doesn't copy other thread lib files (ttrace.tcl).

  --

  Forth, pkg.vc reflects an old version (2.6.3) which is not in sync with configure.in.

  --

  And then you can't even build at all with VC8.

  --

  The attached patch updates the makefile.vc and rules.vc to be in line with the sampleextension in CVS (plus the changes I have submitted in bug [ 1689286 ] nmakehlp not building on Win32 with VC).

  Tested with VC6 and VC8 on Windows 2000.  Builds, installs and runs correctly (including running all tests), so far as I can tell ;)

  There are a number of changes:
    - configure.in: note to change DOTVERSION in makefile.vc when you update the version
    - win/thread.rc: the sampleextension makefile.vc provides different variables/defines to rc
    - tests/thread.test: tcl_PatchLevel ==> tcl_patchLevel
    - win/makefile.vc, win/nmakehlp.c and win/rules.vc: added, based on sampleextension with minimal changes needed to make it work with threads extension

  The files win/vc/makefile.vc, win/vc/nmakehlp.c, win/vc/pkg.vc and win/vc/rules.vc are no longer required and should be deleted.  They have effectively been moved to win/ and replaced with updates based on sampleextension.  If these files stay in win/vc and get modified to be in a "non-standard" location, then we ensure this pain every time they need to be updated (otherwise we just copy from sampleextension).


login: twylite
username: twylite
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=91629
  Originator: NO

  Umm ... I could be being thick here, but I can't see a way to attach a file to a bug ... ;(

  I've submitted my fix as a patch, please refer to patch 1689375.


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  Can you help me with this: What is TLS package?
  If you do not load the TLS package in newly created 
  threads, are you still able to get the crash?


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086
  Originator: NO

  I cannot reproduce this with Tcl / Threa extension.
  It must be something related to external packages.


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  The "lockt" has been removed.
  The "freeCt" is used in the code as the top of free container list 
  (similar to the list of pre-allocated Tcl Objs) and cannot be removed.

  The NUMSPBUCKETS is a count of slots containing shared arrays.
  Instead of locking the array during access, the slot (bucket) containing 
  the array is locked. Arrays are distributed among buckets using a simple
  hash that maps the array name into one of the buckets. 
  The whole logic was "borrowed" from AOL/Naviserver.
  Setting the value larger will reduce locking contention for the bucket
  and thus allow more concurrency. Unfortunately we do not have any
  metrics built into the code that would allow locking introspection and
  finding "hot" buckets. This would be necessary in order to experiment
  with the number of buckets. The value of 32 is found to be "good enough"
  for most of the applications.  


login: dgp
username: dgp
mimetype: text/plain
icomment:
  Thanks for reviewing the request. It's not urgent, but
  good to have others having a look.

  The "lockt" field is still present in the SpBucket struct
  on the trunk.

  I doubt that the "freeCt" is used in the way you claim.
  (Don't confuse it with the field of the same name in
  the Bucket struct in threadSvCmd.h)

  I've had the field commented out for many many weeks
  in a local sandbox with no apparent ill effect.   If I'm overlooking
  something, then adding something to the test suite that would
  notice this field being missing would be a good thing to do.


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  You are right about the freeCt. It is defined twice
  but one of the instances is unnecessary. I overlooked
  that. Will remove that now.

  Apropos lockt... It is removed and I "fossil push"ed so
  I cannot explain why you still see it. In my sandbox it
  is gone.


login: dgp
username: dgp
mimetype: text/plain
icomment:
  File Added - 125622: 1163357.patch

  Logged In: YES 
  user_id=80530


  Attached patch is good enough
  to let the current test suite run
  successfully.  Perhaps better solutions
  involve [tcl_findLibrary] or
  [thread::pkgconfig get scriptdir,source].


login: dgp
username: dgp
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=80530


  BTW, that [subst] scheme within
  the pkgIndex.tcl file will also fail
  whenever $dir has spaces in it,
  or is otherwise "list-unsafe".


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  Fixed in HEAD. Also corrected the problem when directory 
  path contains blanks or other special characters. 


login: elfring
username: elfring
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=572001

  This tool will need some functions in the thread interface to query the dependencies between the running or 
  waiting threads.
  A helpful function would be to return a list of identifiers for deadlocked threads.
  I suggest to offer the function "thread::is_deadlocked id", too.


login: elfring
username: elfring
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=572001

  The solution will be somewhere in the lock APIs. 
  (http://docs.sun.com/db/doc/806-6867/6jfpgdcml?a=view)


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  Detecting deadlock may be done be wrapping the mutex API 
  i.e. providing your own set of mutex operations. 
  This would, however, slow down the application.  
  I might put this into the TIP I'm preparing about the  
  Tcl threading API. This may be then conditionaly compiled 
  for debugging purposes. 
   


login: elfring
username: elfring
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=572001

  Damned - The pthread interface does not offer any function to query the 
  thread and lock usage.
  1. I am wondering now why this was not 
  introduced into POSIX 1003.1c after this standard was available for so many 
  years.
  2. I'm curious for your TIP. I hope that the TCL community will find 
  an other function library to retrieve the requested information.


login: elfring
username: elfring
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=572001

  3. Is there anything different with "Java-platform-level deadlocks" compared 
  to "Tcl-platform-level deadlocks"?
  What are the internal programming 
  interfaces?


login: elfring
username: elfring
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=572001

  4. Another solution:
  A detection is not needed if a system was created 
  deadlock 
  free.
  (http://groups.google.com/groups?dq=&start=150&hl=de&lr=&ie=UTF-
  8&group=comp.hypercube&selm=302%40hubcap.UUCP)


login: elfring
username: elfring
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=572001

  I add some links to helpful information about algorithms and 
  technologies.

  4. Can any experiences from "Communicating 
  Sequential Processes" (http://www.cs.ukc.ac.uk/projects/ofa/jcsp/csp-
  java-model-abstract.html) be applied for TCL?

  5. A book 
  (http://groups.google.de/groups?selm=12785%40star.cs.vu.nl&rnum=159)
  Title: 
      Modern Operating Systems
  Author: Andrew S. Tanenbaum
  ISBN:   
  0-13-588187-0


login: elfring
username: elfring
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=572001

  http://groups.google.de/groups?
  group=comp.programming.threads&selm=40ed1d8f.0301150519.
  60fc9f36%40posting.google.com


login: dkf
username: dkf
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=79902

  Would you guys want me to set up a mailing list for
  discussion of thread-related issues?  It'd come with useful
  things like archiving tools too...


login: elfring
username: elfring
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=572001

  Yes. But I would like a newsgroup (interface) 
  more...

  com.programming.threads.tcl ?


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  The deadlock prevention using exclusive mutexes is
  now handled in the upcomming 2.6 threading extension.
  Instead of deadlock, the Tcl error is thrown.
  Also, the deletion of sync promitives in use is also 
  terminated with Tcl error.


login: elfring
username: elfring
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=572001

  Interesting - But the improvement that you describe is just
  the expected error handling.

  Can a TCL function be added that will be equivalent or
  similar to the method "findMonitorDeadlockedThreads" of the
  "Threading MBean"?
  Would you like to reuse anything from the article "Using
  JConsole to Monitor Applications" by Mandy Chung?
  http://java.sun.com/developer/technicalArticles/J2SE/jconsole.html#DeadlockDetection


login: dgp
username: dgp
mimetype: text/plain
icomment:
  allow_comments - 1

  fix already merged into thread-2-7-branch.
  Now backported to trunk (2.6.*) too.


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  Thanks for the update. I will rather declare those as static
  in the tclXkeylist.c because those are only used from
  within that file.
  I will checkin the fix tomorrow in the cvs head branch.


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  Checked into CVS head branch. Instead of the proposed patch
  I made those symbols static in the tclXkeylist.c file. This should do.


login: matt-newman
username: matt-newman
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=1333796

  Thanks for making this change, however unfortunately I was 
  not complete in the list of  public symbols that class, the 
  complete list - verified by using NM on the resultant objects 
  is:

  #define tclXWrongArgs           thread_tclXWrongArgs
  #define TclX_WrongArgs          thread_TclX_WrongArgs
  #define TclX_AppendObjResult    
  thread_TclX_AppendObjResult
  #define TclX_IsNullObj          thread_TclX_IsNullObj
  #define TclX_KeyedListDelete    thread_TclX_KeyedListDelete
  #define TclX_KeyedListGet       thread_TclX_KeyedListGet
  #define TclX_KeyedListGetKeys   
  thread_TclX_KeyedListGetKeys
  #define TclX_KeyedListInit      thread_TclX_KeyedListInit
  #define TclX_KeyedListSet       thread_TclX_KeyedListSet
  #define TclX_NewKeyedListObj    
  thread_TclX_NewKeyedListObj


login: hobbs
username: hobbs
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=72656

  I don't know when tclXkeylist.c was forked, but I updated
  the TclX version with some mods that significantly improve
  performance on large keylists.  You might also consider
  those when making changes.


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  I took pieces out the TclX for about 2 years ago. I will look
  into your changes and sync them in. Thanks for the hint.


login: jan.nijtmans
mimetype: text/plain
icomment:
  Double-checked that really all relevant symbols are MODULE_SCOPE: Yes they are! this means that this ticked can be closed: there are no possible symbol conflicts any more.


login: dkf
username: dkf
mimetype: text/plain
icomment:
  allow_comments - 1

  Oh, that's Tcl's variable resolution rules. Yuck. Luckily in this case there's a way to write things that avoids the nastiness, changing (effectively) this:

      if {![catch {package require Thread} version]} {
          variable tvers $version
          ...
      }

  into this:

      if {![catch {
          variable tvers [package require Thread]
      }]} {
          ...
      }

  That version is hazard-free.


login: mistachkin
username: mistachkin
mimetype: text/plain
icomment:
  From the code snippet you included above, it seems like you are expecting to have the sending interpreter receive an error when it cancels the interpreter in the other thread?  The sending interpreter does not encounter any error condition by canceling the other thread and the fact that your script completes indicates that the target (receiving) thread is being canceled properly.


login: dgp
username: dgp
mimetype: text/plain
icomment:
  I thought a cancellation got converted into
  a (un-[catch]able?) TCL_ERROR status unwinding
  up through the call stack in the cancelled thread?
  Is the issue really with [thread::create] maybe?
  Not (yet) in an event loop, so don't have thread::errorproc
  handling engaged?


login: nobody
username: nobody
mimetype: text/plain
icomment:
  Logged In: NO 

  I have written a sample script in which I have provided 
  tcltest package in thread send call. Each thread consumes 
  800KB of memory , but while release only 75KB of memory is 
  released.Please find the sample script below.

  package require Thread
  array set ::NewThread {}
  set ::mcount 10

  after 4000
  # Program Mem-Usage here is 2260K
  for  {set count 1} {$count < $::mcount} {incr count} {

     set ::NewThread($count) [thread::create -joinable { 
  thread::wait }]
     puts "Created Thread NewThread($count):  $::NewThread
  ($count)"

     for  {set cnt 1} {$cnt < 100 } {incr cnt} {
        thread::send $::NewThread($count) {
           package require tcltest
           puts "xxxxxxxxxxxxxxhello worldxxxxxxxxxxxxxxxxxx"
        }
     }
  }

  after 4000
  # Program Mem-Usage here is 9880K
  for  {set count 1} {$count < $::mcount} {incr count} {

     thread::release $::NewThread($count)
     puts "Released Thread NewThread($count) : $::NewThread
  ($count)"
     
     thread::join $::NewThread($count)
     puts "NewThread($count) Thread Terminated"

  }

  puts "Quiting............"
  # Program Mem-Usage here is 9180K
  after 4000


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086
  Originator: NO

  The problem is that you are creating threads with "-joinable" 
  but you never join them. Instead you "thread::release" them.
  Either skip "-joinable" or use "thread::join" to collect the 
  exiting thread.

  I agree that it is not clear when to call what and this has something
  to do with the history. I will try to simplify the API and remove 
  some commands out of it to make it more easier to use. Yet, I will
  have to wait for 3.0 to do that. 


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  Sure. I want to add the dictionary type plus the  
  lset command. 
  Already in CVS is the keyed-list datatype. I will 
  release the 2.6 towards the end of the year. 
   
  What I've already done (as in CVS) is the tsv 
  persistence (currently only gdbm-like layer is supported), 
  Users should be able to write their own persistence layer 
  in C and in Tcl (latter is still not checked in) 
   
   
   


login: dkf
username: dkf
mimetype: text/plain
icomment:
  You could try holding a mutex in the main thread, sending a job to each of the workers which briefly grabs the mutex (i.e. which blocks until the main thread is ready). Since you know that none of the jobs will complete, you can be sure that you've got the pool fully populated and that it won't consume more until you're ready. Then load up the pool's queue with the real jobs and release the mutex from the main thread to fire.

  But consider whether this is really the right approach. With only two cores, your optimal pool size is pretty small and if you need the main thread to remain responsive then perhaps your pool should only be size 1! Investing in a bigger machine may be more cost effective. (None of the advice in this paragraph is Tcl-specific; it applies to any serious system of thread pools without nasty locking problems, as it is a matter of system resourcing.)


login: ferrieux
username: ferrieux
mimetype: text/plain
icomment:
  To amplify Donal's "is it the right approach ?" hint, also notice that if the interference with post operations is just a slowdown (rather than a race condition), this would be better handled through priorities. I have no acquaintance with thread-level priorities, but process-level ones work nicely ;-) Just lower the prio of your worker threads relatively to the launcher's and you're done.


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Hi!

  I believe this is trivially possible. I could just flip a bit in the pool structure that will be examined by every pool worker.
  In case bit it set, the worker would spin, otherwise it would service the pool. I could implement this with two commands: [tpool::suspend poolName] and [tpool::resume poolName]. It is also possible to achieve this "from the outside" by making a global mutex and some simple script-coding but it would be more complicated then the C-level implementation. I cannot promise when but if you need it very fast, then please submit the implementation (it is really
  símple) as a diff and I will apply it. Otherwise I will do it but you will need to be patient for some time...

  BTW... I still do not get the reason why you need such functionality...


login: ferrieux
username: ferrieux
mimetype: text/plain
icomment:
  That sounds like a contradiction: if you don't get the reason why he needs it (I don't either), why accept it so quickly ?
  A much more generally useful thing IMO would be to control thread prio...


login: hathawa
username: hathawa
mimetype: text/plain
icomment:
  Zoran, thanks for your reply. I thought it would be trivial to do and handy to have.
  I'm working on a visualization problem that can be broken up into about 50 equal chunks. The total jobs take about 4 hours on a slow machine and about 20 minutes on an 8-core machine using your thread package. I keep a progress bar going so that I can report "n of 50 completed". The natural and easy way of doing this is to do 50 posts (setup time is quick for my problem) and then sit in a wait loop, get the output of each chuck, print some messages, and update the progress bar. This doesn't really work since before the post loop can complete the tasks grab all the cores. As each task finishes, its output get queued, since I have not entered the wait loop yet. Eventually everything completes; however, the progress is not reported in a smooth manner.  I have solved my problem with creating a queue and posting from that until the pool is filled and when a task completes posting another job. This is just a pain. It would be a similar case for mutexes.  You end up managing the threads; instead of letting the threadpool do it for you. Thanks.

  Barry


login: ferrieux
username: ferrieux
mimetype: text/plain
icomment:
  Then clearly the stop-and-go feature is just a kludge and won't solve the general problem of your "manager thread" being unresponsive while the workers are eating all CPU. Again the only clean solution to this is _priorities_. 


login: hathawa
username: hathawa
mimetype: text/plain
icomment:
  I don't agree that a suspend/resume feature is a kludge, but a valid set of pool operations. The issue is not  my "manager thread" being unresponsive, but when it is unresponsive. Priorities, are not clean, easy to use, or easy to implement to solve my issue. That said, I do agree that priorities would be nice to have if you have a lot of time to implement them.  It probably would also be nice to vary the poolsize on the fly. Others could come up with other enhancements. 


login: dkf
username: dkf
mimetype: text/plain
icomment:
  If you have to keep the manager thread responsive at all times, the only real fix is to make the pool size smaller than the number of execution units available to you (what that means is kind of complex as it depends on the system architectural details) so that at least one remains free to service the manager. Whether this is the best choice depends on what you view as valuable: getting all the answers sooner or presenting a progress bar.


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Hi!

  I have just commited the changes for 
  [tpool::suspend] and [tpool::resume].

  First of all: it is really handy! I can imagine 
  many ways how people would benefit from it.
  I even contemplate to add this to the 
  singleton thread, not just thread pools.  

  Second: it is trivial to do at the C-level 
  and (well) clumsy on Tcl.

  Third: I said I did not understand why 
  Barry needed it (specifically) but it was
  no question about the usefulness of 
  it in general. I was merely qurious what 
  specific task he needs to address by
  such constructs.

  Please check out the head from CVS
  and compile. I updated the doc/tpool.man
  but haven't generated the nroff/html
  part (please just look in the file for docs 
  about those two commands). 

  Note that you need to create certain 
  number of worker threads upfront as they
  (i.e. their count) will not be manipulated 
  by the [tpool::resume] NOR [tpoo::post]
  once the work gets posted to the pool.


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  What do you mean by "if you run a process in a thread" ?
  Can you give me a small example in Tcl to illustrate the
  problem? 
  What platform are you using? 


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086
  Originator: NO

  You do not "kill" a thread with thread::release.
  You merely flip a bit that controls what the 
  thread will do when it re-enters its event loop 
  again. If the thread never visits the event loop
  because it is stuck in executing some external
  process, the outcome is undefined. 

  Please note that threads are not of an preemptive,
  rather cooperative "multitasking". You cannot 
  execute the same level of control (i.e. kill) on 
  threads like you can do on processes as this 
  will create whole lotta havoc on shared resources 
  (including deadlocks of various kinds).

  I will close this ticket now. 


login: jan.nijtmans
mimetype: text/x-fossil-wiki
icomment:
  Fixed in [6e420680e0eccd70d19dd9a98fec55e9804eb747|6e420680e0]. Thanks to Cyan Ogilvie for suggested patch.


login: dgp
username: dgp
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=80530


  set ::env(TCL_LIBRARY) $::tcl_library

  in the master interp should be
  a work-around for this issue.


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  I believe that this is out of the scope of the 
  threading extension code. At the point of ignoring
  the result of Tcl_Init()... Yes, I know it is ignored.
  At that point I was pretty unsure what to do as the 
  thread ID has already been reported to the user, so
  I could'nt just exited the new thread. I do still have
  this weird thing on my todo list...

  I will close this ticket with "Won't Fix" but will
  keep an eye on somehow improving the code to signalize
  error if the Tcl_Init does not finish properly.


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  1.  You can now use 
    "--with-gdbm=/my/dir" 
  option to tell where your gdbm files are. Note: both library 
  and include files should be in the same directory for this
  to succeed.  Use just "--with-gdbm" and I will look into 
  some predefined dirs and try to locate it.

  2. Fixed.


login: dkf
username: dkf
mimetype: text/plain
icomment:
  Seems reasonable to me. Raising prio so that this can be dealt with as part of preparing a release.


login: dgp
username: dgp
mimetype: text/plain
icomment:
  I don't have a clear understanding what's being requested.

  I'll take it as request to "bundle a release of Thread under tcl/pkgs/"


login: r_zaumseil
username: r_zaumseil
mimetype: text/plain
icomment:
  I do not mean "bundle a release". Just create a tag for major changes or tag it synchron with tcl releases.
  Thanks for your response.


login: dkf
username: dkf
mimetype: text/plain
icomment:
  If we do go for a bundled release (which we want to do, irrespective of your request) then the tag will get applied for sure.


login: dgp
username: dgp
mimetype: text/plain
icomment:
  If the repository for the Thread package is not
  getting tags corresponding to releases of the
  Thread package, that should be corrected.

  At the moment, the release calendars of Tcl
  and Thread are not synchronized, so common
  tagging doesn't make sense.

  Whenever Thread joins the collection of
  packages bundled with Tcl releases, those
  bundles will be tagged as appropriate.

  If I still have not understood what you are
  asking for, please spell it out in absurd detail.


login: ferrieux
username: ferrieux
mimetype: text/plain
icomment:
  If I may chime in, bundling has definite advantages:
   - many more eyes looking at the Thread code
   - more people testing parts of the Tcl API which are left dormant without Thread
   - [personal dream]: one step towards making threaded builds default (even on unix some day), and possibly phasing out unthreaded builds later one --> one single code path to test.


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Interesting point. Actually I could ask Tcl_IsSafe(interp) of the
  interp that creates the thread (the one that calls thread::create)
  and inherit that state in the new thread so its interp is also safe.
  Would that make sense?


login: dkf
username: dkf
mimetype: text/plain
icomment:
  I suggest making thread creation a restricted operation and that a safe interpreter should only be able to communicate with other interpreters that it's master has permitted it to do so to (which would require a new operation; sorry!)

  But really, it would be better if there was some way for the (non-safe) master to control messages sent (and what variables are shared, what mutexes are available, etc.) more thoroughly. I've tended to assume that it would be best if the thread extension simply wasn't marked as safe at all; let the master expose (safe) aliases to operations as necessary.


login: dkf
username: dkf
mimetype: text/plain
icomment:
  BTW, one safe context isn't the same as another because of the ability to grant exceptions to the rules via aliases. It's possible for there to be two sets of exceptions, each of which is safe on its own, but which are not safe in combination. (Child interpreters of safe interpreters are a special case; they start without any restricted operations at all and the parent safe interpreter can't change that.) It's not ever going to be safe to communicate between safe interps in different threads by default.


login: kjnash
username: kjnash
mimetype: text/plain
icomment:
  > Interesting point. Actually I could ask Tcl_IsSafe(interp) of the
  > interp that creates the thread (the one that calls thread::create)
  > and inherit that state in the new thread so its interp is also safe.
  > Would that make sense?

  That's an interesting suggestion!  However, file/network interaction is normally added to a safe interpreter either by [interp alias] or by sharing sockets: in both cases the slave is configured by its master (or higher ancestor) in the same thread.  If there is no unsafe master, the slave would be of limited value, except for computation.

  If the Thread package were to add new mechanisms for using safe interpreters, I think it would create a lot of work for the developers of the Thread package, in particular the need to always be thinking about whether Thread facilities will allow a safe interpreter to execute commands in an unsafe interpreter (or, as Donal mentioned, a different kind of safe interpreter).

  However, such new mechanisms in Thread would not add much benefit, because safe interpreters are not widely used, and users will tend to use the safe-interpreter commands that they know, and will probably avoid using the new capabilities in Thread because any extra capabilities will introduce additional complications and security risks.

  Also, as far as I know, it is unprecedented to have a safe interpreter as the the root interpreter of a thread or process.  I don't know if it would break anything.  It would impact on development of the core, because the core developers would always have to consider the possibility that the root interpreter may be safe.

  I think the best solution is to remove the safe entry point Thread_SafeInit completely.  As Donal said, let the master expose (safe) aliases to operations as necessary.

  Keith.


login: sebres
username: sebres
mimetype: text/plain
icomment:
  still strange:

  windows x32 / debian x64,
  tcl 8.5.11 with thread 2.6.7
  core-8-5-brunch with thread 2.7

  first script runs without any problem, BUT
  second script runs to his end, hereafter something runs infinity in background  with cpu load 50%. "thread::names" returns main thread only.


login: sebres
username: sebres
mimetype: text/plain
icomment:
  The infinite loop was blame - ("while 1")
  Try following script to prevent an endless running:

  package require Thread
  set id [ thread::id ]
  set tid [
  thread::create {
  while {![info exists ::thstop]} {
  for { set i 0 } { $i < 9999999 } { incr i } {}
  update
  }
  puts end!!!!
  }
  ]
  after 1000
  thread::send -async $tid {set ::thstop 1}
  catch {thread::release -wait $tid}


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086
  Originator: NO

  Great work! Many thanks for the patch
  and your time. I have added this into the
  CVS head. Please checkout and see if
  all is fine. Your test programs work as
  expected now. The fixed will appear first
  in 2.6.6 version.

  Cheers,
  Zoran


login: hobbs
username: hobbs
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=72656

  I have fixed this in the CVS HEAD, but it's not yet in a new
  release version.


login: dgp
username: dgp
mimetype: text/plain
icomment:
  Recently support for the [interp limit] functions
  of Tcl 8.5+ was added to Thread (thanks mistachkin!)

  The implementation of the [tpool::*] commands includes
  several calls to Tcl_DoOneEvent().  IIUC, these should
  also have Tcl_Limit*(..) calls added and maybe more to
  get [interp limit] and [interp cancel] support in place for
  all parts of the Thread package.


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  Uuups? This is a thing! I will check this out immediately
  Thanks for the hint!


login: vasiljevic
username: vasiljevic
mimetype: text/plain
icomment:
  Logged In: YES 
  user_id=95086

  Fixed in CVS head. Some times this will appear in a 
  2.6.x release. Thank you for the report. 


login: anonymous
username: cyan.ogilvie@gmail.com
mimetype: text/plain
icomment:
  Sorry, didn't search for existing tickets before submitting this one.  I see that it's a duplicate of http://core.tcl.tk/thread/tktview?name=3534607fff


login: anonymous
username: cyan.ogilvie@gmail.com
mimetype: text/plain
icomment:
  If I switch the order of setting the interp result and errorcode / errorinfo in SetResult so that the interp result is set after the errorInfo is appended then the problem goes away.


login: anonymous
username: Adrián Medraño Calvo
mimetype: text/plain
icomment:
  The following patch fixes the segmentation fault. It seems this was missing in 355cf8021c98b4bc07c9c97a5a3c74ab17d6ef52. Please, review.

  diff --git i/src/tcl8.6.3/pkgs/thread2.7.1/generic/threadSvCmd.c w/src/tcl8.6.3/pkgs/thread2.7.1/generic/threadSvCm
  index 1250af1..06c4d6a 100644
  --- i/src/tcl8.6.3/pkgs/thread2.7.1/generic/threadSvCmd.c
  +++ w/src/tcl8.6.3/pkgs/thread2.7.1/generic/threadSvCmd.c
  @@ -1505,6 +1505,7 @@ SvNamesObjCmd(
           LOCK_BUCKET(bucketPtr);
           hPtr = Tcl_FirstHashEntry(&bucketPtr->arrays, &search);
           while (hPtr) {
  +            svObj = (Container*)Tcl_GetHashValue(hPtr);
               char *key = Tcl_GetHashKey(&bucketPtr->arrays, hPtr);
               if ((!svObj->aolSpecial || (*key != '.')) /* Hide .<name> arrays for AOL*/ &&
                   (pattern == NULL || Tcl_StringMatch(key, pattern))) {


login: jan.nijtmans
mimetype: text/x-fossil-wiki
icomment:
  Fixed in trunk [f5a0464536f6bc73b0a2a0a1a19e2888eed90b6a|f5a0464536]. Fix will be in thread 2.7.2

  Thanks for the report!


login: dgp
mimetype: text/plain
icomment:
  Checked in as branch bug-a57b6c3fd4 .  Looks good to me,
  but another set of eyes before merge would be good too.


login: sebres
mimetype: text/plain
icomment:
  Slow because of return value (and Sv_DuplicateObj) each time called:

    Tcl_SetObjResult(interp, Sv_DuplicateObj(svObj->tclObj));

  Temporary resolved as Remind.
  Use "tsv::linsert ... end ..." or "tsv::lpush ... elm end" instead.

  Possible to make Sv_DuplicateObj faster for list/dict objects without duplicate over it string representation (faster copied using internal rep. for non simple objects).


login: sebres
mimetype: text/plain
icomment:
  Not so good, that no updateIntRepProc (or something like) in addition to updateStringProc exists...
  DupListObjShared is to "slow", if we have a large list. But it can not be rewritten as on demand, because internal representation should be thread safe and if we should have the same type of object (list). So, something like 'sharedList' may be to unsafe, as long as tcl-core does not can wrap it to list object on demand.


login: gahr
username: gahr
mimetype: text/plain
icomment:
  Good idea. I will provide a [tsv::storages] command to retrieve a list of enabled persistent storage implementations and build constraints around it.


login: gahr
username: gahr
mimetype: text/x-fossil-wiki
icomment:
  Done in branch [bug-72b8ee4c76]. I opted for [tsv::handlers] for consistency with [tsv::bind] documentation.


login: dgp
mimetype: text/plain
icomment:
  Assigned to jenglish not because I want him to
  tackle this, but to demo a bug in fossil ticketing
  where I can assign a ticket to an ID that is not
  even a User in this repository.

  Some kind of weird cross-talk must be in place.


login: dgp
mimetype: text/plain
icomment:
  Assigned to drh now, but that's less
  remarkable since he's an admin-level
  user here.

  The real problem is I want to assign
  to gahr, who is a User here, but who
  does not appear on the drop-down at all.


login: gahr
username: gahr
mimetype: text/x-fossil-wiki
icomment:
  Given that

  * it is likely cheaper and harmless to just always cache the current interpreter, and

  * TCL_TSD_INIT returns zero-initialized memory on the first call

  I think this could be reduced to:

  <pre>
  Index: generic/threadCmd.c
  ==================================================================
  --- generic/threadCmd.c
  +++ generic/threadCmd.c
  @@ -563,4 +563,2 @@
       if (tsdPtr->interp == (Tcl_Interp*)NULL) {
  -        memset(tsdPtr, 0, sizeof(ThreadSpecificData));
  -        tsdPtr->interp = interp;
           ListUpdate(tsdPtr);
  @@ -569,2 +567,3 @@
       }
  +    tsdPtr->interp = interp;
   }
  </pre>

  Thoughts?


login: dah
username: dah
mimetype: text/plain
icomment:
  That makes the most sense to me, I only added the check for InterpDeleted to help demonstrate the issue. Thank you for looking into this, this one is very important to me.


login: gahr
username: gahr
mimetype: text/plain
icomment:
  Alright. I'm still not convinced that changing a Thread's main interp is legitimate. Will have to think a bit more about potential fallout. Thanks for your patience!


login: dah
username: dah
mimetype: text/plain
icomment:
  You seem to be suggesting that once you create your first interp you cannot delete it and create a new one again or else the Tcl boogee men might come after you.

  The question should be, does Tcl support this behavior?  I've yet to find any literature anywhere in code or otherwise that points to the contrary, except for the Thread extension assuming the interp will live as long as itself.

  In this case, I have a long-running app that embeds Tcl. A Tcl interpreter is created and can be deleted and another created again by user demand. The purpose is to re-source scripts on a clean slate, without the user/scripts having to be concerned of old junk leftover from scripts that were modified or removed. 

  It's also easier for users to simply click a button that will load in any new scripts or modified ones, again, without having to be concerned of old code running. Incidentally, I have stress tested this by creating 100s of threads and then killing the main interp without suffering any noticable ill effects.

  You can argue that these scripts could be loaded into a child interp instead, but we don't need anymore than one interp 99% of the time and any such changes have real cost. Besides, the thread extension only communicates with the main interp, so any scripts making use of it will still need to be running in main interp. Perhaps if Tcl documented the issue in Tcl_DeleteInterp API docs, developers could steer clear of this in the future.

  Still, a workaround is no excuse not to fix a bug, if Tcl is designed to support it.

  Also an app may load up Tcl temporarily to eval some scripts and then unload it afterwords, The Thread package won't support that case either I think, if things are left as-is.

  I'm really finding it hard to understand how this isn't legit, thanks for your time.


login: sebres
mimetype: text/plain
icomment:
  JFYI: I've solved similar problem pragmatically in my modified thread module... 

  If some thread have more as one interpreter, and some code should be definitely executed in current interpreter, I do call of `::thread::inscope` in tcl-side so it explicit temporary switch default interpreter:

  Example: 
  ```
  % set th [thread::create]
  % thread::send -async $th {
    thread::id
    set ::wait 0
    set i [interp create]
    ## in other interp:
    interp eval $i {
      set ::wait 0
      puts "in other interp"
      ::thread::inscope {vwait ::wait}
      puts "out other interp - $::wait"
    }
    ## back in main:
    puts "ok - $::wait"
  }
  % thread::send -async $th {incr ::wait}
  % 
  in other interp
  out other interp - 1
  ok - 0
  ```

  I've added implicit switching in all thread/tpool commands expected this, so call of "::thread::inscope" is only expected for standard tcl calls does not having that (and works asynchronously), resp. some foreign async handler.

  But my thread module is totally rewritten (I'm allready 3.0 ))), just time deficiency to make the end code review and check in here or by me in github.

  But I'll try to do this next week.


login: gahr
username: gahr
mimetype: text/plain
icomment:
  dah, don't get me wrong. I am not suggesting that changing a thread's interp shouldn't be allowed, and I agree there is a problem in the current implementation.

  However, the more I think about it, the more I'm convinced that chaning a thread's interp should be an explicit action - not relying on the fact that a [thread::*] command is actually called from the new interp.

  What if a second thread [thread::send]s to the main thread after the main thread's interp has been deleted and a new one has been created. You would end up with the same error ("attempt to call eval in deleted interpreter"). Unless you call any [thread::*] command from the newly created interp.

  I am considering a different approach. What about a new [thread::interp ?tid? ?interp?] command.

  If *tid* is not passed, default to the current thread. If *interp* is not passed, return the currently bound interp. Otherwise, bind *interp* to *tid*.

  This way, once you delete the interp and create a new one, you could *explicitely* state your intent of changing the thread's interp by calling [thread::interp {}].

  How about this?


login: dah
mimetype: text/plain
icomment:
  "What if a second thread [thread::send]s to the main thread after the main thread's interp has been deleted and a new one has been created. You would end up with the same error ("attempt to call eval in deleted interpreter")"

  I'm fine with it in that instance, it is possible for the user to handle it. The interp that created its child threads should join/release its threads before shutting down, otherwise it doesn't seem a legitimate thing to do.

  Your new approach is fine with me but doesn't do anything to help current code, it's a new feature. It'll of course also have to handle the case where there hasn't been an interp created yet.


login: mistachkin
mimetype: text/plain
icomment:
  From my perspective, if an interpreter has been deleted, the native code in the thread package must make every effort to make sure it is not used further.  If that means shutting down the thread or changing the "primary" interpreter for the thread, so be it.

  I think the "right" fix for this issue may be to either:

  1. Clear the "cached" interpreter for the thread and refuse to perform any
     further actions that require it.

  -OR-

  2. Update the "cached" interpreter for the thread to something else?  What?


login: gahr
username: gahr
mimetype: text/plain
icomment:
  I like mistachkin's proposal, this would boild down to:

  1) check for Tcl_InterpDeleted every time the interp is used, and error out if true.
  2) provide a method to update the interp to the caller's, e.g., [thread::updateinterp].

  Note that [thread::updateinterp] would take no arguments, i.e., contrary to my first proposal, it shouldn't be possible to pass any interp for the thread to use. I have tried this approach, but it creates problems with deleting the async handlers when the thread dies.

  generic/tclAsync.c:283
  ├>        Tcl_Panic("Tcl_AsyncDelete: async handler deleted by the wrong thread");


login: dgp
mimetype: text/plain
icomment:
  It's curious that the Init() routine that does the work
  of placing a thread on Thread's list of known threads
  is not called as part of any [package require Thread],
  but as a side effect of a number of [thread::$cmd]
  invocations.  Presumably this avoids creation of
  a "thread ID" in the system until there is some
  need for one to direct messages.  Tends to make
  operations less predictable though. Especially since
  it's the first interp in a thread to call one of these
  commands that gets to be the "main" interp of that
  thread, which may very well not be the top interp
  of an [interp create] hierarchy.

  Overall, Thread seems to be founded on the notion
  that a thread ID is sufficient while to really direct
  a message to a particular interp would need more
  info to accomplish.  I'd expect most coders are
  assuming that an arrangement to send a message
  back to their thread are assuming it will come back
  to their interp too, with any prereqs needed for
  handling the message provided for.  If some 3rd
  party code deletes the interp before the message
  comes back, it's not at all clear to me that sending
  it to any random interp in the same thread that
  may have been created in the interim is a defensible
  idea.  My first impression is that the "attempt to call
  eval in a deleted interpreter" error most accurately
  reflects what's happened.

  There's still a problem here in that subsequent
  interp creation followed by [thread::$cmd] in
  an already active thread will be unable to direct
  messages to it, since the interp slot has already been
  claimed.

  Yeah, this whole thing is just not designed for
  anything other than a single interp per thread
  to participate in Thread-directed messaging.
  Any solution to this is going to essentially be
  a re-design.  Which is good, but disruptive.


login: tombert
mimetype: text/plain
icomment:
  Another one:

  Program received signal SIGSEGV, Segmentation fault.
  [Switching to Thread 0x7fff8c7c8700 (LWP 32470)]
  Sv_DuplicateObj (objPtr=0x8cef0030) at ./generic/threadSvCmd.c:989
  989	    if (objPtr->typePtr != NULL) {
  (gdb) 
  (gdb) bt
  #0  Sv_DuplicateObj (objPtr=0x8cef0030) at ./generic/threadSvCmd.c:989
  #1  0x00007ffff410bf08 in DupListObjShared (srcPtr=0x7fffdcd06470, copyPtr=0x2885140) at ./generic/threadSvListCmd.c:903
  #2  0x00007ffff4101318 in Sv_DuplicateObj (objPtr=0x7fffdcd06470) at ./generic/threadSvCmd.c:1015
  #3  0x00007ffff4102e29 in SvSetObjCmd (arg=0x0, interp=0x7fffdccbe500, objc=4, objv=0x7fffdcccca38)
      at ./generic/threadSvCmd.c:1701
  #4  0x00007ffff76306a6 in Dispatch (data=0x7fffd54c06b8, interp=0x7fffdccbe500, result=0)
      at /home/tombert/cm/tcltk86/dcompile/tcl/generic/tclBasic.c:4358
  #5  0x00007ffff7630734 in TclNRRunCallbacks (interp=0x7fffdccbe500, result=0, rootPtr=0x7fffdcd052d0)
      at /home/tombert/cm/tcltk86/dcompile/tcl/generic/tclBasic.c:4391
  #6  0x00007ffff763336e in TclEvalObjEx (interp=0x7fffdccbe500, objPtr=0x7fffbd5d5710, flags=131072, invoker=0x0, word=0)
      at /home/tombert/cm/tcltk86/dcompile/tcl/generic/tclBasic.c:5957
  #7  0x00007ffff7633307 in Tcl_EvalObjEx (interp=0x7fffdccbe500, objPtr=0x7fffbd5d5710, flags=131072)
      at /home/tombert/cm/tcltk86/dcompile/tcl/generic/tclBasic.c:5938
  #8  0x00007ffff778c4db in AfterProc (clientData=0x7fffdd060690)
      at /home/tombert/cm/tcltk86/dcompile/tcl/generic/tclTimer.c:1191
  #9  0x00007ffff778b44f in TimerHandlerEventProc (evPtr=0x4267f80, flags=-3)
      at /home/tombert/cm/tcltk86/dcompile/tcl/generic/tclTimer.c:593
  #10 0x00007ffff7760ced in Tcl_ServiceEvent (flags=-3) at /home/tombert/cm/tcltk86/dcompile/tcl/generic/tclNotify.c:670
  #11 0x00007ffff7761129 in Tcl_DoOneEvent (flags=-3) at /home/tombert/cm/tcltk86/dcompile/tcl/generic/tclNotify.c:967
  #12 0x00007ffff40fe332 in ThreadWait (interp=0x7fffdccbe500) at ./generic/threadCmd.c:2882
  #13 0x00007ffff40fb5cd in ThreadWaitObjCmd (dummy=0x0, interp=0x7fffdccbe500, objc=1, objv=0x7fffdcccbe00)
      at ./generic/threadCmd.c:1163
  #14 0x00007ffff76306a6 in Dispatch (data=0x7fffdcd05d28, interp=0x7fffdccbe500, result=0)
      at /home/tombert/cm/tcltk86/dcompile/tcl/generic/tclBasic.c:4358
  #15 0x00007ffff7630734 in TclNRRunCallbacks (interp=0x7fffdccbe500, result=0, rootPtr=0x0)
      at /home/tombert/cm/tcltk86/dcompile/tcl/generic/tclBasic.c:4391
  #16 0x00007ffff762fec6 in Tcl_EvalObjv (interp=0x7fffdccbe500, objc=1, objv=0x7fffdcccbe00, flags=2097168)
      at /home/tombert/cm/tcltk86/dcompile/tcl/generic/tclBasic.c:4121


login: sebres
mimetype: text/x-fossil-wiki
icomment:
  I don't see something evil in Sv_DuplicateObj resp. DupListObjShared...

  Can you possibly provide more info which types of objects are set to the TSV there?

  By the tsv, only known types would be duplicated with its internal representation (using typePtr->dupIntRepProc like list/dict resp. scalar like int, etc).
  All other types would be "duplicated" using "safe" way as stringified tcl-objects, so using same string representation (as new object without type).
  If you use some extensions with new object types, that do not support safe multi-threaded duplication (or restoration from string representation), you should avoid to set it to the tsv resp. avoid of its usage in other threads.

  E. g.:
  <code><pre>
  # thread 1:
  set obj [my-tcl-ext::obj ...]; # returns my-own-obj#1
  tsv::set some value $obj

  thread::send $th {process}

  # just 1st example - dual usage of not thread-safe object:
  my-tcl-ext::do-something $obj
  # just 2nd example - even delete it in thread 1:
  my-tcl-ext::delete $obj

  # thread 2:
  proc process {} {
    set obj [tsv::get some value]
    my-tcl-ext::do-something $obj; # BOOM in 2 cases, see below
  }

  </pre></code>
  makes BOOM if:<ul>
  <li>either type of object has no thread-safe duplication functionality;</li>
  <li>or type of object has no duplication functionality (typePtr->dupIntRepProc is NULL), but restoration of the object from string ("my-own-obj#1") tries to find the object in some list, that is not multi-threaded safe (resp. convert it to the unusable object of another thread).</li>
  <li>or the extension is compiled for single-threaded environment (no mutex/etc).</li>
  </ul>


login: tombert
mimetype: text/plain
icomment:
  Your example below is clear to me, but I am not passing handles to threads. 
  No special extension/objects used. I am only passing strings.
  In the code below [obj] is a procedure name being executed when it is queued.

  tsv::lock objlockLockedObjs {
          tsv::set objlockTidPrio [thread::id] $priority
          tsv::lappend objlockTidQueue global [thread::id]
          tsv::lappend objlockPendingObjs [thread::id] {*}$args
          foreach obj $args {$obj -thread queue gthread objlock lock}
      }

  tsv::lock objlockLockedObjs {
          ## corner case - unlock if locking is still pending
          if {[thread::id] in [tsv::get objlockTidQueue global]} {
              tsv::set objlockPendingObjs [thread::id] {}
              tsv::lrem objlockTidQueue global [thread::id]
              ## release from sleep
              thread::send -async [thread::id] [list gthread objlock wakeup]
          }
          ## release owned locks
          foreach {obj tid} [tsv::array get objlockLockedObjs] {if {$tid == [thread::id]} {tsv::set objlockLockedObjs $obj {}}}
      }


login: sebres
mimetype: text/x-fossil-wiki
icomment:
  If this ticket belongs the same issue as [1e3409bf1c] (not thread-safe tkcon) then please close it also.


login: tombert
mimetype: text/plain
icomment:
  I run this test again in the tclsh but this crash still happens.
  Unfort. I am not a C expert (thats why I love TCL) but would it be possible to guide me through debugging?

  I moved the stack up (now in DupListObjShared) and found following interesting:

  (gdb) print llen
  $5 = 895694950
  (gdb) print srcPtr
  $6 = (Tcl_Obj *) 0x7fffb12093a0
  (gdb) whatis srcPtr
  type = Tcl_Obj *
  (gdb) print srcPtr->typePtr
  $7 = (const Tcl_ObjType *) 0x7ffff7dce9c0 <tclListType>
  (gdb) print srcPtr->refCount
  $8 = 247
  (gdb) print srcPtr->bytes
  $9 = 0x7ffff7dd9448 <tclEmptyString> ""
  (gdb) print srcPtr->length
  $10 = 0
  (gdb) print srcPtr->typePtr
  $11 = (const Tcl_ObjType *) 0x7ffff7dce9c0 <tclListType>


  Why is llen so high? How can I find the variable name (can I get it from Tcl_Obj)?
  How can I peek into the list elements to get an idea what makes llen so high?
  etc. etc.

  If llen, for whatever reason, is correct, then Tcl_ListObjIndex(NULL, srcPtr, i, &elObj) should be able to copy the content? But it seems to fail.

  Do you think this is worth trying? Or am I on the wrong path?

  Regards


login: tombert
mimetype: text/plain
icomment:
  Now in SvSetObjCmd:

  print objv[0]
  $32 = (Tcl_Obj * const) 0x7fffb8ed3e10
  (gdb) print objv[0]->typePtr
  $33 = (const Tcl_ObjType *) 0x7ffff7dd8bc0 <tclCmdNameType>
  (gdb) print objv[1]->typePtr
  $34 = (const Tcl_ObjType *) 0x0
  (gdb) print objv[2]->typePtr
  $35 = (const Tcl_ObjType *) 0x0
  (gdb) print objv[3]->typePtr
  $36 = (const Tcl_ObjType *) 0x7ffff7dce9c0 <tclListType>
  (gdb) print objv[4]->typePtr
  $37 = (const Tcl_ObjType *) 0x0
  (gdb) print objv[5]->typePtr
  $38 = (const Tcl_ObjType *) 0x202020200a5d312e
  (gdb) print objc
  $39 = 4
  (gdb) print off
  $40 = 3
  (gdb) print arg
  $41 = (ClientData) 0x0
  (gdb) print svObj
  $42 = (Container *) 0x9bf428


  what puzzles me is that svObj seems to point to an invalid address ...


login: sebres
mimetype: text/plain
icomment:
  > Why is llen so high? 

  I assume that this list is already broken (tcl-object released? or something similar)..

  > How can I find the variable name (can I get it from Tcl_Obj)?

  No, you cannot at all. Also it must not necessarily be a variable, just any sub-list or any list used internally.
  I assume that many of the call-stack dumps you posted are just consequential errors.

  Better would be if you can probide a short test example running in tcl-shell, that seg-faulted by you. If you've such example please post it here.


login: tombert
mimetype: text/plain
icomment:
  I feared that you gone ask for a "short" example - by trying so I stumbled over the TK bug.
  It must be a tsv variable that is corrupted - can't I set a watch to identify the procedure corrupting the memory? somehow by constantly checking boundaries?

  I know it is best to provide an example, but I have constantly failed as the bug might occur in the first minutes or after several hours. How do I know if the bug is still there? Not at all.

  But ok - if you say there is no other way I will do so ...


login: sebres
mimetype: text/x-fossil-wiki
icomment:
  > I stumbled over the TK bug.

  You mean [1e3409bf1c] or yet another one?

  > can't I set a watch to identify the procedure corrupting the memory? somehow by constantly checking boundaries?

  Sorry but I guess not, resp. not so easy without compiling using something like "Rational Purify", but this way (because of the overhead) you can run in the situation where it is not reproducible anymore.


login: tombert
mimetype: text/plain
icomment:
  By running helgrind I get a lot of data races such as:

  ==2614== ----------------------------------------------------------------
  ==2614== 
  ==2614== Possible data race during write of size 4 at 0x5602C170 by thread #78
  ==2614== Locks held: none
  ==2614==    at 0x4FAAFE3: DupListInternalRep (tclListObj.c:1798)
  ==2614==    by 0x4FB7DF5: Tcl_DuplicateObj (tclObj.c:1588)
  ==2614==    by 0x4F615D4: TEBCresume (tclExecute.c:3671)
  ==2614==    by 0x4E86733: TclNRRunCallbacks (tclBasic.c:4391)
  ==2614==    by 0x4E8936D: TclEvalObjEx (tclBasic.c:5957)
  ==2614==    by 0x4E89306: Tcl_EvalObjEx (tclBasic.c:5938)
  ==2614==    by 0x4FE24DA: AfterProc (tclTimer.c:1191)
  ==2614==    by 0x4FE144E: TimerHandlerEventProc (tclTimer.c:593)
  ==2614==    by 0x4FB6CEC: Tcl_ServiceEvent (tclNotify.c:670)
  ==2614==    by 0x4FB7128: Tcl_DoOneEvent (tclNotify.c:967)
  ==2614==    by 0x72A3331: ThreadWait (threadCmd.c:2882)
  ==2614==    by 0x72A05CC: ThreadWaitObjCmd (threadCmd.c:1163)
  ==2614== 
  ==2614== This conflicts with a previous write of size 4 by thread #77
  ==2614== Locks held: none
  ==2614==    at 0x4FAAF1F: FreeListInternalRep (tclListObj.c:1761)
  ==2614==    by 0x4FB7BAC: TclFreeObj (tclObj.c:1461)
  ==2614==    by 0x4F5CE7C: TEBCresume (tclExecute.c:2310)
  ==2614==    by 0x4E86733: TclNRRunCallbacks (tclBasic.c:4391)
  ==2614==    by 0x4E8936D: TclEvalObjEx (tclBasic.c:5957)
  ==2614==    by 0x4E89306: Tcl_EvalObjEx (tclBasic.c:5938)
  ==2614==    by 0x4FE24DA: AfterProc (tclTimer.c:1191)
  ==2614==    by 0x4FE144E: TimerHandlerEventProc (tclTimer.c:593)
  ==2614==  Address 0x5602c170 is 10,384 bytes inside a block of size 16,384 alloc'd
  ==2614==    at 0x4C2BFA0: malloc (in /usr/lib/valgrind/vgpreload_helgrind-amd64-linux.so)
  ==2614==    by 0x4FE08A9: GetBlocks (tclThreadAlloc.c:1044)
  ==2614==    by 0x4FDF68C: TclpAlloc (tclThreadAlloc.c:358)
  ==2614==    by 0x4FDFB94: TclpRealloc (tclThreadAlloc.c:514)
  ==2614==    by 0x4FD4447: GrowStringBuffer (tclStringObj.c:154)
  ==2614==    by 0x4FD5B3B: AppendUtfToUtfRep (tclStringObj.c:1562)
  ==2614==    by 0x4FD540A: Tcl_AppendLimitedToObj (tclStringObj.c:1089)
  ==2614==    by 0x4FD54B1: Tcl_AppendToObj (tclStringObj.c:1130)
  ==2614==    by 0x4F8EC09: ReadChars (tclIO.c:6044)
  ==2614==    by 0x4F8E801: DoReadChars (tclIO.c:5836)
  ==2614==    by 0x4F8E594: Tcl_ReadChars (tclIO.c:5721)
  ==2614==    by 0x4FA48E7: TclNREvalFile (tclIOUtil.c:1937)
  ==2614==  Block was alloc'd by thread #78
  ==2614== 

  Since the SIGSEGV is happening in Sv_DuplicateObj method and helgrind warns about Tcl_DuplicateObj should'nt this be worth analyzing further?


login: sebres
mimetype: text/plain
icomment:
  As already said, you catch here an aftereffects (of something evil previously).

  The last time I asked you for an example, that produces it. It's too complex? Needs something external?

  Another question was "Could you completelly exclude tk-impact here (or some another modules, that are not thread-safe possibly)?"


login: tombert
mimetype: text/plain
icomment:
  I was not able to strip down my program to show that error. Also, and that was confusing, the bug really seems to be Tk related.

  I thought I had removed all Tk stuff from the program, but then I saw another [package re Tk] in the main thread. As soon as I remove this [package re Tk] the program runs without crashing.

  I am having the program running for several hours now. If it continues running over night I think I can be sure about that.


login: tombert
mimetype: text/plain
icomment:
  Still no crash. I thing with peace of conscience this bug report can be closed and assumed that all comes from this bug: 

  http://core.tcl.tk/tk/tktview/c64420e94ac3e6560ead2065533d254beee538a6

  thx


login: sebres
mimetype: text/x-fossil-wiki
icomment:
  I don't know that Tk is completely thread-safe (On the contrary I assume it's not). But I'm not using Tk already several years (early, as I still wrote tk-applications, I've used tk only in the main-thread).
  This was also due to technological (event-driven / tk-event execution across interpreter may be evil).

  So I don't known what you do here exactly, but if you rewrite it with "single-threaded" Tk usage, and it works, then you know what is it.

  I mean something like:
  <code><pre>
  # in 2nd thread
  $tkWidget do $something
  </pre></code>
  rewrite to something like:
  <code><pre>
  # in 2nd thread
  thread::send $mainThreadId [list $tkWidget do $something]
  # or even asynchronous:
  thread::send -async $mainThreadId [list $tkWidget do $something]
  </pre></code>

  May be this issue is rather a tk-issue? 
  This bug-tracker is only for the thread-module.


login: tombert
mimetype: text/plain
icomment:
  I am doing exactly how you suggest by passing TK things to the main single thread.
  Luckily I am now able to reproduce with the fairly simple code below. Have [doMeas] running and it will either SIGSEGV or run into this endless loop. One important thing to note! A few lines are printed more than once! For example:

  puts from tid0x7fffdbfff700:3543
  puts from tid0x7fffdbffd700:3536
  ...
  puts from tid0x7fffdbfff700:3543
  puts from tid0x7fffdbfff700:3543

  As the [incr ::foo] is evaluated in the main caller this seem to be a message queuing issue in the thread package?

  *** code ***

  package re Thread

  ## show tkcon
  set tkconPath [file join [file dirname [file dirname [info nameofexecutable]]] bin tkcon]
  uplevel #0 source -encoding utf-8 $tkconPath
  uplevel #0 tkcon show

  for {set i 0} {$i < 16} {incr i} {
      lappend ::tids [thread::create {thread::wait}]
  }

  proc startMeas {} {
      foreach tid $::tids {
          thread::send -async $tid [list thread::send -async [thread::id] [list puts "puts from $tid:[incr ::foo]"]]
      }
  }

  proc doMeas {} {
      startMeas
      after 1 doMeas
  }

  doMeas

  *** code ***

  Here is the SIGSEGV

  Program received signal SIGSEGV, Segmentation fault.
  DisplayText (clientData=0x7b95c0)
      at /home/tombert/cm/tcltk86/dcompile/tk/unix/../generic/tkTextDisp.c:4285
  4285		    if ((!(dlPtr2->flags & OLD_Y_INVALID))
  (gdb) bt
  #0  DisplayText (clientData=0x7b95c0)
      at /home/tombert/cm/tcltk86/dcompile/tk/unix/../generic/tkTextDisp.c:4285
  #1  0x00007ffff778b627 in TclServiceIdle ()
      at /home/tombert/cm/tcltk86/dcompile/tcl/generic/tclTimer.c:751
  #2  0x00007ffff7761145 in Tcl_DoOneEvent (flags=34)
      at /home/tombert/cm/tcltk86/dcompile/tcl/generic/tclNotify.c:980
  #3  0x00007ffff7a66fa6 in Tk_UpdateObjCmd (clientData=0x669980, interp=0x61ccb0, objc=2, 
      objv=0x621288) at /home/tombert/cm/tcltk86/dcompile/tk/unix/../generic/tkCmds.c:1214
  #4  0x00007ffff76306a6 in Dispatch (data=0x86ccd8, interp=0x61ccb0, result=0)
      at /home/tombert/cm/tcltk86/dcompile/tcl/generic/tclBasic.c:4358
  #5  0x00007ffff7630734 in TclNRRunCallbacks (interp=0x61ccb0, result=0, rootPtr=0x0)
      at /home/tombert/cm/tcltk86/dcompile/tcl/generic/tclBasic.c:4391
  #6  0x00007ffff762fec6 in Tcl_EvalObjv (interp=0x61ccb0, objc=2, objv=0x620fa0, 
      flags=2097168) at /home/tombert/cm/tcltk86/dcompile/tcl/generic/tclBasic.c:4121
  #7  0x00007ffff763271e in TclEvalEx (interp=0x61ccb0, 
      script=0x7fffb006b400 "puts {puts from tid0x7ffff0ce2700:3574}", numBytes=39, 
      flags=131072, line=1, clNextOuter=0x0,


login: sebres
mimetype: text/x-fossil-wiki
icomment:
  <blockquote>> I am doing exactly how you suggest by passing TK things to the main single thread.</blockquote>

  Nope. Not really. E.g. because you do increment via "incr ::foo" in the main thread (not in the workers)... If I correct understood what you want.
  Bellow you'll find a little bit modified script how I've suggested to test it...

  Additionally I would like to known if you do this in tclsh (not in tkcon), how it works then? Just to exclude potential bug on the pipe-handling of the tkcon... (I assume it works then, if so - you can close this here and post this issue to the tk-repository).

  Test script (if this works also, then definitive tk-issue):
  <code><pre>
  package require Thread
  ## show tckon
  ...

  tsv::set test mainThread [thread::id]
  if {![info exists tids]} {
    set tids {}; time { lappend ::tids [thread::create] } 16
  }

  proc startMeas {} {
    foreach tid $::tids {
      # send to worker:
      thread::send -async $tid {
        # send to main the value of my foo-var...
        thread::send -async [tsv::set test mainThread] [list puts "puts from [thread::id]: [incr ::foo]"]
      }
    }
  }

  set ::cnt 0
  proc doMeas {} {
    startMeas
    if {[incr ::cnt] >= 1000} {after 50; puts "\[MAIN\] **stop by $::cnt iterations**"; return}
    after 1 doMeas
  }

  doMeas
  </pre></code>


login: sebres
mimetype: text/plain
icomment:
  I've reopened it in tcl-repository (belongs to windows channel subsystem of tcl)...
  See https://core.tcl.tk/tcl/tktview/1873ea0ee4f01b264620f89621b98bd8895ce886.


login: dgp
mimetype: text/plain
icomment:
  With that background information, and using a --enable-symbols=mem
  build of Tcl, run this demo script:

  package require Thread

  proc myScript mainTid {
      package require Thread
      thread::send -async $mainTid [list puts "ThreadRunning = [tsv::get sharedArray ThreadRunning]"]
      tsv::set sharedArray ThreadRunning 1
      thread::send -async $mainTid [list puts "ThreadRunning = [tsv::get sharedArray ThreadRunning]"]
  }

  proc installProc {tid procName} {
      set procBody [info body $procName]
      set procArgs [info args $procName]
      thread::send $tid [list proc $procName $procArgs $procBody]
  }

  tsv::set sharedArray ThreadRunning 0
  set _threadHandle [thread::create]
  installProc $_threadHandle myScript
  thread::send -async $_threadHandle [list myScript [thread::id]]

  after 1000

  puts "\t Completed tests"


  With any luck, one of those Panic safeguards will be triggered.


login: dgp
mimetype: text/plain
icomment:
  The root of the problem is that the command procedures of
  many tsv commands contain

      Tcl_DecrRefCount(svObj->tclObj);

  without any regard to whether the code is running in the
  same thread that created that Tcl_Obj.

  Near as I can tell this entire sub-package is built on a misuse of the core.


login: dgp
mimetype: text/plain
icomment:
  In tclUtil.c, there's a collection of code supporting ProcessGlobalValues,
  which is one scheme for values to be shared among multiple threads.
  Maybe it is useful as inspiration or as a foundation? Maybe some of its
  routines would be useful to export?


login: sebres
mimetype: text/plain
icomment:
  > without any regard to whether the code is running in the same thread that created that Tcl_Obj.

  Why it is expected? The access to the objects in the tsv-storage is mutex-protected, and each set as well as the result of any tsv-call should always contain a safe duplication of the object (thus safe for any thread).

  BTW I cannot reproduce the segfault with your example.


login: dgp
mimetype: text/plain
icomment:
  The history of the objThreadMap restriction starts with
  these checkins by Mo Dejong. First he tried adding a field
  to the Tcl_Obj struct:

  https://core.tcl.tk/tcl/info/50c5d9ac11c96d30

  And then followed up with the objThreadMap approach:

  https://core.tcl.tk/tcl/info/4dc4a16832209a53

  The checkin and ChangeLog comments indicate the worry was
  about races between multiple threads manipulating the
  refCount field.  This is a valid concern, but with mutex
  care and sufficient thread isolation it can be managed
  without imposing such a strong constraint.


login: dgp
mimetype: text/plain
icomment:
  There's a far bigger concern about general sharing of
  a Tcl_Obj struct by multiple threads, though, and it
  is the volatility of the internal representation. Shimmering.

  Any code that interacts with an internal rep commonly has
  a structure of using objPtr->typePtr to determine the routines
  to use to act on the internal rep. Those routines proceed on
  the belief the internal rep's nature is not going to change
  while the routine runs.  All internal rep access and manipulation
  is written on a foundational assumption of single-thread access.

  If you get multiple threads passing through changing intrep details
  out from under each other, there is going to be serious trouble.


login: sebres
mimetype: text/x-fossil-wiki
icomment:
  Thanks Nathan (pooryorick) for finding and minimalistic fix.
  I've amended with more extensive fix replacing strcpy with memcpy (additionally should avoid segfault by overwrite if string representation would be possibly broken, e. g. without NTS 0-byte) and extended the test-cases to cover exactly this issue.

  So fixed now in 2.8 and trunk, thus close.


login: sebres
mimetype: text/x-fossil-wiki
icomment:
  Although idle-event is indeed not really good strategy for async send to self.

  But I don't see this as issue.

  Please note:
  <ul><li>"after" events are timer-events and will be executed as one chunk (if the timer was reached, and in case of after 0 it is always the case);
  </li><li>this means - you can not insert in-between 0-timer events, unless you don't generate a timer-event also (I don't really see any advantages by generation of 0-timer event in case of async send).
  </li><li>events are cumulated with event source handlers (setupProc/checkProc), so the time where events are reached the evaluation phase is undefined;
  </li><li>if the event will be appended directly to thread-queue, then all your timer-events (after 0) will be executed hereafter (because those will reach the event-queue first after sourcing via setupProc/checkProc);
  </li><li>asynchronous send to self is not the primary usage in thread-module, so I don't think that "much performance being lost" here.</li></ul>

  Just as loud thoughts.


login: adrianmedranocalvo
username: support@prs.de
mimetype: text/plain
icomment:
  Hello sebres,

  thank you for your comments.

  Indeed that's the issue: some [thread::send -async] are being transformed into "after" events, which implies all points you detailed, while other [thread::send -async] do not.

  Mine was a lengthy text, I'm not sure my proposal it's clear.  Let me summarize it:

  - I think that each "thread::send -async $tid $script" should be appended to the same queue, so that events for thread $tid are evaluated on the order they were posted.

  - Other events can be handled in whatever order.

  - I think that transforming some "thread::send -async $tid $script" into "after idle" (as it's currently done in Thread 2.8.0) is a misfeature, because it makes "thread::send -async" no longer work as a useful thread-safe queue.

  - I propose removing the code selected in the following link:
    https://core.tcl-lang.org/thread/artifact?udc=1&ln=2742-2759&name=61003585d6458178&udc=1

  What's your opinion on doing that?


login: sebres
mimetype: text/x-fossil-wiki
icomment:
  But you did still not understand...

  if you'll do that, the event will be appended directly into the queue of the thread, so in this case it will be not LAST (as by idle) but FIRST event (because timer events coming later via processing of event-sources).

  So again, it cannot be "on the order they were posted", because they are another types of events.
  It will be not between your timer-events, but either hereafter (like now) or before (if direct appended to event-queue).
  But definitely not between.

  For this case "thread::send -async [thread::id]" should be quasi rewritten as "after 0", and I'm definitely against this, because it produces unexpected overhead.

  Possibly for my another event-branch (that I wrote in tcl-core-repo), where "after 0" have O(1) by changing of timer-queue, does not generate extra overhead as regards time-compare at all (because creates an immediate timer-event), etc.<br/>
  If this will be accepted sometimes...


login: adrianmedranocalvo
username: support@prs.de
mimetype: text/plain
icomment:
  Hello sebres,

  there's definitely some misunderstanding.  English is not my mother language, it's not the first time I cause a mess...

  I'll try to explain with code.

  One important point is: I don't care about the order of [after idle] events nor [after 0] events with respect to [thread::send -async] events.  I only care about the order of [thread::send -async] events.

  Please, try the following script:

  ~~~
  package require Thread;

  set t(main) [::thread::id]
  set t(other) [::thread::create]

  # Let t(other) know the main thread's id, t(main).
  ::thread::send $t(other) [list array set t [array get t]]

  # Async thread::send to current thread (the main thread).
  # I expect this script to be inserted at the end of the thread event queue.
  ::thread::send -async $t(main) {
      lappend ::trace LOCAL-THREAD-SEND;
  }

  ::thread::send $t(other) {
      # Async thread::send to other thread (the main thread).
      # I expect this script to be inserted at the end of the
      # thread event queue, after the one above.
      ::thread::send -async $t(main) {
  	  lappend ::trace REMOTE-THREAD-SEND;
      }
  }

  after 100 {set forever 1};
  vwait forever;
  puts [join $::trace "\n"];
  ~~~

  This is the output:

  ~~~
  REMOTE-THREAD-SEND
  LOCAL-THREAD-SEND
  ~~~

  This is not expected, because the [thread::send -async] lappending LOCAL-THREAD-SEND ran before the one lappending REMOTE-THREAD-SEND.

  When I remove the following code:
    https://core.tcl-lang.org/thread/artifact?udc=1&ln=2742-2759&name=61003585d6458178&udc=1

  I obtain the expected output:

  ~~~
  LOCAL-THREAD-SEND
  REMOTE-THREAD-SEND
  ~~~

  Are there any downsides in removing the following code?
    https://core.tcl-lang.org/thread/artifact?udc=1&ln=2742-2759&name=61003585d6458178&udc=1

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

  I'll now try to respond to your points.

  > if you'll do that, the event will be appended directly into the queue of the thread, so in this case it will be not LAST (as by idle) but FIRST event (because timer events coming later via processing of event-sources).
  >
  > So again, it cannot be "on the order they were posted", because they are another types of events. It will be not between your timer-events, but either hereafter (like now) or before (if direct appended to event-queue). But definitely not between.

  I think that's what I want and expect, that the [thread::send -async] is appended to the queue of the thread.  I don't expect them to be handled between other types of events.

  > For this case "thread::send -async [thread::id]" should be quasi rewritten as "after 0", and I'm definitely against this, because it produces unexpected overhead.

  I'm also against this.  In fact that's the precise problem: "thread::send -async [thread::id]" is being rewritten into "after idle".  See the part of the code where that happens (https://core.tcl-lang.org/thread/artifact?udc=1&ln=2742-2759&name=61003585d6458178&udc=1):

    2742      /*
    2743       * Short circuit sends to ourself.
    2744       */
    2745  
    2746      if (thrId == Tcl_GetCurrentThread()) {
    2747          Tcl_MutexUnlock(&threadMutex);
    2748          if ((flags & THREAD_SEND_WAIT)) {
    2749              int code = (*send->execProc)(interp, (ClientData)send);
    2750              ThreadFreeProc((ClientData)send);
    2751              return code;
    2752          } else {
    2753              send->interp = interp;
    2754              Tcl_Preserve((ClientData)send->interp);
    2755              Tcl_DoWhenIdle((Tcl_IdleProc*)ThreadIdleProc, (ClientData)send);
    2756              return TCL_OK;
    2757          }
    2758      }

  > Possibly for my another event-branch (that I wrote in tcl-core-repo), where "after 0" have O(1) by changing of timer-queue, does not generate extra overhead as regards time-compare at all (because creates an immediate timer-event), etc.
  If this will be accepted sometimes... 

  More performant event handling is always welcome... thank you for your efforts, I hope it gets merged soon.


login: sebres
mimetype: text/x-fossil-wiki
icomment:
  Now I understand your objection (I was just confused with both after(timer) events from your example and your description).

  Indeed it is possible..., but not what you provided as block "to remove",
  this one would be enough to remove: 

  [https://core.tcl-lang.org/thread/artifact?name=61003585d6458178&ln=2752-2756|61003585d6458178(lines 2752-2756)]

  Of course with move of `Tcl_MutexUnlock(&threadMutex);` (line 2747) 2 lines down, into block `if ((flags & THREAD_SEND_WAIT))`.

  I'll take a look later as regards the test-cases and some other dependencies and behavior etc.


login: adrianmedranocalvo
username: support@prs.de
mimetype: text/plain
icomment:
  Hello sebres,

  great!

  Indeed, the code you selected is correct, and mine was wrong.  Sorry for the confusion.

  Let me know if you need any help.

  Best regards,
  Adrián.


login: adrianmedranocalvo
username: support@prs.de
mimetype: text/plain
icomment:
  Hello,

  Implemented the suggested change, please see patch below.  Tests pass:

  ~~~
  Only running tests that match:  *
  Skipping test files that match:  l.*.test
  Only sourcing test files that match:  *.test
  Tests began at Fri Aug 10 11:57:28 CEST 2018
  Thread 2.8.0
  Mainthread id is tid0x7fff8e159380
  thread.test
  tkt-84be1b5a73.test

  tpool.test
  tsv.test
  ttrace.test

  Tests ended at Fri Aug 10 11:57:43 CEST 2018
  all.tcl:        Total   136     Passed  120     Skipped 16      Failed  0
  Sourced 0 Test Files.
  Number of tests skipped for each constraint:
          8       have_gdbm
          8       have_lmdb
  ~~~

  ~~~
  diff --git c/src/tcl8.6.6/pkgs/thread2.8.0/generic/threadCmd.c i/src/tcl8.6.6/pkgs/thread2.8.0/generic/threadCmd.c
  index 9744230c3..b9af96475 100644
  --- c/src/tcl8.6.6/pkgs/thread2.8.0/generic/threadCmd.c
  +++ i/src/tcl8.6.6/pkgs/thread2.8.0/generic/threadCmd.c
  @@ -343,9 +343,6 @@ ThreadErrorProc(Tcl_Interp *interp);
   static void
   ThreadFreeProc(ClientData clientData);
   
  -static void
  -ThreadIdleProc(ClientData clientData);
  -
   static void
   ThreadExitProc(ClientData clientData);
   
  @@ -2697,21 +2694,13 @@ ThreadSend(interp, thrId, send, clbk, flags)
       }
   
       /*
  -     * Short circuit sends to ourself.
  +     * Short circuit synchronous sends to ourself.
        */
  -
  -    if (thrId == Tcl_GetCurrentThread()) {
  +    if (thrId == Tcl_GetCurrentThread() && (flags & THREAD_SEND_WAIT)) {
           Tcl_MutexUnlock(&threadMutex);
  -        if ((flags & THREAD_SEND_WAIT)) {
  -            int code = (*send->execProc)(interp, (ClientData)send);
  -            ThreadFreeProc((ClientData)send);
  -            return code;
  -        } else {
  -            send->interp = interp;
  -            Tcl_Preserve((ClientData)send->interp);
  -            Tcl_DoWhenIdle((Tcl_IdleProc*)ThreadIdleProc, (ClientData)send);
  -            return TCL_OK;
  -        }
  +        int code = (*send->execProc)(interp, (ClientData)send);
  +        ThreadFreeProc((ClientData)send);
  +        return code;
       }
   
       /*
  @@ -3462,34 +3451,6 @@ ThreadSetOption(interp, thrId, option, value)
       return TCL_OK;
   }
   

  -/*
  - *----------------------------------------------------------------------
  - *
  - * ThreadIdleProc --
  - *
  - * Results:
  - *
  - * Side effects.
  - *
  - *----------------------------------------------------------------------
  - */
  -
  -static void
  -ThreadIdleProc(clientData)
  -    ClientData clientData;
  -{
  -    int ret;
  -    ThreadSendData *sendPtr = (ThreadSendData*)clientData;
  -
  -    ret = (*sendPtr->execProc)(sendPtr->interp, (ClientData)sendPtr);
  -    if (ret != TCL_OK) {
  -        ThreadErrorProc(sendPtr->interp);
  -    }
  -
  -    Tcl_Release((ClientData)sendPtr->interp);
  -    ThreadFreeProc(clientData);
  -}
  -

   /*
    *----------------------------------------------------------------------
    *
  ~~~


login: adrianmedranocalvo
username: amcalvo@prs.de
mimetype: text/plain
icomment:
  Hello everyone,

  could you help me get this merged?  I have a local branch implementing it, but am not allowed to push it to the repository.


login: sebres
mimetype: text/x-fossil-wiki
icomment:
  The subject is rather complicated...

  I found and fixed this already in my fork (which is unfortunately incompatible and it'd be complex to back-port directly), but the test-case covering that is there, so taking a closer look I noticed the fix is not so simple as assumed, because of:<ul>
  <li>the option <code>-head</code> which can be also supplied to the </li>
  <li>synchronous send (if not <code>-head</code>) should consider all previously dispatched events, so they should be processed earlier.</li>
  <li>(test-only) there are tcl-forks, which timer-generation may be retarded (or time point where setup would generate it, is raher undefined), so timer event can be delayed.</li>
  </ul>
  I fixed it now and will check-in (and rebase the test) in trunk soon.


login: adrianmedranocalvo
mimetype: text/plain
icomment:
  Dear sebres,

  thank you for your response.

  If I understand correctly, you are saying that synchronous sends should behave just like asynchronous, only causing the caller to wait for the result.  That is, they should not be evaluated at the earliest opportunity, instead being evaluated after all other ::thread::sends to the target thread.

  In my opinion that is a desirable feature, specially given that it can be "disabled" by using -head.  On the other hand, I think it would be understandable that -async sends and synchronous ones are not ordered.

  Please post a comment once you push your changes; I'll have a look and test them.

  Thank you,
  Adrián.


login: sebres
mimetype: text/x-fossil-wiki
icomment:
  <code><pre>
      after 0                                {lappend res TMR-0}
      after idle                             {lappend res IDLE}
      thread::send -async       [thread::id] {lappend res ASYNC}
      thread::send -async -head [thread::id] {lappend res AHEAD}
      thread::send        -head [thread::id] {lappend res HEAD}
      thread::send              [thread::id] {lappend res SYNC}
  </pre></code>
  generates either:
  <code><pre>
      HEAD TMR-0 AHEAD ASYNC SYNC IDLE
      # ... or (timer event received later up-to following) ...
      HEAD AHEAD ASYNC SYNC TMR-0 IDLE
  </pre></code>
  Note that due to internal handling the time point of TMR-0 event is rather undefined and may deviate on several tcl-versions or forks (e. g. some NRT- resp. RTS-capable), so it can move between HEAD and IDLE.


login: sebres
mimetype: text/x-fossil-wiki
icomment:
  Also note that basically <code>thread::send -head [thread::id] ...</code> is nothing else as <code>namespace inscope :: ...</code> and co (note the global scope), so is more for the consistency (e. g. compatibility) reasons there...

  Just even slower a bit. PoC:
  <code><pre style="padding-left:10pt">
  % set ev [list set v 1]; 
  <b style="color:red;">- timerate {thread::send -head [thread::id] $ev}</b>
  <b style="color:green;">+ timerate {namespace inscope :: $ev}</b>
  <b style="color:red;">- 1.627392 µs/# 599904 # 614480 #/sec 976.279 net-ms</b>
  <b style="color:green;">+ 0.477992 µs/# 1932230 # 2092084 #/sec 923.591 net-ms</b>
  </pre></code>

  But opposed to that, the <code>thread::send [thread::id] ...</code> makes real sense, so to be interpreted as meaning that: add an event to tail of the event queue and wait for completion (also process all other events up to this one).<br/>
  The fact, that the event has not been really added to the queue, changes nothing on the whole handling. This way the new behavior (circuit implementation) is more similar to synchronous send to any other threads (no matter the head or tail of queue it'd target).


login: elfring
mimetype: text/plain
icomment:
  The information “… evaluates the resulting script under the internal mutex protection.” is provided for the command “tsv::lock”.
  May this mutex be passed to the command “thread::cond wait” (within such a TCL script)?


login: sebres
mimetype: text/x-fossil-wiki
icomment:
  In the multi-threaded environment it is very often the case that some things are executed simultaneously.<br/>
  In this case, the thread will be created and the id of the thread will be returned to the caller thread, and the new created worker starts to evaluated initialization script simultaneously.

  If your interpretation of the documentation is "influenced by the detail that something is not explicitly mentioned", this is not necessarily a documentation lack in my opinion (but possibly just your interpretation, perception or expectation).

  But of course we are always fine with an extending of the documentation with more info about some unclear behavior.


login: elfring
mimetype: text/plain
icomment:
  Thanks for your small extension in the command description.
  https://core.tcl-lang.org/thread/info/6eef222a2f66e489

  How do you think about to stress the involved case distinctions any more?


login: sebres
mimetype: text/x-fossil-wiki
icomment:
  As already said, you are wrong here.

  Why did you not understand that you are trying to create several mutexes and would overwrite a tsv-value containg mutex handle with another values within your on demand mutex initialization?

  Still again each time your thread is entering a perform_command and uses <code>tsv::set [tsv::get shared context] pcm [thread::mutex create]</code>:
  <ul>
  <li>it create a new mutex</li>
  <li>and overwrites with this new mutex handle the value in tsv-storage</li>
  <li>the later calls of lock/unlock may read another (overwritten) value here from tsv-storage (this way you introduce a RC here)</li>
  </ul>

  Just to explain what you are trying to do (and what exactly you still misunderstand):
  <code><pre style="padding-left: 10pt">
  package require Thread

  set i 0; time { set ::t([incr i]) [thread::create]; thread::send $::t($i) [list set ::MTH [thread::id]] } 4
  proc test {body} {
    catch {tsv::unset shared pcm}
    set ::ready 0
    # eval body within 4 threads 1000 times:
    set i 0; time {thread::send -async $::t([incr i]) [list time $body 1000]} 4
    # output last known mutex handle in each thread and signal ready:
    set i 0; time {
    thread::send -async $::t([incr i]) {
      puts "[thread::id] uses a mutex [tsv::get shared pcm]"
      thread::send -async $MTH {incr ::ready}
    }} 4
    # wait for ready from all 4 threads:
    while {$::ready < 4} {vwait ::ready}
    puts "MAIN: created mutex [tsv::get shared pcm]"
  }

  # 1. EXAMPLE illustrates CORRECT dynamic (on demand) initialization of mutex:

  proc test1 {} {
    test {
      <b style="color:green;">if {![tsv::exists shared pcm]} {
        tsv::lock shared {
          if {![tsv::exists shared pcm]} {
            tsv::set shared pcm [thread::mutex create]
            puts "[thread::id] created a mutex [tsv::get shared pcm] and set it to tsv"
          }
        }
      }</b>
    }
  }
  test1

  # 2. EXAMPLE illustrates WRONG dynamic (on demand) initialization of mutex:

  proc test2 {} {
    test {
      <b style="color:red;">tsv::set shared pcm [thread::mutex create]</b>
    }
  }
  test2
  </pre></code>

  There are 4 threads which simply execute body in 4 threads 1000 times each thread (e. g. a mutex initialization) in parallel...
  Just in test1 compared to test2 - there is a protection within lock and with checking of the existence for tsv-value which contain mutex.

  As you can see the result of test1 (1. EXAMPLE) - exactly 1 mutex is created and finally stored in tsv:
  <code><pre style="padding-left: 10pt">
  % test1
  tid0000263C created a mutex mid0 and set it to tsv
  tid00002B4C uses a mutex mid0
  tid0000263C uses a mutex mid0
  tid00002AD8 uses a mutex mid0
  tid00000774 uses a mutex mid0
  MAIN: created mutex mid0
  </pre></code>
  And the by the "wrong" initialization (similar to that what you are trying to do) - the result of test2 (2. EXAMPLE) looks like here (4 * 1000 mutex's are created, each thread assume to have a different mutex, the last thread wins with last created mutex):
  <code><pre style="padding-left: 10pt">
  % test2
  tid00002AD8 uses a mutex mid3670
  tid00000774 uses a mutex mid3950
  tid0000263C uses a mutex mid3960
  tid00002B4C uses a mutex mid4000
  MAIN: created mutex mid4000
  </pre></code>

  As already said:
  <ul>
  <li>either you should create a mutex in main thread (before each worker would use it);</li>
  <li>or if on demand mutex creation is expected, you should check the mutex is already created by other worker (and this should occur in locked area, to avoid overwriting of tsv-value, like in example 1).
  </li></ul>
  Anyway in all cases it should be exactly the same mutex handle to supply it later to lock/unlock/whatever in order to protect some resources.


login: elfring
mimetype: text/plain
icomment:
  > Why did you not understand that you are trying to create several mutexes and would overwrite a tsv-value containg mutex handle with another values within your on demand mutex initialization? 

  I get the impression from this question that more efforts will be needed to achieve a better common understanding (also for the shown test script).
  The created mutex handle should not be overwritten in this test variant.


  > it create a new mutex

  This is intended here (for each loop iteration).


  > and overwrites with this new mutex handle the value in tsv-storage

  The identifiers will be stored in different thread shared variables, won't they?


  > the later calls of lock/unlock may read another (overwritten) value here from tsv-storage

  I suggest to take another look at the desired implementation for the mutual exclusion.


  > 1. EXAMPLE illustrates CORRECT dynamic (on demand) initialization of mutex:

  Your procedure “test1” refers to the constant identifier “shared” for a thread shared variable.
  (I have constructed also a test variant before which works in this way.)


  > 2. EXAMPLE illustrates WRONG dynamic (on demand) initialization of mutex:

  Why do you find the command from the procedure “test2” wrong when it is identical to the previous one?


  > There are 4 threads which simply execute body …

  Our software design approaches are different in the way that you would like to show an approach for the handling of the readyness for these threads while I want to check the proper termination of them by the programming means of corresponding condition variables.


  > Anyway in all cases it should be exactly the same mutex handle to supply it later to lock/unlock/whatever in order to protect some resources.

  I suggest to increase the precision for the appropriate parameter passing.
  A single combination of mutexes and condition variables should be sufficient for our simple test examples, shouldn't it?


  Did you dare to try my test script out?


login: sebres
mimetype: text/plain
icomment:
  > The identifiers will be stored in different thread shared variables, won't they?

  What do you mean here as "different"? Why it should be?

  Value of <code>[tsv::get shared context]</code> is the same across ALL THREADS INSIDE THE PROCESS and would change, if you'd set it somewhere.<br/>
  This way all your threads would change the "context" to the last one.


login: elfring
mimetype: text/plain
icomment:
  > What do you mean here as "different"?

  I suggest to take another look at the code specification “tsv::set [tsv::get shared context] pcm [thread::mutex create]” from my procedure “perform_command” (which picks an interation number up that was set by my procedure “repeat_command” in the published example).


  > Why it should be?

  I tried the programming opportunity out for the maximum separation between these data synchronisation objects.


  > Value of <code>[tsv::get shared context]</code> is the same across ALL THREADS INSIDE THE PROCESS

  It seems that you will need another moment to understand my test cast and corresponding data processing goals better.
  The value should be the same for a while (during execution of the procedure “perform_command”).


  > and would change,

  Yes, this should happen a few times for the shown test.


  > if you'd set it somewhere.

  Is the intented modification be triggered by the code specification “tsv::set shared context $x” in a for loop of the procedure “repeat_command”?


  > This way all your threads would change the "context" to the last one.

  I have got an other expectation on how this design approach should work finally.


login: elfring
mimetype: text/html
icomment:
  Do you find the following test script variant easier to understand for the clarification of desirable software behaviour?

  <pre>
  puts stderr "Thread package version: [package require Thread]"
  set results {}

  proc perform_command {} \
  {
  thread::mutex lock [set pcm [tsv::set [set context [tsv::get shared context]] pcm [thread::mutex create]]]
  puts stderr "Initial pcm: ${pcm}"
  tsv::set ${context} pcc [thread::cond create]

  try \
     {
     tsv::set ${context} working 0
     tsv::set ${context} finish_notification 0
     } \
  finally \
     {
     proc main_thread_unlock_start {} \
     {
     puts stderr "main_thread_unlock_start pcm: [set pcm [tsv::get [tsv::get shared context] pcm]]"
     thread::mutex unlock ${pcm}
     }
     main_thread_unlock_start
     }

  for {set x 1} {$x <= 3} {incr x} \
     {
     thread::create {
                    proc handle_input {} \
                    {
                    puts stderr "[tsv::incr [set context [tsv::get shared context]] working] threads are working."
                    thread::mutex lock [tsv::get [tsv::get shared context] pcm]

                    try \
                       {
                       tsv::incr ${context} working -1

                       if [tsv::get ${context} finish_notification] \
                          {
                          puts stderr {thread::cond notify finish}
                          thread::cond notify [tsv::get ${context} pcc]
                          }
                       } \
                    finally \
                       {
                       proc handle_input_unlock_end {} \
                       {
                       puts stderr "handle_input_unlock_end pcm: [set pcm [tsv::get [tsv::get shared context] pcm]]"
                       thread::mutex unlock ${pcm}
                       }
                       handle_input_unlock_end
                       }
                    }

                    handle_input
                    }
     }

  puts stderr "Worker threads were started."
  thread::mutex lock ${pcm}

  try \
     {
     tsv::set ${context} finish_notification 1

     while {[set w [tsv::get ${context} working]] > 0} \
        {
        puts stderr "cond wait finish for $w threads"
        thread::cond wait [tsv::get [tsv::get shared context] pcc] ${pcm}
        }
     } \
  finally \
     {
     proc main_thread_unlock_finish {} \
     {
     puts stderr "main_thread_unlock_finish pcm: [set pcm [tsv::get [tsv::get shared context] pcm]]"
     thread::mutex unlock ${pcm}
     }
     main_thread_unlock_finish
     }

  global results
  lappend results [llength [thread::names]]
  #thread::cond destroy [tsv::get [tsv::get shared context] pcc]
  #thread::mutex destroy [tsv::get [tsv::get shared context] pcm]
  }

  proc repeat_command {} \
  {
  for {set x 1} {$x <= 2} {incr x} \
     {
     puts stderr "$x. run"
     tsv::set shared context $x
     perform_command
     }
  }

  proc run_analysis {} \
  {
  repeat_command
  global results

  if [llength ${results}] \
     {
     proc report {} \
     {
     global results

     foreach k ${results} \
        {
        dict incr counts $k
        }

     set delimiter |
     puts [join {{"still running threads"} incidence} ${delimiter}]

     dict for {k v} ${counts} \
        {
        puts "$k${delimiter}$v"
        }
     }

     report
     } \
  else \
     {
     puts stderr {No result for this analysis!}
     }
  }

  run_analysis
  </pre>

  <hr/>

  <b>Example for a test result:</b>
  …
  <pre>
  "still running threads"|incidence
  1|2
  </pre>


login: sebres
mimetype: text/x-fossil-wiki
icomment:
  > Do you find the following test script variant easier to understand for the clarification of desirable software behaviour?

  I don't need to "decrypt" your entire script to see that several things are basically wrong here:
  <ul>
  <li>your shared context could be overwritten in repeat_command... better use some identity value in order to target a thread-pool or the context like <code>[tsv::get shared context $ctxid]</code> if you cannot guarantee safe work (wait for completion in repeat_command)</li>
  <li>if it is intended to wait (in perform_command) until all tasks are done, it is not really succeeded (due to wrong implementation of conditional wait in case of all 6 workers, as well as the wait for end it completelly wrong)</li>
  <li>why you're trying to implement your own thread-pool if it is already there (see tpool)?</li>
  <li>or why you won't use "thread::send" with answer via variable? etc pp</li>
  </ul>

  Also note:
  <ul>
  <li>thread creation (and initialization) as well as destruction are expensive operations, so normally one tries to avoid that</li>
  <li>tsv::* are atomic operations (you don't need to protect it with mutex if you don't need to change consequently a lot of members there, but also in this case you can use tsv::lock)</li>
  <li>you don't need a conditional wait/notify here (I don't recognize any reason why it may be expected in your code)... to signal stop thread you can send to thread something, call thread::release (if thread is waiting) or simply set your finish_notification in tsv context... to signal main thread, all workers are done, just increment some tsv-value or even wait for a variable (and send from threads any command to main thread signaling it goes down)</li>
  </ul>

  Anyway, because of basic misconception and too many errors in your example script, I'll close this ticket as invalid.

  Although it would be simplier with tpool and other constructs (but you want this solution and I don't know what exactly is needed), I'll attach a "proper" asynchronous pool implementation similar yours, but without conditional wait and extra mutex (the mutex is created there and you can use it, but it is not used internally).


login: elfring
mimetype: text/plain
icomment:
  > I don't need to "decrypt" your entire script

  It seems to be very challenging to achieve a better common understanding for the shown use case.


  > to see that several things are basically wrong here:

  I disagree to specific details from the observable software behaviour.


  >your shared context could be overwritten in repeat_command

  We might interpret this information in different ways.

  1. The command “tsv::set shared context $x” is executed in a for loop as expected.

  2. It is demonstrated that the iteration number is used by the called functions in the intended way for data access with thread shared variables.


  > if it is intended to wait (in perform_command) until all tasks are done,

  This is one of the main goals of this test case.


  > it is not really succeeded

  I observed that the success depends on some configuration parameters here.


  > (due to wrong implementation of conditional wait in case of all 6 workers, as well as the wait for end it completelly wrong)

  How should the wait be correctly performed (together with condition variables) according to your software development experience?


  > why you're trying to implement your own thread-pool if it is already there (see tpool)?

  I do not try a test development for the TCL programming language in this direction.


  > or why you won't use "thread::send" with answer via variable?

  This detail is an important difference for my test variant. Each started thread should eventually notify the main control thread about its usual termination.


  > tsv::* are atomic operations

  This information is generally helpful.


  > (you don't need to protect it with mutex

  A single mutex is created at the beginning of the procedure “perform_command” so that it can be used for the command “thread::cond wait [tsv::get [tsv::get shared context] pcc] ${pcm}” there.
  Would you like to suggest a reuse of any other mutex for this data synchonisation interface?


  > if you don't need to change consequently a lot of members there,

  The number of shared variables is reasonably small for such a test approach, isn't it?


  > but also in this case you can use tsv::lock)

  This command can be better used when more statements should be unconditionally combined.


  > you don't need a conditional wait/notify here (I don't recognize any reason why it may be expected in your code)

  Are you familiar also with the application of the synchonisation primitive “condition variable”?


  > or simply set your finish_notification in tsv context

  Does the command “tsv::set ${context} finish_notification 1” (line 63) fit already to this suggestion?


  > Although it would be simplier with tpool and other constructs

  These programming tools will become more interesting for other data processing approaches than the small test case which we try to discuss (and clarify) here.


login: sebres
mimetype: text/x-fossil-wiki
icomment:
  > We might interpret this information in different ways.

  There is nothing to interpret:

  You use a mutex reference saved in the shared tsv, and then another thread pool overwrites this tsv-value, so a conditional notify and/or unlock mutex later would access another mutex initialized for another run (so for example you're trying to unlock a mutex which was never locked in this worker).

  Just try to log mutex name (or rather a value of <code>[tsv::get shared context]</code>) in case of error (catch around unlock if "mutex is not locked" error throw) - you would see the context is incremented in-between and the mutex is not the same.
  As already multiple times said you switches the context after unlock in main thread too fast so some of 6 threads entering the RC (doing its unlock after switch of context tsv).

  > Are you familiar also with the application of the synchonisation primitive “condition variable”?

  Yes I'm familiar (but it looks like you are not familiar with multi-threaded environments completely, because you try to use it totally wrong way).
  And yes, you can rewrite my attached script to use conditional vars and mutex instead of asynchronous handling.

  The main point (you are missing to understand) is the usage of unique identifier ($ctx in my case) to avoid overwriting of the context by next run.<br/>
  Or even to wait until all threads going down before you continue next run.

  As already said, your approach is wrong per definition, so it can not work properly at all.

  There is nothing to discuss or to repair, no matter which primitives or facilities you would use.

  See new attachment where you can see how it may be simply repaired - either if you would resolve ALL race conditions you have introduced there, or with a simple trick I fixed that (marked as IMPORTANT part).<br/>
  Note all places marked with "****" which explaining the possible RC.<br/>
  Especially IMPORTANT part at end waiting for the threads exited (if you switch `if 1 {...}` to `if 0 {...}` you will see your errors again).<br/>
  But it is only one possible solution (or even workaround), another would be to change your entirely wrong approach (or resolve all RC's or even use unique context id to avoid replacements, etc pp).

  Still one (last) try from my side: A protection of context with mutex does not help if your context (which contains mutex reference) switches in-between (because your main thread does not really know how many threads are started, obtained the mutex and context or are ended before).<br/>

  So I'll close it again due to basic misconception and too many errors in your example script.<br/>
  <b>Please read what is a race condition before you post here again!</b>


login: elfring
mimetype: text/plain
icomment:
  > I'll attach a "proper" asynchronous pool implementation similar yours,

  Thanks for your attached example script.

  It can demonstrate also specific ideas.


  > but without conditional wait and extra mutex

  This difference is relevant for the understanding of the really wanted results while noticing undesirable software behaviour.


  I suggest to reconsider data processing properties around the following piece of code from your variant of the command “handle_input”.


  …
  >        while {$::status ni {"endoftask" "shutdown"}} {
  >          vwait ::status; # wait for end of work
  >        }
  …


  These “worker threads” should not wait for an end because they should perform only their own work before they will eventually inform the main control thread about their immediate termination.




  >You use a mutex reference saved in the shared tsv,

  Yes, exactly.


  > and then another thread pool overwrites this tsv-value, 

  There are no thread pools involved in the test scripts for this issue.
  Each mutex handle is stored in an other thread shared variable (which is selected by the shared variable “context” or the command parameter “ctx”), isn't it?


  > so a conditional notify and/or unlock mutex later would access another mutex initialized for another run

  I would disagree to your conclusion in the concrete case while I can follow this information in the direction that inappropriate data accesses can happen after software failures.


  > - you would see the context is incremented in-between and the mutex is not the same

  I did not get such an impression from my test scripts for this issue so far.


  > Yes I'm familiar

  This is nice.


  > (but it looks like you are not familiar with multi-threaded environments completely,

  I am used to them for several years (as you can also determine from public archives).


  > because you try to use it totally wrong way).

  We disagree still about specific implementation details.


  > The main point (you are missing to understand) is the usage of unique identifier ($ctx in my case) to avoid overwriting of the context by next run.

  I am using the thread shared variable “context” for this purpose.


  > Or even to wait until all threads going down before you continue next run.

  This should usually happen.


  > As already said, your approach is wrong per definition, so it can not work properly at all.

  I disagree to this view.


  > There is nothing to discuss or to repair, no matter which primitives or facilities you would use.

  We present different perspectives for this software (and communication) situation.


  > See new attachment where you can see how it may be simply repaired - either if you would resolve ALL race conditions you have introduced there,

  The proper handling of race conditions is a general challenge for multi-threaded programming.


  …
  >        ## **** HERE IS A RACE CONDITION: YOU CAN OBTAIN MUTEX AFTER MAIN THREAD LEAVE perform_command

  This information is mostly appropriate.

  But all mutexes (which were created for the variable “pcm”) are kept usable for this test (because I omitted the resource clean-up for them at the moment).


  >        log "[tsv::incr [tsv::get shared context] working] threads are working."
  …

  I find that the counter for the working extra threads is properly managed.


  …
  >        ## **** HERE IS A RACE CONDITION: YOU CAN NOTIFY A CALLER THAT THE LAST THREAD IS GOING, BUT SOME MAY BE STILL THERE
  >        tsv::incr [tsv::get shared context] working -1
  …

  If this counter is consistent, then no additional threads will be left over.


  …
  >    ## **** HERE IS A RACE CONDITION: IF working IS 0 (YOU DON'T ENTER WAIT AT ALL)
  >    while {[set w [tsv::get [tsv::get shared context] working]] > 0} {
  …

  Your information is appropriate.

  If it was determined that no worker threads were left over, the next loop iteration can be executed finally.



  > or with a simple trick I fixed that (marked as IMPORTANT part).

  I find the call of the command “thread::release -wait $t($x)” unnecessary because the affected threads should have been immediately finished after a possible termination notification.


login: sebres
mimetype: text/x-fossil-wiki
icomment:
  > It can demonstrate also specific ideas.

  No, there is still one attached script demonstrated four ! RC you introduced.

  So forget the first script, and take a look at the second attachment.

  > There are no thread pools involved in the test scripts for this issue

  What? Your script creates your own thread pool (6 threads).

  And even due to mentioned race conditions (BUGS IN YOUR SCRIPT marked from me wiith ****) does it wrong (a second pool started before first "releases" the context).

  > We disagree still about specific implementation details.

  No, I told you that your script is totally and basically wrong, because it has at least 4 bugs.
  You trying to say me something about different approach. 

  BTW take a look in your mailbox (possibly spam-folder) I have send you a mail yesterday in German possibly better explaining your RC's.


login: elfring
mimetype: text/plain
icomment:
  > So forget the first script, and take a look at the second attachment.

  I responded also to information in both TCL scripts a moment ago.


  > Your script creates your own thread pool 

  Some threads are just repeatedly created. - But they are not kept in a “pool”.


  > I have send you a mail yesterday in German possibly better explaining your RC's.

  I am curious on how the clarification will evolve further (eventually also with the help of a telephone call).


login: sebres
mimetype: text/x-fossil-wiki
icomment:
  As discussed today by telephone (that tsv::incr +1 followed by -1 in all 6 threads should supposedly set the value to negative, I analysed it and can confirm it is possible even due to one of your RC which I already mentioned in my script examples and by telephone.

  This occurs because of the mistakenly switch of the content in the main thread (next iteration in repeat_command) where still one or more active worker are in the second lock area of the handle_input.

  Here are the problem places (some of that are already marked with **** in second attachment): 
  <ol>
  <li>your thread pool (workers) operate within context (holding the mutex and variables like "working") identified by <code>[tsv::get shared context]</code>, thereby it does no matter which <code>$x</code> is currently used, because if main thread would switch the context, the content of <code>[tsv::get shared context]</code> is incremented surely together with $x, but "old" still alive workers would simply write/read all the values in/from "new" context.</li>
  <li>The main thread (due to several RC) does not really wait unless all workers are going down.</li>
  <li>The context self is not identified in workers (so if main thread switch it within lock, the old still alive thread uses new context as it would be own, what is wrong.</li>
  </ol>

  Here is one example of possible scenarios where it can take place:
  <ol>
  <li>let the iteration in repeat_command be 11 (x = 11), main thread set "shared context" to 11 and starts 6 threads, 4 incrementing and decrementing "working" and going down. But 2 are still in the obtaining of first lock (waiting for unlock, main thread holds the mutex), so have not incremented "working".</li>
  <li>The value of "working" is now 0 (4*+1 and 4*-1).</li>
  <li>Now main thread want to enter while cycle, but the condition checking <code>[tsv::get [tsv::get shared context] working] > 0</code> is false, because it is 0.</li>
  <li>The main thread does unlock of mutex, ends perform_command and returns to repeat_command and start next iteration (so x will be 12)</li>
  <li>Before it begins to switch context, because the previous mutex was unlocked, 2 "old" alive threads entering locking area one after the other and incrementing the value of "working" (now it is 2).</li>
  <li>Now the main thread (in the next iteration enters perform_command and sets new "shared context" to 12 and then overwrite global tsv context with new mutex and cond-var, set "working" and "finish_notification" to 0.<br/>
  So the value of <code>[tsv::get shared context]</code> is 12 (but it does no matter because the workers never check that).</li>
  <li>Before the main thread begins to start new 6 threads, 2 "old" alive threads entering now second locking area one after the other and now decrementing the value of "working". But now it is a new context, and its "working" was 0.<br/>
  So after <code>tsv::incr [tsv::get shared context] working -1</code> of thread-5 it will be -1.<br/>
  And after <code>tsv::incr [tsv::get shared context] working -1</code> of thread-6 it will be -2.<br/></li>
  </ol>
  This way the mutex (and missing identifier and wait for end of thread-pool) are blame for the race condition and you get negative value in tsv-value "working" (but if one does not see this RC, one would say it is impossible).

  This is pretty good reproducible and can be easily comprehensible if one adds logging messages (value $x as well as value of [tsv::get shared context] in threads before decrementing).

  One will see something like that:
  <code><pre style="padding-left: 10pt">
  MAIN:  ctx: 11 | Start of perform_command.
  TH01:  ctx: 11 | working +1 = 1 threads are working.
  TH01:  ctx: 11 | working -1 = 0 threads are working.
  TH02:  ctx: 11 | working +1 = 1 threads are working.
  TH03:  ctx: 11 | working +1 = 2 threads are working.
  TH04:  ctx: 11 | working +1 = 3 threads are working.
  TH02:  ctx: 11 | working -1 = 2 threads are working.
  TH04:  ctx: 11 | working -1 = 1 threads are working.
  TH03:  ctx: 11 | working -1 = 0 threads are working.
  MAIN:  ctx: 11 | Worker threads were started.
  MAIN:  ctx: 11 | End of perform_command.
  TH05:  ctx: 11 | working +1 = 1 threads are working.
  TH06:  ctx: 11 | working +1 = 2 threads are working.
  *********** here is the switch of the context (next iteration) ***********
  MAIN:  ctx: 12 | Start of perform_command.
  TH06:  ctx: 12 | working -1 = -1 threads are working.
  TH05:  ctx: 12 | working -1 = -2 threads are working.
  </pre></code>

  So I'm pretty sure your script is wrong (due to many RC's) and therefore the issue is invalid here.
  Neither conditional/mutex, nor the tsv-storage have any synchronization issue in the direction you're trying to claim.

  Regards,
  Serg.


login: elfring
mimetype: text/plain
icomment:
  >… threads should supposedly set the value to negative,

  I would expect that a counter variable like “working” would be kept non-negative
  here because of the system properties of the command “tsv::incr”.


  >…, but "old" still alive workers would simply write/read all the values in/from "new" context.

  This is a possible data processing concern.


  > The main thread (due to several RC) does not really wait unless all workers are going down.

  I got other expectations. It is attempted to achieve the result that no worker threads will be left over running at the end of the procedure “perform_command”.


  >The context self is not identified in workers.

  Not for this test variant so far.


  > so if main thread switch it within lock,

  This should not happen here.


  > the old still alive thread uses new context as it would be own, what is wrong.

  Your conclusion would be appropriate for the unexpected case.


  > (but if one does not see this RC,

  One race condition might not be appropriately taken into account for this test case version.


  > one would say it is impossible). 

  The software situation might be still unclear for discussed variations in implementation details.


  > This is pretty good reproducible

  Interesting, isn't it? - Thanks for your test demonstration.


  > and can be easily comprehensible if one adds logging messages

  The logging approach can trigger significant differences for the data flow comparison.


  > So I'm pretty sure your script is wrong

  The shown approach can be incomplete. It corresponds to a software development stage around a try for the safe application of a (single) condition variable.


  > Neither conditional/mutex, nor the tsv-storage have any synchronization issue in the direction you're trying to claim.

  I imagine that our different views can contribute to a better solution finally.
  Are there any more design options to reconsider?


login: sebres
mimetype: text/x-fossil-wiki
icomment:
  > I would expect that a counter variable like “working” would be kept non-negative
  here because of the system properties of the command “tsv::incr”.

  Yes, but you increment or decrement values in different storages!
  Firstly in my example it is 11, then it is 12.<br/>
  So <code>[tsv::incr 11 working +1]</code> and hereafter <code>[tsv::incr 12 working -1]</code>.

  > This is a possible data processing concern.

  No. Still again - this is a result of your wrong protection algorithm.

  > I got other expectations. It is attempted to achieve the result that no worker threads will be left over running at the end of the procedure “perform_command”.

  But this expectation is wrong by design - your worker pool (context) is protected by "working" counter, which is not guarantee at the moment that all workers really counted (because of the RC).

  Either you would increment it before starting of each thread in main thread, and each worker would decrement it. Or something similar.<br/>
  Bur right now as it build, it is rather sporadic value and main thread get "working" == 0 because workers still not made the increment (suspended in lock before).

  > Interesting, isn't it? - Thanks for your test demonstration.

  Not really, because as already said - it is wrong by design.

  > The logging approach can trigger significant differences for the data flow comparison.

  Sure (and with each additional log-message it will be harder to reproduce). But try to follow my explanation - it is clear a wrong design. The situation of steps 1..6 is imaginable but should be avoided, therefore in step 7 it going wrong.

  > The shown approach can be incomplete. It corresponds to a software development stage around a try for the safe application of a (single) condition variable.

  No. This is wrong. If you cannot guarantee that the pool working properly and does not overwrite some values - this results to bugs/RC you see.<br/>
  And then to describe this as an issue of tcl-thread package is at least weird in my opinion.<br/>
  You will get this issue if you would write this with ANY language supporting multi-threading facilities and the approach remind the same.

  > I imagine that our different views can contribute to a better solution finally.

  No. There are basic differences in our views. I'm developing in multi-threaded environments already 20 years with several languages (C, tcl, python, ruby, java/scala, etc).
  The protection of the pool (and primitives used within) is always a primary task and should be organized properly. Your protection in script is wrong by design.<br/>
  Nowhere one would expect the code:
  <code><pre>
  tsv::set shared context 11
  tsv::incr [tsv::get shared context] working +1
  tsv::set shared context 12
  tsv::incr [tsv::get shared context] working -1
  puts [tsv::get [tsv::get shared context] working]
  </pre></code>
  would result to something other as -1.

  So if you cannot implement basic protection to guarantee working counter always corresponds to the real counter of workers and trying to implement a wait for the completion on the basis of this wrong protection, it is doomed to fail.


login: elfring
mimetype: text/plain
icomment:
  > so if main thread switch it within lock,

  Now I imagine that this undesired action can be prevented by referring to the mentioned context information by a local TCL variable instead of using the command “[tsv::get shared context]” in the worker threads.
  This can be achieved by a concatenation of two code parts like the following.

     thread::create [string cat "set context $x\n" $suffix]


  > … your worker pool (context) is protected by "working" counter, which is not guarantee at the moment that all workers really counted …

  I have changed the software design to an approach which works with termination notifications from TCL worker threads.
  Unfortunately, it contains also open issues for further development considerations.


