When Virtual Functions Aren’t Virtual

Let’s take a look at some innocuous-looking code and see if you can spot the bugs. If you can, you’re doing great!

class Base {
private:
	void Cleanup() {
		// Do some cleaning stuff here
		LogCleanup();
	}
	
protected:
	// The derived classes determine where the logging file lives
	virtual FILE *GetLoggingFile() = 0;
	
	// Perform some basic logging, but derived classes can overload
	// the function to do log more informatively if they choose.
	virtual void LogInit() {
		FILE *fp = GetLoggingFile();
		if (fp) {
			::fprintf( fp, "Initializing the base class\n" );
		}
	}
	
	virtual void LogCleanup() {
		FILE *fp = GetLoggingFile();
		if (fp) {
			::fprintf( fp, "Finalizing the base class\n" );
		}	
	}
	
public:
	Base() {
		LogInit();
	}
	
	virtual ~Base() {
		Cleanup();
	}	
};

class Derived : public Base {
private:
	FILE *mLogFile;
protected:
	virtual FILE *GetLoggingFile() {
		return mLogFile;
	}
	
	virtual void LogCleanup() {
		if (mLogFile)	::fprintf( mLogFile, "Closing the log file after finalizing the base class.\n" );
		Base::LogCleanup();
	}
	
public:
	Derived() : Base(), mLogFile( NULL ) {
		mLogFile = ::fopen( "MyLogFile.txt", "at" );	
	}
	
	virtual ~Derived() {
		if (mLogFile)	::fclose( mLogFile );
	}
};

While this code may look reasonable, it is dangerously incorrect due to what’s happening in the constructors and destructors.

In C++, an object is not fully constructed until after all of its constructor methods have run. This means that you cannot call virtual methods from within your constructor, such as we’re doing in Base::Base. When you look at the way constructors work in C++ (as laid out in 12.6.2.5 of the C++98 spec), this will make a lot of sense. When you call Derived *d = new Derived, the object’s constructor methods are called from the least derived class to the most derived class. In our example, it means that Base::Base is called before Derived::Derived. Now you can understand why virtual methods would be dangerous — because Derived’s constructor hasn’t had the chance to run, any virtual functions that would be called on Derived from within Base’s constructor would be run on an object that hasn’t had the opportunity to initialize anything! So C++ simply disallows the chance to get into that situation in the first place. While a constructor method is executing, the type of the object is that of the currently-scoped class. While in Base::Base, the typeid of “this” is Base, even if it’s part of constructing a Derived object. So virtual methods are dispatched as though they’re not really virtual.

Going back to our example, if the user attempts to create an instance of Derived, then Base::Base is called first. It calls LogInit, which is a virtual method — that’s our first bug. But it’s a “harmless” bug by chance, since Derived doesn’t override the method. So Base::LogInit is called. It then attempts to call GetLoggingFile, which is a pure virtual function. However, because there is no way to get to Derived::GetLoggingFile, this is a major crashing bug — the infamous “pure virtual function call” crash! Base::GetLoggingFile is the only method in scope, but it has no implementation (it’s purely virtual). So there’s no method to actually call, and the application crashes.

The same issues happen during destruction as during construction, just in reverse order. When an object is being destroyed, it is destroyed from the most derived class down to the least derived class. This means that Derived::~Derived is called first, then Base::~Base. So there are actually three bugs here: two logic issues and a crashing issue. The logic issue is that Derived::~Derived is closing the log file before Base::~Base has the chance to log to it. But worse than that, you have the same pure virtual function call crash as the constructor. Derived::~Derived is fired first, and after leaving the destructor method, the Derived object no longer exists. So Base::~Base calls Cleanup (which is fine, because it is not a virtual function), which then calls LogCleanup (which is not fine, because it is a virtual function). However, instead of calling Derived::LogCleanup, like you may expect, it actually calls Base::LogCleanup because that’s the only possible method to call. This method then attempts to call Base::GetLoggingFile, which never existed in the first place, and so you crash.

As you can see, there’s inherent danger in attempting to call virtual functions from a constructor or destructor. The worst case scenario is attempting to call a pure virtual function which results in a crash. The middling case scenario is that you call the function successfully, but it calls something on the base class instead of the derived class like you might expect. The best case scenario is that the function calls the base class as expected because no derived classes override it (in which case, the function likely shouldn’t be virtual anyway). Regardless, calling a virtual function from a constructor or destructor rarely works as expected. Remember that this actually affects more than just virtual methods. During construction and destruction, the this pointer’s type is that of the object being created/destroyed. So things like dynamic_cast and typeid are affected as well!

You may be thinking “this is why compilers have error reporting.” Wrong. It’s not required to be considered an error by the compiler. But even worse than that, it is frequently not even reported as a warning by the compiler either! For instance, in Visual Studio 2010, the above code compiles without warning (aside from the security warning for the call to fopen). It compiles entirely without warning in gcc 4.2 and Clang 2.0 used by XCode 4. Even the venerable PC-Lint doesn’t flag this as being a warning. So it is likely that you will be on your own for spotting this dangerous activity and ensuring it doesn’t happen. Unfortunately, it can be difficult to spot — in our example, the Base destructor calls only a non-virtual method. It just so happens that the non-virtual method that is called ends up calling a virtual method.

Tenacity is the only cure for this sin. When you are writing a constructor or destructor method, trace the function calls and their definitions to ensure they don’t call virtual methods. If you find that they do call virtual methods, then find a different way to perform the same functionality without the virtual requirement. For instance, with our example code, it would be cleaner to have the base class hold onto the logging file, and have the derived class simply pass it in during construction. It may not always be easy to restructure your code in this way, but in cases where it’s hard, you are likely to have a design issue with your classes that may need to be rectified. Another danger area comes from refactoring existing code — if you are adding virtual function calls to an existing method, you should ensure that the method isn’t eventually called from a constructor or destructor.

tl;dr: When in a constructor or destructor, do not call virtual methods as they won’t behave in the way you expect.

This entry was posted in C/C++ and tagged . Bookmark the permalink.

Leave a Reply

Your email address will not be published. Required fields are marked *