Destructor throws exception when deriving from unique_ptr

Question

I tried to hack into unique_ptr's deleter, but without compromising construct by right value (we can't override deleter since it's associated with the right value). So I decided to try deriving from unique_ptr, here is my code.

using OStreamPtr = std::unique_ptr<std::ostream>;

class MockOStreamPtr : public OStreamPtr {

 public:
  MockOStreamPtr(OStreamPtr&& rhs, MockOutputSystem* sys) : OStreamPtr(std::move(rhs)), sys_(sys) {}
  MockOStreamPtr(std::ostream* p, MockOutputSystem* sys) : OStreamPtr(p), sys_(sys) {}
  ~MockOStreamPtr() {
    std::cout << "~MockOStreamPtr" << std::endl;
    if (sys_) {
      std::cout << get()->good() << std::endl;  // this failed already
      // sys_->manage(*this);
    }
  }

 protected:
  MockOutputSystem* sys_ = nullptr;
};

MSVC gives me an SEH exception on accessing ostream pointer during destruction, which I can't understand at all.

compact test case:

#include <iostream>
#include <sstream>
#include <istream>
#include <string>
#include <memory>
#include <cassert>

using namespace std;

using OStreamPtr = std::unique_ptr<std::ostream>;

class MockOutputSystem {

 public:
  template <typename T>
  static OStreamPtr MakeStream(T&& rhs) {
    return std::make_unique<T>(std::move(rhs));
  }
  OStreamPtr fopen(const std::string& file);
};

class MockOStreamPtr : public OStreamPtr {

 public:
  MockOStreamPtr(OStreamPtr&& rhs, MockOutputSystem* sys) : OStreamPtr(std::move(rhs)), sys_(sys) {}
  MockOStreamPtr(std::ostream* p, MockOutputSystem* sys) : OStreamPtr(p), sys_(sys) {}
  ~MockOStreamPtr() {
    std::cout << "~MockOStreamPtr" << std::endl;
    if (sys_) {
      std::cout << get()->good() << std::endl;  // this failed already
      // sys_->manage(*this);
    }
  }

 protected:
  MockOutputSystem* sys_ = nullptr;
};

OStreamPtr MockOutputSystem::fopen(const std::string& file) {
  auto s = std::ostringstream();
  s << file << ":" ;
  return MockOStreamPtr(std::move(MakeStream(std::move(s))), this);
}

int main(void) {
  MockOutputSystem sys;
  OStreamPtr s(sys.fopen("test_file.b"));
  (*s) << "hello world";
  s.release();  // failed here
}

Answer 1

You have an inadvisable mix of strategies in play. The first thing to address is deriving from a class that was not designed as a base class – don't do that. Ask yourself: is MockOStreamPtr supposed to be a pointer to an ostream or is it supposed to have a pointer to an ostream? The former suggests inheritance, but it also suggests not introducing members (like sys_) that have nothing to do with being a pointer. The latter is closer to what you seem to want: a class that makes ostream and MockOutputSystem work together.

Option A

Make the pointer a class member, putting the output stream at the same level as the output system.

class MockOStreamPtr {
    // Stuff here. I'll leave the details to you.

    protected: // Sure you don't want "private" here?
        OStreamPtr  stream_;
        MockOutputSystem* sys_ = nullptr;
};

This at least has better organization, but it can be more cumbersome since you now have two pointers to track. Also, you might run into the same problem in the destructor. Unless you spot the issue when writing a wrapper for release(). You may find your project simplified if you go with option B.

Option B

You could derive from std::ostream instead of from a pointer. This would be OK since an ostream is designed to be part of an inheritance tree. On the other hand, it is designed to be a non-leaf in that tree, so this particular inheritance would not be that useful. To have a useful inheritance, you would want to derive from something derived from ostream, perhaps derive from std::ofstream.

If ofstream is the only class that needs to be extended with knowledge of your output system, then this could work out simply. If you need to extend other classes, a template could work out almost as simply.

class MockOStream : public std::ofstream {
    // Stuff here. I'll leave the details to you.

    protected: // Sure you don't want "private" here?
        MockOutputSystem* sys_ = nullptr; // Maybe you want a reference instead?
};

// *** OR  ***

template <class S>
class Mock : public S {
    // Stuff here. I'll leave the details to you. Write code as if S is ostream.

    protected: // Sure you don't want "private" here?
        MockOutputSystem* sys_ = nullptr; // Maybe you want a reference instead?
};

A comprehensive, robust output system should provide this sort of wrapper for you, so that you do not have to be concerned about such details. All you would need to be concerned about is avoiding object slicing. However, if MockOutputSystem::fopen must return an ofstream instead of a MockOStream, you could inject your wrapper into the construction process.

int main(void) {
    MockOutputSystem sys;
    // The next line is awkward; don't use it as written.
    // The point is that a MockOStream's address can be used as a pointer-to-ostream.
    std::unique_ptr<std::ostream> s = std::make_unique<MockOStream>(sys.fopen("test_file.b"));
    s << "hello world";
    s.release();  // <-- destroy the stream, using the virtual destructor
}

This better encapsulates what you want, doesn't it? When your stream is destroyed, you want something to happen based upon the associated output system. Not when the pointer is destroyed, but when the stream is destroyed.

Which leads into the specific reason your code crashed. A call to get()->good() is a memory violation when get() is null. And in your sample program, get() will be null since release() was called before the pointer's destructor. The call to release() will destroy the pointed-to object (the stream) and set the stored pointer to null. By the time the pointer's destructor is called, it is too late to do anything with that stream.