Chiebot-Mirror
/
grpc
mirror of https://github.com/grpc/grpc.git
8
0
Fork 0
Code Issues Projects Releases Wiki Activity

Revert "[chaotic-good] Fix some ref leaks (#37865)"

Browse Source 
This reverts commit e8aa408bba.
pull/37882/head
Craig Tiller 2 months ago
parent 4cf2759096
commit 6e666cd985
3 changed files with 95 additions and 100 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Unified View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 88
      src/core/ext/transport/chaotic_good/server_transport.cc
  2. 4
      src/core/ext/transport/chaotic_good/server_transport.h
  3. 103
      src/core/server/server.cc

88
src/core/ext/transport/chaotic_good/server_transport.cc
Unescape View File

@ -125,13 +125,14 @@ auto ChaoticGoodServerTransport::MaybePushFragmentIntoCall(
} }
auto ChaoticGoodServerTransport::SendFragment( auto ChaoticGoodServerTransport::SendFragment(
ServerFragmentFrame frame, MpscSender<ServerFrame> outgoing_frames) { ServerFragmentFrame frame, MpscSender<ServerFrame> outgoing_frames,
CallInitiator call_initiator) {
GRPC_TRACE_LOG(chaotic_good, INFO) GRPC_TRACE_LOG(chaotic_good, INFO)
<< "CHAOTIC_GOOD: SendFragment: frame=" << frame.ToString(); << "CHAOTIC_GOOD: SendFragment: frame=" << frame.ToString();
// Capture the call_initiator to ensure the underlying call spine is alive // Capture the call_initiator to ensure the underlying call spine is alive
// until the outgoing_frames.Send promise completes. // until the outgoing_frames.Send promise completes.
return Map(outgoing_frames.Send(std::move(frame)), return Map(outgoing_frames.Send(std::move(frame)),
[](bool success) -> absl::Status { [call_initiator](bool success) -> absl::Status {
if (!success) { if (!success) {
// Failed to send outgoing frame. // Failed to send outgoing frame.
return absl::UnavailableError("Transport closed."); return absl::UnavailableError("Transport closed.");
@ -145,26 +146,27 @@ auto ChaoticGoodServerTransport::SendCallBody(
CallInitiator call_initiator) { CallInitiator call_initiator) {
// Continuously send client frame with client to server // Continuously send client frame with client to server
// messages. // messages.
return ForEach(OutgoingMessages(call_initiator), return ForEach(
// Capture the call_initator to ensure the underlying call OutgoingMessages(call_initiator),
// spine is alive until the SendFragment promise completes. // Capture the call_initator to ensure the underlying call
[stream_id, outgoing_frames, aligned_bytes = aligned_bytes_]( // spine is alive until the SendFragment promise completes.
MessageHandle message) mutable { [stream_id, outgoing_frames, call_initiator,
ServerFragmentFrame frame; aligned_bytes = aligned_bytes_](MessageHandle message) mutable {
// Construct frame header (flags, header_length ServerFragmentFrame frame;
// and trailer_length will be added in // Construct frame header (flags, header_length
// serialization). // and trailer_length will be added in
const uint32_t message_length = message->payload()->Length(); // serialization).
const uint32_t padding = const uint32_t message_length = message->payload()->Length();
message_length % aligned_bytes == 0 const uint32_t padding =
? 0 message_length % aligned_bytes == 0
: aligned_bytes - (message_length % aligned_bytes); ? 0
CHECK_EQ((message_length + padding) % aligned_bytes, 0u); : aligned_bytes - message_length % aligned_bytes;
frame.message = FragmentMessage(std::move(message), padding, CHECK_EQ((message_length + padding) % aligned_bytes, 0u);
message_length); frame.message =
frame.stream_id = stream_id; FragmentMessage(std::move(message), padding, message_length);
return SendFragment(std::move(frame), outgoing_frames); frame.stream_id = stream_id;
}); return SendFragment(std::move(frame), outgoing_frames, call_initiator);
});
} }
auto ChaoticGoodServerTransport::SendCallInitialMetadataAndBody( auto ChaoticGoodServerTransport::SendCallInitialMetadataAndBody(
@ -185,7 +187,8 @@ auto ChaoticGoodServerTransport::SendCallInitialMetadataAndBody(
frame.headers = std::move(*md); frame.headers = std::move(*md);
frame.stream_id = stream_id; frame.stream_id = stream_id;
return TrySeq( return TrySeq(
SendFragment(std::move(frame), outgoing_frames), SendFragment(std::move(frame), outgoing_frames,
call_initiator),
SendCallBody(stream_id, outgoing_frames, call_initiator)); SendCallBody(stream_id, outgoing_frames, call_initiator));
}, },
[]() { return absl::OkStatus(); }); []() { return absl::OkStatus(); });
@ -206,11 +209,15 @@ auto ChaoticGoodServerTransport::CallOutboundLoop(
return Empty{}; return Empty{};
}), }),
call_initiator.PullServerTrailingMetadata(), call_initiator.PullServerTrailingMetadata(),
[stream_id, outgoing_frames](ServerMetadataHandle md) mutable { // Capture the call_initator to ensure the underlying call_spine
// is alive until the SendFragment promise completes.
[stream_id, outgoing_frames,
call_initiator](ServerMetadataHandle md) mutable {
ServerFragmentFrame frame; ServerFragmentFrame frame;
frame.trailers = std::move(md); frame.trailers = std::move(md);
frame.stream_id = stream_id; frame.stream_id = stream_id;
return SendFragment(std::move(frame), outgoing_frames); return SendFragment(std::move(frame), outgoing_frames,
call_initiator);
})); }));
} }
@ -302,11 +309,10 @@ auto ChaoticGoodServerTransport::ReadOneFrame(ChaoticGoodTransport& transport) {
call_initiator.has_value(), call_initiator.has_value(),
[&call_initiator]() { [&call_initiator]() {
auto c = std::move(*call_initiator); auto c = std::move(*call_initiator);
return c.SpawnWaitable("cancel_from_read", return c.SpawnWaitable("cancel", [c]() mutable {
[c]() mutable { c.Cancel();
c.Cancel(); return absl::OkStatus();
return absl::OkStatus(); });
});
}, },
[]() -> absl::Status { return absl::OkStatus(); }); []() -> absl::Status { return absl::OkStatus(); });
}), }),
@ -391,8 +397,6 @@ void ChaoticGoodServerTransport::AbortWithError() {
// Close all the available pipes. // Close all the available pipes.
outgoing_frames_.MarkClosed(); outgoing_frames_.MarkClosed();
ReleasableMutexLock lock(&mu_); ReleasableMutexLock lock(&mu_);
if (aborted_with_error_) return;
aborted_with_error_ = true;
StreamMap stream_map = std::move(stream_map_); StreamMap stream_map = std::move(stream_map_);
stream_map_.clear(); stream_map_.clear();
state_tracker_.SetState(GRPC_CHANNEL_SHUTDOWN, state_tracker_.SetState(GRPC_CHANNEL_SHUTDOWN,
@ -401,19 +405,18 @@ void ChaoticGoodServerTransport::AbortWithError() {
lock.Release(); lock.Release();
for (const auto& pair : stream_map) { for (const auto& pair : stream_map) {
auto call_initiator = pair.second; auto call_initiator = pair.second;
call_initiator.SpawnInfallible("cancel_from_transport_closed", call_initiator.SpawnInfallible("cancel", [call_initiator]() mutable {
[call_initiator]() mutable { call_initiator.Cancel();
call_initiator.Cancel(); return Empty{};
return Empty{}; });
});
} }
} }
absl::optional<CallInitiator> ChaoticGoodServerTransport::LookupStream( absl::optional<CallInitiator> ChaoticGoodServerTransport::LookupStream(
uint32_t stream_id) { uint32_t stream_id) {
MutexLock lock(&mu_);
GRPC_TRACE_LOG(chaotic_good, INFO) GRPC_TRACE_LOG(chaotic_good, INFO)
<< "CHAOTIC_GOOD " << this << " LookupStream " << stream_id; << "CHAOTIC_GOOD " << this << " LookupStream " << stream_id;
MutexLock lock(&mu_);
auto it = stream_map_.find(stream_id); auto it = stream_map_.find(stream_id);
if (it == stream_map_.end()) return absl::nullopt; if (it == stream_map_.end()) return absl::nullopt;
return it->second; return it->second;
@ -421,9 +424,9 @@ absl::optional<CallInitiator> ChaoticGoodServerTransport::LookupStream(
absl::optional<CallInitiator> ChaoticGoodServerTransport::ExtractStream( absl::optional<CallInitiator> ChaoticGoodServerTransport::ExtractStream(
uint32_t stream_id) { uint32_t stream_id) {
MutexLock lock(&mu_);
GRPC_TRACE_LOG(chaotic_good, INFO) GRPC_TRACE_LOG(chaotic_good, INFO)
<< "CHAOTIC_GOOD " << this << " ExtractStream " << stream_id; << "CHAOTIC_GOOD " << this << " ExtractStream " << stream_id;
MutexLock lock(&mu_);
auto it = stream_map_.find(stream_id); auto it = stream_map_.find(stream_id);
if (it == stream_map_.end()) return absl::nullopt; if (it == stream_map_.end()) return absl::nullopt;
auto r = std::move(it->second); auto r = std::move(it->second);
@ -433,12 +436,9 @@ absl::optional<CallInitiator> ChaoticGoodServerTransport::ExtractStream(
absl::Status ChaoticGoodServerTransport::NewStream( absl::Status ChaoticGoodServerTransport::NewStream(
uint32_t stream_id, CallInitiator call_initiator) { uint32_t stream_id, CallInitiator call_initiator) {
MutexLock lock(&mu_);
GRPC_TRACE_LOG(chaotic_good, INFO) GRPC_TRACE_LOG(chaotic_good, INFO)
<< "CHAOTIC_GOOD " << this << " NewStream " << stream_id; << "CHAOTIC_GOOD " << this << " NewStream " << stream_id;
if (aborted_with_error_) { MutexLock lock(&mu_);
return absl::UnavailableError("Transport closed");
}
auto it = stream_map_.find(stream_id); auto it = stream_map_.find(stream_id);
if (it != stream_map_.end()) { if (it != stream_map_.end()) {
return absl::InternalError("Stream already exists"); return absl::InternalError("Stream already exists");
@ -454,7 +454,7 @@ absl::Status ChaoticGoodServerTransport::NewStream(
self->ExtractStream(stream_id); self->ExtractStream(stream_id);
if (call_initiator.has_value()) { if (call_initiator.has_value()) {
auto c = std::move(*call_initiator); auto c = std::move(*call_initiator);
c.SpawnInfallible("cancel_from_on_done", [c]() mutable { c.SpawnInfallible("cancel", [c]() mutable {
c.Cancel(); c.Cancel();
return Empty{}; return Empty{};
}); });

4
src/core/ext/transport/chaotic_good/server_transport.h
Unescape View File

@ -109,7 +109,8 @@ class ChaoticGoodServerTransport final : public ServerTransport {
auto SendCallBody(uint32_t stream_id, MpscSender<ServerFrame> outgoing_frames, auto SendCallBody(uint32_t stream_id, MpscSender<ServerFrame> outgoing_frames,
CallInitiator call_initiator); CallInitiator call_initiator);
static auto SendFragment(ServerFragmentFrame frame, static auto SendFragment(ServerFragmentFrame frame,
MpscSender<ServerFrame> outgoing_frames); MpscSender<ServerFrame> outgoing_frames,
CallInitiator call_initiator);
auto CallOutboundLoop(uint32_t stream_id, CallInitiator call_initiator); auto CallOutboundLoop(uint32_t stream_id, CallInitiator call_initiator);
auto OnTransportActivityDone(absl::string_view activity); auto OnTransportActivityDone(absl::string_view activity);
auto TransportReadLoop(RefCountedPtr<ChaoticGoodTransport> transport); auto TransportReadLoop(RefCountedPtr<ChaoticGoodTransport> transport);
@ -143,7 +144,6 @@ class ChaoticGoodServerTransport final : public ServerTransport {
// Map of stream incoming server frames, key is stream_id. // Map of stream incoming server frames, key is stream_id.
StreamMap stream_map_ ABSL_GUARDED_BY(mu_); StreamMap stream_map_ ABSL_GUARDED_BY(mu_);
uint32_t last_seen_new_stream_id_ = 0; uint32_t last_seen_new_stream_id_ = 0;
bool aborted_with_error_ ABSL_GUARDED_BY(mu_) = false;
RefCountedPtr<Party> party_; RefCountedPtr<Party> party_;
ConnectivityStateTracker state_tracker_ ABSL_GUARDED_BY(mu_){ ConnectivityStateTracker state_tracker_ ABSL_GUARDED_BY(mu_){
"chaotic_good_server", GRPC_CHANNEL_READY}; "chaotic_good_server", GRPC_CHANNEL_READY};

103
src/core/server/server.cc
Unescape View File

@ -813,60 +813,55 @@ absl::StatusOr<ClientMetadataHandle> CheckClientMetadata(
} // namespace } // namespace
auto Server::MatchAndPublishCall(CallHandler call_handler) { auto Server::MatchAndPublishCall(CallHandler call_handler) {
call_handler.SpawnGuardedUntilCallCompletes( call_handler.SpawnGuarded("request_matcher", [this, call_handler]() mutable {
"request_matcher", [this, call_handler]() mutable { return TrySeq(
return TrySeq( // Wait for initial metadata to pass through all filters
// Wait for initial metadata to pass through all filters Map(call_handler.PullClientInitialMetadata(), CheckClientMetadata),
Map(call_handler.PullClientInitialMetadata(), CheckClientMetadata), // Match request with requested call
// Match request with requested call [this, call_handler](ClientMetadataHandle md) mutable {
[this, call_handler](ClientMetadataHandle md) mutable { auto* registered_method = static_cast<RegisteredMethod*>(
auto* registered_method = static_cast<RegisteredMethod*>( md->get(GrpcRegisteredMethod()).value_or(nullptr));
md->get(GrpcRegisteredMethod()).value_or(nullptr)); RequestMatcherInterface* rm;
RequestMatcherInterface* rm; grpc_server_register_method_payload_handling payload_handling =
grpc_server_register_method_payload_handling payload_handling = GRPC_SRM_PAYLOAD_NONE;
GRPC_SRM_PAYLOAD_NONE; if (registered_method == nullptr) {
if (registered_method == nullptr) { rm = unregistered_request_matcher_.get();
rm = unregistered_request_matcher_.get(); } else {
} else { payload_handling = registered_method->payload_handling;
payload_handling = registered_method->payload_handling; rm = registered_method->matcher.get();
rm = registered_method->matcher.get(); }
} auto maybe_read_first_message = If(
auto maybe_read_first_message = If( payload_handling == GRPC_SRM_PAYLOAD_READ_INITIAL_BYTE_BUFFER,
payload_handling == GRPC_SRM_PAYLOAD_READ_INITIAL_BYTE_BUFFER, [call_handler]() mutable { return call_handler.PullMessage(); },
[call_handler]() mutable { []() -> ValueOrFailure<absl::optional<MessageHandle>> {
return call_handler.PullMessage(); return ValueOrFailure<absl::optional<MessageHandle>>(
}, absl::nullopt);
[]() -> ValueOrFailure<absl::optional<MessageHandle>> { });
return ValueOrFailure<absl::optional<MessageHandle>>( return TryJoin<absl::StatusOr>(
absl::nullopt); std::move(maybe_read_first_message), rm->MatchRequest(0),
}); [md = std::move(md)]() mutable {
return TryJoin<absl::StatusOr>( return ValueOrFailure<ClientMetadataHandle>(std::move(md));
std::move(maybe_read_first_message), rm->MatchRequest(0), });
[md = std::move(md)]() mutable { },
return ValueOrFailure<ClientMetadataHandle>(std::move(md)); // Publish call to cq
}); [call_handler, this](std::tuple<absl::optional<MessageHandle>,
}, RequestMatcherInterface::MatchResult,
// Publish call to cq ClientMetadataHandle>
[call_handler, r) {
this](std::tuple<absl::optional<MessageHandle>, RequestMatcherInterface::MatchResult& mr = std::get<1>(r);
RequestMatcherInterface::MatchResult, auto md = std::move(std::get<2>(r));
ClientMetadataHandle> auto* rc = mr.TakeCall();
r) { rc->Complete(std::move(std::get<0>(r)), *md);
RequestMatcherInterface::MatchResult& mr = std::get<1>(r); grpc_call* call =
auto md = std::move(std::get<2>(r)); MakeServerCall(call_handler, std::move(md), this,
auto* rc = mr.TakeCall(); rc->cq_bound_to_call, rc->initial_metadata);
rc->Complete(std::move(std::get<0>(r)), *md); *rc->call = call;
grpc_call* call = return Map(WaitForCqEndOp(false, rc->tag, absl::OkStatus(), mr.cq()),
MakeServerCall(call_handler, std::move(md), this, [rc = std::unique_ptr<RequestedCall>(rc)](Empty) {
rc->cq_bound_to_call, rc->initial_metadata); return absl::OkStatus();
*rc->call = call; });
return Map( });
WaitForCqEndOp(false, rc->tag, absl::OkStatus(), mr.cq()), });
[rc = std::unique_ptr<RequestedCall>(rc)](Empty) {
return absl::OkStatus();
});
});
});
} }
absl::StatusOr<RefCountedPtr<UnstartedCallDestination>> absl::StatusOr<RefCountedPtr<UnstartedCallDestination>>

Write Preview
Loading…
Cancel
Save