Is pointer dereferencing thread safe?

Question

I have implemented a heap memory management, and now I'm unsure at two points (explained after code):

So the heap memory management is basically implemented like this:

struct memory {
     // Mutex for thread safe operations
     pthread_mutex_t _mutex;
     // Memory size
     size_t _size;
};

int allocMemory(void **memoryPointer, const size memorySize)
{
     void *memory;
     struct memory *internal;

     /* Checking arguments... */
     memory = malloc(memorySize + sizeof(struct memory));

     if (memory == NULL) return ENOMEM;

     internal = (struct memory *)memory;

     pthread_mutex_init(&internal->_mutex);
     internal->_size = memorySize;

     // Set pointer with offset
     *memoryPointer = memory + sizeof(struct memory);

     return 0;
}

int reallocMemory(void **memoryPointer, const size newMemorySize)
{
     /* Checking arguments... */
     {
         void *memory;
         void *memoryWithoutOffset;
         struct memory *internal;

         // Get pointer         v--- is this thread safe?
         memory               = *memoryPointer;
         // Subtract offset
         memoryWithoutOffset  = memory - sizeof(struct memory);
         // Get internal data (for _mutex)
         internal             = (struct memory *)memoryWithoutOffset;

         pthread_mutex_lock(&internal->_mutex);
         {
             void *newMemory;
             newMemory = realloc(memoryWithoutOffset, sizeof(struct memory) + internal->_size + newMemorySize);
             if (newMemory != NULL) {
                  // Refresh pointer to "internal" because its location may have been changed
                  internal = (struct memory *)newMemory;
                  internal->_size += newMemorySize;

                  // Add offset
                  *memoryPointer = newMemory + sizeof(struct memory);
             }
         }
         pthread_mutex_unlock(&internal->_mutex);
     }

     return 0;
}

In my main file I have something like this:

void *myMemory;

void *threadFunction(void *arg)
{
     //             v--- reference to `myMemory`
     reallocMemory(&myMemory, 100);

     return NULL;
}

int main(int argc, const char **argv)
{
     pthread_t thread;

     allocMemory(&myMemory, 10);

     /* ... */
     for (int i = 0; i < 5; ++i) {
          pthread_create(&thread, NULL, threadFunction, NULL);
     }
     /* ... joining threads etc. */
     return EXIT_SUCCESS;
}

My question is, before I can lock the mutex I have to "extract" it from the given pointer reference. Does this pointer dereferencing destroy thread safety? memory = *memoryPointer

And is this "hiding" a struct in a pointer a safe / good practice?

BTW: I have tested it with valgrind and it shows no invalid reads or writes.

Thanks for your help ahead...

Answer 1

No, it is not thread safe. Consider two threads with a pointer to myMemory, if one thread reallocates myMemory, how does the other thread know that it's myMemory pointer is now invalid (realloc may move the memory block).

You may want to consider having a fixed-location structure like:

struct memory {
     pthread_mutex_t _mutex;    // Mutex for thread safe operations
     size_t _size;              // Memory size
     void *memory_ptr;          // pointer to memory
};

then allocate, reallocate and access your memory with functions like (pseudo-code):

alloc_memory()
   malloc struct memory
   memory_ptr= malloc memory reqd
   fill in memory struct

reallocMemory( struct memory *, new_size )
   obtain mutex
   reallocate memory pointed to by memory_ptr
   release mutex

void * start_access_memory( struct memory *)
   obtain mutex
   return memory_ptr

end_access_memory( struct memory *)
   release mutex

This way each thread may have a persistent pointer to a memory structure and its mutex.

Answer 2

No, it is not thread-safe.

Two threads can access the memory location myMemory at the same time, and at least one operation is a write access. That's called a data race, and C11 states, that programs with data races have undefined behaviour–exactly like in C++11.

The two relevant lines are:

  memory               = *memoryPointer;
  *memoryPointer = newMemory + sizeof(struct memory);

You have to use an atomic variable for the variable myMemory. Unfortunately, AFAIK GCC-4.7 doesn't support it for C11.