# Thread-safe Shared Pointer implementation in C

I've been working on a C project where I needed a small, thread-safe shared pointer, and so I wrote this implementation.

It's primary use in the project is as a way of allowing multiple threads to have ownership of a resource at once. This means it's very important that it is thread safe (and also doesn't leak, obviously)

SharedPtr.h
#ifndef SHARED_PTR_H
#define SHARED_PTR_H

typedef struct shared_ptr* SharedPtr;

typedef void(*RawPtrDestructor_t)(void*);

SharedPtr SharedPtr_create(void* rawPtr, RawPtrDestructor_t destructorFunc);

SharedPtr SharedPtr_copy(restrict SharedPtr rhs);

void* SharedPtr_get(restrict SharedPtr sharedPtr);

void SharedPtr_lockMutex(restrict SharedPtr sharedPtr);

void SharedPtr_unlockMutex(restrict SharedPtr sharedPtr);

void SharedPtr_free(SharedPtr sharedPtr);

#endif //SHARED_PTR_H

SharedPtr.c
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#include "SharedPtr.h"

struct shared_ptr
{
struct shared_ptr* previous;

struct shared_ptr* next;

void* rawPtr;

RawPtrDestructor_t destructor;
};

static void genericDestructor(void* ptr)
{
free(ptr);
ptr = NULL;
}

static SharedPtr mallocSharedPtr(void)
{
//Create the shared pointer
SharedPtr sharedPtr = malloc(sizeof(struct shared_ptr));
//Check to make sure the malloc didn't fail
if (!sharedPtr)
{
fprintf(stderr, "Alloc error at %s:%i. Aborting...\n", __FILE__, __LINE__);
abort();
}
return sharedPtr;
}

SharedPtr SharedPtr_create(void* rawPtr, RawPtrDestructor_t destructorFunc)
{
//Create the shared pointer
SharedPtr sharedPtr = mallocSharedPtr();

//Create the mutex
//Check that it didn't fail
if (result != 0)
{
fprintf(stderr, "Pthread mutex init error '%s' at %s:%i. Aborting...\n", strerror(result), __FILE__, __LINE__);
abort();
}

//Set the raw pointer
sharedPtr->rawPtr = rawPtr;
//If the destructor isn't specified, use the generic destructor
if (!destructorFunc) {sharedPtr->destructor = &genericDestructor;}
//Otherwise just use the one that's given
else {sharedPtr->destructor = destructorFunc;}

//Make sure previous and next are NULL
sharedPtr->previous = NULL;
sharedPtr->next = NULL;

//Return the shared pointer
return sharedPtr;
}

SharedPtr SharedPtr_copy(restrict SharedPtr rhs)
{
//Create the shared pointer
SharedPtr newSharedPtr = mallocSharedPtr();

//Set the mutex
newSharedPtr->sharedMutex = rhs->sharedMutex;

//Lock the mutex

//Set the raw pointer and destructor
newSharedPtr->rawPtr = rhs->rawPtr;
newSharedPtr->destructor = rhs->destructor;

//Set the 'previous' pointer to this shared pointer
newSharedPtr->previous = rhs;

//Set the 'next' pointer to NULL
newSharedPtr->next = NULL;

//Set the 'next' pointer of this shared pointer to the new shared pointer
rhs->next = newSharedPtr;

//Unlock the mutex

//Return the new shared pointer
return newSharedPtr;
}

void* SharedPtr_get(restrict SharedPtr sharedPtr)
{
//Create a variable to store the pointer
void* ret = NULL;
//Lock the mutex
pthread_mutex_lock(sharedPtr->sharedMutex);  //Is the mutex locking/unlocking here even necessary?
//Set the pointer
ret = sharedPtr->rawPtr;
//Unlock the mutex
//Return the pointer
return ret;
#else
//Return the raw pointer
return sharedPtr->rawPtr;
}

void SharedPtr_lockMutex(restrict SharedPtr sharedPtr)
{
//Lock the shared mutex
}

void SharedPtr_unlockMutex(restrict SharedPtr sharedPtr)
{
//Unlock the shared mutex
}

void SharedPtr_free(restrict SharedPtr sharedPtr)
{
//Lock the mutex

//If this is the last shared pointer
if (!sharedPtr->previous && !sharedPtr->next)
{
//Call the destructor on the raw pointer
sharedPtr->destructor(sharedPtr->rawPtr);

//Unlock the mutex

//Destroy the mutex
if (result != 0)
{
fprintf(stderr, "Pthread mutex destroy error '%s' at %s:%i. Aborting...\n", strerror(result), __FILE__, __LINE__);
abort();
}
//Free the mutex
free(sharedPtr->sharedMutex);
}
//If there are more shared pointers
else
{
//Remove the pointer from the linked list
if (sharedPtr->next) {sharedPtr->next->previous = sharedPtr->previous;}
if (sharedPtr->previous) {sharedPtr->previous->next = sharedPtr->next;}

//Unlock the mutex
}

//Free the shared pointer's memory
free(sharedPtr);
sharedPtr = NULL;
}


I decided to use void pointers rather than a macro system (where calling a macro defines a version of the shared pointer with a specific pointer type) as I feel it's simpler and easier to write.

And a little example to showcase what it does:

#include "SharedPtr.h"
#include <stdio.h>
#include <stdlib.h>

void destructor(void* ptr)
{
(void)ptr;
puts("Destructor called!");
}

int main(void)
{
void* myRawPtr = NULL;

SharedPtr ptr1 = SharedPtr_create(myRawPtr, &destructor);
SharedPtr ptr2 = SharedPtr_copy(ptr1);

SharedPtr_free(ptr1);
SharedPtr_free(ptr2);
}


As expected, the program only outputs "Destructor called!" once.

I've written code in C++ for a few years but I've only just started writing in C, so any input would be helpful!

This is an interesting idea! I wouldn't have thought to do this in straight C.

# Don't Repeat Yourself

I found this a little confusing. You're keeping a linked list of struct shared_ptrs. Every time you copy a shared pointer, you add another node in the list. In doing so, you copy the pointer to the mutex, the raw pointer, and the destructor pointer into every new node in the list. It might be better to make the list be a separate struct from the nodes so you don't have to carry around as much stuff in each node. For example, it could look more like this:

typedef struct shared_ptr_list {
SharedPtr    tail;
RawPtrDestructor_t    destructor;
void* rawPtr;
} shared_ptr_list;


and then nodes in the list would look like this:

typedef struct shared_ptr {
SharedPtr    next;
SharedPtr    prev;
shared_ptr_list* list;
} shared_ptr;


Then in the various functions, you'll need to dereference the shared_ptr->list member to get at the raw pointer, the mutex, or the destructor. But there will only be 1 copy of each. This would be opaque to a user of the shared pointers - they'd never see the list structure. When you destroy the last shared pointer, you also destroy the list.

# Bugs

Also, you have a bug in your list creation. In SharedPtr_copy(), you always set the previous member to rhs and next to NULL, but what happens if a caller does this:

SharedPtr ptr1;
...
ptr1 = <set to something>
...
SharedPtr ptr2 = SharedPtr_copy(ptr1);
...


At this point, things are fine, as ptr1->next is ptr2, and ptr2->prev is ptr1'. But if we then do this:

SharedPtr ptr3 = SharedPtr_copy(ptr1);


we have a problem. ptr3->previous is ptr1. ptr2->previous is also ptr1. ptr1->next is ptr3 and there's no way to reach ptr2 by traversing the list.

This is why I recommended having a head and tail in my implementation. You can simply always add to the list by doing:

tail->next = newNode;
newNode->previous = tail;
newNode->next = NULL;
tail = newNode;


# Handling Failure

I notice in several places if something fails you simply abort(). That seems like a really bad idea. You should definitely think about ways that you can handle errors like running out of memory without simply killing the process. It's not clear to me what could go wrong in destroying a mutex, but it reminds me of when closing a read-only file fails. There's not a lot you can do about it, but it doesn't warrant exiting the application!

# Is This Mutex Lock/Unlock Necessary?

You have a comment in SharedPtr_get() asking whether you need to lock when getting the raw pointer. The answer is yes, you do. Think of this case: You have 1 copy of a shared pointer left and there are 2 threads operating on the same copy. (Maybe this violates your design, but it's not enforced in the code.) It is the last copy left in the linked list. You call SharedPtr_free() on thread 1, and at the exact same time call SharedPtr_get()` on thread 2. If you don't lock on thread 2, you may access the raw pointer just as it's being freed on the other thread.

Obviously, you shouldn't be using the same copy of a shared pointer on 2 different threads, but I don't know of a way (off the top of my head) to enforce that. You can easily allocate one on one thread and then pass it over to another thread.