Why does std::shared_ptr not increment reference count during the operator ->?

580 views
Skip to first unread message

karste...@gmail.com

unread,
Jun 15, 2018, 10:34:39 AM6/15/18
to ISO C++ Standard - Discussion

Hi all,


I am very interested in C++ code safety and believe that the smart pointers since tr1 go a long way as far as guaranteeing memory correctness. However I feel there has been a lost opportunity when it comes to the std::shared_ptr<T>::operator ->. I have attempted to create a very simple example to demonstrate this:


https://pastebin.com/JSUUAtCZ


This program design is flawed in that we are wiping the last reference count whilst inside a method of the object being deleted. As far as C++ goes, this is not a problem. However, when we then try to access a member variable after this has happened, we will encounter a use after free scenario. In this simple example, the flawed design is easy to detect but if the class leading to the decreased reference count was hidden much further down a number of calls, I would personally find it hard to detect and instead have to rely on some memory checking tools such as Valgrind. I would have thought something more mechanical using RAII would have been good to put in place here.


For example, in a toy wrapper around std::shared_ptr<T> I demonstrate that the operator-> has been overloaded to temporarily increase the reference count during the lifetime of that call.


https://pastebin.com/9h2awBDy


It does this by taking advantage of the special case of operator -> recursing down until a pointer is found. With this in place, the lifetime of the object is guaranteed whilst we are inside one of its methods.


I use something similar to this whilst debugging my own projects for both smart pointers and iterators but instead, to achieve the same API / behavior compatibility with standard C++, I make it guarantee an abort rather than protect against the use after free so I can attempt to find the broken design via the stack trace.


Is there any reason why this functionality is not included in the design of shared_ptr? Does it cause other issues I have not yet anticipated?


Interestingly std::weak_ptr is immune to this error scenario described above because of the explicit need to call lock() before you can use operator -> to access the method. This performs that necessary increase to reference count. This seems to work well so I am assuming that the same would have also been a good addition to shared_ptr?


Best regards,


Karsten

Nicol Bolas

unread,
Jun 15, 2018, 11:11:50 AM6/15/18
to ISO C++ Standard - Discussion, karste...@gmail.com
What you want would require doing an atomic increment/decrement every time someone does `->` or `*`. That is an unacceptable performance loss for a circumstance that, quite frankly, represents terrible design on the user's part.

If you have some location in your code where this circumstance might happen, you are free to create a temporary `shared_ptr` manually to impose this performance penalty. But the rest of us should not have to suffer for poor design decisions by a few people.

`shared_ptr` is not a magical salve that makes all your code safe. It's a useful tool, and it only remains so as long as its performance tradeoffs remain reasonable.

karste...@gmail.com

unread,
Jun 15, 2018, 12:00:31 PM6/15/18
to ISO C++ Standard - Discussion, karste...@gmail.com
I agree that it would be wasteful in a release build but as a debugging aid, it could be extremely valuable to find these design errors (or at least to help guarantee that they do not exist as part of a test suite).

There is a cost in performing the atomic increment but not in instead emitting an error / exception if the underlying object is attempting to be deleted. I believe this "assertion" behavior is outside of the C++ standard's scope though.

I suppose then that this kind of stuff is more for compiler / implementation specific versions of a "debug standard library" rather than the standard library itself. Similar to the Microsoft debug runtime's safe iterators.

OK, thanks for your time, apologies for the noise :)
Reply all
Reply to author
Forward
0 new messages