3

I am reading about std::shared_ptr in the Effective Modern C++14 book by Scott Meyers. Here is a code in which the author says a potentital resource leak can be:

int computePriority(); // this function always throw an exception
// ...
processWidget(std::shared_ptr<Widget>(new Widget), // (S.M): potentital
              computePriority());                  // resource
                                                   // leak

Scott Meyers says that computePriority() function may be called between new Widget and std::shared_ptr<Widget>(expr) expressions are evaluated what will lead to a memory leak caused by new. Should not be there a one sequence point that can guarantee if the new Widget expression is evaluated then std::shared_ptr<Widget>(expr) will be evaluated next? I think about this because It would be correct in my opinion: sequence point is in std::shared_ptr<Widget>(expr) will evaluate all of it's arguments(sub expressions) without sequencing and will make shared pointer be ready, then do something else (evalute other arguments without sequencing). Just if this is untrue and Scott Meyers is right (I still believe him, obviously) , would not be this behavior incorrect?

Is there a rule that explains why is this possible? I am not good at sequence points, but people who told me about them says this should be valid.

VP.
  • 15,509
  • 17
  • 91
  • 161

4 Answers4

4

Sequencing does not imply immediacy.
(My waking up is sequenced before my going to work, even if I get dressed and have breakfast inbetween.)

The evaluation of the arguments are not sequenced with respect to one another.

It's also not the case that one argument must be fully evaluated before the other, regardless of sequence.

That is, the allocation is sequenced before the shared_ptr constructor, but neither of those is sequenced with respect to the other argument and there is no guarantee that nothing happens inbetween the allocation and the constructor call.

molbdnilo
  • 64,751
  • 3
  • 43
  • 82
2

Should not be there a one sequence point that can guarantee if the new Widget expression is evaluated then std::shared_ptr<Widget>(expr) will be evaluated next?

No, it is not the case. There is no sequence point between new Widget and shared_ptr construction.

See Sequence point. (Which is obsolete term in C++11 BTW, but the logic stays pretty much the same.)

sequence points occur
...

  1. Between evaluation of the left and right operands of the && (logical AND), || (logical OR) ....

  2. Between the evaluation of the first operand of the ternary "question-mark" operator and the second or third operand. ....

  3. At the end of a full expression ....

  4. Before a function is entered in a function call ....

  5. At a function return ....

  6. At the end of an initializer; ....

  7. Between each declarator in each declarator sequence ....

  8. 8.After the action associated with input/output conversion format specifier. ....

Community
  • 1
  • 1
AlexD
  • 32,156
  • 3
  • 71
  • 65
1

Should not be there a one sequence point that can guarantee if the new Widget expression is evaluated then std::shared_ptr(expr) will be evaluated next?

Well, yes. A semi-colon would do nicely, since it marks the end of a full expression.

auto ptr = new Widget; // may throw bad_alloc
std::shared_ptr<Widget> shared(ptr); // may throw bad_alloc
processWidget(std::move(shared), computePriority()); // computePriority may throw

One benefit of this approach (separating the construction of the shared ptr from new) is that such a constructor makes a guarantee to delete the pointer you handed to it upon exception.

However, it's preferable to avoid naked new if you can, by taking advantage of std::make_shared because it can do more efficient allocation (control block and data block in single contiguous allocation), and if an exception is thrown the end result is that the function has no effect (and no memory leak)

So do prefer the following syntax if you must

processWidget(std::make_shared<Widget>(), computePriority());

because two function calls are never interleaved, computePriority() may be called before make_shared or vice-versa, and if either throws there is no memory leak. However, it may be in your interest to surround throwing calls with try-catch blocks rather than have a hard crash via inlining them.

AndyG
  • 39,700
  • 8
  • 109
  • 143
0

see http://en.cppreference.com/w/cpp/memory/shared_ptr/shared_ptr for reference
using new A in the constructor is fine, as long as the constructor won't throw bad_alloc.
make_shared does not have this issue (see std::shared_ptr initialization: make_shared<Foo>() vs shared_ptr<T>(new Foo) )

Community
  • 1
  • 1
I3ck
  • 433
  • 3
  • 10
  • "totally fine" is incorrect. The constructor is permitted to throw. If you use this constructor you should be prepared to catch a `bad_alloc`. – Richard Hodges Dec 29 '15 at 12:29