Support deallocating kernel timers in timer handlers.

kernel/clock.cpp

@@ -235,6 +235,17 @@ void Clock::Cancel(Timer* timer)
 	UnlockClock();
 }
 
+bool Clock::TryCancel(Timer* timer)
+{
+	LockClock();
+	bool active = timer->flags & TIMER_ACTIVE;
+	if ( active )
+		Unlink(timer);
+	UnlockClock();
+	return active;
+}
+
+
 // TODO: We need some method for threads to sleep for real but still be
 //       interrupted by signals.
 struct timespec Clock::SleepDelay(struct timespec duration)
@@ -350,10 +361,16 @@ static void Clock__FireTimer(void* timer_ptr)
 	timer->clock->LockClock();
 	timer->num_overrun_events = timer->num_firings_scheduled;
 	timer->num_firings_scheduled = 0;
+	bool may_deallocate = timer->flags & TIMER_FUNC_MAY_DEALLOCATE_TIMER;
 	timer->clock->UnlockClock();
 
 	Clock__DoFireTimer(timer);
 
+	// The handler may have deallocated the storage for the timer, don't touch
+	// it again.
+	if ( may_deallocate )
+		return;
+
 	// If additional events happened during the time of the event handler, we'll
 	// have to handle them because the firing bit is set. We'll schedule another
 	// worker thread job and resume there, so this worker thread can continue to
@@ -376,6 +393,7 @@ static void Clock__FireTimer_InterruptWorker(void* timer_ptr, void*, size_t)
 void Clock::FireTimer(Timer* timer)
 {
 	timer->flags &= ~TIMER_ACTIVE;
+	bool may_deallocate = timer->flags & TIMER_FUNC_MAY_DEALLOCATE_TIMER;
 
 	// If the CPU is currently interrupted, we call the timer callback directly
 	// only if it is known to work when the interrupts are disabled on this CPU.
@@ -389,7 +407,8 @@ void Clock::FireTimer(Timer* timer)
 			timer->num_firings_scheduled++;
 		else
 		{
-			timer->flags |= TIMER_FIRING;
+			if ( !may_deallocate )
+				timer->flags |= TIMER_FIRING;
 			Interrupt::ScheduleWork(Clock__FireTimer_InterruptWorker, timer, NULL, 0);
 		}
 	}
@@ -405,13 +424,15 @@ void Clock::FireTimer(Timer* timer)
 			timer->num_firings_scheduled++;
 		else
 		{
-			timer->flags |= TIMER_FIRING;
+			if ( !may_deallocate )
+				timer->flags |= TIMER_FIRING;
 			Worker::Schedule(Clock__FireTimer, timer);
 		}
 	}
 
 	// Rearm the timer only if it is periodic.
-	if ( timespec_le(timer->value.it_interval, timespec_nul()) )
+	if ( may_deallocate ||
+	     timespec_le(timer->value.it_interval, timespec_nul()) )
 		return;
 
 	// TODO: If the period is too short (such a single nanosecond) on a delay

kernel/include/sortix/kernel/clock.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2013 Jonas 'Sortie' Termansen.
+ * Copyright (c) 2013, 2016 Jonas 'Sortie' Termansen.
  *
  * Permission to use, copy, modify, and distribute this software for any
  * purpose with or without fee is hereby granted, provided that the above
@@ -55,6 +55,7 @@ public:
 	void Register(Timer* timer);
 	void Unlink(Timer* timer);
 	void Cancel(Timer* timer);
+	bool TryCancel(Timer* timer);
 	void LockClock();
 	void UnlockClock();
 	struct timespec SleepDelay(struct timespec duration);

kernel/include/sortix/kernel/timer.h

@@ -35,6 +35,28 @@ static const int TIMER_ACTIVE = 1 << 1;
 static const int TIMER_FIRING = 1 << 2;
 static const int TIMER_FUNC_INTERRUPT_HANDLER = 1 << 3;
 static const int TIMER_FUNC_ADVANCE_THREAD = 1 << 4;
+// The timer callback may deallocate the timer itself. The timer data structure
+// will not be touched by the timer clock after running the callback and the
+// clock and timer implementation will not have any issues with deallocating it.
+// This feature cannot be combined with periodic timers. The Cancel method may
+// not be called, as it ensures consistency (the timer is cancelled if pending,
+// and if it's firing, then waiting for it to complete). Instead, use TryCancel
+// which will return false if the timer wasn't pending. If the timer has been
+// armed, and the handler has not yet run, that means the handler is scheduled
+// to run and it's not safe to deallocate until the handler runs. It is not
+// possible call the Set method on an armed timer, unless the timer has been
+// successfully TryCancelled, or the handler has run. It's the user's
+// responsibility to ensure deallocation of the timer only happens if no other
+// threads will use the timer data structure. I.e. if some code wants to
+// TryCancel a timer, it must synchronize with the timer handler, so the timer
+// handler doesn't deallocate the timer and then the other thread calls
+// TryCancel on a freed pointer.
+// The object containing the timer could contain a mutex and a bool of whether
+// the timer is armed and the handler has not run. If there's a need to destroy
+// the object, attempt to TryCancel and timer and do so if it succeeds,
+// otherwise delay the destruction until the timer handler, which also grabs the
+// mutex and checks whether object destruction is supposed to happen.
+static const int TIMER_FUNC_MAY_DEALLOCATE_TIMER = 1 << 5;
 
 class Timer
 {
@@ -62,6 +84,7 @@ public:
 	void Detach();
 	bool IsAttached() const { return clock; }
 	void Cancel();
+	bool TryCancel();
 	Clock* GetClock() const { return clock; }
 	void Get(struct itimerspec* current);
 	void Set(struct itimerspec* value, struct itimerspec* ovalue, int flags,

kernel/timer.cpp

@@ -77,6 +77,13 @@ void Timer::Cancel()
 		clock->Cancel(this);
 }
 
+bool Timer::TryCancel()
+{
+	if ( clock )
+		return clock->TryCancel(this);
+	return true;
+}
+
 void Timer::GetInternal(struct itimerspec* current)
 {
 	if ( !(this->flags & TIMER_ACTIVE ) )
@@ -103,6 +110,8 @@ void Timer::Set(struct itimerspec* new_value, struct itimerspec* old_value,
                 void* new_user)
 {
 	assert(clock);
+	assert(!(flags & TIMER_FUNC_MAY_DEALLOCATE_TIMER) ||
+	       timespec_le(new_value->it_interval, timespec_nul()));
 
 	clock->LockClock();