Make Scheme hooks and timers threadsafe

* Timers are now maintained as a sorted list
* Timer callbacks and other threads can call bot:[add|del]timer
* Running hooks presently *cannot* call bot:addhook.

diff --git a/source/BotInterp.C b/source/BotInterp.C
index 0a1419c..9a07be6 100644 (file)
--- a/source/BotInterp.C
+++ b/source/BotInterp.C
@@ -21,6 +21,8 @@
 #include "config.h"
 #endif
 
+#include <algorithm>
+
 #include "Utils.H"
 #include "Bot.H"
 #include "BotInterp.H"
@@ -34,7 +36,7 @@ extern "C"
 }
 
 BotInterp::BotInterp(Bot *b, String fn)
-  : bot(b), counter(0)
+  : bot(b), counter(0), timer_mutex (true)
 {
   logPort = scm_open_file (Utils::str2scm (fn),
                            Utils::str2scm ("a"));
@@ -70,7 +72,8 @@ namespace
 
 bool
 BotInterp::AddHook(int hooktype, SCM regex, SCM function, int pri, bool fall,
-                  String name) {
+                  String name) 
+{
   if (scm_string_p(regex) == SCM_BOOL_F)
     return false;
   String rx = Utils::to_upper (Utils::scm2str (regex));
@@ -80,6 +83,9 @@ BotInterp::AddHook(int hooktype, SCM regex, SCM function, int pri, bool fall,
                                       SCM_UNDEFINED));
   scm_gc_protect_object(r);
   scm_gc_protect_object(function);
+
+  BotLock hook_lock (hook_mutex);
+
   // First, we check if an hook doesn't exist yet
   std::list<Hook *>::iterator it = hooksMap[hooktype].begin();
   std::list<Hook *>::iterator it2 = hooksMap[hooktype].end();
@@ -105,13 +111,18 @@ BotInterp::AddHook(int hooktype, SCM regex, SCM function, int pri, bool fall,
 bool
 BotInterp::RunHooks(int hooktype, String match, SCM args)
 {
+  BotLock hook_lock (hook_mutex);
+
   SCM result;
+
   // We want to execute higher priority hooks first, so we start at
   // the end of the list instead of the beggining
   std::list<Hook *>::reverse_iterator it = hooksMap[hooktype].rbegin();
   std::list<Hook *>::reverse_iterator it2 = hooksMap[hooktype].rend();
+
   wrapper_data wd;
   wd.args = args;
+
   for ( ; it != it2; ++it) {
     if (scm_regexp_exec((*it)->regex, Utils::str2scm (match),
                         SCM_UNDEFINED, SCM_UNDEFINED) != SCM_BOOL_F)
@@ -126,64 +137,101 @@ BotInterp::RunHooks(int hooktype, String match, SCM args)
          break;
     }
   }
+
   return true;
 }
 
 SCM
 BotInterp::AddTimer(int delay, SCM function)
 {
+  BotLock timer_lock (timer_mutex);  
   int when = time(NULL) + delay;
-  int c = ++counter;
+
   scm_gc_protect_object(function);
-  Timer *t = new Timer(c, when, function);
-  timersList.push_back(t);
-  return scm_from_int (c);
+
+  Timer *timer = new Timer (++counter, when, function);
+  TimerList::iterator it = std::find_if (timers.begin (), timers.end (),
+                                        std::bind1st (timer_sort_p, timer));
+  
+  if (it != timers.end ())
+    timers.insert (it, timer);
+  else
+    timers.push_back (timer);
+
+  return scm_from_int (counter);
+
 }
 
 bool
 BotInterp::DelTimer(SCM timer)
 {
-  int count = scm_to_int (timer);
-  std::list<Timer *>::iterator it = timersList.begin();
-  std::list<Timer *>::iterator it2 = timersList.end();
+  BotLock timer_lock (timer_mutex);
 
-  for ( ; it != it2; ++it) {
-    if ((*it)->count == count) {
-      scm_gc_unprotect_object((*it)->function);
-      delete (*it);
-      timersList.erase(it);
-      return true;
+  int count = scm_to_int (timer);
+  TimerList::iterator it = timers.begin();
+  TimerList::iterator end = timers.end();
+
+  for ( ; it != end; ++it) 
+    {
+      if ((*it)->count == count) 
+       {
+         scm_gc_unprotect_object((*it)->function);
+         delete (*it);
+         timers.erase(it);
+
+         return true;
+       }
     }
-  }
+
   return false;
 }
 
 bool
 BotInterp::RunTimers(int now)
 {
-  std::list<Timer *>::iterator it = timersList.begin();
-  std::list<Timer *>::iterator it2 = timersList.end();
-  std::list<Timer *>::iterator it3;
-
+  BotLock timer_lock (timer_mutex);
   struct wrapper_data wd;
   wd.args = scm_list_n (SCM_UNDEFINED);
 
-  while (it != it2) {
-    if ((*it)->when <= now) {
-      wd.func = (*it)->function;
-      scm_internal_catch(SCM_BOOL_T,
-                    (scm_t_catch_body) Interp::LazyApplyWrapper, (void *)&wd,
-                    (scm_t_catch_handler) Interp::EmptyHandler, 0);
-      scm_gc_unprotect_object(wd.func);
-      it3 = it;
-      ++it3;
-      delete (*it);
-      timersList.erase(it);
-      it = it3;
-    } else {
-      ++it;
+  while (!timers.empty ())
+    {
+      // Keep a stack allocated copy of the front of the timer queue
+      // just in case the timer is deleted while being executed (which
+      // is very unlikely as the only place this could occur is if the
+      // timer deleted itself)
+      Timer current_timer = *timers.front () ; 
+
+      if (current_timer.when <= now) 
+       {
+         wd.func = current_timer.function;
+
+         scm_internal_catch (SCM_BOOL_T,
+                             (scm_t_catch_body) Interp::LazyApplyWrapper,
+                             (void *)&wd,
+                             (scm_t_catch_handler) Interp::EmptyHandler, 
+                             0);
+
+         // The timer list may have been modified by the timer
+         // callback; if it has in such a way that the first queue
+         // item has changed (adding a timer in the past) then we
+         // switch the slow path for deleting a timer
+         if (current_timer.count == timers.front()->count)
+           {
+             scm_gc_unprotect_object (current_timer.function);
+             delete timers.front ();
+             timers.pop_front ();
+           }
+         else
+           {
+             DelTimer (scm_from_int (current_timer.count));
+           }
+       } 
+      else 
+       {
+         break;
+       }
     }
-  }
+
   return true;
 }
 

diff --git a/source/BotInterp.H b/source/BotInterp.H
index b0978ae..4929058 100644 (file)
--- a/source/BotInterp.H
+++ b/source/BotInterp.H
@@ -1,6 +1,6 @@
 // BotInterp.H  -*- C++ -*-
 // Copyright (c) 1998 Etienne BERNARD
-// Copyright (C) 2002,2005 Clinton Ebadi
+// Copyright (C) 2002,2005,2008 Clinton Ebadi
 
 // This program is free software; you can redistribute it and/or modify
 // it under the terms of the GNU General Public License as published by
@@ -27,8 +27,15 @@
 #ifdef USESCRIPTS
 
 #include <ctime>
+#include <map>
+#include <list>
+#include <functional>
+
 #include <libguile.h>
+
+#include "BotThreading.H"
 #include "String.H"
+#include "Utils.H"
 
 class Bot;
 
@@ -95,14 +102,22 @@ struct Timer {
 
   Timer(int c, std::time_t t, SCM f)
     : count(c), when(t), function(f) { }
+
+  bool operator< (const Timer & other) const
+  { return when < other.when; }
 };
 
 class BotInterp {
+  typedef std::list<Timer *> TimerList;
+  
   Bot * bot;
   SCM logPort;
   std::map<int, std::list<Hook *>, std::less<int> > hooksMap;
-  std::list<Timer *> timersList;
+  TimerList timers;
+  Utils::IndirectPred<Timer, std::less<Timer> > timer_sort_p;
   int counter;
+  BotMutex hook_mutex;
+  BotMutex timer_mutex; // NOTE: recursive lock
 
 public:
   BotInterp(Bot *, String);