John
John

Reputation: 6648

Is the following C++ code thread safe?

Is the following

int BlkArray::GetNthBlockA(unsigned int n, const Block *&pfb, int &maxIndex) const {
    if (n + 1 >= (unsigned int)formattingPivots.GetCount()) return -1;
    pfb = formattingPivots.GetNthBlckB(n);
    maxIndex = formattingPivots.GetNthInt(n + 1) - 1;
    return formattingPivots.GetNthInt(n);
}

thread safe considering:

  1. formattingPivots.GetNthBlckB(n), formattingPivots.GetNthInt(n + 1), formattingPivots.GetNthInt(n) and formattingPivots.GetCount() are all const methods.
  2. I call GetNthBlock() from 2 threads, when thread1 calls and returns an usual Block I notice a side effect in thread2.
  3. const Block *&pfb is passed as follows from each thread's worker method:

    int maxIndex;
    const Block *pfb = null;
    pStoredBlcks->GetNthBlockA(blockBreakIndex, pfb, maxIndex);
    

I'm concerned const might be causing an unintended effect in persisting between both workers' bodies. I'm 98% the bugs I get are from the code above but, being peculiar to multithreading I can't get much more sure.

I'm getting near my question limit for 24 hrs, on one more thing, if it might help. Is static_cast<> thread safe? (Silly? yeah but I wrote C for years) I ask because of:

const Block *GetNthblckB(int n) const {
    return static_cast<const Block*>(Blocks.GetAt(n));//Returns `Object`* without cast.
}

3am___ Thanks for the encouragement guys. I just surrounded that call with a CritSecMonitor and I still have the side effect. Short of reading the valgrind manual I better catch some zz's.

Upvotes: 1

Views: 423

Answers (3)

John
John

Reputation: 6648

In answer to my question, I thought someone else had already said this:

Don't assume any library function is thread safe unless it says it is.

My 98% guess was wrong and the thread unsafe method lay elsewhere in a library instance method using completely seperate objects but being called from two threads. There must have been a static variable in there somewhere as the call stacks where it would crash (very rarely) looked to be deep inside library code.

Upvotes: 0

kfmfe04
kfmfe04

Reputation: 15327

Is Blocks.GetAt() an immutable method (doesn't change any internal state)? It may not be, if it is using a cache to read from a database or from a file, for instance.

Also, the answer to your question would also depend on when the data was initialized.

Is it before any threads are spawned?

Also, I recommend that you using valgrind's drd and helgrind regularly on your project to help you find current bugs as well as preventing future threading bugs from getting into your project.

Last Recommendation

One last suggestion, when in doubt about thread-safety, put in your own mutex. If you can show that it runs fine with the mutex then you can isolate the bug/false assumptions/critical section(s).

Upvotes: 0

Dietrich Epp
Dietrich Epp

Reputation: 213258

The #1 fact of thread safety: If two functions f() and g() are both thread safe, then the following function is not necessarily thread safe:

// h might not be thread-safe!
void h()
{
    f(); // f is thread-safe
    g(); // g is thread-safe
}

So you will have to prove thread-safety based on the contents of the functions GetNthBlckB, GetNthInt, etc. I don't know what these methods do, so I don't know if they are thread-safe or not (const has nothing to do with it). It looks like it is not thread-safe to me.

Upvotes: 3

Related Questions