Enhance threadsafety of Hooks and improve generally

* bot:addhook may now be called from within a hook
  - hook mutex is recursive
  - It is unspecified whether a newly added hook will first run on the
    current or next hook match; depending on the properties of the
    hook either may occur
* Use general sorted list utilities instead of pushing to the back and
  doing a full sort on each add
* Incidentally replace String usage with std::string

diff --git a/source/BotInterp.C b/source/BotInterp.C
index 2388cd4..de65e93 100644 (file)
--- a/source/BotInterp.C
+++ b/source/BotInterp.C
@@ -66,7 +66,8 @@ Hook::operator< (const Hook & h) const
 }
 
 BotInterp::BotInterp(Bot *b, String fn)
-  : bot(b), counter(0), timer_mutex (true)
+  : bot(b), counter(0), 
+    hook_mutex (true), timer_mutex (true)
 {
   logPort = scm_open_file (Utils::str2scm (fn),
                            Utils::str2scm ("a"));
@@ -94,80 +95,101 @@ BotInterp::ScriptLog()
 
 namespace
 {
-  bool hptr_lt (const Hook* h, const Hook* h1)
-    // Hook Pointer less than
-    // Used to sort the Hooks list
-  { return *h < *h1; }
+  struct HookFind
+  {
+    std::string rx;
+    std::string name;
+
+    HookFind (std::string r, std::string n)
+      : rx (r), name (n)
+    { }
+
+    bool operator() (const Hook * hook) const
+    { return hook->regex_str == rx && hook->name == name; }
+  };
 }
 
 bool
 BotInterp::AddHook(int hooktype, SCM regex, SCM function, int pri, bool fall,
-                  String name) 
+                  std::string name) 
 {
   if (scm_string_p(regex) == SCM_BOOL_F)
     return false;
-  String rx = Utils::to_upper (Utils::scm2str (regex));
-  // fixme: really ought to use scm_c_module_lookup with bot module
+
+  BotLock hook_lock (hook_mutex);
+  std::string rx = Utils::to_upper (Utils::scm2str (regex));
   SCM r = scm_make_regexp (regex,
                           scm_list_n (scm_variable_ref (scm_c_lookup ("regexp/icase")),
                                       SCM_UNDEFINED));
+  HookFind hook_find (rx, name);
+  HookList& hook_list = hooks[hooktype];
+
   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();
+  HookList::iterator it = std::find_if (hook_list.begin (),
+                                       hook_list.end (),
+                                       hook_find);
+                                                
+  if (it != hook_list.end())
+    {
+      Hook * found = *it;
 
-  for ( ; it != it2; ++it)
-    // It exists, we replace it.
-    if ((*it)->regex_str == rx && (*it)->name == name) {
-      scm_gc_unprotect_object((*it)->function);
+      scm_gc_unprotect_object(found->function);
       scm_gc_unprotect_object (r);
-      (*it)->function = function;
-      (*it)->priority = pri;
-      (*it)->fallthru = fall;
-      hooksMap[hooktype].sort (hptr_lt);
+
+      found->function = function;
+      found->priority = pri;
+      found->fallthru = fall;
+
+      hook_list.erase (it);
+      Utils::push_sorted (hook_list, found, hook_sort_p);
+
+      return true;
+    }
+  else
+    {
+      Utils::push_sorted (hook_list,
+                         new Hook(hooktype, rx, r, function, pri, fall, name),
+                         hook_sort_p);
+      
       return true;
     }
-  // It does not exist, we create it
-  hooksMap[hooktype].push_back (new Hook(hooktype, rx, r, 
-                                        function, pri, fall, name));
-  hooksMap[hooktype].sort (hptr_lt);
-  return true;
 }
 
 bool
-BotInterp::RunHooks(int hooktype, String match, SCM args)
+BotInterp::RunHooks(int hooktype, std::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)
-      {
-       wd.func = (*it)->function;
-       result = scm_internal_catch(SCM_BOOL_T, 
-                                   (scm_t_catch_body) 
-                                   Interp::LazyApplyWrapper,
-                                   static_cast<void *> (&wd), 
-                                   (scm_t_catch_handler) Interp::EmptyHandler, 0);
-       if (! (*it)->fallthru)
-         break;
+  // We want to execute higher priority hooks first, so we start at
+  // the end of the list instead of the beginning
+  
+  for (HookList::reverse_iterator it = hooks[hooktype].rbegin();
+       it != hooks[hooktype].rend();
+       ++it) 
+    {
+      std::cerr << "Matching...\n";
+      if (scm_regexp_exec((*it)->regex, Utils::str2scm (match),
+                         SCM_UNDEFINED, SCM_UNDEFINED) != SCM_BOOL_F)
+       {
+         std::cerr << " Match " << (*it)->regex_str << std::endl;
+         bool fallthru_p = (*it)->fallthru;
+         wd.func = (*it)->function;
+         result = scm_internal_catch(SCM_BOOL_T, 
+                                     (scm_t_catch_body) 
+                                     Interp::LazyApplyWrapper,
+                                     static_cast<void *> (&wd), 
+                                     (scm_t_catch_handler) Interp::EmptyHandler, 0);
+         if (!fallthru_p)
+           break;
+       }
     }
-  }
-
+  
   return true;
 }
 

diff --git a/source/BotInterp.H b/source/BotInterp.H
index f63a7b1..4329ee4 100644 (file)
--- a/source/BotInterp.H
+++ b/source/BotInterp.H
@@ -27,9 +27,10 @@
 #ifdef USESCRIPTS
 
 #include <ctime>
-#include <map>
-#include <list>
 #include <functional>
+#include <list>
+#include <map>
+#include <string>
 
 #include <libguile.h>
 
@@ -44,12 +45,12 @@ struct Hook {
   int priority;
   bool fallthru;
 
-  String regex_str;
-  String name;
+  std::string regex_str;
+  std::string name;
   SCM regex;
   SCM function;
 
-  Hook(int t, String rs, SCM r, SCM f, int p, bool ft, String n="DEFAULT")
+  Hook(int t, std::string rs, SCM r, SCM f, int p, bool ft, std::string n="DEFAULT")
     : type(t), priority (p), fallthru (ft), regex_str(rs), 
       name (n), regex(r), function(f)  { }
 
@@ -82,13 +83,19 @@ struct Timer {
 
 class BotInterp {
   typedef std::list<Timer *> TimerList;
+  typedef std::list<Hook *> HookList;
+  typedef std::map<int, HookList, std::less<int> > HookMap;
   
   Bot * bot;
   SCM logPort;
-  std::map<int, std::list<Hook *>, std::less<int> > hooksMap;
+  int counter;
+
+  HookMap hooks;
   TimerList timers;
+
   Utils::IndirectPred<Timer, std::less<Timer> > timer_sort_p;
-  int counter;
+  Utils::IndirectPred<Hook, std::less<Hook> > hook_sort_p;
+
   BotMutex hook_mutex;
   BotMutex timer_mutex; // NOTE: recursive lock
 
@@ -100,8 +107,8 @@ public:
   void Execute(String);
   void LoadScript(String);
 
-  bool AddHook(int, SCM, SCM, int, bool, String);
-  bool RunHooks(int, String, SCM);
+  bool AddHook(int, SCM, SCM, int, bool, std::string);
+  bool RunHooks(int, std::string, SCM);
 
   SCM AddTimer(int, SCM);
   bool DelTimer(SCM);