Skip to content

Commit 5009049

Browse files
Faruk Barotovfacebook-github-bot
authored andcommitted
Fix a race in fibers::BatchSemaphore
Summary: D66114255 fixed a data race in `fibers::Semaphore` occuring due to lack of 'happens-before' signal-wait synchronization. The very same issue exists in `fibers::BatchedSemaphore` which I came across today in a flaky unit test. Hence porting the fix. Reviewed By: Gownta Differential Revision: D68594868 fbshipit-source-id: 752a7b15198015928a7fe040fecc8b3c9e56a889
1 parent fed498b commit 5009049

File tree

3 files changed

+48
-1
lines changed

3 files changed

+48
-1
lines changed

folly/fibers/SemaphoreBase.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ bool SemaphoreBase::signalSlow(int64_t tokens) {
3434
// waitlist, ensure the token count increments. No need for CAS here as
3535
// we will always be under the mutex
3636
if (tokens_.compare_exchange_strong(
37-
testVal, testVal + tokens, std::memory_order_relaxed)) {
37+
testVal, testVal + tokens, std::memory_order_release)) {
3838
return true;
3939
}
4040
continue;

folly/fibers/test/BUCK

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,3 +85,12 @@ cpp_unittest(
8585
"//folly/synchronization/detail:sleeper",
8686
],
8787
)
88+
89+
cpp_unittest(
90+
name = "batch_semaphore_test",
91+
srcs = ["BatchSemaphoreTest.cpp"],
92+
deps = [
93+
"//folly/fibers:batch_semaphore",
94+
"//folly/portability:gtest",
95+
],
96+
)
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
/*
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
#include <folly/fibers/BatchSemaphore.h>
18+
#include <folly/portability/GTest.h>
19+
20+
TEST(BatchSemaphoreTest, WaitSignalSynchronization) {
21+
folly::fibers::BatchSemaphore sem{0};
22+
23+
int64_t data = 0;
24+
folly::relaxed_atomic_bool signalled = false;
25+
26+
std::jthread t{[&]() {
27+
while (!signalled) {
28+
std::this_thread::yield();
29+
}
30+
31+
sem.wait(1);
32+
EXPECT_NE(data, 0);
33+
}};
34+
35+
data = 1;
36+
sem.signal(1);
37+
signalled = true;
38+
}

0 commit comments

Comments
 (0)