Unhandled Exception due to Class slicing

Question

I'm getting an unhandled exception reading location 0x00000008 (reading NULL value) on the noted line below, relevant methods leading up to the error are included (continued below examples):

Event Methods:

Event::Event(Event::EVENTTYPE type) : eventType(type) { }

KeyEvent Methods:

class KeyboardKeyEvent : public Event {
public:
    //...
    int GetKey() const;
protected:
//...
};

int KeyboardKeyEvent::GetKey() const {
    return this->_scancode; //Errors out here. "this" returns 0x000000
}
KeyboardKeyEvent::KeyboardKeyEvent(int key, Event::EVENTTYPE type) : Event(type), _scancode(key) { }

KeyDownEvent Methods:

KeyboardKeyDownEvent::KeyboardKeyDownEvent(int scancode) : KeyboardKeyEvent(scancode, Event::KEYBOARD_KEYDOWN) { }

Event Handler Methods:

bool EventHandler::EnqueueEvent(Event* event) {
    if(event == NULL) return false;
    try {
        this->_eventQueue.push(event);
    } catch (...) {
        return false;
    }
    return true;
}

Event* EventHandler::DequeueEvent() {
    if(this->_eventQueue.empty() == false) {
        Event* result = new Event(*this->_eventQueue.front());
        delete this->_eventQueue.front();
        this->_eventQueue.pop();
        return result;
    }
    return NULL;
}

Main Loop Sequence:

if(_eh->HasEvents()) {
    Event* nxtEvent = _eh->DequeueEvent();
    switch(nxtEvent->GetType()) {
        case Event::KEYBOARD_KEYDOWN:
            allegro_message("You pressed the %d key!", dynamic_cast<KeyboardKeyDownEvent*>(nxtEvent)->GetKey());
            break;
        default:
            /* DO NOTHING */;
    }
    delete nxtEvent;
    nxtEvent = NULL;
}

I know this is a slicing problem I just don't see why it's happening or how to fix it (Actually, now that I think about it, it's probably a "Can not convert to requested type" error). All throughout when I step through the program _scancode is the appropriate value, but the second the line dynamic_cast<KeyboardKeyDownEvent*>(nxtEvent)->GetKey() runs it throws the error. Double casting as dynamic_cast<KeyboardKeyDownEvent*>(dynamic_cast<KeyboardKeyEvent*>(nxtEvent))->GetKey() fails with the same error as well.

EDIT:

After some tweaking, this variant works perfectly:

if(_eh->HasEvents()) {
    switch(_eh->PeekEvent()->GetType()) {
    case Event::KEYBOARD_KEYDOWN:
        allegro_message("You pressed the %s key!", scancode_to_name(dynamic_cast<KeyboardKeyDownEvent*>(_eh->PeekEvent())->GetKey()));
        break;
    case Event::MOUSE_BUTTONDOWN:{
        Mouse::BUTTONS btn = dynamic_cast<MouseButtonDownEvent*>(_eh->PeekEvent())->GetButton();
        if(btn == Mouse::BUTTON2) {
            allegro_message("You pressed the %d button!", dynamic_cast<MouseButtonDownEvent*>(_eh->PeekEvent())->GetButton());
        }
                                 }
        break;
        default:
            /* DO NOTHING */;
    }
}

This can't be your real code. The prototype of `GetKey` and your definition don't match. This shouldn't compile. — pmr, Jul 28 '11 at 17:39

Nawaz · Accepted Answer · 2011-07-28T17:45:48.073

1

One solution to avoid slicing is to make the destructor of base class virtual, so in your case you can make ~Event() virtual:

class Event
{
 public:
    //...    
    virtual ~Event() {}
};

By the way, I'm wondering why you do the following:

//YOUR CODE : its causing the problem!
Event* EventHandler::DequeueEvent() {
    if(this->_eventQueue.empty() == false) {
        Event* result = new Event(*this->_eventQueue.front()); // WHY?
        delete this->_eventQueue.front();  //WHY?
        this->_eventQueue.pop();
        return result;
    }
    return NULL;
}

Why don't you simply do this:

//Use it. Because it should not cause that probem
Event* EventHandler::DequeueEvent() {
    if(this->_eventQueue.empty() == false) {
        Event* result = this->_eventQueue.front();
        this->_eventQueue.pop();
        return result;
    }
    return NULL;
}

edited Jul 28 '11 at 17:45

answered Jul 28 '11 at 17:38

Nawaz

353,942
115
666
851

Your solution worked. As to your question, I have no idea why I was attempting to returning a copy. It violates the definition of a queue when that is done. Though, how would I prevent a leak when Events are declared like so: `_handler->EnqueueEvent(new KeyboardKeyDownEvent(KEY_A))` ? – Casey Jul 28 '11 at 18:01
@Casey: After calling `DequeueEvent`, you must be doing something with the returned `Event*`, right? So you're done with the event, and you don't need that anymore, just write `delete event;`, it will deallocate the memory. You don't need to worry about the actual type of `event`. By making `~Event()` virtual, everything would work just fine! – Nawaz Jul 28 '11 at 18:07
What does a virtual destructor have to do with slicing? – UncleBens Jul 28 '11 at 21:09
Besides, I never create a destructor that *isn't* virtual – Casey Jul 30 '11 at 01:31

score 1 · Answer 2 · answered Jul 28 '11 at 17:41

In Event* EventHandler::DequeueEvent() you have line Event* result = new Event(*this->_eventQueue.front()); Here the slicing occurs. You can do the following:

class Event {
 public:
 virtual Event* clone() {
  // create a new instance and copy all the fields
}

}

Then override clone() in derived classes, e.g.

class KeyboardKeyEvent :public Event {
 public: 
 ... 
 virtual KeyboardKeyEvent* clone(); // note - it returns different type
}

Then change Event* EventHandler::DequeueEvent() : Event* result = (*this->_eventQueue.front()).clone();

score 1 · Answer 3 · answered Jul 28 '11 at 17:41

Your DequeueEvent method will always return an Event object, not any of the sub-classes that you are expecting.

    Event* result = new Event(*this->_eventQueue.front());

Your Dequeue event should either return the actual reference it is caching, or your base Event class need to provide some sort of virtual copy operation that will provide a real clone.

score 1 · Answer 4 · answered Jul 28 '11 at 17:43

Why are you copying the Event when you remove it from the queue? That's what is doing the slicing, since you're constructing the base class. Instead, return the pointer that was on the queue to the user.

As noted above, Event should have a virtual ~Event(), so that the recipient of the event can delete it properly. Otherwise, the concrete class destructor will not be properly run.

Event* EventHandler::DequeueEvent() {
    if(this->_eventQueue.empty() == false) {
        Event* result = this->_eventQueue.front();
        this->_eventQueue.pop();
        return result;
    }
    return NULL;
}

Unhandled Exception due to Class slicing

4 Answers4