1

I have a shared_ptr stored in a central place which can be accessed by multiple threads through a method getPointer(). I want to make sure that only one thread uses the pointer at one time. Thus, whenever a thread wants to get the pointer I test if the central copy is the only one via std::shared_ptr::unique() method. If it returns yes, I return the copy assuming that unique()==false as long as that thread works on the copy. Other threads trying to access the pointer at the same time receive a nullptr and have to try again in the future.

Now my question:

Is it theoretically possible that two different threads calling getPointer() can get mutual access to the pointer despite the mutex guard and the testing via unique() ? 

std::shared_ptr<int> myPointer; // my pointer is initialized somewhere else but before the first call to getPointer()
std::mutex myMutex;

std::shared_ptr<int> getPointer()
{
    std::lock_guard<std::mutex> guard(myMutex);
    std::shared_ptr<int> returnValue;

    if ( myPointer.unique() )
        returnValue = myPointer;
    else
        returnValue = nullptr;

    return returnValue;
}

Regards

xaxxon
  • 19,189
  • 5
  • 50
  • 80
Desperado17
  • 835
  • 6
  • 12
  • 1
    The linked question does not cover std::shared_ptr::unique and very little about mutexes. It also doesn't explicitly discuss the scenario I listed here, especially the scope problematic. Please remove the duplicate mark. Regards. – Desperado17 Jul 29 '18 at 12:39
  • I cannot provide the original code because it is confident. The code presented is basically the original code stripped of everything that distracts from the problem or is private. – Desperado17 Jul 29 '18 at 13:02
  • Then modify your question to ask about the code provided instead of referring to some nebulous unprovided code. – xaxxon Jul 29 '18 at 13:02
  • 1
    `std::shared_ptr::unique` is *deprecated* in C++17 and *removed* in C++20. I'd suggest avoiding its use. – Jesper Juhl Jul 29 '18 at 13:03
  • @xaxxon yes, but only because the temporary return value is immediately destroyed after the first call so the reference count of myMutex returns to 1 I presume? – Desperado17 Jul 29 '18 at 13:04
  • @JesperJuhl it just means use_count == 1, so you can imagine the same question just checking that.. – xaxxon Jul 29 '18 at 13:04
  • @Desperado17 yes, but your question is whether it can be given out twice.. so the answer is "yes". If you have a different question, then you should be more specific as to what it is. – xaxxon Jul 29 '18 at 13:05
  • @Jesper Juhl As far as I know it is deprecated mostly because it doesn't think of weak_ptrs. The question is if std::shared_ptr::unique() is reliable if combined with a mutex guard in the way I describe above. – Desperado17 Jul 29 '18 at 13:05
  • @Desperado17 my *point* is that once we get to C++20 `std::shared_ptr::unique` will no longer exist. So long term it makes the most sense to simply not use it. – Jesper Juhl Jul 29 '18 at 13:08
  • answer to current question: "no" – xaxxon Jul 29 '18 at 13:11
  • I reformulated my question. It basically comes down to this: Can a thread gain access to the central shared_ptr after a different thread has received a copy to it which has not yet been destroyed? – Desperado17 Jul 29 '18 at 13:11
  • @JesperJuhl "_std::shared_ptr::unique is deprecated_" for no good reason – curiousguy Jul 29 '18 at 16:59

2 Answers2

2

Only one "active" copy can exist at a time.

It is protected by the mutex until after a second shared_ptr is created at which point a subsequent call (once it gets the mutex after the first call has exited) will fail the unique test until the initial caller's returned shared_ptr is destroyed.

As noted in the comments, unique is going away in c++20, but you can test use_count == 1 instead, as that is what unique does.

xaxxon
  • 19,189
  • 5
  • 50
  • 80
  • Is it guaranteed that the use count of myPointer is incremented before the guard gets destroyed? – Desperado17 Jul 29 '18 at 13:17
  • Yes. It is incremented as soon as you assign it to returnValue, but even if you didn't do it explicitly, the return value is created before the mutex is released. You could demonstrate this to yourself by creating a type that prints logging info in its constructor/destructor to show the relative order of construction/destruction – xaxxon Jul 29 '18 at 13:18
  • What if I remove the returnValue and return immediately? May return value optimization throw a wrench in my gears? – Desperado17 Jul 29 '18 at 13:23
  • @Desperado17 no because the returned shared_ptr object still has to be created, which increments use_count before the mutex_guard is destroyed releasing the mutex – xaxxon Jul 29 '18 at 13:28
-1

Your solution seems overly complicated. It exploits the internal workings of the shared pointer to deduce a flag value. Why not just make the flag explicit?

std::shared_ptr<int> myPointer;
std::mutex myMutex;
bool myPointerIsInUse = false;

bool GetPermissionToUseMyPointer() {
    std::lock_guard<std::mutex guard(myMutex);
    auto r = (! myPointerIsInUse);
    myPointerIsInUse ||= myPointerIsInUse;
    return r;
}

bool RelinquishPermissionToUseMyPointer() {
    std::lock_guard<std::mutex guard(myMutex);
    myPointerIsInUse = false;
}

P.S., If you wrap that in a class with a few extra bells and whistles, it'll start to look a lot like a semaphore.

xaxxon
  • 19,189
  • 5
  • 50
  • 80
Solomon Slow
  • 25,130
  • 5
  • 37
  • 57
  • You say that the proposed solution is overly complicated, yet you add additional state information and then require explicit cleanup calls when the handed-out shared_ptr is destroyed. Additionally, ||= isn't a c++ operator (that I know of) and even if it were, that wouldn't be correct logic as it would always remain at its current value. just assigning it true would be correct. – xaxxon Jul 30 '18 at 00:27
  • To be very specific, my primary issue with this answer is that it requires an explicit release of the lock instead of relying on the "guard"-ness of the shared_ptr to maintain the use count. – xaxxon Jul 30 '18 at 00:45
  • @xaxxon, That's a fair assessment. If I put a bit more effort into this, I could turn it into an RAII class, and address one of your concerns. But as for the other, I really don't like seeing the `shared_ptr` being used in this way: It's being made to serve two different purposes: (1) it's usual role, managing the lifetime of the object to which it refers, and (2) as a sort of weird adjunct to a mutex. In fact, now that I think about it, Why does the OP need `getPointer()` at all? Why doesn't the OP simply call `myMutex::try_lock()` instead? – Solomon Slow Jul 30 '18 at 12:27
  • The whole situation has a bit of a code smell to it, so exactly the best way to approach it is a bit nebulous.. it's likely there's a fundamental redesign that addresses the issue better than any answer to this specific question. I agree that a simple mutex seems like it would be sufficient. – xaxxon Jul 30 '18 at 13:23