Segfault on deleting class member pointer to a stream

Question

I'm trying to write a class for handling exceptions. I thought it would be useful to add a << operator to give me an easy way to throw error messages. Bellow is my solution to the problem so far. One thing that is bugging me, is that if I delete the stream in the destructor (as shown in my code) I get a segfault. Bellow is the entire code of my exception handling class. I have no idea what may be deleting the stream before the destructor gets to it.

class prc_exception : public std::exception {
  private:
    int line_num;
    std::ostringstream* msg;

  public:
    prc_exception(const char* part, const int line) throw()
    : exception() {
      msg = new std::ostringstream();
      *msg << "<parcer:" << part << ":" << line << ">: ";
    }
    ~prc_exception() throw() { delete msg; }
    virtual const char* what() const throw() { return msg->str().c_str(); }
    virtual const int line() const throw() { return line_num; }
    template <class T> prc_exception& operator<<(const T& rhs)
    { *msg << rhs; return (*this); }
};

Also, if anybody can suggest a better way of handling what I want to do, please give me an advise. What I want to have is a something usable like this:

throw prc_exception("code_part",__LINE__) << "More info";

which when caught will have it's what() function return a string like:

<parcer:code_part:36>: More info

Thanks a lot.

Solved: As @polkadotcadaver suggested, adding a Copy constructor and a copy assignment operator fixed the issue. The fixed code functions as intended: #include #include

using namespace std;

class prc_exception : public std::exception {
  private:
    int line_num;
    std::ostringstream msg;

  public:
    prc_exception(const char* part, const int line) throw()
    : exception() {
      msg << "<parcer:" << part << ":" << line << ">: ";
    }
    /** Copy Constructor */
    prc_exception (const prc_exception& other)
    {
      line_num = other.line();
      msg << other.what();
    }
    /** Copy Assignment Operator */
    prc_exception& operator= (const prc_exception& other)
    {
      line_num = other.line();
      msg << other.what();
      return *this;
    }
    ~prc_exception() throw() { }

    virtual const char* what() const throw() { return msg.str().c_str(); }
    virtual const int line() const throw() { return line_num; }
    template <class T> prc_exception& operator<<(const T& rhs)
    { msg << rhs; return (*this); }
};

int main()
{
  try
  {
    throw prc_exception("hello", 5) << "text." << " more text.";
  } catch (exception& e) {
    cout << e.what() << endl;
  }

  return 0;
}

Answer 1

The following answer is something of a supposition, as I don't have the code where you're using your class. If I'm barking up the wrong tree, please say!

OK, here's a compilable, runnable test program which completed without any problem.

#include <iostream>
#include <sstream>

using namespace std;

class prc_exception : public std::exception {
  private:
    int line_num;
    std::ostringstream* msg;

  public:
    prc_exception(const char* part, const int line) throw()
    : exception() {
      msg = new std::ostringstream();
      *msg << "<parcer:" << part << ":" << line << ">: ";
    }
    ~prc_exception() throw() { delete msg; }
    virtual const char* what() const throw() { return msg->str().c_str(); }
    virtual const int line() const throw() { return line_num; }
    template <class T> 
    prc_exception& operator<<(const T& rhs)
    { *msg << rhs; return (*this); }
};

int main()
{
    try
    {
        throw prc_exception("hello", 5);
    }
    catch (prc_exception& e)
    {
        e << "Oh dear";

        cout << e.what() << endl;
    }

    return 0;
}

What I think you may be doing is catching an exception by value as opposed to by reference:

    try
    {
        throw prc_exception("hello", 5);
    }
    catch (prc_exception& e)
    {
        e << "Oh dear";

        cout << e.what() << endl;
    }

... NOW it segfaults, after it prints the output as expected. The reason is a combination of two things.

First, you have a faulty copy constructor. As you didn't define a copy constructor yourself, the compiler generated one for you. Its default behaviour is to copy each member of your class - and this is bad when you have a heap-allocated member like your ostringstream.

Think of it like this (don't take this to mean exceptions should be new'd, they shouldn't!):

// This creates an ostringstream, let's call it Bob
prc_exception* an_exception = new prc_exception{"hello", 5}; 

// This SHARES the first ostringstream, Bob, as it just copied the pointer, 
// NOT what it was pointing to. 
prc_exception* a_second_exception = new prc_exception{an_exception};

// Delete the original exception. This calls ~prc_exception and deletes Bob
delete an_exception;

// WHOOPS we just deleted Bob again, because we were pointing to it. 
// This causes the crash.
delete a_second_exception

When you catch an exception by value, as I suspect you are, this will copy the exception. Best practice is to always catch exceptions by reference.

But crucially, if you create a class which has one or more heap-allocated members, you MUST also write your own copy constructor, copy assignment operator, and destructor. Google for the Rule of Three (see references below, other subtleties apply).

I don't actually see a reason for heap-allocating your stream at all. Just have an ostringstream as a member - I suspect you did this originally but got a compilation error where you caught the exception by value. This is because ostringstream is NOT copyable!

So your fix is clear:

• Make the ostringstream a member without new-ing it. Scrap the destructor.
• Catch exceptions by reference.

Reference material:

Problem with ostringstream and copy constructor

http://www.gotw.ca/gotw/069.htm

https://en.wikipedia.org/wiki/Rule_of_three_(C++_programming)