Re: [boost] Request for 1.34.1 milestone (bug #906)

From: Anthony Williams (anthony_w.geo_at_[hidden])
Date: 2007-05-22 07:06:23

"Johan Nilsson" <r.johan.nilsson_at_[hidden]> writes:

> Anthony Williams wrote:
>> Thomas Witt <witt_at_[hidden]> writes:
>>
>>> Any input on this one. Seems like too complicated a fix for 1.34.1
>>
>> I believe the patch below will fix Johan's immediate problems; I
>> would be grateful if he could try it.
>
> There was a typo at (patched) line 82: "#if define(BOOST_HAS_WINTHREADS)"
> instead of "#if defined(BOOST_HAS_WINTHREADS)".

Oops. Thanks for spotting it.

> IIRC we earlier discussed checking for a valid TSS index before calling
> TlsSetValue when in the cleanup_slots context. 

That was when you suggested calling TlsSetValue *after* cleanup slots --- at
which point the TSS index may not be valid. In cleanup_slots (where it is now)
it is always valid, so we don't need the check.

> Further, I'd suggest statically initializing tss_data_native_key to
> 0xFFFFFFFF instead of having it zero initialized.

Good idea.

> The attached patch file (against RC_1_34_0) has these things
> fixed/added. Please review. I've applied the patch and successfully run all
> tests under libs/thread/test for current RC_1_34_0 under WinXP SP2 (SMP +
> HT). My own tests also keep running successfully with the patch applied.
>
> Even though the patch "couldn't possibly" have any effects under non-windows
> platforms I also applied the patch and ran the tests under OpenSUSE 10.2
> using gcc 4.1.2.

Thanks for doing this.

> Adding a named constant instead of hardcoding 0xFFFFFFFF all over the place
> would also be great. However, as I believe this is the last Boost release
> containing the "original" thread code I guess the point is moot?

Actually, this is TLS_OUT_OF_INDEXES (the return value from TlsAlloc if
there's not a free slot), so I've replaced it with that constant.

> [A bit off-topic] Is it intentional to have the 1.34.1 updates go under the
> RC_1_34_0 branch?

Yes.

Attached is a revised patch.

Anthony
-- 
Anthony Williams
Just Software Solutions Ltd - http://www.justsoftwaresolutions.co.uk
Registered in England, Company Number 5478976.
Registered Office: 15 Carrallack Mews, St Just, Cornwall, TR19 7UL
Index: tss.cpp
===================================================================
RCS file: /cvsroot/boost/boost/libs/thread/src/tss.cpp,v
retrieving revision 1.16.10.4
diff -u -r1.16.10.4 tss.cpp
--- tss.cpp     1 Oct 2006 12:57:18 -0000       1.16.10.4
+++ tss.cpp     22 May 2007 10:55:06 -0000
@@ -31,7 +31,7 @@
 boost::mutex* tss_data_mutex = 0;
 tss_data_cleanup_handlers_type* tss_data_cleanup_handlers = 0;
 #if defined(BOOST_HAS_WINTHREADS)
-    DWORD tss_data_native_key;
+    DWORD tss_data_native_key=TLS_OUT_OF_INDEXES;
 #elif defined(BOOST_HAS_PTHREADS)
     pthread_key_t tss_data_native_key;
 #elif defined(BOOST_HAS_MPTASKS)
@@ -60,6 +60,7 @@
         tss_data_mutex = 0;
 #if defined(BOOST_HAS_WINTHREADS)
         TlsFree(tss_data_native_key);
+        tss_data_native_key=TLS_OUT_OF_INDEXES;
 #elif defined(BOOST_HAS_PTHREADS)
         pthread_key_delete(tss_data_native_key);
 #elif defined(BOOST_HAS_MPTASKS)
@@ -78,6 +79,9 @@
         (*(*tss_data_cleanup_handlers)[i])((*slots)[i]);
         (*slots)[i] = 0;
     }
+#if defined(BOOST_HAS_WINTHREADS)
+    TlsSetValue(tss_data_native_key,0);
+#endif
     tss_data_dec_use(lock);
     delete slots;
 }
@@ -97,7 +101,7 @@
     //Allocate tls slot
     tss_data_native_key = TlsAlloc();
-    if (tss_data_native_key == 0xFFFFFFFF)
+    if (tss_data_native_key == TLS_OUT_OF_INDEXES)
         return;
 #elif defined(BOOST_HAS_PTHREADS)
     int res = pthread_key_create(&tss_data_native_key, &cleanup_slots);