I've received a message from Przemyslaw that I attach here, I think we should
seriously fix these problems, right now xharbour is not useable anymore, not
only in MT mode but even a plain ST code with a destructor (which does not
uses statics nor creates now objects inside destructor).

Maurilio.


Message from Przemyslaw follows
--------8<-----------------------------

It's causes by:
   STATIC lInErr
inside rtl/terror.prg

below I'm forwarding the last message I sent to Phil with a patch
which resolves the problem by simple removing lInErr.
Please note that there is s_iLaunchCount for protection against recursive
error calls inside source/vm/errorapi.c which in MT version is part of HVM
stack.

The problem with destructors your reported is probably caused by
executing destructors also for statics variables. Because class
definition is stored inside such variables then it means that now
you cannot create any new object inside destructor even if you want
to remove it immediately. It also means that you should not expect
any valid values in static variables you will want to access from
destructors so you should probably not use them in destructors at all.

best regards,
Przemek


----- Forwarded message from Przemyslaw Czerpak <dru...@acn.waw.pl> -----

From: Przemyslaw Czerpak <dru...@acn.waw.pl>
Subject: Re: [xHarbour-developers] ChangeLog 2008-12-23 01:00 UTC+0300
        PhilKrylov <phil a t newstar.rinet.ru>
To: Phil Krylov <p...@newstar.rinet.ru>
Date: Wed, 24 Dec 2008 03:25:48 +0100
Lines: 131

On Wed, 24 Dec 2008, Przemyslaw Czerpak wrote:

Hi Phil,

Finally I located the problem with reference counters and why
it was crashing in T004. Just simply in some syncing after CVS
update I had to add by mistake HB_ATOMIC_INC()/HB_ATOMIC_DEC()
macros definition to the section which is used for ST mode.
If you moved it then readonly access to complex variables
begins to work without crashes. Looking for the problem I also
found that hb_itemUnShareString() was copied from Harbour as
is without adopting it to xHarbour. I have serious doubts if
Miguel well knows what he is doing porting Harbour code only
partially. Someone should verify such modifications.
I'm attaching patch which resolves this problems and speedtst.prg
begins to work with xHarbour. But it was rather trivial things.
Now you should look for much more complicated ones. F.e. race
conditions when threads are started/finished which causes that
programs creating simultaneously many short life threads crashes,
memvar variables which causes unreported memory leaks so in practice
cannot be use in MT programs which have to be executed for long time
creating many threads (each new thread allocates new dynamic symbols
for each used memvar which is never released), add MT protection
for classy code, remove all global static variables like
hb_vm_bQuitRequest, s_iBaseLine, hb_vm_iTry, ...
check the .prg code which also uses many static variables
(probably you will have to add support for THREAD STATIC if
you will want to make some things like GETLIST working in MT
mode), make SETs thread local (or any code which have to use
them cannot be ported to MT), updated GT code and enable GT
locking (f.e. see gtapi.c in Harbour and xHarbour), clean
basic thread API to eliminate race conditions which xHarbour
thread functions have (see some notes in speedtst.prg), fix
some compiler constructions like multivalue macros (f.e.:
a[&("1,2")], qout(&("3,4")), a:={&("5,6,7"})] ) or TRY/CATCH
which are not MT safe, update debugger for MT mode, reduce
number of internal locks which kill scalability - you probably
noticed in speedtst results that on real multiCPU machine is
more efficient to execute simultaneously 2 * <numCPU> instances
of single thread xHarbour program then <numCPU> threads in MT one.
Here it will be necessary to strongly change some subsystems.
F.e. whole macro compiler is protected by MUTEX so only one thread
can access it. To resolve it it's necessary to rewrite compiler,
macrocompiler and lexer code to be MT and reentrant safe.
And probably many other things I do not remember know. If you
are interested then I can try to collect modifications I had
to made in Harbour when I was working on MT mode. I still should
have a list I created over year ago.
They should help you to locate places which should be updated.
HTH

