From a4dee22e8fa625b01ad6d92a6af81fd39c8e9ca8 Mon Sep 17 00:00:00 2001 From: Sam Whittle Date: Thu, 6 Jun 2024 15:38:56 +0200 Subject: [PATCH] Fix race due to use of TryLock where grpc_event_engine::experimental::TimerList::TimerCheck could miss an existing timer and leave the wakeup time to InfiniteFuture. --- src/core/lib/event_engine/posix_engine/timer.cc | 10 ++++------ src/core/lib/event_engine/posix_engine/timer.h | 4 ++-- third_party/benchmark | 2 +- third_party/bloaty | 2 +- third_party/boringssl-with-bazel | 2 +- third_party/cares/cares | 2 +- third_party/envoy-api | 2 +- third_party/googleapis | 2 +- third_party/googletest | 2 +- third_party/opencensus-proto | 2 +- third_party/opentelemetry | 2 +- third_party/opentelemetry-cpp | 2 +- third_party/protobuf | 2 +- third_party/protoc-gen-validate | 2 +- third_party/re2 | 2 +- third_party/xds | 2 +- third_party/zlib | 2 +- 17 files changed, 21 insertions(+), 23 deletions(-) diff --git a/src/core/lib/event_engine/posix_engine/timer.cc b/src/core/lib/event_engine/posix_engine/timer.cc index 6f152ce494a375..cb57e1e75d8c3d 100644 --- a/src/core/lib/event_engine/posix_engine/timer.cc +++ b/src/core/lib/event_engine/posix_engine/timer.cc @@ -298,12 +298,10 @@ TimerList::TimerCheck(grpc_core::Timestamp* next) { return std::vector(); } - if (!checker_mu_.TryLock()) return absl::nullopt; - std::vector run = - FindExpiredTimers(now, next); - checker_mu_.Unlock(); - - return std::move(run); + { + absl::MutexLock lock(&checker_mu_); + return FindExpiredTimers(now, next); + } } } // namespace experimental diff --git a/src/core/lib/event_engine/posix_engine/timer.h b/src/core/lib/event_engine/posix_engine/timer.h index 30e4bb2eb6c0f8..e3164858507811 100644 --- a/src/core/lib/event_engine/posix_engine/timer.h +++ b/src/core/lib/event_engine/posix_engine/timer.h @@ -149,8 +149,8 @@ class TimerList { grpc_core::Mutex mu_; // The deadline of the next timer due across all timer shards std::atomic min_timer_; - // Allow only one FindExpiredTimers at once (used as a TryLock, protects no - // fields but ensures limits on concurrency) + // Allow only one FindExpiredTimers at once (protects no fields but ensures + // limits on concurrency) grpc_core::Mutex checker_mu_; // Array of timer shards. Whenever a timer (Timer *) is added, its address // is hashed to select the timer shard to add the timer to diff --git a/third_party/benchmark b/third_party/benchmark index 344117638c8ff7..7f0e99af540a33 160000 --- a/third_party/benchmark +++ b/third_party/benchmark @@ -1 +1 @@ -Subproject commit 344117638c8ff7e239044fd0fa7085839fc03021 +Subproject commit 7f0e99af540a333108b92d792923ec7fc9e9fad9 diff --git a/third_party/bloaty b/third_party/bloaty index 60209eb1ccc34d..34f4a66559ad49 160000 --- a/third_party/bloaty +++ b/third_party/bloaty @@ -1 +1 @@ -Subproject commit 60209eb1ccc34d5deefb002d1b7f37545204f7f2 +Subproject commit 34f4a66559ad4938c1e629e9b5f54630b2b4d7b0 diff --git a/third_party/boringssl-with-bazel b/third_party/boringssl-with-bazel index b8a2bffc598f23..c1d9ac02514a13 160000 --- a/third_party/boringssl-with-bazel +++ b/third_party/boringssl-with-bazel @@ -1 +1 @@ -Subproject commit b8a2bffc598f230484ff48a247526a9820facfc2 +Subproject commit c1d9ac02514a138129872a036e3f8a1074dcb8bd diff --git a/third_party/cares/cares b/third_party/cares/cares index 6360e96b5cf8e5..91fa52c41ed44f 160000 --- a/third_party/cares/cares +++ b/third_party/cares/cares @@ -1 +1 @@ -Subproject commit 6360e96b5cf8e5980c887ce58ef727e53d77243a +Subproject commit 91fa52c41ed44f6bd0b54d7aca1ae48c1d422f5b diff --git a/third_party/envoy-api b/third_party/envoy-api index 78f198cf96ecdc..bf59f2d652ca95 160000 --- a/third_party/envoy-api +++ b/third_party/envoy-api @@ -1 +1 @@ -Subproject commit 78f198cf96ecdc7120ef640406770aa01af775c4 +Subproject commit bf59f2d652ca9558fc4dcce6fc60b169f7fc02c7 diff --git a/third_party/googleapis b/third_party/googleapis index 2f9af297c84c55..21c839628513a6 160000 --- a/third_party/googleapis +++ b/third_party/googleapis @@ -1 +1 @@ -Subproject commit 2f9af297c84c55c8b871ba4495e01ade42476c92 +Subproject commit 21c839628513a6b8d6ce99ae6911d4d2094c0767 diff --git a/third_party/googletest b/third_party/googletest index 2dd1c131950043..a7f443b80b105f 160000 --- a/third_party/googletest +++ b/third_party/googletest @@ -1 +1 @@ -Subproject commit 2dd1c131950043a8ad5ab0d2dda0e0970596586a +Subproject commit a7f443b80b105f940225332ed3c31f2790092f47 diff --git a/third_party/opencensus-proto b/third_party/opencensus-proto index 4aa53e15cbf1a4..1664cc961550be 160000 --- a/third_party/opencensus-proto +++ b/third_party/opencensus-proto @@ -1 +1 @@ -Subproject commit 4aa53e15cbf1a47bc9087e6cfdca214c1eea4e89 +Subproject commit 1664cc961550be8f3058ddd29390350242f44f1f diff --git a/third_party/opentelemetry b/third_party/opentelemetry index 60fa8754d890b5..bd7cf55b6d45f3 160000 --- a/third_party/opentelemetry +++ b/third_party/opentelemetry @@ -1 +1 @@ -Subproject commit 60fa8754d890b5c55949a8c68dcfd7ab5c2395df +Subproject commit bd7cf55b6d45f3c587d2131b68a7e5a501bdb10c diff --git a/third_party/opentelemetry-cpp b/third_party/opentelemetry-cpp index 4bd64c9a336fd4..cb9cd1d1d13a5c 160000 --- a/third_party/opentelemetry-cpp +++ b/third_party/opentelemetry-cpp @@ -1 +1 @@ -Subproject commit 4bd64c9a336fd438d6c4c9dad2e6b61b0585311f +Subproject commit cb9cd1d1d13a5c6b6a99287dcb8b0d7bb62f277e diff --git a/third_party/protobuf b/third_party/protobuf index 2434ef2adf0c74..f3c140f96c9a7c 160000 --- a/third_party/protobuf +++ b/third_party/protobuf @@ -1 +1 @@ -Subproject commit 2434ef2adf0c74149b9d547ac5fb545a1ff8b6b5 +Subproject commit f3c140f96c9a7cb59f02289261c70c91bb9c009f diff --git a/third_party/protoc-gen-validate b/third_party/protoc-gen-validate index fab737efbb4b4d..ca115cfd131364 160000 --- a/third_party/protoc-gen-validate +++ b/third_party/protoc-gen-validate @@ -1 +1 @@ -Subproject commit fab737efbb4b4d03e7c771393708f75594b121e4 +Subproject commit ca115cfd131364a094044faab592858151fd291c diff --git a/third_party/re2 b/third_party/re2 index 0c5616df9c0aaa..b91097e3989a8c 160000 --- a/third_party/re2 +++ b/third_party/re2 @@ -1 +1 @@ -Subproject commit 0c5616df9c0aaa44c9440d87422012423d91c7d1 +Subproject commit b91097e3989a8c8b463c0b6d7ffe96dafa5d6fad diff --git a/third_party/xds b/third_party/xds index 3a472e524827f7..555b57ec207be8 160000 --- a/third_party/xds +++ b/third_party/xds @@ -1 +1 @@ -Subproject commit 3a472e524827f72d1ad621c4983dd5af54c46776 +Subproject commit 555b57ec207be86f811fb0c04752db6f85e3d7e2 diff --git a/third_party/zlib b/third_party/zlib index 09155eaa2f9270..0f51fb4933fc9c 160000 --- a/third_party/zlib +++ b/third_party/zlib @@ -1 +1 @@ -Subproject commit 09155eaa2f9270dc4ed1fa13e2b4b2613e6e4851 +Subproject commit 0f51fb4933fc9ce18199cb2554dacea8033e7fd3