From: kalman on
Hi all,
I've recently stepped in a project of medium size, and at the easy
question: "are those class instances shared between two or more
threads" the response was: "no... wait... yes, well I'm not sure... I
don't know...". Riiiight.
I used the following technique that should permit to detect (at
runtime, sigh!) if two or more threads are using concurrently a class.

Suppose we have the following class:

struct Shared {
void foo() { ... }
};

and we are unsure if two threads are calling the Shared::foo() at same
time. One way is to add a mutex to the class Shared and then attempt a
"try lock" as first thing to do inside the foo and raise an error in
case the try lock failed.
Something like:

class Shared {
void foo() {
TryScopedLock aLock(theMutex);

if (!aLock) { throw std::runtime_error("BOOM"); }
...
}
private:
volatile mutex theMutex;
};

this approach works but it will slow down your software, hiding other
problems around and, most of all, introduces useless synchronization;
a mutex lock is not exactly cheap.

The idea is to use the technique above but lock less, GCC gives us
some functions for atomic memory access, and we can use for example:

bool __sync_bool_compare_and_swap (type *ptr, type oldval type
newval, ...)

for our very purpose. That function assigns at *ptr the value newval
only if the current value of *ptr is oldval, it returns true if the
comparison is successful and newval was written. We can use it to
store the threadId when we enter the critical section, "zeroing" the
value when we exit.

Basically I wrote a class that store (with an atomic operation) the
threadID of the thread entering the critical section, and when it
leaves forgets about the threadID. This was the result:

/////////////////////////////
#ifndef THREAD_COLLISION_WARNING
#define THREAD_COLLISION_WARNING

#include <stdexcept>

#ifdef NDEBUG

#define THREAD_WATCH(obj)
#define SCOPED_WATCH(obj)
#define WATCH(obj)

#else

#define THREAD_WATCH(obj) ThreadCollisionWarning _##obj;
#define SCOPED_WATCH(obj) ThreadCollisionWarning::ScopedWatch
scoped_watch_##obj(_##obj);
#define WATCH(obj) ThreadCollisionWarning::Watch
watch_##obj(_##obj);

#endif

class ThreadCollisionWarning {
public:
ThreadCollisionWarning()
:theActiveThread(0)
{ }

~ThreadCollisionWarning() { }

class Watch {
public:
Watch(ThreadCollisionWarning& aTCW)
:theWarner(aTCW)
{ theWarner.enter_self(); }

~Watch() { }

private:
ThreadCollisionWarning& theWarner;
};

class ScopedWatch {
public:
ScopedWatch(ThreadCollisionWarning& aTCW)
:theWarner(aTCW)
{ theWarner.enter(); }

~ScopedWatch() { theWarner.leave(); }

private:
ThreadCollisionWarning& theWarner;
};

private:

void enter_self() {
//If the active thread is 0 then I'll write the current thread
ID
//if two or more threads arrive here only one will success to
write on theActiveThread
//the current thread ID
if (! __sync_bool_compare_and_swap(&theActiveThread, 0,
pthread_self())) {

//Last chance! may be is the thread itself calling from a
critical section
//another critical section
if (!__sync_bool_compare_and_swap(&theActiveThread,
pthread_self(), theActiveThread)) {
throw std::runtime_error("Thread Collision");
}
}
}

void enter() {
if (!__sync_bool_compare_and_swap(&theActiveThread, 0,
pthread_self())) {
//gotcha! another thread is trying to use the same class
throw std::runtime_error("Thread Collision");
}
}

void leave() {
__sync_fetch_and_xor(&theActiveThread, theActiveThread);
}

pthread_t theActiveThread;
};

#endif
///////////////////////

The class ThreadCollisionWarning has the responsibility to store the
thread using the class (or more in general entering a critical
section) while the nested class ScopedWatch is used to notify the
entering and the leaving the critical section, the nested class
Watchjust during his constructor initializes theActiveThread member
with the current id thread if it isn't still initialized, in case it
gives another chance to check if the active thread is itself.

I'm now using those classes in the following way:

Case #1: Check that one thread ever uses some critical section
(recursion allowed)

struct Shared {
void foo() {
WATCH(CriticaSectionA);
bar();
}

void bar() {
WATCH(CriticaSectionA);
}

THREAD_WATCH(CriticaSectionA);
};


Case #2: Check that a class is constructed and destroyed inside the
same thread

struct Shared {
Shared() {
WATCH(CTOR_DTOR_SECTION);
...
}

~Shared() {
WATCH(CTOR_DTOR_SECTION);
...
}

THREAD_WATCH(CTOR_DTOR_SECTION);
};

note that doing so the Shared destructor can throw an exception, so do
not use this in a production code (put the WATCH between a try-catch
and just notify it in some way).

Case #3: Two or more different threads can enter a critical section
but in exclusive way (useful to check if external sync mechanism are
working).

struct Shared {
foo() {
SCOPED_WATCH(CriticalSectionA);
}

THREAD_WATCH(CriticalSectionA);
};


Any comment or suggestions are welcome.


--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

From: wasti.redl on
On Jun 20, 9:38 pm, kalman <mend...(a)gmail.com> wrote:
> Hi all,
> I've recently stepped in a project of medium size, and at the easy
> question: "are those class instances shared between two or more
> threads" the response was: "no... wait... yes, well I'm not sure... I
> don't know...". Riiiight.
> I used the following technique that should permit to detect (at
> runtime, sigh!) if two or more threads are using concurrently a class.

I think the approach is faulty. If you want to know if an instance is
used by more than one thread, you shouln't rely on it being actually
used concurrently in one particular execution - threading being what
it is, you might miss the shared usage.

