On Fri, Apr 6, 2018 at 4:52 PM, Chuck Rolke <cro...@redhat.com> wrote:
> There has been another solution waiting in a github pull request. > > https://github.com/apache/qpid-cpp/pull/11 > > This fix looks like it would work, but I put a slightly clearer fix on https://github.com/apache/qpid-cpp/pull/13 It gets rid of the bogus assignment rather than working around it by adding a dummy constructor. Chuck can you commit this? I don't have a local windows box set up and Appveyor is telling me that the master branch is broken before applying the fix. > ----- Original Message ----- > > From: "Alan Conway" <acon...@redhat.com> > > To: "qpid" <dev@qpid.apache.org> > > Sent: Friday, April 6, 2018 3:33:17 PM > > Subject: Re: Wrt QPID-7926 > > > > On Fri, Apr 6, 2018 at 4:33 AM, Michael Arnold <myk...@gmail.com> wrote: > > > > > Hi, > > > > > > Have a couple of questions wrt QPID-7926 > > > > > > Question 1: what is the relevance of the is_pod<> result? > > > > > > JIRA- 7926 (https://issues.apache.org/jira/browse/QPID-7926) states > that: > > > "In a stand-alone windows program > > > std::is_pod<PODMutex>::value > > > returns false. In Linux the same statement in qpidd broker returns > true." > > > > > > If I execute: > > > grep -r is_pod > > > in qpid-cpp-1.38.0 directory I get nothing i.e. from what I can see the > > > broker never uses "is_pod". What am I missing? > > > > > > > There is no requirement for PODMutex to be a POD, only that it can be > > initialized at file scope in a thread-safe way. In linux we handle that > by > > making it a POD, which gets initialized at library load (before any > > possible calls) rather than at C++-global-constructor time, when the > order > > of constructors between compilation units is undefined, so one > compilation > > units (C++ source file) might start using un-constructed global variables > > in another. > > > > > > > Question 2: Is this a possible approach? > > > I find 3 non-comment lines using QPID_MUTEX_INITIALIZER: > > > src/qpid/sys/posix/Mutex.h:#define QPID_MUTEX_INITIALIZER { > > > PTHREAD_MUTEX_INITIALIZER } > > > src/qpid/sys/windows/Mutex.h:#define QPID_MUTEX_INITIALIZER 0 > > > src/qpid/log/Logger.cpp:sys::PODMutex loggerLock = > QPID_MUTEX_INITIALIZER; > > > > > > Where the last line is generating the compiler error under windows, > due to > > > the type mismatch. > > > > > > For windows PODMutex, is a thin wrapper around boost::recusive_mutex, > while > > > under linux PODMutex is wrapper around pthread_mutex_t. > > > > > > From what I can see boost::recusive_mutex does not need to be > initalised, > > > but pthread_mutex_t does, hence under windows+boost the initialisation > on > > > the line: > > > sys::PODMutex loggerLock = QPID_MUTEX_INITIALIZER; > > > is not required. So possibly src/qpid/log/Logger.cpp can become: > > > #if defined(BOOST_WINDOWS) > > > sys::PODMutex loggerLock; > > > #else > > > sys::PODMutex loggerLock = QPID_MUTEX_INITIALIZER; > > > #endif > > > > > > > > That would compile but has potential race conditions if the loggerLock > > variable can be used from other compilation units before C++ global > > constructors are run. For example if there are other C++ files that log > > start-up information in their own global C++ constructors. It probably > > won't bite you but if it does it will be a horror to debug. > > > > I'm sure that windows, or boost, or both, have a safe solution to this > > problem. Probably similar to the pthreads/POD aggregate-initialiation > > approach in Linux. Check the docs, it might require a special type of > mutex. > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org > For additional commands, e-mail: dev-h...@qpid.apache.org > >