Marry Christmas and Happy New Year,
Przemek


  * xharbour/config/rules.cf
    * give PRG_USR higher priority then HB_FLAGS

  * xharbour/include/thread.h
    ! fixed declaration of HB_ATOMIC_INC()/HB_ATOMIC_DEC() macros
      for 32bit x86 *nixes - by mistake it was added to ST section

  * xharbour/source/rtl/terror.prg
    ! removed lInErr static variable - it breaks using errorNew() in
      MT programs

  * xharbour/source/vm/itemapi.c
    ! fixed copied as is from Harbour hb_itemUnShareString()
      Harbour uses different HVM internals and such code cannot
      be copied without adopting to xHarbour

  * xharbour/tests/speedtst.prg
    ! fixed possible race condition in initialization
    + added SET WORKAREA PRIVATE

Index: config/rules.cf
===================================================================
RCS file: /cvsroot/xharbour/xharbour/config/rules.cf,v
retrieving revision 1.13
diff -u -r1.13 rules.cf
--- config/rules.cf     16 Mar 2008 19:15:57 -0000      1.13
+++ config/rules.cf     24 Dec 2008 02:19:44 -0000
@@ -85,7 +85,7 @@
 else
 # Rule to generate a C file from a PRG file.
 %.c : $(GRANDP)%.prg
-       $(HB) $? $(PRG_USR) $(HB_FLAGS)
+       $(HB) $? $(HB_FLAGS) $(PRG_USR)
 endif

 ifeq ($(SHLVL),) # COMMAND.COM
Index: include/thread.h
===================================================================
RCS file: /cvsroot/xharbour/xharbour/include/thread.h,v
retrieving revision 1.126
diff -u -r1.126 thread.h
--- include/thread.h    23 Dec 2008 18:06:33 -0000      1.126
+++ include/thread.h    24 Dec 2008 02:19:44 -0000
@@ -396,8 +396,37 @@

 #endif

-   #define HB_ATOMIC_INC( x )          ( ++(x) )
-   #define HB_ATOMIC_DEC( x )          ( --(x) )
+   #if HB_COUNTER_SIZE == 4 && defined( __GNUC__ ) && 1 && \
+       ( defined( i386 ) || defined( __i386__ ) || defined( __x86_64__ ) )
+
+      static __inline__ void hb_atomic_inc32( volatile int * p )
+      {
+         __asm__ __volatile__(
+            "lock; incl %0\n"
+            :"=m" (*p) :"m" (*p)
+         );
+      }
+
+      static __inline__ int hb_atomic_dec32( volatile int * p )
+      {
+         unsigned char c;
+         __asm__ __volatile__(
+            "lock; decl %0\n"
+            "sete %1\n"
+            :"=m" (*p), "=qm" (c) :"m" (*p) : "memory"
+         );
+         return c == 0;
+      }
+
+      #define HB_ATOMIC_INC( x )    ( hb_atomic_inc32( ( volatile int * )
&(x) ) )
+      #define HB_ATOMIC_DEC( x )    ( hb_atomic_dec32( ( volatile int * )
&(x) ) )
+
+   #else
+
+      #define HB_ATOMIC_INC( x )    ( ++(x) )
+      #define HB_ATOMIC_DEC( x )    ( --(x) )
+
+   #endif

    #define HB_MUTEX_T                  HB_CRITICAL_T
    #define HB_MUTEX_INIT( x )          HB_CRITICAL_INIT( x )
@@ -875,37 +904,8 @@

 #else

