3

I am reading "C++ Concurrency in action" book and trying to comprehend exception safety in thread-safe data structures (for example stack). Author states that to avoid race condition, pop should do both operations - pop and return item from the stack, however:

If the pop() function was defined to return the value popped, as well as remove it from the stack, you have a potential problem: the value being popped is returned to the caller only after the stack has been modified, but the process of copying the data to return to the caller might throw an exception.

Here is proposed solution:

std::shared_ptr<T> pop()
{
    std::lock_guard<std::mutex> lock(m);
    if(data.empty()) throw runtime_error("empty");
    std::shared_ptr<T> const res(std::make_shared<T>(data.top()));
    data.pop();
    return res;
}

In this case we got invocation of copy constructor on make_shared line. If copy constructor throws an exception, stack is not yet modified and we are good.

However I don't see how it differs much from this snippet:

T pop2() {
    std::lock_guard<std::mutex> lock(m);
    if(data.empty()) throw runtime_error("empty");
    auto res = data.top();
    data.pop();
    return res;
}

Here we have copy constructor invocation at data.top() line and move constructor on return. Again, if copy constructor throws an exception we are good since stack is not yet modified. Move contstructor is not supposed to throw exceptions.

Am I missing something? What is the benefit of returning shared_ptr comparison to returning (movable) T ?

lstipakov
  • 3,138
  • 5
  • 31
  • 46
  • 1
    Move constructors are allowed to throw. – Simple Apr 14 '16 at 07:22
  • My PDF copy of the book has `throw empty_stack()` instead of `throw runtime_error("empty")`. The `empty_stack` class in the book inherits directly from `std::exception`. If the `runtime_error` in your question is a `std::runtime_error`, then note that `throw runtime_error("empty");` [might throw other exceptions as well](https://stackoverflow.com/questions/36106747/how-to-construct-a-stdexcept-exception-without-throwing). – jotik Apr 14 '16 at 08:18
  • Yep, mine too has `empty_stack`. I just replaced it with `runtime_exception` in the example. – lstipakov Apr 14 '16 at 08:21
  • Note that `res` being `const` prevents move operations and will force a copy (unless copy elision happens which is not guaranteed). – Chnossos Dec 07 '20 at 14:58

2 Answers2

1

If T is not movable, your code might end up doing multiple copy-constructions. Using a shared_ptr to allocate the copy on the heap might be more efficient in this case.

And even if T is movable, your code might still do multiple (potentially costly) move-constructions. Move-constructing and/or copy-constructing a shared_ptr is known to be relatively cheap compared to move- or copy-constructing any possible type T.

Of course your mileage may vary depending on the exact types, your compiler and on your operating environment and hardware.

jotik
  • 17,044
  • 13
  • 58
  • 123
  • So, assuming that `T` is movable and move `constructor` does not throw exceptions, *only* benefit of returning `shared_ptr` would be saving `move constructor` invocation and nothing related to exception safety? – lstipakov Apr 14 '16 at 08:29
  • 1
    Yes, I think so. To be more precise, it saves one move-construction of `T`, but adds a move of `shared_ptr`. The second difference is that `std::make_shared` allocates the value in the **heap**, while in `pop2()` you construct and return the value on the **stack**, which might also have some performance implications relating to memory locality. – jotik Apr 14 '16 at 08:34
1

move-constructor may throw (in general). std::shared_ptr<T> has a noexcept move constructor.

So in first case return res; cannot throw, but in second case return res; may throw (and data.pop() has already been called).

Jarod42
  • 203,559
  • 14
  • 181
  • 302