[Bro-Dev] [Bro-Commits] [git/bro] master: Fix for input readers occasionally dead-locking. (c980d10)

[Clark, Gilbert]

// Thoughts?

/* 
 * Since we know that read_ctr is only incremented after a successful read, and write_ctr is only incremented after a successful write, the two values should be equal iff the queue is empty.  I think it could also help if these two items were 
*  declared volatile.
*/
bool MaybeReady() { return read_ctr != write_ctr; }

// Alternatively, replacing random() with a per-instance counter and saying the function returns true every Xth time it is called might mean you weren't having to hit the RNG every time you checked on the status of a queue.
// Also, should the call be to random_r() instead of random() ?

// --Gilbert
________________________________________
From: bro-commits-bounces at bro.org [bro-commits-bounces at bro.org] On Behalf Of Robin Sommer [robin at icir.org]
Sent: Thursday, October 24, 2013 9:28 PM
To: bro-commits at bro.org
Subject: [Bro-Commits] [git/bro] master: Fix for input readers occasionally     dead-locking. (c980d10)

Repository : ssh://git@bro-ids.icir.org/bro

On branch  : master
Link       : https://github.com/bro/bro/commit/c980d1055e1e17da4867e3fab1ee10f604b242b0

>---------------------------------------------------------------

commit c980d1055e1e17da4867e3fab1ee10f604b242b0
Author: Robin Sommer <robin at icir.org>
Date:   Thu Oct 24 18:16:49 2013 -0700

    Fix for input readers occasionally dead-locking.

    Bernhard and I tracked it down we believe: the thread queue could
    deadlock in certain cases. As a fix we tuned the heuristic for telling
    if a queue might have input to occasionaly err on the safe side by
    flagging "yes", so that processing will proceed.

    It's a bit unfortunate to apply this fix last minute before the
    release as it could potentially impact performance if the heuristic
    fails to often. We believe the chosen parmaterization should be fine ...


>---------------------------------------------------------------

c980d1055e1e17da4867e3fab1ee10f604b242b0
 CHANGES               |  4 ++++
 VERSION               |  2 +-
 src/threading/Queue.h | 10 ++++++----
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/CHANGES b/CHANGES
index 10bc187..1ae192f 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,4 +1,8 @@

+2.2-beta-152 | 2013-10-24 18:16:49 -0700
+
+  * Fix for input readers occasionally dead-locking. (Robin Sommer)
+
 2.2-beta-151 | 2013-10-24 16:52:26 -0700

   * Updating submodule(s).
diff --git a/VERSION b/VERSION
index 2b5702f..26d1beb 100644
--- a/VERSION
+++ b/VERSION
@@ -1 +1 @@
-2.2-beta-151
+2.2-beta-152
diff --git a/src/threading/Queue.h b/src/threading/Queue.h
index 792fb63..c4f2bfa 100644
--- a/src/threading/Queue.h
+++ b/src/threading/Queue.h
@@ -61,11 +61,13 @@ public:
        bool Ready();

        /**
-        * Returns true if the next Get() operation might succeed.
-        * This function may occasionally return a value not
-        * indicating the actual state, but won't do so very often.
+        * Returns true if the next Get() operation might succeed. This
+        * function may occasionally return a value not indicating the actual
+        * state, but won't do so very often. Occasionally we also return a
+        * true unconditionally to avoid a deadlock when both pointers happen
+        * to be equal even though there's stuff queued.
         */
-       bool MaybeReady() { return ( ( read_ptr - write_ptr) != 0 ); }
+       bool MaybeReady() { return (read_ptr != write_ptr) || (random() % 10000 == 0); }

        /** Wake up the reader if it's currently blocked for input. This is
         primarily to give it a chance to check termination quickly.

_______________________________________________
bro-commits mailing list
bro-commits at bro.org
http://mailman.icsi.berkeley.edu/mailman/listinfo/bro-commits