Instead, remember the current thread ID in the constructor and never
reset it. Test at every function entry. This way you'll definitely see
if more than one thread accesses an instance during its lifetime, even
if the accesses happen to not occur at exactly the same time.


That said, I wonder about the design of a program where you can't tell
if an instance is shared between threads.


--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

From: kalman on
On Jun 21, 2:27 pm, wasti.r...(a)gmx.net wrote:
> On Jun 20, 9:38 pm, kalman <mend...(a)gmail.com> wrote:
>
> > Hi all,
> > I've recently stepped in a project of medium size, and at the easy
> > question: "are those class instances shared between two or more
> > threads" the response was: "no... wait... yes, well I'm not sure... I
> > don't know...". Riiiight.
> > I used the following technique that should permit to detect (at
> > runtime, sigh!) if two or more threads are using concurrently a class.
>
> I think the approach is faulty. If you want to know if an instance is
> used by more than one thread, you shouln't rely on it being actually
> used concurrently in one particular execution - threading being what
> it is, you might miss the shared usage.

If I want in general to know if a class is used in more than one thread I
use the macro WATCH (see the example below), to check if an external sync
mechanism is working (or at least is not failing) then I use the
SCOPED_WATCH.

> Instead, remember the current thread ID in the constructor and never
> reset it. Test at every function entry. This way you'll definitely see
> if more than one thread accesses an instance during its lifetime, even
> if the accesses happen to not occur at exactly the same time.

This is exactly what you will obtain using the nested class Watch trough
the macro WATCH(...), like this:

class MayBeShared {
MayBeShared() {
WATCH(single_thread_usage);
...
}

foo() {
WATCH(single_thread_usage);
}

private:
THREAD_WATCH(single_thread_usage);
};

> That said, I wonder about the design of a program where you can't tell
> if an instance is shared between threads.

I wonder that too, but you know how real world works.


--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

From: Maxim Yegorushkin on
On Jun 21, 1:27 pm, wasti.r...(a)gmx.net wrote:
> On Jun 20, 9:38 pm, kalman <mend...(a)gmail.com> wrote:
>
> > Hi all,
> > I've recently stepped in a project of medium size, and at the easy
> > question: "are those class instances shared between two or more
> > threads" the response was: "no... wait... yes, well I'm not sure... I
> > don't know...". Riiiight.
> > I used the following technique that should permit to detect (at
> > runtime, sigh!) if two or more threads are using concurrently a class.
>
> I think the approach is faulty. If you want to know if an instance is
> used by more than one thread, you shouln't rely on it being actually
> used concurrently in one particular execution - threading being what
> it is, you might miss the shared usage.
>
> Instead, remember the current thread ID in the constructor and never
> reset it. Test at every function entry. This way you'll definitely see
> if more than one thread accesses an instance during its lifetime, even
> if the accesses happen to not occur at exactly the same time.

The global objects are initialised by the thread that enters main()
(in a typical implementation). If some global objects are only ever
used by another thread this approach may not work.

Max

--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

From: kalman on
On Jun 22, 11:27 pm, Maxim Yegorushkin <maxim.yegorush...(a)gmail.com>
wrote:
> On Jun 21, 1:27 pm, wasti.r...(a)gmx.net wrote:
>
>
>
> > On Jun 20, 9:38 pm, kalman <mend...(a)gmail.com> wrote:
>
> > > Hi all,
> > > I've recently stepped in a project of medium size, and at the easy
> > > question: "are those class instances shared between two or more
> > > threads" the response was: "no... wait... yes, well I'm not sure... I
> > > don't know...". Riiiight.
> > > I used the following technique that should permit to detect (at
> > > runtime, sigh!) if two or more threads are using concurrently a class.
>
> > I think the approach is faulty. If you want to know if an instance is
> > used by more than one thread, you shouln't rely on it being actually
> > used concurrently in one particular execution - threading being what
> > it is, you might miss the shared usage.
>
> > Instead, remember the current thread ID in the constructor and never
> > reset it. Test at every function entry. This way you'll definitely see
> > if more than one thread accesses an instance during its lifetime, even
> > if the accesses happen to not occur at exactly the same time.
>
> The global objects are initialised by the thread that enters main()
> (in a typical implementation). If some global objects are only ever
> used by another thread this approach may not work.

In that case you can set the watches like this:

class MayBeShared {
MayBeShared() {
WATCH(ctor_dtor)
...
}
~MayBeShared() {
WATCH(ctor_dtor)
...
}

foo() {
WATCH(unic_thread)
...
}

bar() {
WATCH(unic_thread)
...
}

do_some_work() {
WATCH(multiple_thread_external_sync_mechanism)
...
}

private:
THREAD_WATCH(ctor_dtor)
THREAD_WATCH(unic_thread)
THREAD_WATCH(multiple_thread_external_sync_mechanism)
};

the "ctor_dtor watch" checks that the instances are created and
destroyed on same thread,
the "unic_thread watch" checks that a single thread ever is using
those methods while
the "multiple_thread_external_sync_mechanism watch" checks that class
external synchronization
mechanism are working or in place.

Consider this case:

class SharedForSure
{
foo() {
Mutex::scoped_lock lock(theMutex);
theClassToProtect.do_some_work();
}

bar() {
<=== Here is missing a lock
theClassToProtect.do_some_work();
}

private:
MayBeShared theClassToProtect;
Mutex theMutex;
};

the class SharedForSure has a bug missing a lock on his public bar
method,
hence the instance of MayBeShared can detect (if happens) the wrong
usage.


--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]