Skip to content

Commit

Permalink
Fix concurrency issue in RibUpdateMonitor::MergeUpdate
Browse files Browse the repository at this point in the history
The return value of RibUpdateMonitor::EnqueueUpdateUnlocked could be
incorrect if it's called without holding the monitor mutex.  That in
turn prevents BgpExport::Join from kicking SchedulingGroup to start
a tail dequeue for the bulk UpdateQueue.

Change-Id: I1d2a4418346bb0e90eaed68a3a535d9bc741825e
Closes-Bug: #1451306
  • Loading branch information
Nischal Sheth committed May 4, 2015
1 parent b7f6c4f commit 9810b26
Showing 1 changed file with 6 additions and 13 deletions.
19 changes: 6 additions & 13 deletions src/bgp/bgp_update_monitor.cc
Expand Up @@ -369,8 +369,8 @@ bool RibUpdateMonitor::GetPeerSetCurrentAndScheduled(DBEntryBase *db_entry,

//
// Concurrency: Called in the context of the DB partition task. There's no
// entry lock as such since the DBState is a RouteState. We may or may not
// hold the monitor lock, but in either case we don't try to acquire it.
// entry lock since the DBState is a RouteState, but the monitor lock must
// be held.
//
// Helper routine for MergeUpdate to handle case where the current DBState
// is a RouteState.
Expand Down Expand Up @@ -470,17 +470,10 @@ bool RibUpdateMonitor::MergeUpdate(DBEntryBase *db_entry,
RouteUpdate *rt_update) {
CHECK_CONCURRENCY("db::DBTable");

// Don't need to bother going through the monitor if there's no DBState.
// The scheduling group task can't hold a lock on the entry in this case.
DBState *dbstate =
db_entry->GetState(ribout_->table(), ribout_->listener_id());
if (dbstate == NULL) {
return EnqueueUpdateUnlocked(db_entry, rt_update);
}

// Go through the monitor and handle the DBState as appropriate. Note
// that we still need to check for the DBState being NULL as things may
// have changed after the check made above.
// Go through the monitor and handle the DBState as necessary. Need
// to use the monitor even if there's no DBState in order to protect
// against race conditions which could result in an incorrect return
// value.
while (true) {
tbb::interface5::unique_lock<tbb::mutex> toplock(mutex_);
DBState *dbstate =
Expand Down

0 comments on commit 9810b26

Please sign in to comment.