🔒 Locked

Prevent pure virtual function call

Started by Vexator Apr 2, 2009 at 10:18 AM 8 replies 2.1k views

Original Post

Apr 2, 2009 at 10:18 AM

I have a class TaskManager to which tasks (which derive from the base class Task) are added. When added, the task manager calls the task's Initialize() function. This function is pure virtual in the task base class and has to be implemented by deriving classes (-> InputTask). So far, I create a task and then add in to the task manager afterwards, like this:

m_pkInputTask = shared_ptr<InputTask>( new InputTask() );
Engine::GetTaskManager()->AddTask( shared_ptr<Task>(m_pkInputTask) );

however, i'd like to do this in the task's constructor already, like this:

Task::Task( long priority )
{
	m_iPriority = priority;

	Engine::GetTaskManager()->AddTask( shared_ptr<Task>(this) );
}

this compiles fine, but when I run the app, it reports a pure virtual function call. here's how I call Initialize():

bool TaskManager::AddTask( const shared_ptr<Task> &task )
{
	if( !task->Initialize() )
		return false;

	// ...
}

how can I make sure that the function of the derived class is being called instead of the base class's fucntion? As I said, it's declared pure virtual. Thank you!

Antheus

Apr 2, 2009 at 10:28 AM

When you're creating base class, derived classes don't necessarily exist yet, neither does vtable. As such, calling virtual function in constructor is conceptually undefined and compiler should at least emit a warning.

If you need to do some processing, that is what constructor is for - let derived classes use it. Initialize is definition of constructor.

For same reason, the use of 'this' in constructor is generally discouraged, since it's passing potentially incomplete instance. While not invalid and useful in some cases, care must be taken to never invoke a virtual function in this case (which is where your problem stems from).

stonemetal

Apr 2, 2009 at 10:57 AM

visualize it as a stack. First the base class is constructed then the next class in the chain and the one after that until the class that was instantiated is reached. So you can not call a pure virtual function because that part of the function hasn't been created yet. Infact if you call a virtual function in a base class constructor it will call the base class implementation of the function.

class a {    public:    a(){hi();}    virtual void hi(){std::cout <<"hi bob\n";}};class b : public a{public:    virtual void hi() {std::cout <<"hi joe\n";}};int main(){    b myb;myb.hi();}

prints
hi bob
hi joe

Vexator

Apr 2, 2009 at 11:18 AM

what I do not understand why it worked before, when I created the task and then added it manually. What's the difference? Thank you for your quick replies!

SiCrane

Apr 2, 2009 at 11:55 AM

It doesn't work because until control flow execution reaches the first brace of the derived class constructor, the class doesn't actually have the type of the derived class. It's dynamic type is still that of the base type.

Waterwalker

Apr 2, 2009 at 1:27 PM

Quote:
Original post by Vexator
what I do not understand why it worked before, when I created the task and then added it manually. What's the difference? Thank you for your quick replies!

In the first case you have a readily constructed InputTask instance. The instance already knows that it is a InputTask object and it also knows which overloaded virtual function to call (namely the one from the InputTask class).

In the second case you are inside the Task constructor. Even if you are about to create a InputTask instance the object does not know that it will be a InputTask object. At least not while it is inside the Task constructor. Hence you can (unfortunately) compile that without a warning but at runtime the code tries to call Task::Initialize and not InputTask::Initialize.

The object does only know that it is a InputTask object once it reaches the InputTask constructor which is in fact *after* going through the Task constructor.

As a bottom line: Never ever call a virtual function inside a constructor - this results most likely in the wrong implementation being called. And never ever ever call a pure virtual function inside a constructor - this results in the program crashing.

By the way, it seems that some compilers such as gcc ignore pure virtual function calls. So even if there is no crash it does not do what you wanted it to do.

Codarki

Apr 3, 2009 at 12:03 AM

Also having shared_ptr(this) in constructor is invalid usage of shared_ptr. Check boost::enable_shared_from_this<>.

Doggan

Apr 3, 2009 at 2:58 AM

C++ FAQ == good:

http://www.parashift.com/c++-faq-lite/ctors.html#faq-10.7
http://www.parashift.com/c++-faq-lite/strange-inheritance.html#faq-23.5

I highly recommend reading through the entire thing at least once. You will learn many important things about the language, which will save you headaches in the long run.

Vexator

Apr 4, 2009 at 8:46 AM

Thank you guys, I think I understand now. I removed the virtual function call and instead fully initialize the object in its c'tor. one problem remains: i create a shared_ptr from "this" here:

Engine::GetTaskManager()->AddTask( shared_ptr<Task>(this) );

this works fine, but the app crashes when the task's d'tor is called, so i guess I should do as Ftn said and derive Task from enable_shared_from_this and call AddTask() like this:

Engine::GetTaskManager()->AddTask( shared_from_this() );

This results in a crash when I run the app though:

boost::exception_detail::clone_impl<boost::exception_detail::error_info_injector<boost::bad_weak_ptr> > at memory location 0x0012f508..

what's wrong? thank you for your patience!
(I could of course just call AddTask() outisde of the Task's c'tor, but I want to create objects from Lua, and I don't want to expose the task manager to Lua)

SiS-Shadowman

Apr 4, 2009 at 9:33 AM

The thing you are doing wrong is that you should NOT create a shared_ptr from within the c'tor with the this pointer. You will never really know how your class (even if you're the only one) is used.

Foo f( bar );

f constructs a shared_ptr of this... now the shared_ptr owns f, but f gets destructed when it is out of scope. The class itself shall not limit itself to be only placed on the heap: the programmer has to use whatever is more appropriate. In any way it's not up to the c'tor.

If you do now want to expose your task manager to lua, offer a function that does the following:

Foo *getFoo(){    Foo *f = new Foo();    addTask( shared_ptr<Foo>( f ) );    return f;}

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.