CODEWITHSUNDEEP
c++multithreadingthread-safetyshared-ptrrace-condition
AndrewP
AndrewP

Reputation: 45

C++ Using Raw pointer to std::shared_ptr

So I'm trying to pass a shared pointer between threads via a messaging mechanism that I'm using. Due to how the serialization/deserialization works, I cannot directly embed a shared_ptr into a message that I send. So I effectively need to send a raw pointer of a shared_ptr. See below:

Thread 1:

auto objPtr = std::make_shared<ObjectClass>();
uint64_t serializedPtr = reinterpret_cast<uint64_t>(&objPtr);

Thread 2:

std::shared_ptr<ObjectClass> objPtrT2 = *(reinterpret_cast<std::shared_ptr<ObjectClass>*>(serializedPtr));

This sometimes crashes in thread 2 when it goes to increment the reference count of the shared pointer. I can only assume some race condition is at play here but haven't been able to sort out a solution. Note that it doesn't always crash and the deserialization is seemingly always successful.

Do I need to synchronize access to this shared_ptr (the shared_ptr itself, not what the share_ptr points to)? I'm concerned that the manner in which I am transferring this shared_ptr is breaking how the reference count is being managed.

I'm still debating whether the use of a shared_ptr is appropriate here for other performance related reasons but I'd like to know what I'm doing wrong for my own benefit.

Thanks

EDIT: Just to note, thread 1 and thread 2 are in the same process/host. I am emplacing the shared_ptr into a map managed by thread1 (I tried to leave out details that I initially thought to be unimportant). What I didn't realize, however, was that the manner in which I retrieve from said map was incorrect. I was copying the contents of the map into a temp shared_ptr and then sending the address of the temp shared_ptr to thread2. So I was unwittingly sending over the address of a variable on the stack. Silly mistake, but I think the commentary in this thread has been quite instructive/helpful nonetheless. The following seems to fix my issue.

auto& objPtr = m_objMap[index];
uint64_t serializedPtr = reinterpret_cast<uint64_t>(&objPtr);

Upvotes: 2

Views: 1367

Answers (2)

Breealzibub
Breealzibub

Reputation: 8095

shared_ptr automatically increases and decreases its internally-stored reference count when you copy it (using the assignment operator=), and - importantly - when a shared_ptr is destroyed (by going out of scope). Your approach for transmitting a pointer-to-shared-pointer is fundamentally flawed in the code above, because you are transmitting the address of a temporary shared pointer, but not the ownership or lifespan. The shared_ptr - which is still owned by Thread A - might go out of scope and be destroyed before Thread B can use it.

In order to transfer ownership of a shared_ptr instance, I would suggest creating a heap-allocated/dynamic shared_ptr for transfer. This can be accomplished using new or (even better) make_unique. Using a unique_ptr (i.e.: unique_ptr<shared_ptr<ObjectClass>>), you would use the 'release' method, before passing the pointer across the thread barrier in your message.

Thread A:

auto sharedPtr = std::make_shared<ObjectClass>();

// This line creates a heap-allocated copy of a 
// shared_ptr (incrementing reference count)
// And places ownership inside a unique_ptr
auto sharedPtrDynamicCopy = std::make_unique<decltype(sharedPtr)>(sharedPtr);

// This 'releases' ownership of the heap-allocated shared_ptr,
// returning a raw pointer; it is now a potential
// memory leak!!!  It must be 'captured' in Thread B.
auto rawPtrToPass = sharedPtrDynamicCopy.release();

Thread B:

// Here, we immediately 'capture' the raw pointer back
// inside a unique_ptr, closing the loop on the potential
// memory leak
auto sharedPtrDynamicCopy = unique_ptr<shared_ptr<ObjectClass>>(rawPtrFromThreadA);

// Now we can make a copy of the shared_ptr, if we like.
// This sharedCopy will live on, even after recvdPtr goes
// out of scope.
auto sharedCopy = *sharedPtrDynamicCopy;

You could perhaps shorten this further by simply 'new'-ing a raw shared_ptr instead of capturing it inside a unique_ptr<shared_ptr<T>>, but I personally prefer this approach since it has clear "capture" and "release" semantics for the pointer-in-flight.

Upvotes: 4

David Schwartz
David Schwartz

Reputation: 182883

Your code sends the address of a temporary shared pointer from one thread to another. You create objPtr as an object on the stack and you pass the address of objPtr (not the object's address!) to the thread.

That is not what you want to do. You have two sane choices:

  1. Send the address of the object from one thread to the other and ensure that at least one shared pointer to the object exists at all times (as you claim your code already does). This is a scary fix because it doesn't ensure the object's lifetime is sufficient.

  2. Use new to dynamically allocated an additional shared pointer to the object and pass the pointer to that shared pointer to the thread. Have the thread delete the shared pointer when it's done with the object. This is probably the simplest fix that ensures the object's lifetime is sufficient.

A general pattern for passing objects to threads is to create a class or structure that carries all the information you want to pass to the thread. Before you create the thread, use new to create a new instance of the structure and fill it in as needed. Then pass the address of that structure to the thread. When the thread is done with the structure, have the thread use delete to free the structure. You can use this to pass any collection of objects or data to a thread. In this case, you want to pass an actual shared_ptr to the thread -- not the address of one.

Upvotes: 2

Related Questions