Fix memory leak in GHOST Event Manager.

The events are allocated on the heap, then pushed on a stack. Before
being processed, they are popped from the stack, and deleted after
processing is done. When the manager is destroyed (e.g. application
closing), any remaining event in the stack is detroyed.

Issue is that when the "application closing" event is processed, it is
never freed, because the manager gets destroyed before the call to
`delete` is made and the event is not on the stack anymore.

Now events are left on the stack while they are processed, and only
popped and deleted after processing is done.

As a slight bonus refactor: use void as return type for dispatch events
functions, as no caller is checking the return value, and it is not
clear what it means (suggested by the reviewer).

Reviewers: brecht

Differential Revision: https://developer.blender.org/D1695

7 files changed, 29 insertions, 87 deletions

diff --git a/intern/ghost/GHOST_C-api.h b/intern/ghost/GHOST_C-api.h
index ff0dc575005..f1484a298d3 100644
--- a/intern/ghost/GHOST_C-api.h
+++ b/intern/ghost/GHOST_C-api.h
@@ -263,9 +263,8 @@ extern int GHOST_ProcessEvents(GHOST_SystemHandle systemhandle, int waitForEvent
 /**
  * Retrieves events from the queue and send them to the event consumers.
  * \param systemhandle The handle to the system
- * \return Indication of the presence of events.
  */
-extern int GHOST_DispatchEvents(GHOST_SystemHandle systemhandle);
+extern void GHOST_DispatchEvents(GHOST_SystemHandle systemhandle);
 
 /**
  * Adds the given event consumer to our list.
diff --git a/intern/ghost/GHOST_ISystem.h b/intern/ghost/GHOST_ISystem.h
index 6825e8a0384..4c48473c7b8 100644
--- a/intern/ghost/GHOST_ISystem.h
+++ b/intern/ghost/GHOST_ISystem.h
@@ -319,9 +319,8 @@ public:
 
 	/**
 	 * Retrieves events from the queue and send them to the event consumers.
-	 * \return Indication of the presence of events.
 	 */
-	virtual bool dispatchEvents() = 0;
+	virtual void dispatchEvents() = 0;
 
 	/**
 	 * Adds the given event consumer to our list.
diff --git a/intern/ghost/intern/GHOST_C-api.cpp b/intern/ghost/intern/GHOST_C-api.cpp
index 9baa66abad9..ccd7f57f9a4 100644
--- a/intern/ghost/intern/GHOST_C-api.cpp
+++ b/intern/ghost/intern/GHOST_C-api.cpp
@@ -230,11 +230,11 @@ int GHOST_ProcessEvents(GHOST_SystemHandle systemhandle, int waitForEvent)
 
 
 
-int GHOST_DispatchEvents(GHOST_SystemHandle systemhandle)
+void GHOST_DispatchEvents(GHOST_SystemHandle systemhandle)
 {
 	GHOST_ISystem *system = (GHOST_ISystem *) systemhandle;
 	
-	return (int) system->dispatchEvents();
+	system->dispatchEvents();
 }
 
 
diff --git a/intern/ghost/intern/GHOST_EventManager.cpp b/intern/ghost/intern/GHOST_EventManager.cpp
index 7dc8b8a19bf..44b57d11a9f 100644
--- a/intern/ghost/intern/GHOST_EventManager.cpp
+++ b/intern/ghost/intern/GHOST_EventManager.cpp
@@ -78,16 +78,6 @@ GHOST_TUns32 GHOST_EventManager::getNumEvents(GHOST_TEventType type)
 }
 
 
-GHOST_IEvent *GHOST_EventManager::peekEvent()
-{
-	GHOST_IEvent *event = NULL;
-	if (m_events.empty() == false) {
-		event = m_events.back();
-	}
-	return event;
-}
-
-
 GHOST_TSuccess GHOST_EventManager::pushEvent(GHOST_IEvent *event)
 {
 	GHOST_TSuccess success;
@@ -103,52 +93,36 @@ GHOST_TSuccess GHOST_EventManager::pushEvent(GHOST_IEvent *event)
 }
 
 
-bool GHOST_EventManager::dispatchEvent(GHOST_IEvent *event)
+void GHOST_EventManager::dispatchEvent(GHOST_IEvent *event)
 {
-	bool handled;
-	if (event) {
-		handled = true;
-		TConsumerVector::iterator iter;
-		for (iter = m_consumers.begin(); iter != m_consumers.end(); ++iter) {
-			if ((*iter)->processEvent(event)) {
-				handled = false;
-			}
-		}
-	}
-	else {
-		handled = false;
+	TConsumerVector::iterator iter;
+
+	for (iter = m_consumers.begin(); iter != m_consumers.end(); ++iter) {
+		(*iter)->processEvent(event);
 	}
-	return handled;
 }
 
 
-bool GHOST_EventManager::dispatchEvent()
+void GHOST_EventManager::dispatchEvent()
 {
-	GHOST_IEvent *event = popEvent();
-	bool handled = false;
-	if (event) {
-		handled = dispatchEvent(event);
-		delete event;
-	}
-	return handled;
+	GHOST_IEvent *event = m_events.back();
+
+	dispatchEvent(event);
+
+	m_events.pop_back();
+	delete event;
 }
 
 
-bool GHOST_EventManager::dispatchEvents()
+void GHOST_EventManager::dispatchEvents()
 {
-	bool handled;
-	if (getNumEvents()) {
-		handled = true;
-		while (getNumEvents()) {
-			if (!dispatchEvent()) {
-				handled = false;
-			}
-		}
+	if (m_events.empty()) {
+		return;
 	}
-	else {
-		handled = false;
+
+	while (!m_events.empty()) {
+		dispatchEvent();
 	}
-	return handled;
 }
 
 
@@ -241,16 +215,6 @@ void GHOST_EventManager::removeTypeEvents(GHOST_TEventType type, GHOST_IWindow *
 }
 
 
-GHOST_IEvent *GHOST_EventManager::popEvent()
-{
-	GHOST_IEvent *event = peekEvent();
-	if (event) {
-		m_events.pop_back();
-	}
-	return event;
-}
-
-
 void GHOST_EventManager::disposeEvents()
 {
 	while (m_events.empty() == false) {
diff --git a/intern/ghost/intern/GHOST_EventManager.h b/intern/ghost/intern/GHOST_EventManager.h
index c8b5d1debe5..958fc5f9310 100644
--- a/intern/ghost/intern/GHOST_EventManager.h
+++ b/intern/ghost/intern/GHOST_EventManager.h
@@ -74,13 +74,6 @@ public:
 	GHOST_TUns32 getNumEvents(GHOST_TEventType type);
 
 	/**
-	 * Return the event at the top of the stack without removal.
-	 * Do not delete the event!
-	 * \return The event at the top of the stack.
-	 */
-	GHOST_IEvent *peekEvent();
-
-	/**
 	 * Pushes an event on the stack.
 	 * To dispatch it, call dispatchEvent() or dispatchEvents().
 	 * Do not delete the event!
@@ -90,23 +83,20 @@ public:
 
 	/**
 	 * Dispatches the given event directly, bypassing the event stack.
-	 * \return Indication as to whether any of the consumers handled the event.
 	 */
-	bool dispatchEvent(GHOST_IEvent *event);
+	void dispatchEvent(GHOST_IEvent *event);
 
 	/**
 	 * Dispatches the event at the back of the stack.
 	 * The event will be removed from the stack.
-	 * \return Indication as to whether any of the consumers handled the event.
 	 */
-	bool dispatchEvent();
+	void dispatchEvent();
 
 	/**
 	 * Dispatches all the events on the stack.
 	 * The event stack will be empty afterwards.
-	 * \return Indication as to whether any of the consumers handled the events.
 	 */
-	bool dispatchEvents();
+	void dispatchEvents();
 
 	/**
 	 * Adds a consumer to the list of event consumers.
@@ -145,12 +135,6 @@ public:
 	    );
 
 protected:
-	/**
-	 * Returns the event at the top of the stack and removes it.
-	 * Delete the event after use!
-	 * \return The event at the top of the stack.
-	 */
-	GHOST_IEvent *popEvent();
 
 	/**
 	 * Removes all events from the stack.
diff --git a/intern/ghost/intern/GHOST_System.cpp b/intern/ghost/intern/GHOST_System.cpp
index 1698d2f0f31..0f5c822f4b6 100644
--- a/intern/ghost/intern/GHOST_System.cpp
+++ b/intern/ghost/intern/GHOST_System.cpp
@@ -214,23 +214,20 @@ bool GHOST_System::getFullScreen(void)
 }
 
 
-bool GHOST_System::dispatchEvents()
+void GHOST_System::dispatchEvents()
 {
-	bool handled = false;
-
 #ifdef WITH_INPUT_NDOF
 	// NDOF Motion event is sent only once per dispatch, so do it now:
 	if (m_ndofManager) {
-		handled |= m_ndofManager->sendMotionEvent();
+		m_ndofManager->sendMotionEvent();
 	}
 #endif
 
 	if (m_eventManager) {
-		handled |= m_eventManager->dispatchEvents();
+		m_eventManager->dispatchEvents();
 	}
 
 	m_timerManager->fireTimers(getMilliSeconds());
-	return handled;
 }
 
 
diff --git a/intern/ghost/intern/GHOST_System.h b/intern/ghost/intern/GHOST_System.h
index 9dcba11527c..c4951adb4fd 100644
--- a/intern/ghost/intern/GHOST_System.h
+++ b/intern/ghost/intern/GHOST_System.h
@@ -190,9 +190,8 @@ public:
 	/**
 	 * Dispatches all the events on the stack.
 	 * The event stack will be empty afterwards.
-	 * \return Indication as to whether any of the consumers handled the events.
 	 */
-	bool dispatchEvents();
+	void dispatchEvents();
 
 	/**
 	 * Adds the given event consumer to our list.