From 26524edbf42b31666b507d10a3c54c3f419f36cf Mon Sep 17 00:00:00 2001
From: Norman Feske <norman.feske@genode-labs.com>
Date: Thu, 13 Aug 2015 22:53:03 +0200
Subject: [PATCH] alarm: reposition reprogrammed alarms in queue

The alarm library failed to handle the case properly where an already
scheduled alarm gets rescheduled before it triggered. Even though the
attempt to reschedule the alarm (twice insertion into alarm queue) was
detected, this condition resulted in the mere modification of the
alarm's parameters while keeping the alarm's queue position unchanged.
This, in turn, may violate the invariant that all enqueued alarm objects
are strictly ordered by their deadlines. The patch handles the case by
dequeuing the alarm object before reinserting it into the queue at the
right position.

Fixes #1646
---
 repos/os/include/os/alarm.h     |  5 +++++
 repos/os/src/lib/alarm/alarm.cc | 38 +++++++++++++++++++++++++++------
 2 files changed, 37 insertions(+), 6 deletions(-)

diff --git a/repos/os/include/os/alarm.h b/repos/os/include/os/alarm.h
index 0bfcf1aa0..717d6e9e2 100644
--- a/repos/os/include/os/alarm.h
+++ b/repos/os/include/os/alarm.h
@@ -92,6 +92,11 @@ class Genode::Alarm_scheduler
 		 */
 		Alarm *_get_pending_alarm();
 
+		/**
+		 * Assign timeout values to alarm object and add it to the schedule
+		 */
+		void _setup_alarm(Alarm &alarm, Alarm::Time period, Alarm::Time deadline);
+
 	public:
 
 		Alarm_scheduler() : _head(0), _now(0) { }
diff --git a/repos/os/src/lib/alarm/alarm.cc b/repos/os/src/lib/alarm/alarm.cc
index c8f2b64e9..ef4c7364c 100644
--- a/repos/os/src/lib/alarm/alarm.cc
+++ b/repos/os/src/lib/alarm/alarm.cc
@@ -19,9 +19,10 @@ using namespace Genode;
 
 void Alarm_scheduler::_unsynchronized_enqueue(Alarm *alarm)
 {
-	/* do not enqueue twice */
-	if (alarm->_active)
+	if (alarm->_active) {
+		PERR("trying to insert the same alarm twice!");
 		return;
+	}
 
 	alarm->_active++;
 
@@ -146,12 +147,27 @@ void Alarm_scheduler::handle(Alarm::Time curr_time)
 }
 
 
+void Alarm_scheduler::_setup_alarm(Alarm &alarm, Alarm::Time period, Alarm::Time deadline)
+{
+	/*
+	 * If the alarm is already present in the queue, re-consider its queue
+	 * position because its deadline might have changed. I.e., if an alarm is
+	 * rescheduled with a new timeout before the original timeout triggered.
+	 */
+	if (alarm._active)
+		_unsynchronized_dequeue(&alarm);
+
+	alarm._assign(period, deadline, this);
+
+	_unsynchronized_enqueue(&alarm);
+}
+
+
 void Alarm_scheduler::schedule_absolute(Alarm *alarm, Alarm::Time timeout)
 {
 	Lock::Guard alarm_list_lock_guard(_lock);
 
-	alarm->_assign(0, timeout, this);
-	_unsynchronized_enqueue(alarm);
+	_setup_alarm(*alarm, 0, timeout);
 }
 
 
@@ -159,9 +175,19 @@ void Alarm_scheduler::schedule(Alarm *alarm, Alarm::Time period)
 {
 	Lock::Guard alarm_list_lock_guard(_lock);
 
+	/*
+	 * Refuse to schedule a periodic timeout of 0 because it would trigger
+	 * infinitely in the 'handle' function. To account for the case where the
+	 * alarm object was already scheduled, we make sure to remove it from the
+	 * queue.
+	 */
+	if (period == 0) {
+		_unsynchronized_dequeue(alarm);
+		return;
+	}
+
 	/* first deadline is overdue */
-	alarm->_assign(period, _now, this);
-	_unsynchronized_enqueue(alarm);
+	_setup_alarm(*alarm, period, _now);
 }