From 2335a62a075b551e6bd1bca2bdc02be605bc6987 Mon Sep 17 00:00:00 2001
From: Robin Sommer <robin@icir.org>
Date: Tue, 14 Jun 2016 18:10:37 -0700
Subject: [PATCH] Preventing the event processing from looping endlessly when
 an event reraised itself during execution of its handlers.

---
 CHANGES                                       |  6 +++
 VERSION                                       |  2 +-
 src/Event.cc                                  | 50 +++++++++++--------
 src/Event.h                                   |  2 -
 .../Baseline/core.recursive-event/output      |  1 +
 testing/btest/core/recursive-event.bro        | 31 ++++++++++++
 6 files changed, 67 insertions(+), 25 deletions(-)
 create mode 100644 testing/btest/Baseline/core.recursive-event/output
 create mode 100644 testing/btest/core/recursive-event.bro

diff --git a/CHANGES b/CHANGES
index 94ba4ddf4e..d702d85b3f 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,4 +1,10 @@
 
+2.4-613 | 2016-06-14 18:10:37 -0700
+
+  * Preventing the event processing from looping endlessly when an
+    event reraised itself during execution of its handlers. (Robin
+    Sommer)
+
 2.4-612 | 2016-06-14 17:42:52 -0700
 
   * Improved handling of 802.11 headers. (Jan Grashoefer)
diff --git a/VERSION b/VERSION
index 4f4e5c1be1..ac2ea9d11f 100644
--- a/VERSION
+++ b/VERSION
@@ -1 +1 @@
-2.4-612
+2.4-613
diff --git a/src/Event.cc b/src/Event.cc
index 5d54752a5a..6371a69248 100644
--- a/src/Event.cc
+++ b/src/Event.cc
@@ -94,26 +94,6 @@ void EventMgr::QueueEvent(Event* event)
 	++num_events_queued;
 	}
 
-void EventMgr::Dispatch()
-	{
-	if ( ! head )
-		reporter->InternalError("EventMgr::Dispatch underflow");
-
-	Event* current = head;
-
-	head = head->NextEvent();
-	if ( ! head )
-		tail = head;
-
-	current_src = current->Source();
-	current_mgr = current->Mgr();
-	current_aid = current->Analyzer();
-	current->Dispatch();
-	Unref(current);
-
-	++num_events_dispatched;
-	}
-
 void EventMgr::Drain()
 	{
 	if ( event_queue_flush_point )
@@ -124,8 +104,34 @@ void EventMgr::Drain()
 	PLUGIN_HOOK_VOID(HOOK_DRAIN_EVENTS, HookDrainEvents());
 
 	draining = true;
-	while ( head )
-		Dispatch();
+
+	// Past Bro versions drained as long as there events, including when
+	// a handler queued new events during its execution. This could lead
+	// to endless loops in case a handler kept triggering its own event.
+	// We now limit this to just a couple of rounds. We do more than
+	// just one round to make it less likley to break existing scripts
+	// that expect the old behavior to trigger something quickly.
+
+	for ( int round = 0; head && round < 2; round++ )
+		{
+		Event* current = head;
+		head = 0;
+		tail = 0;
+
+		while ( current )
+			{
+			Event* next = current->NextEvent();
+
+			current_src = current->Source();
+			current_mgr = current->Mgr();
+			current_aid = current->Analyzer();
+			current->Dispatch();
+			Unref(current);
+
+			++num_events_dispatched;
+			current = next;
+			}
+		}
 
 	// Note: we might eventually need a general way to specify things to
 	// do after draining events.
diff --git a/src/Event.h b/src/Event.h
index 0d004d526c..1b76928f10 100644
--- a/src/Event.h
+++ b/src/Event.h
@@ -90,8 +90,6 @@ public:
 			delete_vals(vl);
 		}
 
-	void Dispatch();
-
 	void Dispatch(Event* event, bool no_remote = false)
 		{
 		current_src = event->Source();
diff --git a/testing/btest/Baseline/core.recursive-event/output b/testing/btest/Baseline/core.recursive-event/output
new file mode 100644
index 0000000000..f599e28b8a
--- /dev/null
+++ b/testing/btest/Baseline/core.recursive-event/output
@@ -0,0 +1 @@
+10
diff --git a/testing/btest/core/recursive-event.bro b/testing/btest/core/recursive-event.bro
new file mode 100644
index 0000000000..cc7a2be8eb
--- /dev/null
+++ b/testing/btest/core/recursive-event.bro
@@ -0,0 +1,31 @@
+# @TEST-EXEC: bro %INPUT 2>&1 | grep -v termination | sort | uniq | wc -l >output
+# @TEST-EXEC: btest-diff output
+
+# In old version, the event would keep triggering endlessely, with the network
+# time not moving forward and Bro not terminating. 
+# 
+# Note that the output will be 10 (not 20) because we still execute two rounds
+# of events every time we drain.
+
+redef exit_only_after_terminate=T;
+
+global c = 0;
+
+event test()
+        {
+	c += 1;
+
+	if ( c == 20 ) 
+		{
+		terminate();
+		return;
+		}
+	
+        print network_time();
+        event test();
+        }
+
+event bro_init()
+        {
+        event test();
+        }