0

I build a little application which has a render thread and some worker threads for tasks which can be made nearby the rendering, e.g. uploading files onto some server. Now in those worker threads I use different objects to store feedback information and share these with the render thread to read them for output purpose. So render = output, worker = input. Those shared objects are int, float, bool, STL string and STL list.

I had this running a few months and all was fine except 2 random crashes during output, but I learned about thread syncing now. I read int, bool, etc do not require syncing and I think it makes sense, but when I look at string and list I fear potential crashes if 2 threads attempt to read/write an object the same time. Basically I expect one thread changes the size of the string while the other might use the outdated size to loop through its characters and then read from unallocated memory. Today evening I want to build a little test scenario with 2 threads writing/reading the same object in a loop, however I was hoping to get some ideas here aswell.

I was reading about the CriticalSection in Win32 and thought it may be worth a try. Yet I am unsure what the best way would be to implement it. If I put it at the start and at the end of a read/function it feels like some time was wasted. And if I wrap EnterCriticalSection and LeaveCriticalSection in Set and Get Functions for each object I want to have synced across the threads, it is alot of adminstration.

I think I must crawl through more references.

Okay I am still not sure how to proceed. I was studying the links provided by StackedCrooked but do still have no image of how to do this.

I put copied/modified together this now and have no idea how to continue or what to do: someone has ideas?

class CSync
{
public:
    CSync()
    : m_isEnter(false)
    { InitializeCriticalSection(&m_CriticalSection); }
    ~CSync()
    { DeleteCriticalSection(&m_CriticalSection); }
    bool TryEnter()
    {
        m_isEnter = TryEnterCriticalSection(&m_CriticalSection)==0 ? false:true;
        return m_isEnter;
    }
    void Enter()
    {
        if(!m_isEnter)
        {
            EnterCriticalSection(&m_CriticalSection);
            m_isEnter=true;
        }
    }
    void Leave()
    {
        if(m_isEnter)
        {
            LeaveCriticalSection(&m_CriticalSection);
            m_isEnter=false;
        }
    }

private:
    CRITICAL_SECTION m_CriticalSection;
    bool m_isEnter;
};

/* not needed

class CLockGuard
{
public:
    CLockGuard(CSync& refSync) : m_refSync(refSync) { Lock(); }
    ~CLockGuard() { Unlock(); }

private:
    CSync& m_refSync;

    CLockGuard(const CLockGuard &refcSource);
    CLockGuard& operator=(const CLockGuard& refcSource);
    void Lock() { m_refSync.Enter(); }
    void Unlock() { m_refSync.Leave(); }
};*/

template<class T> class Wrap
{
public:
    Wrap(T* pp, const CSync& sync)
        : p(pp)
        , m_refSync(refSync)
    {}
    Call_proxy<T> operator->() { m_refSync.Enter(); return Call_proxy<T>(p); }
private:
    T* p;
    CSync& m_refSync;
};

template<class T> class Call_proxy
{
public:
    Call_proxy(T* pp, const CSync& sync)
        : p(pp)
        , m_refSync(refSync)
    {}
    ~Call_proxy() { m_refSync.Leave(); }
    T* operator->() { return p; }
private:
    T* p;
    CSync& m_refSync;
};


int main
{
    CSync sync;
    Wrap<string> safeVar(new string);
    // safeVar what now?
    return 0;
};

Okay so I was preparing a little test now to see if my attempts do something good, so first I created a setup to make the application crash, I believed...

But that does not crash!? Does that mean now I need no syncing? What does the program need to effectively crash? And if it does not crash why do I even bother. It seems I am missing some point again. Any ideas?

string gl_str, str_test;

void thread1()
{
    while(true)
    {
        gl_str = "12345";
        str_test = gl_str;
    }
};

void thread2()
{
    while(true)
    {
        gl_str = "123456789";
        str_test = gl_str;
    }
};

CreateThread( NULL, 0, (LPTHREAD_START_ROUTINE)thread1, NULL, 0, NULL );
CreateThread( NULL, 0, (LPTHREAD_START_ROUTINE)thread2, NULL, 0, NULL );

Just added more stuff and now it crashes when calling clear(). Good.

void thread1()
{
    while(true)
    {
        gl_str = "12345";
        str_test = gl_str;
        gl_str.clear();
        gl_int = 124;
    }
};

void thread2()
{
    while(true)
    {
        gl_str = "123456789";
        str_test = gl_str;
        gl_str.clear();
        if(gl_str.empty())
            gl_str = "aaaaaaaaaaaaa";
        gl_int = 244;
        if(gl_int==124)
            gl_str.clear();
    }
};
xezon
  • 91
  • 5
  • Use the synchronization primitives provided by ``. Essentially, you should always synchronize access to any container that is used by more than one thread. – Kerrek SB Dec 14 '11 at 15:30
  • I read about mutexes too and the essence of it was that it would be an overkill to use if I need it inside one process only. CriticalSection is faster. http://msdn.microsoft.com/en-us/library/ms810428.aspx – xezon Dec 14 '11 at 16:22

3 Answers3

5

The rules is simple: if the object can be modified in any thread, all accesses to it require synchronization. The type of object doesn't matter: even bool or int require external synchronization of some sort (possibly by means of a special, system dependent function, rather than with a lock). There are no exceptions, at least in C++. (If you're willing to use inline assembler, and understand the implications of fences and memory barriers, you may be able to avoid a lock.)

