Is this safe across threads?

Question

I have some code that looks like this:

Thread 0:

CMyOtherClass *m_myclass;

CMyClass::SomeFunc(DWORD SomeParam)
{

m_myclass = new CMyOtherClass(SomeParam);

}

Thread 1:

CMyClass::InsideSecondThread()
{

   MySecondThreadFunc(*m_myclass);

}

CMyClass::MySecondThreadFunc(MyOtherClass& myclass)  
{

// do something with myclass variable....
// What about thread safety???  Is this not now a shared variable?
// or have we passed a copy and are we safe?
// SomeParam should be the same while in this thread, however it can be changed by Thread 0


}

So my question is, if you are passing this m_myclass variable across threads, is this threadsafe or not?

Answer 1

It is not threadsafe.

Answer 2

It is not threadsafe. If thread 1 gets executed before the thread 0 creates the object then you end up in accessing NULL pointer in thread 1. ( Assuming m_myclass is initialised to NULL in the constructor)

Another point is :

CMyClass::MySecondThreadFunc(MyOtherClass& myclass) 

the myclass object is a shared object and any operations from different threads can create problem. Unless there is a shared static member, local copy should work for thread safe here.

Answer 3

It's not threadsafe because both threads are accessing the same bit of memory.

As your code stands, as long as thread 0 allocates the memory before thread 1 starts then you should have no problems because thread 0 doesn't do anything except the allocation. If you change the thread 0 code so it starts modifying the class instance then you'll start to run into issues.

[Edit] Removed my example of what I thought ought to be thread-safe. There's another post here on SO which asks whether changing a pointer is considered an atomic action which is worth a read.

Answer 4

You can create a new CMyOtherThreadSafeClass which has two members CMyOtherClass *m_myclass and a CRITICAL_SECTION cs. You should InitializeCriticalSection or better with InitializeCriticalSectionAndSpinCount function in the CMyOtherThreadSafeClass constructor and use EnterCriticalSection and LeaveCriticalSection (see http://msdn.microsoft.com/en-us/library/aa910712.aspx and http://msdn.microsoft.com/en-us/library/ms686908.aspx) everywhere if you need access CMyOtherClass *m_myclass member (read or modify, allocate or free).

You can include a method in CMyOtherThreadSafeClass to make easier to use of EnterCriticalSection and LeaveCriticalSection. If all threads of your program will use EnterCriticalSection and LeaveCriticalSection then your program will be thread safe.

Answer 5

If you call CMyClass::SomeFunc only prior to the second thread being created, then there is no race condition.

Otherwise, it's basically the same problem that prevents double-checked locking from working: The pointer could be assigned to m_myclass prior to the object actually being constructed, and the thread then uses an unconstructed (or partially constructed) object.