-   #if HB_COUNTER_SIZE == 4 && defined( __GNUC__ ) && \
-       ( defined( i386 ) || defined( __i386__ ) || defined( __x86_64__ ) )
-
-      static __inline__ void hb_atomic_inc32( volatile int * p )
-      {
-         __asm__ __volatile__(
-            "lock; incl %0\n"
-            :"=m" (*p) :"m" (*p)
-         );
-      }
-
-      static __inline__ int hb_atomic_dec32( volatile int * p )
-      {
-         unsigned char c;
-         __asm__ __volatile__(
-            "lock; decl %0\n"
-            "sete %1\n"
-            :"=m" (*p), "=qm" (c) :"m" (*p) : "memory"
-         );
-         return c == 0;
-      }
-
-      #define HB_ATOMIC_INC( x )    ( hb_atomic_inc32( ( volatile int * )
&(x) ) )
-      #define HB_ATOMIC_DEC( x )    ( hb_atomic_dec32( ( volatile int * )
&(x) ) )
-
-   #else
-
-      #define HB_ATOMIC_INC( x )    ( ++(x) )
-      #define HB_ATOMIC_DEC( x )    ( --(x) )
-
-   #endif
+   #define HB_ATOMIC_INC( x )    ( ++(x) )
+   #define HB_ATOMIC_DEC( x )    ( --(x) )

    #define HB_CRITICAL_LOCK( x )
    #define HB_CRITICAL_TRYLOCK( x )
Index: source/rtl/terror.prg
===================================================================
RCS file: /cvsroot/xharbour/xharbour/source/rtl/terror.prg,v
retrieving revision 1.19
diff -u -r1.19 terror.prg
--- source/rtl/terror.prg       27 Jun 2008 06:21:49 -0000      1.19
+++ source/rtl/terror.prg       24 Dec 2008 02:19:44 -0000
@@ -61,19 +61,12 @@

 FUNCTION ErrorNew( SubSystem, GenCode, SubCode, Operation, Description, Args,
ModuleName, ProcName, ProcLine )

-   STATIC lInErr := .F., s_oClass
+   STATIC s_oClass
    LOCAL oErr
    LOCAL nLevel, aaStack

    //TraceLog( SubSystem, GenCode, SubCode, Operation, Description, Args,
ModuleName, ProcName, ProcLine )

-   // Avoid RECURSIVE Errors.
-   IF lInErr
-      RETURN NIL
-   ELSE
-      lInErr := .T.
-   ENDIF
-
    IF s_oClass == NIL
       s_oClass := HBClass():New( "ERROR" )

@@ -160,8 +153,6 @@
       oErr:VMThreadId     := ThreadGetCurrentInternal()
    #endif

-   lInErr := .F.
-
 RETURN oErr

 FUNCTION __eInstVar53( oVar, cMethod, xValue, cType, nSubCode, bValid )
Index: source/vm/itemapi.c
===================================================================
RCS file: /cvsroot/xharbour/xharbour/source/vm/itemapi.c,v
retrieving revision 1.155
diff -u -r1.155 itemapi.c
--- source/vm/itemapi.c 3 Dec 2008 11:09:45 -0000       1.155
+++ source/vm/itemapi.c 24 Dec 2008 02:19:44 -0000
@@ -1521,14 +1521,19 @@
    HB_TRACE_STEALTH(HB_TR_DEBUG, ("hb_itemUnShareString(%p)", pItem));

    if( pItem->item.asString.allocated == 0 ||
-       hb_xRefCount( pItem->item.asString.value ) > 1 )
+       *( pItem->item.asString.pulHolders ) > 1 )
    {
       ULONG ulLen = pItem->item.asString.length + 1;
       char *szText = ( char* ) hb_xgrab( ulLen );

       hb_xmemcpy( szText, pItem->item.asString.value, ulLen );
       if( pItem->item.asString.allocated )
-         hb_xRefDec( pItem->item.asString.value );
+      {
+         if( HB_ATOMIC_DEC( *( pItem->item.asString.pulHolders ) ) == 0 )
+         {
+            hb_xfree( pItem->item.asString.value );
+         }
+      }
       pItem->item.asString.value = szText;
       pItem->item.asString.allocated = ulLen;
    }
