'C - Thread Sanitizer Reporting Data Race With Mutex Locks
I have a blocking synchronized queue that is being used in a thread pool (the main thread is spawning the thread pools) and spawning the queue. Everything in my program is working as expected, there are no memory leaks or errors reported by asan, but thread sanitizer is reporting a data race warning on the following line free(tempNode); Nothing else in any other file can access tempNode. There are "setter" and "getter" functions, but I don't think those will impact any data races that may be occurring...
I have implemented a queue like so:
struct Node { // These are private and not meant to be accessed by client code.
void *data;
size_t dataSize;
struct Node *next;
};
struct Queue {
bool queueDone, mainThreadTermination;
struct Node *head, *tail;
size_t sleepingThreads, queueSize, numOfThreads;;
pthread_mutex_t lock;
pthread_cond_t dequeueReady;
};
struct Queue *initQueue() {
struct Queue *queue = (struct Queue *) malloc(sizeof(struct Queue));
if (queue == NULL) {
perror("Init Queue Memory Allocation Failed:");
exit(2);
}
queue->head = queue->tail = NULL;
queue->queueDone = false;
queue->sleepingThreads = 0;
queue->queueSize = 0;
queue->mainThreadTermination = true; // By default, at least
queue->numOfThreads = -1; // This is -1 unless main thread termination is false.
pthread_mutex_init(&queue->lock, NULL);
pthread_cond_init(&queue->dequeueReady, NULL);
return queue;
}
void *enqueue(struct Queue *queue, void *item, size_t itemSize) {
pthread_mutex_lock(&queue->lock); // FIXME: CHECK RETURN VALUES OF ALL THIS! IF IT FAILS,
struct Node *tempNode = (struct Node *) malloc(sizeof(struct Node));
tempNode->data = malloc(itemSize); // Don't forget your null terminator
tempNode->data = memcpy(tempNode->data, item, itemSize);
tempNode->dataSize = itemSize;
tempNode->next = NULL;
if (isEmpty(queue)) {
queue->head = queue->tail = tempNode; // If empty, head and tail point to the same thing.
queue->queueSize++;
pthread_cond_signal(&queue->dequeueReady);
pthread_mutex_unlock(&queue->lock);
return tempNode->data;
}
queue->tail->next = tempNode; // tail points to old tempNode. This says old tempNode now points to new tempNode
queue->tail = tempNode; // Current tail also needs to point to new tempNode
queue->queueSize++;
pthread_cond_signal(&queue->dequeueReady);
pthread_mutex_unlock(&queue->lock); // FIXME: Check return value
return item;
}
void *dequeue(struct Queue *queue) {
pthread_mutex_lock(&queue->lock); // FIXME: CHECK RETURN VALUES OF ALL THIS! IF IT FAILS, EXIT
while (isEmpty(queue)) {
if (queue->queueDone) {
pthread_mutex_unlock(&queue->lock);
return NULL;
}
queue->sleepingThreads++;
while (queue->mainThreadTermination == false && queue->sleepingThreads == queue->numOfThreads && isEmpty(queue)) {
pthread_mutex_unlock(&queue->lock);
jobComplete(queue);
pthread_mutex_lock(&queue->lock);
}
pthread_cond_wait(&queue->dequeueReady, &queue->lock);
queue->sleepingThreads--;
}
struct Node *tempNode = queue->head; // Get node to dequeue
void *data = malloc(tempNode->dataSize); // Need to store the data, so we can use it later. Client is responsible for freeing this because it points to dynamic memory. This could be improved so the client doesn't have to do anything, but it's fine for now.
data = memcpy(data, tempNode->data, tempNode->dataSize);
queue->head = queue->head->next;
if (queue->head == NULL) {
queue->tail = NULL;
}
free(tempNode->data);
free(tempNode); // For some reason, a data race is occurring here.
queue->queueSize--;
pthread_mutex_unlock(&queue->lock); // FIXME: CHECK RETURN VALUES OF ALL THIS! IF IT FAILS,
return data;
}
void jobComplete(struct Queue *queue) {
pthread_mutex_lock(&queue->lock);
queue->queueDone = true;
pthread_cond_broadcast(&queue->dequeueReady);
pthread_mutex_unlock(&queue->lock);
}
void mainThreadTermination(struct Queue *queue, int numThreads) { // Yes, this is a setter function like Java. I want to avoid using global variables since that is bad practice.
pthread_mutex_lock(&queue->lock);
queue->mainThreadTermination = false;
queue->numOfThreads = numThreads;
pthread_mutex_unlock(&queue->lock);
}
bool isEmpty(struct Queue *queue) {
return queue->queueSize == 0;
}
I am not sure how to fix this. I've made sure that the queue is always locked when sensitive memory is being accessed and I've made sure that mutex is unlocked when each function is done. I am not sure what else may be causing this data race. Is it possible that it is a false positive?
Edit: Here is the output from thread sanitizer...
WARNING: ThreadSanitizer: data race (pid=2530159)
Write of size 8 at 0x7b0800000020 by thread T10 (mutexes: write M0):
#0 free ../../../../src/libsanitizer/tsan/tsan_interceptors_posix.cpp:707 (libtsan.so.0+0x35f25)
#1 dequeue src/unbounded_queue.c:105 (ww+0x2e1b)
#2 startDirectoryThreads program.c:357 (ww+0x3d8e)
Previous read of size 8 at 0x7b0800000020 by main thread:
#0 enqueue /src/unbounded_queue.c:58 (ww+0x2c0b)
#1 recursiveThreading program.c:427 (ww+0x4077)
#2 main program.c:498 (ww+0x2893)
Mutex M0 (0x7b2400000030) created at:
#0 pthread_mutex_init ../../../../src/libsanitizer/tsan/tsan_interceptors_posix.cpp:1220 (libtsan.so.0+0x4a616)
#1 initQueue /src/unbounded_queue.c:37 (ww+0x2a5c)
#2 recursiveThreading /src/program.c:373 (ww+0x3eb2)
#3 main program.c:498 (ww+0x2893)
Thread T10 (tid=2530176, running) created by main thread at:
#0 pthread_create ../../../../src/libsanitizer/tsan/tsan_interceptors_posix.cpp:962 (libtsan.so.0+0x5ea79)
#1 recursiveThreading program.c:406 (ww+0x4341)
#2 main program.c:498 (ww+0x2893)
SUMMARY: ThreadSanitizer: data race /src/unbounded_queue.c:105 in dequeue
==================
==================
WARNING: ThreadSanitizer: data race (pid=2530159)
Write of size 8 at 0x7b080000a140 by thread T1 (mutexes: write M1):
#0 free ../../../../src/libsanitizer/tsan/tsan_interceptors_posix.cpp:707 (libtsan.so.0+0x35f25)
#1 dequeue /src/unbounded_queue.c:105 (ww+0x2e1b)
#2 startFileThreads program.c:349 (ww+0x38e6)
Previous read of size 8 at 0x7b080000a140 by thread T10:
#0 enqueue /src/unbounded_queue.c:58 (ww+0x2c0b)
#1 wrapDirectory program.c:301 (ww+0x3b66)
#2 startDirectoryThreads program.c:360 (ww+0x3d9f)
Mutex M1 (0x7b24000000c0) created at:
#0 pthread_mutex_init ../../../../src/libsanitizer/tsan/tsan_interceptors_posix.cpp:1220 (libtsan.so.0+0x4a616)
#1 initQueue /src/unbounded_queue.c:37 (ww+0x2a5c)
#2 recursiveThreading program.c:390 (ww+0x4246)
#3 main program.c:498 (ww+0x2893)
Thread T1 (tid=2530165, running) created by main thread at:
#0 pthread_create ../../../../src/libsanitizer/tsan/tsan_interceptors_posix.cpp:962 (libtsan.so.0+0x5ea79)
#1 recursiveThreading program.c:396 (ww+0x42ad)
#2 main program.c:498 (ww+0x2893)
Thread T10 (tid=2530176, running) created by main thread at:
#0 pthread_create ../../../../src/libsanitizer/tsan/tsan_interceptors_posix.cpp:962 (libtsan.so.0+0x5ea79)
#1 recursiveThreading program.c:406 (ww+0x4341)
#2 main program.c:498 (ww+0x2893)
SUMMARY: ThreadSanitizer: data race /src/unbounded_queue.c:105 in dequeue
Sources
This article follows the attribution requirements of Stack Overflow and is licensed under CC BY-SA 3.0.
Source: Stack Overflow
| Solution | Source |
|---|
