From 536bbbf18191d8e0659f8e414dbab10263ed1720 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 13 Sep 2024 19:28:33 -0700 Subject: [PATCH] [party] Race fix (#37726) Party contains a state field that contains ref count *and some other things*... so RefIfNonZero needs to pay attention to only consider the ref count portion. Closes #37726 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/37726 from ctiller:flake-fightas-7 f2b3b3ff6dfb92475bfe9cca1d41dc183734c011 PiperOrigin-RevId: 674514201 --- src/core/ext/transport/chaotic_good/server_transport.cc | 2 +- src/core/lib/promise/party.cc | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/core/ext/transport/chaotic_good/server_transport.cc b/src/core/ext/transport/chaotic_good/server_transport.cc index b40e0290b0a..d6c5faf3574 100644 --- a/src/core/ext/transport/chaotic_good/server_transport.cc +++ b/src/core/ext/transport/chaotic_good/server_transport.cc @@ -240,7 +240,7 @@ auto ChaoticGoodServerTransport::DeserializeAndPushFragmentToNewCall( call_initiator->SpawnGuarded( "server-write", [this, stream_id = frame_header.stream_id, call_initiator = *call_initiator, - call_handler = std::move(call.handler)]() { + call_handler = std::move(call.handler)]() mutable { call_destination_->StartCall(std::move(call_handler)); return CallOutboundLoop(stream_id, call_initiator); }); diff --git a/src/core/lib/promise/party.cc b/src/core/lib/promise/party.cc index b8b6a899a48..1fbc01fd3da 100644 --- a/src/core/lib/promise/party.cc +++ b/src/core/lib/promise/party.cc @@ -41,18 +41,18 @@ namespace grpc_core { // PartySyncUsingAtomics GRPC_MUST_USE_RESULT bool Party::RefIfNonZero() { - auto count = state_.load(std::memory_order_relaxed); + auto state = state_.load(std::memory_order_relaxed); do { // If zero, we are done (without an increment). If not, we must do a CAS // to maintain the contract: do not increment the counter if it is already // zero - if (count == 0) { + if ((state & kRefMask) == 0) { return false; } - } while (!state_.compare_exchange_weak(count, count + kOneRef, + } while (!state_.compare_exchange_weak(state, state + kOneRef, std::memory_order_acq_rel, std::memory_order_relaxed)); - LogStateChange("RefIfNonZero", count, count + kOneRef); + LogStateChange("RefIfNonZero", state, state + kOneRef); return true; }