OpenMP: Causes for heap corruption, anyone?

Question

EDIT: I can run the same program twice, simultaneously without any problem - how can I duplicate this with OpenMP or with some other method?

This is the basic framework of the problem.

//Defined elsewhere
class SomeClass
{
public:
  void Function()
  {
    // Allocate some memory
    float *Data;
    Data = new float[1024];

    // Declare a struct which will be used by functions defined in the DLL
    SomeStruct Obj;
    Obj = MemAllocFunctionInDLL(Obj);

    // Call it
    FunctionDefinedInDLL(Data,Obj);

    // Clean up
    MemDeallocFunctionInDLL(Obj);
    delete [] Data;        
  }
}

void Bar()
{
   #pragma omp parallel for
   for(int j = 0;j<10;++j)
   {
     SomeClass X;
     X.Function();
   }
}

I've verified that when some memory is attempted to be deallocated through MemDeallocFunctionInDLL(), the _CrtIsValidHeapPointer() assertion fails.

Is this because both threads are writing to the same memory?

So to fix this, I thought I'd make SomeClass private (this is totally alien to me, so any help is appreciated).

void Bar()
{
   SomeClass X;
   #pragma omp parallel for default(shared) private(X)
   for(int j = 0;j<10;++j)
   {         
     X.Function();
   }
}

And now it fails when it tries to allocate memory in the beginning for Data.

Note: I can make changes to the DLL if required

Note: It runs perfectly without #pragma omp parallel for

EDIT: Now Bar looks like this:

void Bar()
{
   int j
   #pragma omp parallel for default(none) private(j)
   for(j = 0;j<10;++j)
   {
     SomeClass X;         
     X.Function();
   }
}

Still no luck.

Answer 1

Instead of

delete Data

you must write

delete [] Data;

Wherever you do new [], make sure to use delete [].

It looks like your problem is not specific to openmp. Did you try to run your application without including #pragma parallel?

Answer 2

default(shared) means all variables are shared between threads, which is not what you want. Change that to default(none).

Private(X) will make a copy of X for each thread, however, none of them will be initialised so any construction will not necessarily be performed.

I think you'd be better with your initial approach, put a breakpoint in the Dealloc call, and see what the memory pointer is and what it contains. You can see the guard bytes to tell if the memory has been overwritten at the end of a single call, or after a thread.

Incidentally, I am assuming this works if you run it once, without the omp loop?

Answer 3

Check out MemAllocFunctionInDLL, FunctionDefinedInDLL, MemDeallocFunctionInDLL are thread-safe, or re-entrant. In other words, do these functions static variables or shared variables? In such case, you need to make it sure these variables are not corrupted by other threads.

The fact without omp-for is fine could mean you didn't correctly write some functions to be thread-safe.

I'd like to see what kind of memory allocation/free functions has been used in Mem(Alloc|Dealloc)FunctionInDLL.

Added: I'm pretty sure your functions in DLL is not thread-safe. You can run this program concurrently without problem. Yes, it should be okay unless your program uses system-wide shared resources (such as global memory or shared memory among processes), which is very rare. In this case, no shared variables in threads, so your program works fine.

But, invoking these functions in mutithreads (that means in a single process) crashes your program. It means there are some shared variables among threads, and it could have been corrupted.

It's not a problem of OpenMP, but just a multithreading bug. It could be simple to solve this problem. Please take a look the DLL functions whether they are safe to be called in concurrent by many threads.

How to privatize static variables

Say that we have such global variables:

static int  g_data;
static int* g_vector = new int[100];

Privatization is nothing but a creating private copy for each thread.

int  g_data[num_threads];
int* g_vector[num_threads];
for (int i = 0; i < num_threads; ++i)
  g_vector[i] = new int[100];

And, then any references on such variables are

// Thread: tid
g_data[tid] = ...
.. = g_vector[tid][..]

Yes, it's pretty simple. However, this sort of code may have a false sharing problem. But, false sharing is a matter of performance, not correctness.

First, just try to privatize any static and global variables. Then, check it correctness. Next, see the speedup you would get. If the speedup is scalable (say 3.7x faster on quad core), then it's okay. But, in case of low speedup (such as 2x speedup on quad core), then you probably look at the false sharing problem. To solve false sharing problem, all you need to do is just putting some padding in data structures.