Index: tests/speedtst.prg
===================================================================
RCS file: /cvsroot/xharbour/xharbour/tests/speedtst.prg,v
retrieving revision 1.8
diff -u -r1.8 speedtst.prg
--- tests/speedtst.prg  23 Dec 2008 18:06:33 -0000      1.8
+++ tests/speedtst.prg  24 Dec 2008 02:19:45 -0000
@@ -11,7 +11,6 @@
  *
  */

-
 #define N_TESTS 54
 #define N_LOOPS 1000000
 #define ARR_LEN 16
@@ -241,7 +240,13 @@

 /* initialize mutex in hb_trheadDoOnce() */
 init proc once_init()
+   set workarea private
    hb_threadOnce()
+   /* initialize error object to reduce possible crashes when two
+    * threads will try to create new error class simultaneously
+    * xHarbour does not have any protection against such situation
+    */
+   errorNew()
 return

 function hb_threadOnce( xOnceControl, bAction )
@@ -253,11 +258,11 @@
    if xOnceControl == NIL
       hb_mutexLock( s_mutex )
       if xOnceControl == NIL
-         xOnceControl := .t.
-         lFirstCall := .t.
          if bAction != NIL
             eval( bAction )
          endif
+         xOnceControl := .t.
+         lFirstCall := .t.
       endif
       hb_mutexUnlock( s_mutex )
    endif
@@ -275,15 +280,15 @@
 TEST t003 WITH L_D:=date()       CODE x := L_D

 TEST t004 INIT _( static s_once, S_C ) ;
-          INIT iif( hb_threadOnce( @s_once ), S_C := dtos(date()), ) ;
+          INIT hb_threadOnce( @s_once, {|| S_C := dtos( date() ) } ) ;
           CODE x := S_C

 TEST t005 INIT _( static s_once, S_N ) ;
-          INIT iif( hb_threadOnce( @s_once ), S_N := 112345.67, ) ;
+          INIT hb_threadOnce( @s_once, {|| S_N := 112345.67 } ) ;
           CODE x := S_N

 TEST t006 INIT _( static s_once, S_D ) ;
-          INIT iif( hb_threadOnce( @s_once ), S_D := date(), ) ;
+          INIT hb_threadOnce( @s_once, {|| S_D := date() } ) ;
           CODE x := S_D

 TEST t007 INIT _( memvar M_C ) INIT _( private M_C := dtos( date() ) ) ;
@@ -298,19 +303,19 @@
 TEST t010 INIT _( memvar P_C ) ;
           INIT _( static s_once ) ;
           INIT _( public P_C ) ;
-          INIT iif( hb_threadOnce( @s_once ), P_C := dtos( date() ), ) ;
+          INIT hb_threadOnce( @s_once, {|| P_C := dtos( date() ) } ) ;
           CODE x := P_C

 TEST t011 INIT _( memvar P_N ) ;
           INIT _( static s_once ) ;
           INIT _( public P_N ) ;
-          INIT iif( hb_threadOnce( @s_once ), P_N := 112345.67, ) ;
+          INIT hb_threadOnce( @s_once, {|| P_N := 112345.67 } ) ;
           CODE x := P_N

 TEST t012 INIT _( memvar P_D ) ;
           INIT _( static s_once ) ;
           INIT _( public P_D ) ;
-          INIT iif( hb_threadOnce( @s_once ), P_D := date(), ) ;
+          INIT hb_threadOnce( @s_once, {|| P_D := date() } ) ;
           CODE x := P_D

 TEST t013 INIT _( field F_C ) INIT use_dbsh() EXIT close_db() ;


----- End forwarded message -----

-- 
 __________
|  |  | |__| Maurilio Longo
|_|_|_|____| farmaconsult s.r.l.



------------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It is the best place to buy or sell services for
just about anything Open Source.
http://p.sf.net/sfu/Xq1LFB
_______________________________________________
xHarbour-developers mailing list
xHarbour-developers@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/xharbour-developers

Reply via email to