The GCC bug affects you

This is a follow-up to my previous post about a bug in GCC. I was reading comments, and observed that some readers say that the example I used is rare, or artificial, or that it does not follow “good programming practices”. I want to give you another example.

I will be using a vector of shared_ptrs:

std::vector<std::shared_ptr<R>>;

I do not consider it a good idea in general, but I know people often do it: sometimes because this is simply the right thing to do, and sometimes because this gives them the feeling that they are relieved of any memory management issues.

Also, I will be using std::make_shared to avoid subtle memory leaks, as described in Herb Sutter’s GotW #89. Let’s construct a vector storing two pointers:

std::vector<std::shared_ptr<R>> v {std::make_shared<R>(1),
                                   std::make_shared<R>(2)};

Now, what is R? Something that is a resource-managing class, or at least looks like one, but also something that helps us test exception safety:

struct R
{
  explicit R(int)
  {
      acquire_resource();
      std::puts("create"); 
  }

  R(R const&) = delete; // no copying, no moving

  ~R() { std::puts("destroy"); }
}; 

Function acquire_resource is implemented so that it succeeds upon the first call, and fails upon the second call: this emulates resource exhaustion:

void acquire_resource()
{
  static int resources_exhausted = 0;
  if (resources_exhausted) 
    throw std::runtime_error("failed");
  else
    ++resources_exhausted;
}

And this is enough to re-create our bug in GCC! Here is the full example:

#include <cstdio>
#include <memory>
#include <stdexcept>
#include <vector>

void acquire_resource()
{
  static int resources_exhausted = 0;
  if (resources_exhausted) 
    throw std::runtime_error("failed");
  else
    ++resources_exhausted;
}

struct R
{
  explicit R(int)
  {
      acquire_resource();
      std::puts("create"); 
  }

  R(R const&) = delete; // no copying, no moving

  ~R() { std::puts("destroy"); }
};
 
int main()
{
  try {
    std::vector<std::shared_ptr<R>> v {std::make_shared<R>(1),
                                       std::make_shared<R>(2)};
  }
  catch (...) {}
}

Here again, there is a temporary involved, although we cannot see it explicitly: a temporary of type std::initialized_list<std::shared_ptr<R>>. Similarly, the destructor of a std::shared_ptr<R> is skipped…

Advertisements
This entry was posted in programming and tagged , , . Bookmark the permalink.

10 Responses to The GCC bug affects you

  1. with some modification to your original code it works fine… I suspect that move semantics are not well defined in your example for class User and it might be a problem, also process was taking by value, which implied a copy

    https://wandbox.org/permlink/ueoKwNy44h5dfvKe

    • Thanks. As you observe, there is a way to work around the bug. In fact, many ways. But in order to do it, you first need to know that you have hit the bug and are leaking resources. I am convinced that in a big application, when you rely on the C++ rules, you will not be able to spot that you have the problem.

      BTW, there is nothing wrong with move semantics of User: it follows the “Rule of Zero”: inherit special constructors and assignments from members. Your fix works, because instead of creating a temporary, you create an automatic variable u. If use a temporary the bug is back:

      https://wandbox.org/permlink/730IOc1OgoUZZ63a.

  2. wrazik says:

    Bjarne said on cppcast, that this bug should be fixed yesterday 🙂

  3. Matt says:

    I’m curious, why do you say that vector<shared_ptr> is not a good idea in general?

    • It is just a special case of my claim that using shared_ptr<T>. I know I will be hated for this. Using shared_ptr sends a message: a number of people will be using (also modifying) this object from multiple places, possibly multiple threads, and it is impossible to predict which of these usages will be last, so I have to employ the costly scheme “the last person to leave the room turns the light off”.

      The above is a valid use-case. But often when I see people use shared_ptrs, they are often using it for other purposes:

      1. Because I did not know that you also have unique_ptr,
      2. Because I was told auto_ptr is deprecated (read: evil) and the only other smart pointer I knew of was shared_ptr,
      3. Because when I used unique_ptr it did not compile,
      4. Because I did not know returning by value does not copy objects,
      5. Because I thought using shared_ptr is like using GC: I no longer have to bother about object lifetimes,
      6. Because otherwise Gmock would not work,
      7. etc…

      And if this is the case, they make the code work slower, fixing it more difficult, and the intent less clear.

      • Matt says:

        Thanks for the response. I think I understand what you’re saying, though it seems your real concern is people writing poor code and not understanding the tools they are using, not specifically that shared_ptr is an issue. If a person is being careless, I’m not sure restricting the types being used will make them more careful!

        • Yes, you summarized it quite well. The only thing I would add is that shared_ptr is somehow more prone to these abuses than any other type I have seen. Maybe because it is so close to GC (understood as panacea to any memory management issues), or maybe because people do not realize its performance implications. For instance, how many people realize that:

           
          sizeof(shared_ptr<T>) == 2 * sizeof(T*)
          
  4. David Stone says:

    Presumably this same bug would happen if you use std::unique_ptr and std::make_unique

  5. Pingback: C++ Annotated: Apr – Aug 2017 - ReSharper C++ BlogReSharper C++ Blog

  6. Pingback: C++ Annotated: Apr – Aug 2017 | CLion Blog

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s