views:

1067

answers:

6

EDIT: SOLVED

I'm working on a multi-threaded project right now where I have a base worker class, with varying worker classes that inherit from it. At runtime, the worker classes become threads, which then perform work as needed.

Now, I have a Director I've written which is supposed to maintain an array of pointers to all of the workers, so that it can retrieve information from them, as well as modify variables within them later.

I did this by creating a pointer to a pointer of the base class:

baseWorkerClass** workerPtrArray;

Then in the constructor of the Director, I dynamically allocate an array of pointers to the base worker class:

workerPtrArray = new baseWorkerClass*[numWorkers];

In the constructor of each worker thread, the worker calls a function in the director which is meant to store the pointer of that worker in the array.

Here's how the director stores the pointers:

Director::manageWorker(baseWorkerClass* worker)
{
    workerPtrArray[worker->getThreadID()] = worker;
}

Here is an example of a worker variant. Each worker inherits from the base worker class, and the base worker class contains pure virtual functions which should exist in all worker variants, as well as a few variables which are shared between all workers.

class workerVariant : protected baseWorkerClass
{
    public:

    workerVariant(int id)
    : id(id)
    {
        Director::manageWorker(this);
    }

    ~workerVariant()
    {
    }

    int getThreadID()
    {
        return id;
    }

    int getSomeVariable()
    {
        return someVariable;
    }

    protected:

    int id;
    int someVariable
};

Then the baseWorkerClass looks something like this:

class baseWorkerClass
{
public:

    baseWorkerClass()
    {
    }

    ~baseWorkerClass()
    {
    }

    virtual int getThreadID() = 0;
    virtual int getSomeVariable() = 0;
};

After each worker variant is done initializing, I should end up with an array of pointers to baseWorkerClass objects. This means I should be able to, for example, get the value of a given variable in a certain worker using its ID as the index to the array, like so:

workerPtrArray[5]->getSomeVariable(); // Get someVariable from worker thread 5

The problem is that this code causes a crash in a Windows executable, without any explanation of why, and in Linux, it says:

pure virtual method called
terminate called without an active exception
Aborted

I could've sworn I had this working at some point, so I'm confused as to what I've screwed up.


Actual unmodified code that has the problem:

Worker variant header: http://pastebin.com/f4bb055c8
Worker variant source file: http://pastebin.com/f25c9e9e3

Base worker class header: http://pastebin.com/f2effac5
Base worker class source file: http://pastebin.com/f3506095b

Director header: http://pastebin.com/f6ab1767a
Director source file: http://pastebin.com/f5f460aae


EDIT: Extra information, in the manageWorker function, I can call any of the pure virtual functions from the pointer "worker," and it works just fine. Outside of the manageWorker function, when I try to use the pointer array, it fails.

EDIT: Now that I think about it, the thread's entry point is operator(). The Director thread is created before the workers, which may mean that the overloaded parenthesis operator is calling pure virtual functions before they can be overridden by the child classes. I'm looking into this.

+3  A: 

The problem appears to be that Director::manageWorker is called in the constructor of workerVariant instances:

Director::manageWorker(baseWorkerClass* worker) {
    workerPtrArray[worker->getThreadID()] = worker;
}

Presumably getThreadID() isn't a pure virtual function or you would have (hopefully!) gotten a compiler error about not overriding it in workerVariant. But getThreadID() might call other functions which you should override, but are being invoked in the abstract class. You should double check the definition of getThreadID() to make sure you're not doing anything untoward that would depend on the child class before it's properly initialized.

A better solution might be to separate this sort of multi-stage initialization into a separate method, or to design Director and baseWorkerClass such that they don't have this sort of initialization-time interdependency.

John Feminella
Thanks for the input.Actually, about the getThreadID method, that was just an inconsistency on my part. This isn't the actual code I'm using, it's just stuff I wrote a few minutes ago to clearly and concisely illustrate my point. the getThreadID method _is_ pure virtual, and _is_ overridden in every worker variant. All the getThreadID method does is return the "id" variable of the thread.
GenTiradentes
When an object is under construction (while the constructor is executed), the virtual function mechanism will never call into virtual functions of a class derived from the object's type. This is because constructors of derived classes have yet to be executed. So calling a derived class' method from a constructor won't work. A way out of this is two-phase constructing, preferably hidden behind a wrapper object. (The same holds for destruction, BTW.)
sbi
Oh, crap, you're right, I didn't think of that. Let me do a quick test here, and I'll report back. (There goes another hour of sleep.)
GenTiradentes
Didn't work, same error. Thanks a bunch for the help though.
GenTiradentes
Maybe you could post an actual complete example that reproduces the problem and incorporates the changes?
Georg Fritzsche
A: 

