bugfix: fixed a race condition that would crash lighthtouse when saving compositions

Author: gaasedelen

plugin/lighthouse/director.py

@@ -154,6 +154,7 @@ def __init__(self, palette):
         #----------------------------------------------------------------------
 
         self._ast_queue = Queue.Queue()
+        self._composition_lock = threading.Lock()
         self._composition_cache = CompositionCache()
 
         self._composition_worker = threading.Thread(
@@ -693,8 +694,6 @@ def add_composition(self, composite_name, ast):
 
         # evaluate the last AST into a coverage set
         composite_coverage = self._evaluate_composition(ast)
-        composite_coverage.update_metadata(self.metadata)
-        composite_coverage.refresh() # TODO: hash refresh?
 
         # save the evaluated coverage under the given name
         self._update_coverage(composite_name, composite_coverage)
@@ -741,15 +740,14 @@ def _async_evaluate_ast(self):
             # produce a single composite coverage object as described by the AST
             composite_coverage = self._evaluate_composition(ast)
 
-            # map the composited coverage data to the database metadata
-            composite_coverage.update_metadata(self.metadata)
-            composite_coverage.refresh()
-
             # we always save the most recent composite to the hotshell entry
             self._special_coverage[HOT_SHELL] = composite_coverage
 
+            #
             # if the hotshell entry is the active coverage selection, notify
             # listeners of its update
+            #
+
             if self.coverage_name == HOT_SHELL:
                 self._notify_coverage_modified()
 
@@ -767,8 +765,37 @@ def _evaluate_composition(self, ast):
         if isinstance(ast, TokenNull):
             return self._NULL_COVERAGE
 
+        #
+        # the director's composition evaluation code (this function) is most
+        # generally called via the background caching evaluation thread known
+        # as self._composition_worker. But this function can also be called
+        # inline via the 'add_composition' function from a different thread
+        # (namely, the main thread)
+        #
+        # because of this, we must control access to the resources the AST
+        # evaluation code operates by restricting the code to one thread
+        # at a time.
+        #
+        # should we call _evaluate_composition from the context of the main
+        # IDA thread, it is important that we do so in a pseudo non-blocking
+        # such that we don't hang IDA. await_lock(...) will allow the Qt/IDA
+        # main thread to yield to other threads while waiting for the lock
+        #
+
+        await_lock(self._composition_lock)
+
         # recursively evaluate the AST
-        return self._evaluate_composition_recursive(ast)
+        composite_coverage = self._evaluate_composition_recursive(ast)
+
+        # map the composited coverage data to the database metadata
+        composite_coverage.update_metadata(self.metadata)
+        composite_coverage.refresh() # TODO: hash refresh?
+
+        # done operating on shared data (coverage), release the lock
+        self._composition_lock.release()
+
+        # return the evaluated composition
+        return composite_coverage
 
     def _evaluate_composition_recursive(self, node):
         """

plugin/lighthouse/util/ida.py

@@ -475,23 +475,19 @@ def thunk():
 # IDA Async Magic
 #------------------------------------------------------------------------------
 
-@mainthread
-def await_future(future, block=True, timeout=1.0):
+def await_future(future):
     """
     This is effectively a technique I use to get around completely blocking
     IDA's mainthread while waiting for a threaded result that may need to make
-    use of the sync operators.
+    use of the execute_sync operators.
 
     Waiting for a 'future' thread result to come through via this function
     lets other execute_sync actions to slip through (at least Read, Fast).
     """
-
-    elapsed  = 0       # total time elapsed processing this future object
     interval = 0.02    # the interval which we wait for a response
-    end_time = time.time() + timeout
 
-    # run until the the future completes or the timeout elapses
-    while block or (time.time() < end_time):
+    # run until the the future arrives
+    while True:
 
         # block for a brief period to see if the future completes
         try:
@@ -503,26 +499,75 @@ def await_future(future, block=True, timeout=1.0):
         #
 
         except Queue.Empty as e:
-            logger.debug("Flushing future...")
+            pass
+
+        logger.debug("Awaiting future...")
+
+        #
+        # if we are executing (well, blocking) as the main thread, we need
+        # to flush the event loop so IDA does not hang
+        #
+
+        if idaapi.is_main_thread():
+            flush_ida_sync_requests()
+
+def await_lock(lock):
+    """
+    Attempt to acquire a lock without blocking the IDA mainthread.
+
+    See await_future() for more details.
+    """
+
+    elapsed  = 0       # total time elapsed waiting for the lock
+    interval = 0.02    # the interval (in seconds) between acquire attempts
+    timeout  = 60.0    # the total time allotted to acquiring the lock
+    end_time = time.time() + timeout
+
+    # wait until the the lock is available
+    while time.time() < end_time:
+
+        #
+        # attempt to acquire the given lock without blocking (via 'False').
+        # if we succesfully aquire the lock, then we can return (success)
+        #
+
+        if lock.acquire(False):
+            logger.debug("Acquired lock!")
+            return
+
+        #
+        # the lock is not available yet. we need to sleep so we don't choke
+        # the cpu, and try to acquire the lock again next time through...
+        #
+
+        logger.debug("Awaiting lock...")
+        time.sleep(interval)
+
+        #
+        # if we are executing (well, blocking) as the main thread, we need
+        # to flush the event loop so IDA does not hang
+        #
+
+        if idaapi.is_main_thread():
             flush_ida_sync_requests()
 
+    #
+    # we spent 60 seconds trying to acquire the lock, but never got it...
+    # to avoid hanging IDA indefinitely (or worse), we abort via signal
+    #
+
+    raise RuntimeError("Failed to acquire lock after %f seconds!" % timeout)
+
 @mainthread
 def flush_ida_sync_requests():
     """
     Flush all execute_sync requests.
-
-    NOTE: This MUST be called from the IDA Mainthread to be effective.
     """
-    if not idaapi.is_main_thread():
-        return False
 
     # this will trigger/flush the IDA UI loop
     qta = QtCore.QCoreApplication.instance()
     qta.processEvents()
 
-    # done
-    return True
-
 @mainthread
 def prompt_string(label, title, default=""):
     """