Re: [boost] Boost.Threads - Open Questions from the Review

From: Jens Maurer (Jens.Maurer_at_[hidden])
Date: 2001-09-16 14:35:05

William Kempf wrote:
> --- In boost_at_y..., Jens Maurer <Jens.Maurer_at_g...> wrote:
> >  - xtime.hpp uses "boost/stdint.h", which is deprecated
> >and fails with como on my Linux box.  Use "boost/cstdint.hpp"
> >instead (need to fix uintXX_t usage in the code as well).
> 
> I don't understand the part in parens above.  xtime.hpp uses int_fastXX_t
> types but not uintXX_t types, and I don't know of anything in the usage that
> needs fixed.

Currently, there are two options to get to the int_fastXX_t typedefs:
You can #include "boost/stdint.h" and use std::int_fastXX_t
(that's what you do now), or you can #include "boost/cstdint.hpp"
and use boost:int_fastXX_t.   I'm saying that the former option
is less portable than the latter.  The reason is that "boost/stdint.h"
tries to add definitions to namespace std, which is reserved to the
implementation. 

If you do switch to boost/cstdint.hpp, you need to be aware of
the std -> boost namespace change for the int_fastXX_t (or uintXX_t
or whatever) types.  So the places in the source code where you
use these typedefs need syntactic adaptations to account for
the namespace change.

If I'm still talking non-intelligibly, holler (and please state
which of the two paragraphs above need further detailing).

[re-ordered]
> >  - If you need to add "extern C" to some function definitions, do
> make
> >sure to give those implementation-detail functions "static" linkage.
> >An unnamed namespace won't cut it, because the namespace is not
> >coded in the function name for "extern C" functions.
> 
> I need some help with all of this.

Let me start with a bit of explanation:  The pthread functions are
defined in POSIX, which specifies a C API.  The implementation in the
pthread library expects the callbacks to be callable using the C
calling conventions on the machine.  The C++ calling conventions
could be radically different from those.  Thus, we need to make sure
that all function pointers passed to pthread functions can be called
with the C calling convention, by explicitly declaring them as
"extern C".

(This same issue might or might not arise on Windows, and the Windows
code should be hand-checked for the issue, because Windows compilers
might not show appropriate errors.)

> --- In boost_at_y..., Jens Maurer <Jens.Maurer_at_g...> wrote:
> >  - Something needs to be declared "extern C", or a wrapper added:
> >"once.cpp", line 45: error: argument of type "void (*)()" is
> incompatible with
> >           parameter of type "void (*)() C"
> >         pthread_once(&flag, func);

While developing a work-around, I wondered why call_once() doesn't
take a function object?  (This is more a request for adding rationale
than for changing the implementation.)

I think this patch (nearly) does what you want.  It uses a similar
trick than the Windows variant, but doesn't spread the mutex contention.

--- once.cpp.orig       Fri Aug 10 15:46:24 2001
+++ once.cpp    Sun Sep 16 21:18:25 2001
@@ -18,6 +18,23 @@
 
 namespace boost {
 
+#if defined BOOST_HAS_PTHREADS
+
+static pthread_once_t once_control = PTHREAD_ONCE_INIT;
+// A single critical section serializes access to all "flag"s passed
+// to call_once()
+static pthread_mutex_t once_mutex;
+
+
+extern "C" {
+static void init_once_mutex()
+{
+  pthread_mutex_init(&once_mutex, 0);
+}
+}
+
+#endif
+
 void call_once(void (*func)(), once_flag& flag)
 {
 #if defined(BOOST_HAS_WINTHREADS)
@@ -42,7 +59,31 @@
         assert(res);
        }
 #elif defined(BOOST_HAS_PTHREADS)
-       pthread_once(&flag, func);
+        if(!flag) {
+          // ensure mutex is initialized
+          pthread_once(&once_control, &init_once_mutex);
+          // now lock mutex
+          pthread_mutex_lock(&once_mutex);
+          if(!flag) {
+            // Setting the flag must be within the critical section.
+            flag = true;
+            pthread_mutex_unlock(&once_mutex);
+            try {
+              func();
+            } catch(...) {
+              // Emulate "setting the flag only afterwards" behaviour from
+              // the Windows section.
+              // The documentation should state that it's undefined to
+              // access "flag" within "func".
+              pthread_mutex_lock(&once_mutex);
+              flag = false;
+              pthread_mutex_unlock(&once_mutex);
+              throw;
+            }
+          } else {
+            pthread_mutex_unlock(&once_mutex);
+          }
+        }
 #endif
 }
 
> >"thread.cpp", line 103: error: argument of type "void *(*)(void *)"
> is
> >           incompatible with parameter of type "void *(*)(void *) C"
> >       int res = pthread_create(&m_thread, 0, &thread_proxy, &param);
> >                                              ^

This looks easy to fix: Just add "extern C { static ... }" at the
definition of thread_proxy().

> >"tss.cpp", line 134: error: argument of type "void (*)(void *)" is
> >           incompatible with parameter of type "void (*)(void *) C"
> >       int res = pthread_key_create(&m_key, cleanup);
> >                                            ^

Why doesn't "tss" take a function object?

> First, you can't declare the linkage as both extern "C" and static, though
> it's also true that namespaces won't help prevent conflicts for extern "C"
> functions.  So I'm not sure what to do in this case to avoid name collisions
> but use the proper language linkage specification.

See above and other's comments on the list.

> Third, boost::thread_specific_ptr<> poses the most difficulty for me.  The
> cleanup routine must be a templated routine, which AFAICT can't be given C
> linkage.

I'm looking at threadcorrect1.zip, and the tss class is not a template.

I'm confused.

(The version of tss.cpp I'm looking at can be fixed by an additional
level of indirection around the "cleanup" pointer, I believe.)

As a general remark, when throwing thread_resource_error(), it would
be nice to pass an explanatory string *what* system function failed,
and possibly with what system error code.

Jens Maurer