When delegating constructors may stab you in the back

Filed under Programming
Tagged as ,

Delegating constructors is one of the cool new features of C++11. I don’t want to repeat the information that’s already on countless other blogs and sites, so here’s a quick recap:

The olden way

The C++03 example, as in the official proposal:

class X {
	void CommonInit();
	Y y_;
	Z z_;
public:
	X();
	X(int);
	X(W);
};
 
X::X() : y_(42), z_(3.14) { CommonInit(); }
X::X(int i) : y_(i), z_(3.14) { CommonInit(); }
X::X(W e) : y_(53), z_( e ) { CommonInit(); }

How it’s supposed to be done now

With delegating constructors, this can be replaced with the much more concise and clear

class X {
	Y y_;
	Z z_;
 
	X(Y y, Z z) : y_(y), z_(z){ /* CommonInit stuff */ }
 
public:
	X();
	X(int);
	X(W);
};
 
X::X() : X(42, 3.14) {}
X::X(int i) : X(i, 3.14) {}
X::X(W e) : X(53, e) {}

The reason for this post

While this is all good, let’s try converting the following struct A03 to the new and shiny C++11.

struct A03
{
	A03(){
		common_init(42);
		message = NULL;
	}
 
	A03(std::string const& msg){
		common_init(42);
		message = new char[msg.size()+1];
		strncpy(message,msg.c_str(),msg.size()+1);
	}
 
	~A03(){
		std::cout << "~A03" << std::endl;
		delete[] message;
	}
 
	int answer;
 
	char* message;
 
private:
	void common_init(int val){
		answer = val;
	}
 
	A03(A03 const&);
	A03& operator=(A03 const&);
};

The effect should look like this:

struct A11
{
	A11() : A11(42){
		message = nullptr;
	}
 
	A11(std::string const& msg) : A11(42){
		message = new char[msg.size()+1];
		strncpy(message,msg.c_str(),msg.size()+1);
	}
 
	~A11(){
		std::cout << "~A11" << std::endl;
		delete[] message;
	}
 
	int answer;
 
	char* message;
 
private:
	A11(int ans) : answer(ans) {}
 
	A11(A11&&)=delete;
	A11(A11 const&)=delete;
	A11& operator=(A11&&)=delete;
	A11& operator=(A11 const&)=delete;
};

The catch? An object is considered constructed if any of its constructors finishes. What does it change? Should the outside constructor throw, the destructor will be called.

Let’s consider what happens when new[] throws std::bad_alloc. In the C++03 version the destructor isn’t called, because the object hasn’t been fully initialized. All is okay (if the exception can be caught). The C++11 version, however, is considered fully constructed at the point where std::bad_alloc is thrown, so the destructor will be called and, since A11::message isn’t initialized at this point, the behaviour is undefined.
The C++03 solution.
The C++11 solution.
In this case, the solution is pretty simple: at the cost of one assignment (possibly optimized out), simply initialize A11::message to nullptr in the default constructor. Unfortunately it might get uglier when you’re converting legacy code, especially when there is heavy logic involved and exceptions may be thrown in extremely non-obvious ways.

Conclusion

I think delegating constructors is a great feature and definitely should be used when writing new code, but it shouldn’t be considered a drop-in replacement of the old-school common initialization methods – it simply isn’t worth the risk.

edit: This is what I managed to recover from a database crash. There were some less than enthusiastic comments pointing several potential flaws in the above examples, but they were necessary to show the unwanted behaviour.

2 Comments

  1. Danijel says:

    I know this is an old post, but nevertheless. I propose that the default constructor is to be converted to a in-class initializer and you can in this case avoid delegation. I know that you wanted to show the bad things that can happen with delegated constructors, but still.

    Code:

    struct A11
    {
        //Use in-class defaults
    	A11() = default;
     
        //Use in-class defaults for answer
    	A11(std::string const& msg) : message(new char[msg.size()+1]){
    		strncpy(message,msg.c_str(),msg.size()+1);
    	}
     
    	~A11(){
    		std::cout << "~A11" << std::endl;
    		delete[] message;
    	}
     
    	int answer = 42;
    	char* message = nullptr;
     
    private:
    	A11(A11&&)=delete;
    	A11(A11 const&)=delete;
    	A11& operator=(A11&&)=delete;
    	A11& operator=(A11 const&)=delete;
    };
    • krzaq says:

      I agree and I like your example than my solution, but the main point I wanted to make (though it doesn’t seem like I did a particularly good job of it) was that the object is considered constructed once the first constructor finishes.

      I edited your post to account for code tags, I hope you don’t mind.

Post a Comment

Your email is never published nor shared.