Commit ed684e76 authored by Gunjan Sharma's avatar Gunjan Sharma Committed by Tudor Bosman

Fix race in Notification Queue

Summary: When we are changing value of numActiveConsumers_ with setActive from handlerReady at the SCOPE_EXIT we have a race with reading of the same variable in putMessageImpl.

Test Plan: Had a local race which works fine now.

Reviewed By: davejwatson@fb.com

Subscribers: trunkagent

FB internal diff: D1424674
parent fca9fccb
/* /*
* Licensed to the Apache Software Foundation (ASF) under one * Copyright 2014 Facebook, Inc.
* or more contributor license agreements. See the NOTICE file *
* distributed with this work for additional information * Licensed under the Apache License, Version 2.0 (the "License");
* regarding copyright ownership. The ASF licenses this file * you may not use this file except in compliance with the License.
* to you under the Apache License, Version 2.0 (the * You may obtain a copy of the License at
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
* *
* http://www.apache.org/licenses/LICENSE-2.0 * http://www.apache.org/licenses/LICENSE-2.0
* *
* Unless required by applicable law or agreed to in writing, * Unless required by applicable law or agreed to in writing, software
* software distributed under the License is distributed on an * distributed under the License is distributed on an "AS IS" BASIS,
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* KIND, either express or implied. See the License for the * See the License for the specific language governing permissions and
* specific language governing permissions and limitations * limitations under the License.
* under the License.
*/ */
#pragma once #pragma once
#include <fcntl.h> #include <fcntl.h>
...@@ -143,14 +141,23 @@ class NotificationQueue { ...@@ -143,14 +141,23 @@ class NotificationQueue {
private: private:
void setActive(bool active) { void setActive(bool active, bool shouldLock = false) {
DCHECK(queue_); if (!queue_) {
active_ = active;
return;
}
if (shouldLock) {
queue_->spinlock_.lock();
}
if (!active_ && active) { if (!active_ && active) {
++queue_->numActiveConsumers_; ++queue_->numActiveConsumers_;
} else if (active_ && !active) { } else if (active_ && !active) {
--queue_->numActiveConsumers_; --queue_->numActiveConsumers_;
} }
active_ = active; active_ = active;
if (shouldLock) {
queue_->spinlock_.unlock();
}
} }
void init(EventBase* eventBase, NotificationQueue* queue); void init(EventBase* eventBase, NotificationQueue* queue);
...@@ -542,7 +549,7 @@ void NotificationQueue<MessageT>::Consumer::handlerReady(uint16_t events) ...@@ -542,7 +549,7 @@ void NotificationQueue<MessageT>::Consumer::handlerReady(uint16_t events)
uint32_t numProcessed = 0; uint32_t numProcessed = 0;
bool firstRun = true; bool firstRun = true;
setActive(true); setActive(true);
SCOPE_EXIT { setActive(false); }; SCOPE_EXIT { setActive(false, /* shouldLock = */ true); };
while (true) { while (true) {
// Try to decrement the eventfd. // Try to decrement the eventfd.
// //
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment