I just committed an alternative fix to topic/bernhard/alternative-deadlock-fix - this removes the random from Queue again and places another random into the threading manager. However, this one is only called if no communication whatsoever occurs - and only by the main thread, so random() should be fine.
Bernhard On Oct 24, 2013, at 9:04 PM, Clark, Gilbert <[email protected]> wrote: > // 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: [email protected] [[email protected]] On Behalf Of > Robin Sommer [[email protected]] > Sent: Thursday, October 24, 2013 9:28 PM > To: [email protected] > Subject: [Bro-Commits] [git/bro] master: Fix for input readers occasionally > dead-locking. (c980d10) > > Repository : ssh://[email protected]/bro > > On branch : master > Link : > https://github.com/bro/bro/commit/c980d1055e1e17da4867e3fab1ee10f604b242b0 > >> --------------------------------------------------------------- > > commit c980d1055e1e17da4867e3fab1ee10f604b242b0 > Author: Robin Sommer <[email protected]> > 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 > [email protected] > http://mailman.icsi.berkeley.edu/mailman/listinfo/bro-commits > _______________________________________________ > bro-dev mailing list > [email protected] > http://mailman.icsi.berkeley.edu/mailman/listinfo/bro-dev
smime.p7s
Description: S/MIME cryptographic signature
_______________________________________________ bro-dev mailing list [email protected] http://mailman.icsi.berkeley.edu/mailman/listinfo/bro-dev
