From e433e70d3dd015db5124ee72085e758635260168 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Remko=20Tron=C3=A7on?= <git@el-tramo.be>
Date: Thu, 7 Oct 2010 20:35:10 +0200
Subject: Avoid recursive calling of event callbacks.

When EventLoop::handleEvent() was called recursively (i.e. by calling
processEvents() from a slot), weird things happened, especially in the
XMPP parser (assertion triggers, parse error from server, ...). Now, callbacks
are put in a queue handled by the topmost handleEvent.

Resolves: #592, #568

diff --git a/Swiften/EventLoop/EventLoop.cpp b/Swiften/EventLoop/EventLoop.cpp
index 25dd19a..2e9e021 100644
--- a/Swiften/EventLoop/EventLoop.cpp
+++ b/Swiften/EventLoop/EventLoop.cpp
@@ -8,12 +8,13 @@
 
 #include <algorithm>
 #include <boost/bind.hpp>
+#include <iostream>
 
 #include "Swiften/EventLoop/MainEventLoop.h"
 
 namespace Swift {
 
-EventLoop::EventLoop() : nextEventID_(0) {
+EventLoop::EventLoop() : nextEventID_(0), handlingEvents_(false) {
 	MainEventLoop::setInstance(this);
 }
 
@@ -22,6 +23,13 @@ EventLoop::~EventLoop() {
 }
 
 void EventLoop::handleEvent(const Event& event) {
+	if (handlingEvents_) {
+		// We're being called recursively. Push in the list of events to
+		// handle in the parent handleEvent()
+		eventsToHandle_.push_back(event);
+		return;
+	}
+
 	bool doCallback = false;
 	{
 		boost::lock_guard<boost::mutex> lock(eventsMutex_);
@@ -32,7 +40,16 @@ void EventLoop::handleEvent(const Event& event) {
 		}
 	}
 	if (doCallback) {
+		handlingEvents_ = true;
 		event.callback();
+		// Process events that were passed to handleEvent during the callback
+		// (i.e. through recursive calls of handleEvent)
+		while (!eventsToHandle_.empty()) {
+			Event nextEvent = eventsToHandle_.front();
+			eventsToHandle_.pop_front();
+			nextEvent.callback();
+		}
+		handlingEvents_ = false;
 	}
 }
 
diff --git a/Swiften/EventLoop/EventLoop.h b/Swiften/EventLoop/EventLoop.h
index efc68ea..ab58ffc 100644
--- a/Swiften/EventLoop/EventLoop.h
+++ b/Swiften/EventLoop/EventLoop.h
@@ -9,6 +9,7 @@
 #include <boost/function.hpp>
 #include <boost/thread/mutex.hpp>
 #include <list>
+#include <deque>
 
 #include "Swiften/EventLoop/Event.h"
 
@@ -40,5 +41,7 @@ namespace Swift {
 			boost::mutex eventsMutex_;
 			unsigned int nextEventID_;
 			std::list<Event> events_;
+			bool handlingEvents_;
+			std::deque<Event> eventsToHandle_;
 	};
 }
diff --git a/Swiften/EventLoop/UnitTest/EventLoopTest.cpp b/Swiften/EventLoop/UnitTest/EventLoopTest.cpp
index b6023ff..b777c1b 100644
--- a/Swiften/EventLoop/UnitTest/EventLoopTest.cpp
+++ b/Swiften/EventLoop/UnitTest/EventLoopTest.cpp
@@ -11,20 +11,19 @@
 
 #include "Swiften/EventLoop/EventOwner.h"
 #include "Swiften/EventLoop/SimpleEventLoop.h"
+#include "Swiften/EventLoop/DummyEventLoop.h"
 #include "Swiften/Base/sleep.h"
 
 using namespace Swift;
 
-class EventLoopTest : public CppUnit::TestFixture
-{
+class EventLoopTest : public CppUnit::TestFixture {
 		CPPUNIT_TEST_SUITE(EventLoopTest);
 		CPPUNIT_TEST(testPost);
 		CPPUNIT_TEST(testRemove);
+		CPPUNIT_TEST(testHandleEvent_Recursive);
 		CPPUNIT_TEST_SUITE_END();
 
 	public:
-		EventLoopTest() {}
-
 		void setUp() {
 			events_.clear();
 		}
@@ -59,12 +58,30 @@ class EventLoopTest : public CppUnit::TestFixture
 			CPPUNIT_ASSERT_EQUAL(1, events_[0]);
 			CPPUNIT_ASSERT_EQUAL(3, events_[1]);
 		}
+
+		void testHandleEvent_Recursive() {
+			DummyEventLoop testling;
+			boost::shared_ptr<MyEventOwner> eventOwner(new MyEventOwner());
+
+			testling.postEvent(boost::bind(&EventLoopTest::runEventLoop, this, &testling, eventOwner), eventOwner);
+			testling.postEvent(boost::bind(&EventLoopTest::logEvent, this, 0), eventOwner);
+			testling.processEvents();
+
+			CPPUNIT_ASSERT_EQUAL(2, static_cast<int>(events_.size()));
+			CPPUNIT_ASSERT_EQUAL(0, events_[0]);
+			CPPUNIT_ASSERT_EQUAL(1, events_[1]);
+		}
 	
 	private:
 		struct MyEventOwner : public EventOwner {};
 		void logEvent(int i) {
 			events_.push_back(i);
 		}
+		void runEventLoop(DummyEventLoop* loop, boost::shared_ptr<MyEventOwner> eventOwner) {
+			loop->processEvents();
+			CPPUNIT_ASSERT_EQUAL(0, static_cast<int>(events_.size()));
+			loop->postEvent(boost::bind(&EventLoopTest::logEvent, this, 1), eventOwner);
+		}
 
 	private:
 		std::vector<int> events_;
-- 
cgit v0.10.2-6-g49f6