James Kanze
  • 150,581
  • 18
  • 184
  • 329
  • 1
    There are exceptions in the form of `std::atomic` – PlasmaHH Dec 14 '11 at 15:49
  • Well the objects are read in one thread and modified in another. I do not have my program code infront of me now, but a list may be indeed modified by 2 threads, but the list I want to sync anyway. However I don't think the inline assembler is of good use for me here. – xezon Dec 14 '11 at 16:27
  • @PlasmaHH In C++11. I keep forgetting about that, because I still have to write to compilers which don't support C++11 completely. Pre-C++11, some OS's offered similar functionality (but like everything else involving threads, it was system dependent). – James Kanze Dec 14 '11 at 17:53
  • @xezon There are two possibilities: the list can ensure that all of its accesses (to the objects it contains) are synchronized, or the list can require that client code treat it as a basic object, and supply the synchronization. Experience (a lot of experience, in many different languages) has shown that the first is almost always useless; the granularity is too fine, and the extra locking is extra cost, for nothing. In general---there are cases where it can be useful. The usual solution is no locking in list, and a decorator which handles the locking if needed. – James Kanze Dec 14 '11 at 18:00
1

I read int, bool, etc do not require syncing

This is not true:

  • A thread may store a copy of the variable in a CPU register and keep using the old value even in the original variable has been modified by another thread.
  • Simple operations like i++ are not atomic.
  • The compiler may reorder reads and writes to the variable. This may cause synchronization issues in multithreaded scenarios.
  • See Lockless Programming Considerations for more details.

You should use mutexes to protect against race conditions. See this article for a quick introduction to the boost threading library.

StackedCrooked
  • 34,653
  • 44
  • 154
  • 278
  • I read it from this page: http://www.flounder.com/no_synchronization.htm What do you think about it? I will check out your links now. – xezon Dec 14 '11 at 16:18
  • @xezon the author promotes lockless threading, which is an advanced topic and very easy to get wrong. If you want to play on the safe side I recommend you to use mutexes. – StackedCrooked Dec 14 '11 at 16:28
  • Okay. On your linked msdn article I read that atomic use with a variable is okay. I often use stuff like bVar=true and then if(bVar){...} so I think these do not need further attention. I am not sure though why you recommend the mutex. I fail to understand what makes them superior to CriticalSection when I just need thread syncing inside 1 process. http://blogs.msdn.com/b/ce_base/archive/2007/03/26/critical-section-vs-mutex.aspx – xezon Dec 14 '11 at 16:50
  • @xezon Critical sections are also safe. I'm just more used to using mutexes since I do mostly cross-platform development. – StackedCrooked Dec 14 '11 at 17:04
  • @xezon The author of that link doesn't really understand what is going on, since he recommends code that isn't guaranteed to work (but will work 99% of the time---so all of your tests will pass, but it will fail in the critical demo before the most important client). And he doesn't seem to realize that what Microsoft calls a `CriticalSection` is a mutex (and not a _section_; a section is what it protects). – James Kanze Dec 14 '11 at 18:04
  • @StackedCrooked If you're using `boost::threads` for multi-platform, you're fine---under Windows, Boost uses `CriticalSection` to implement the simpler mutexes. And under Linux, `pthread_mutex` is closer to a `CriticalSection` than to a Windows `Mutex`; it just doesn't have all of the restrictions. – James Kanze Dec 14 '11 at 18:07
  • Hmm I was thinking about a smart implementation for the locking of variables and thought a template datatype wrapping the lock inside operator functions could do the trick. But if this was applicable, I am sure someone did that before. I googled a bit and found stuff like this: http://orocos.org/stable/documentation/rtt/v2.x/api/html/classRTT_1_1base_1_1DataObjectLockFree.html Is such stuff good to use or is there something bad I missed? – xezon Dec 14 '11 at 19:35
  • @xezon see [this](http://www2.research.att.com/~bs/wrapper.pdf) or [this](http://stackoverflow.com/questions/3482352/need-some-feedback-on-how-to-make-a-class-thread-safe). – StackedCrooked Dec 14 '11 at 20:33
0

First, you do need protection even for accessing the most primitive of data types. If you have an int x somewhere, you can write

x += 42;

... but that will mean, at the lowest level: read the old value of x, calculate a new value, write the new value to the variable x. If two threads do that at about the same time, strange things will happen. You need a lock/critical section.

I'd recommend using the C++11 and related interfaces, or, if that is not available, the corresponding things from the boost::thread library. If that is not an option either, critical sections on Win32 and pthread_mutex_* for Unix.

NO, Don't Start Writing Multithreaded Programs Yet!

Let's talk about invariants first. In a (hypothetical) well-defined program, every class has an invariant. The invariant is some logical statement that is always true about an instance's state, i.e. about the values of all its member variables. If the invariant ever becomes false, the object is broken, corrupted, your program may crash, bad things have already happened. All your functions assume that the invariant is true when they are called, and they make sure that it is still true afterwards.

When a member function changes a member variable, the invariant might temporarily become false, but that is OK because the member function will make sure that everything "fits together" again before it exits.

You need a lock that protects the invariant - whenever you do something that might affect the invariant, take the lock and do not release it until you've made sure that the invariant is restored.

wolfgang
  • 4,883
  • 22
  • 27