basic ofEvents design question

advanced
#1

I ran into a problem and I’m asking me if there is a bug or something else in OF’s Event System or if I’ve done bad things with ofEvents. :slight_smile:

I’m generating many objects which are all created/destroyed like this:

  
  
Object::Object()  
{  
  ofRegisterMouseEvents(this);  
  // ...  
}  
  
Object::~Object()  
{  
  ofUnregisterMouseEvents(this);  
  // ...  
}

Every object has all the methods like mouseDragged, mouseMoved, … .
All objects are stored in a vector called “objects” in my App. But if I’m running code like this

  
  
void Core::mousePressed(int x, int y, int button)  
{  
objects.erase(objects.begin(),objects.end()); //objects are deleted in this way because they are boost::shared_ptr's  
}

my app crashes without error.

Debugging:
Points me to Delegate.h line 144

  
  
bool notify(const void*, TArgs& arguments)  
	{  
		(_receiverObject->*_receiverMethod)(arguments); // Line 144  
		return true; // a "standard" delegate never expires  
	}  


  
#0 (	0x000000000000038d in ??() (??:??)  
#1 0x437a0a	Poco::Delegate<ds::BaseNode, ofMouseEventArgs, false>::notify(this=0x97baf0, arguments=...) (../openFrameworks/libs/poco/include/Poco/Delegate.h:144)  
#2 0x44b13d	Poco::FIFOStrategy<ofMouseEventArgs, Poco::AbstractDelegate<ofMouseEventArgs>, Poco::p_less<Poco::AbstractDelegate<ofMouseEventArgs> > >::notify(this=0x957d70, sender=0x0, arguments=...) (../../../poco/include/Poco/FIFOStrategy.h:85)  
#3 0x449223	Poco::AbstractEvent<ofMouseEventArgs, Poco::FIFOStrategy<ofMouseEventArgs, Poco::AbstractDelegate<ofMouseEventArgs>, Poco::p_less<Poco::AbstractDelegate<ofMouseEventArgs> > >, Poco::AbstractDelegate<ofMouseEventArgs>, Poco::FastMutex>::notify(this=0x86c518, pSender=0x0, args=...) (../../../poco/include/Poco/AbstractEvent.h:229)  
#4 0x447587	ofNotifyEvent<ofEvent<ofMouseEventArgs>, ofMouseEventArgs>(event=..., args=...) (../../../openFrameworks/events/ofEventUtils.h:106)  
#5 0x447002	ofNotifyMousePressed(x=422, y=296, button=0) (../../../openFrameworks/events/ofEvents.cpp:161)  
#6 0x460f9c	ofAppGlutWindow::mouse_cb(button=0, state=0, x=422, y=296) (../../../openFrameworks/app/ofAppGlutWindow.cpp:595)  
#7 0x7ffff3c21716	glutMainLoopEvent() (/usr/lib/libglut.so.3:??)  
#8 0x7ffff3c21b17	glutMainLoop() (/usr/lib/libglut.so.3:??)  
#9 0x4608b6	ofAppGlutWindow::runAppViaInfiniteLoop(this=0x7fffffffe620, appPtr=0x956250) (../../../openFrameworks/app/ofAppGlutWindow.cpp:303)  
#10 0x461a20	ofRunApp(OFSA=0x956250) (../../../openFrameworks/app/ofAppRunner.cpp:87)  
#11 0x430c87	main() (src/main.cpp:16)

So is it because I’m destroying elements with Listeners just when I’m calling a listener?! Or is it totally dump to put ofRegisterMouseEvents(this); in every object?

thanks for your help!

ben

ofRegisterMouseEvent on multiple instance
#2

the problem can be that events internally store pointers to the objects they need to notify. When you use a vector, there’s a problem with storing pointers to objects contained in it as those pointers can become invalid.

When a vector is resized it needs to maintain the memory contiguous so sometimes it moves all the contents to a different position in memory where there’s enough space and all the pointers become invalid cause the memory addresses change.

You can use an std::list which doesn’t invalidate memory addresses ever. Or just a vector of pointers that way the vector can move the pointers but the objects will remain at the same addresses where they were created

#3

get the same error with a list :frowning:

#4

found a workaround for this.

  
  
void Core::update()  
{  
  if(bDelete)  
  {  
    objects.erase(objects.begin(),objects.end()); //objects are deleted in this way because they are boost::shared_ptr's  
  }  
}  
void Core::mousePressed(int x, int y, int button)  
{  
bDelete = true;  
}

what do you think?

#5

which platform is this in?

#6

arch linux 64bit, latest master from github

#7

oh, ok yes, the event actually makes a copy of the listeners when it’s notified, in order to avoid problems with multithreading so even if you unregister the object the copy of the current notification still have them in it.

which makes me think that it would be good to have a version of the events that allows to pass a smart pointer instead of a raw one.

btw, there’s smart pointers already in 007, ofPtr, the use is exactly the same as boost’s ones

#8

I didn’t realize that’s how the event handling dealt with multi-threading & would love to see an ofPtr version that would handle the deallocation automagically :slight_smile:

#9

yeah this was my assumption that there is something with threads going on :slight_smile:

oh great! didn’t notice that, we need to get up a list “whats new in 007” :slight_smile:

#10

@josh yes, it locks while making the copy and then unlocks and use the copy to notify in order to not lock for too much time, but it seems problematic

@beben yes, kind of, although there’s actually only one thread in this case, the problem is something like this:

  • mousePressed -> notifies event

  • event makes a copy of the vector of listeners and begins to notify them all but using the copy

  • first notification goes to the core which deletes all objects which in turn unregister them as listeners

  • event notifies next listener in the copy of the vector of listeners which still has the original, since they’ve been deleted -> crash

#11

yes, you are right. I’ve done all the tests for this in my app now, but it would be better if ofEvents would do that. I looked in the code of ofEvents but had no idea what to change to provide a fix :frowning:

PS: threads are evil! :wink:

#12

yes, the problem is this code is actually in the base AbstractEvent in poco so we can’t really change it from OF but would need to modify POCO or make a new implementation.

#13

sounds bad. what can we do? file a bug at poco?!