Pure virtual function called means that a member which is pure in the base class is called before the execution of the constructor of the descendant is started. In non multi-thread programs, this means directly or indirectly in the constructor of the base class. In a multi-threaded program, this can also happen when the constructor launch the thread in the constructor and the system execute the thread before having terminated the constructor.

AProgrammer
+2  A: 

Without seeing the full code I would hazard a guess that you are stepping out of the boundary of the memory block pointed to by workerPtrArray. It would certainly make sense since it complains about pure virtual function being called. If the memory being dereferenced is garbage then the runtime can't make sense of it at all and weird things happen.

Try to put asserts into critical places where you are dereferencing the array to make sure that indices make sense. I.e. limit to 4 workers and make sure the id is less than 4.

Igor Zevaka
Now that you mention it, I remember the Visual Studio debugger complaining about array elements being accessed that were out of bounds. I don't see why this is happening though, because I've checked and double checked my indices, and they are all within bounds. I'll be posting my actual code in a few minutes.
GenTiradentes
You might try using a std::vector<baseWorkerClass *> instead of the double pointer. I always have to look up the "new" syntax for allocating arrays like that. Also, this will be a little more clear and alleviates you from managing the array.
David Smith
A: 

During initialization classes are only partially constructed. Specifically, constructors have to be executed from the most ancestor class up, so that each derived class'es constructor can safely access its base members.

this means that the vtable of a partially constructed class can not be in its final state - if virtual methods were allowed to call derived classes, those methods would be called before that classes constructor had been called.

Which means that, during construction, pure virtual functions are in fact, pure virtual. Modern c++ compilers are getting better at catching this - but its possible to in many cases "bury" the illegal call in such a way that the compiler doesn't notice the error.

Moral of the story: dont do anything in your constructor that will invoke a virtual function. It just won't do what you expect. Even when it isn't pure.

Chris Becke
+1  A: 

I didn't see the variant class being constructed in any of your code samples. Are you sure the id being passed is within range for the worker array? Also, you're constructing the objects using 'new', right? If you constructed the object on the stack, it would register itself with the Director, but after the constructor returns the object will be immediately destroyed, but the Director will retain it's pointer to the object that was on the stack.

Also, your baseWorkerClass destructor should be virtual along with the workerVariant destructor to make sure they get called when you delete the array of baseWorkerClass.

From my comment to another question, consider using std::vector instead of the double pointer. It's easier to maintain and understand and removes your need to maintain the array.

It seems like you've added an unnecessary layer of abstraction here. I don't think the id should really be part of the subclass interface. I think something like this might work better for you:

class baseWorkerClass
{
public:

    baseWorkerClass(int id) :
        id( id )
    {
    }

    virtual ~baseWorkerClass()
    {
    }

    int getThreadID(){ return id; };
    virtual int getSomeVariable() = 0;

protected:
    int id;
};

class workerVariant : protected baseWorkerClass
{
    public:

    workerVariant(int id) :
        baseWorkerClass( id )
    {
        Director::manageWorker(this);
    }

    virtual ~workerVariant()
    {
    }

    int getSomeVariable()
    {
        return someVariable;
    }

protected:
    int someVariable
};
David Smith
A: 

Aren't you by any chance accessing the objects after they are destructed? Because during destruction the vtable pointers are gradually "rolled back" so that vtable entries will point to methods of the base class, some of which are abstract. After deleting the object, the memory could be left as it was during the destructor of the base class.

I suggest you try memory debugging tools, such as valgrind or MALLOC_CHECK_=2. Also on unix it is quite easy to get a stacktrace for such fatal errors. Just run your application under gdb, or TotalView, and at the point the error happens it will automatically stop, and you can look at the stack.

shojtsy