0

So I've been cleaning up a bit of code, and I noticed that the desctructor of a class was being called directly after the constructer is called. Effectively, the object does nothing. Im pretty sure the object is still in scope, because I can access its members still. In the constructer I've printed out this and in the destructor I've printed out "deleted: " << this. Here is what the output looks like:

x7fff5fbff380
0x7fff5fbff3d0
deleted: 0x7fff5fbff3d0
deleted: 0x7fff5fbff380
0x7fff5fbff280
0x7fff5fbff2d0
deleted: 0x7fff5fbff2d0
deleted: 0x7fff5fbff280
0x7fff5fbff190
0x7fff5fbff1e0
deleted: 0x7fff5fbff1e0
deleted: 0x7fff5fbff190

Obviously, this isn't enough to help solve the problem, so here is some code, involving how the object is created, how it is used and how it is destroyed.

//event listener constructor
EventListener::EventListener(EventTypes typeEvent,EventFunction functionPointer)
    {
        this->typeEvent = typeEvent;
        this->functionPointer = functionPointer;
        //add it to the tick handler
        this->listenerID = EngineEventDispacher.addEventListener(this);

        std::cout << this << std::endl;
    }
void EventListener::removeListener()
    {
        //remove it from the tickHandler
        EngineEventDispacher.removeEventListener(this->listenerID);
    }

//we add the event listener here
int EventDispatcher::addEventListener(EventListener* listener)
    {
        EventListeners.push_back(listener);
        return (int)EventListeners.size() - 1;
    }
//get rid of a listener
void EventDispatcher::removeEventListener(int id)
    {

        //std::vector<EventListener*>::iterator it;
        //it = EventListeners.begin() + id;
        //EventListeners.erase(it);
       // EventListeners.shrink_to_fit();

        //this isnt very memory efficiant, but it is the best solution for the CPU
        EventListeners[id] = nullptr;
    }
//send an event to all the listeners that can have it
void EventDispatcher::dispatchEvent(EventTypes eventType, Event* event)
    {
        for (int i = 0; i < EventListeners.size(); i++)
            {
                //we check if the current listener is subscribed to the event we are calling
                if (EventListeners[i] != nullptr)
                    if (EventListeners[i]->typeEvent == eventType && EventListeners[i]->functionPointer != 0 )
                        {
                            //it was subscribed, so we are going to call it
                                EventListeners[i]->functionPointer(event);
                        }
            }
    }

//make sure that we can't call this
EventListener::~EventListener()
    {
        EngineEventDispacher.removeEventListener(this->listenerID);
        std::cout << "deleted: " << this << std::endl;
    }

What the classes look like:

//This will recive events
class EventListener
    {
        //this is what type of event it will repsond to
    public:
        EventTypes typeEvent;
        EventListener(EventTypes typeEvent, EventFunction);
        EventListener();
        ~EventListener();
        EventFunction functionPointer;
        void removeListener();
    private:
        int listenerID;
};

//her we define the event dispatcher
class EventDispatcher
    {
    public:
        int addEventListener(EventListener*);
        void removeEventListener(int);
        void dispatchEvent(EventTypes, Event*);

    private:
        std::vector<EventListener*>EventListeners;
    };

And finally how the event listener is declared and constructed:

class Scene
    {
        public:

            Scene();

            std::vector<StaticGeometry>GameObjects;
            void addStaticGeometry(StaticGeometry object);
            void renderSceneWithCamera(camera cam);
            void renderSceneWithCameraAndProgram(camera cam,GLuint program);

            void pickObjectFromScene();
            void pickObjectFromSceneWithScreenCoords(int x, int y);
            int selectedObject;


        private:
                //listen for the left click
                EventListener leftClickEventListener;
                void leftClick(Event* eventPtr);

    };

Scene::Scene() : leftClickEventListener(EventTypeLeftMouseClick,std::bind(&Scene::leftClick,this,std::placeholders::_1))
    {
        //default constructor, we just need to make sure that the selected thing is -1
        selectedObject = -1;
    }

As far as I know, members aren't supposed to call the deconstructor until the parent calls theirs. The Scene class most definitely isn't calling its reconstructor, and thats what really has me puzzled. Everything should be fine, but its not. Nothing I've found says that things should just randomly decide to deconstruct themselves. Any help would be appreciated. Thanks.

BlueSpud
  • 1,555
  • 3
  • 19
  • 44
  • You've included everything except the kitchen sink, where's the code that creates an `EventListener` / `Scene` ? – user657267 Jul 23 '14 at 09:06
  • @user657267 Its in the constructer of Scene. If you meant declared, its in the Scene as well. – BlueSpud Jul 23 '14 at 09:07
  • How do you create a Scene? – Paolo Brandoli Jul 23 '14 at 09:20
  • @PaoloBrandoli its just a member of another class. – BlueSpud Jul 23 '14 at 09:21
  • @BlueSpud And is the destructor of the Scene class being called when you least expect it? – Paolo Brandoli Jul 23 '14 at 09:40
  • @PaoloBrandoli No, the destructor of the event listener is being called when I least expect it, Literally right after its constructed. – BlueSpud Jul 23 '14 at 09:41
  • @Medinoc No, there isn't an exception being thrown as far as I know. – BlueSpud Jul 23 '14 at 09:45
  • 1
    You don't happen to create your EventListeners with *function* scope? In that case, the object would be destroyed as soon as the function returns even though it may still be referenced from elsewhere (- which it shouldn't in the first place). See also: http://stackoverflow.com/a/6337327/1015327 – JimmyB Jul 23 '14 at 09:50
  • @BlueSpud You could simply put a breakpoint in the destructor and when it is hit, look at the call stack. It isn't magic, just debug the issue using a debugger (forego the primitive print statements). – PaulMcKenzie Jul 23 '14 at 09:51
  • 1
    @BlueSpud - Also, it seems a much better design would have been to use a `std::map` or better yet, a `std::map>` instead of std::vector to hold all of your listeners. Half that code you wrote wouldn't need to be written. – PaulMcKenzie Jul 23 '14 at 09:58
  • @PaulMcKenzie Never really messed around with maps. I'll try that out after I solve this problem. – BlueSpud Jul 23 '14 at 10:00
  • @BlueSpud you need to post a small, but complete `main` example that duplicates the error. Classes don't do anything until we see the code that uses them. – PaulMcKenzie Jul 23 '14 at 10:02
  • @HannoBinder was correct. The problem was not having a function scope. The desctructor for the Scene was being called. Adding in a function scope fixed the problem. If you post an answer, I'll mark it as correct. – BlueSpud Jul 23 '14 at 10:08

1 Answers1

1

Problem:

If you create an object inside a block or function with automatic storage duration, like

{
  // ...
  EventListener myListener();
  // ...
}

the object will be destroyed as soon as execution leaves the block/function, even though it may still be referenced from elsewhere. See also:

Creating an object: with or without `new`

Generally, you should never pass a pointer to an object with such scope anywhere where it might be stored internally.

Solution:

You'll have to explicitly use new if you want your object to live beyond the current block:

{
  // ...
  EventListener* myListener = new EventListener();
  // ...
}

Im pretty sure the object is still in scope, because I can access its members still.

Beware: A pointer to an object may still seem to be usable even after the object has been (implicitly) destroyed, but dereferencing this pointer is a severe, though not always obvious, bug.

Community
  • 1
  • 1
JimmyB
  • 12,101
  • 2
  • 28
  • 44