4

This code

#include <iostream>
#include <memory>

template <typename T>
class ScopeGuard final {
 public:
  explicit ScopeGuard(T callback) : callback_(std::move(callback)) {}

  ScopeGuard(const ScopeGuard&) = delete;
  ScopeGuard(ScopeGuard&&) = delete;

  ScopeGuard& operator=(const ScopeGuard&) = delete;
  ScopeGuard& operator=(ScopeGuard&&) = delete;

  ~ScopeGuard() noexcept(false) { callback_(); }

 private:
  T callback_;
};

static int a_instances = 0;

struct A {
  A() {
    ++a_instances;
    std::cout << "ctor" << std::endl;
  }
  A(A&&) = delete;
  A(const A&) = delete;
  ~A() {
    --a_instances;
    std::cout << "dtor" << std::endl;
  }
};

struct Res {
  std::shared_ptr<A> shared;

  explicit Res(std::shared_ptr<A> shared) : shared(shared) {
    std::cout << "res ctor" << std::endl;
  }

  Res(Res&& o) : shared(std::move(o.shared)) {
    std::cout << "res mv" << std::endl;
  }

  Res(const Res& o) : shared(o.shared) { std::cout << "res cp" << std::endl; }

  ~Res() { std::cout << "res dtor" << std::endl; }
};

Res LeakImpl() {
  ScopeGuard on_exit{[] { throw std::runtime_error("error"); }};
  Res res{std::make_shared<A>()};
  return res;
}

void Leak() {
  try {
    auto res = LeakImpl();
  } catch (const std::exception& e) {
    std::cout << e.what() << std::endl;
  }
}

int main() {
  Leak();
  if (a_instances) {
    std::cout << "leak, instances count: " << a_instances << std::endl;
  }
  return 0;
}

Outputs this (GCC 9.1, GCC 11.1 and Clang 9.0.1)

ctor
res ctor
error
leak, instances count: 1

Which means that neither A::~A(), nor Res::~Res() were called, resulting in this case in memory leak.

I suppose inside LeakImpl() Res::~Res() wasn't called before ScopeGuard::~ScopeGuard because of copy elision, but why wasn't it called when exception that was thrown from ScopeGuard::~ScopeGuard left try block?

If I change LeakImpl() like this:

Res LeakImpl() {
  ScopeGuard on_exit{[] { throw std::runtime_error("error"); }};
  return Res{std::make_shared<A>()};
}

then GCC 11.1 generated binary outputs this:

ctor
res ctor
res dtor
dtor
error

no leak

while on GCC 9.1 and Clang 9.0.1 the bug remains.

To me this looks like a bug in a compiler, what are your thoughts?

Islam Boziev
  • 167
  • 1
  • 8
  • 4
    [Do not allow a destructor to emit an exception](https://stackoverflow.com/q/130117/10077). – Fred Larson Jun 30 '21 at 14:30
  • 1
    @FredLarson Thanks for the link! Read it. The only objective (not conceptual) argumentation I saw there for never throwing from destructors is a potential throw during stack unwinding, resulting in process termination. I intentionally posted simplified version of `ScopeGuard` to make example easier to read. In reality my `ScopeGuard` checks if stack unwinding is in progress, and if it is, it just logs exception, so it can never directly cause `std::terminate`. If you think that I missed other objective reasons why destructors should never throw please, let me know. – Islam Boziev Jun 30 '21 at 15:07
  • 1
    There is very widely held a rule of thumb that destructors must always be `nothrow`, so it is plausible that there is a compiler bug related to throwing destructors. It would be less likely to be discovered than more mainstream problems, and it may have a lower bugfix priority. As far as I can tell, there is no reason for the leak. – François Andrieux Jun 30 '21 at 15:22

1 Answers1

2

Seems like a bug, already reported and fixed for gcc: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66139

I suppose, std::shared_ptr<> is partially created during exception throw

As a workaround, you could prevent copy elision:

Res LeakImpl() {
  ScopeGuard on_exit{[] { throw std::runtime_error("error"); }};
  Res res{std::make_shared<A>()};
  return std::move(res);
}

or create ScopeGuard before Res (destruction order is matters in this case):

Res LeakImpl() {
  Res res{std::make_shared<A>()};
  ScopeGuard on_exit{[] { throw std::runtime_error("error"); }};
  return res;
}
Sergey Prika
  • 36
  • 2