Skip to content

Commit

Permalink
Revert "[transport] Centralize ref-counting between transports (grpc#…
Browse files Browse the repository at this point in the history
…36460)"

This reverts commit 03e2bf2.
  • Loading branch information
yashykt committed May 8, 2024
1 parent 03e6617 commit a7461bc
Show file tree
Hide file tree
Showing 6 changed files with 26 additions and 29 deletions.
2 changes: 1 addition & 1 deletion src/core/ext/transport/chaotic_good/client_transport.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ class ChaoticGoodClientTransport final : public ClientTransport {
grpc_endpoint* GetEndpoint() override { return nullptr; }
void Orphan() override {
AbortWithError();
Unref();
delete this;
}

void StartCall(CallHandler call_handler) override;
Expand Down
2 changes: 1 addition & 1 deletion src/core/ext/transport/chaotic_good/server_transport.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ class ChaoticGoodServerTransport final : public ServerTransport {
void SetPollsetSet(grpc_stream*, grpc_pollset_set*) override {}
void PerformOp(grpc_transport_op*) override;
grpc_endpoint* GetEndpoint() override { return nullptr; }
void Orphan() override { Unref(); }
void Orphan() override { delete this; }

void SetAcceptor(Acceptor* acceptor) override;
void AbortWithError();
Expand Down
9 changes: 8 additions & 1 deletion src/core/ext/transport/chttp2/transport/chttp2_transport.cc
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,8 @@ static bool g_default_server_keepalive_permit_without_calls = false;

#define MAX_CLIENT_STREAM_ID 0x7fffffffu
grpc_core::TraceFlag grpc_keepalive_trace(false, "http_keepalive");
grpc_core::DebugOnlyTraceFlag grpc_trace_chttp2_refcount(false,
"chttp2_refcount");

// forward declarations of various callbacks that we'll build closures around
static void write_action_begin_locked(
Expand Down Expand Up @@ -593,7 +595,12 @@ static void init_keepalive_pings_if_enabled_locked(
grpc_chttp2_transport::grpc_chttp2_transport(
const grpc_core::ChannelArgs& channel_args, grpc_endpoint* ep,
bool is_client)
: ep(ep),
: grpc_core::RefCounted<grpc_chttp2_transport,
grpc_core::NonPolymorphicRefCount>(
GRPC_TRACE_FLAG_ENABLED(grpc_trace_chttp2_refcount)
? "chttp2_refcount"
: nullptr),
ep(ep),
peer_string(
grpc_core::Slice::FromCopiedString(grpc_endpoint_get_peer(ep))),
memory_owner(channel_args.GetObject<grpc_core::ResourceQuota>()
Expand Down
12 changes: 5 additions & 7 deletions src/core/ext/transport/chttp2/transport/internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -223,19 +223,17 @@ typedef enum {
GRPC_CHTTP2_KEEPALIVE_STATE_DISABLED,
} grpc_chttp2_keepalive_state;

struct grpc_chttp2_transport final : public grpc_core::FilterStackTransport,
public grpc_core::KeepsGrpcInitialized {
struct grpc_chttp2_transport final
: public grpc_core::FilterStackTransport,
public grpc_core::RefCounted<grpc_chttp2_transport,
grpc_core::NonPolymorphicRefCount>,
public grpc_core::KeepsGrpcInitialized {
grpc_chttp2_transport(const grpc_core::ChannelArgs& channel_args,
grpc_endpoint* ep, bool is_client);
~grpc_chttp2_transport() override;

void Orphan() override;

grpc_core::RefCountedPtr<grpc_chttp2_transport> Ref() {
return grpc_core::FilterStackTransport::RefAsSubclass<
grpc_chttp2_transport>();
}

size_t SizeOfStream() const override;
bool HackyDisableStreamOpBatchCoalescingInConnectedChannel() const override;
void PerformStreamOp(grpc_stream* gs,
Expand Down
28 changes: 10 additions & 18 deletions src/core/ext/transport/inproc/inproc_transport.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,8 @@
namespace grpc_core {

namespace {
class InprocClientTransport;

class InprocServerTransport final : public ServerTransport {
class InprocServerTransport final : public RefCounted<InprocServerTransport>,
public ServerTransport {
public:
void SetAcceptor(Acceptor* acceptor) override {
acceptor_ = acceptor;
Expand Down Expand Up @@ -98,8 +97,6 @@ class InprocServerTransport final : public ServerTransport {
return acceptor_->CreateCall(std::move(md), acceptor_->CreateArena());
}

OrphanablePtr<InprocClientTransport> MakeClientTransport();

private:
enum class ConnectionState : uint8_t { kInitial, kReady, kDisconnected };

Expand All @@ -114,10 +111,6 @@ class InprocServerTransport final : public ServerTransport {

class InprocClientTransport final : public ClientTransport {
public:
explicit InprocClientTransport(
RefCountedPtr<InprocServerTransport> server_transport)
: server_transport_(std::move(server_transport)) {}

void StartCall(CallHandler call_handler) override {
call_handler.SpawnGuarded(
"pull_initial_metadata",
Expand All @@ -134,6 +127,10 @@ class InprocClientTransport final : public ClientTransport {

void Orphan() override { delete this; }

OrphanablePtr<Transport> GetServerTransport() {
return OrphanablePtr<Transport>(server_transport_->Ref().release());
}

FilterStackTransport* filter_stack_transport() override { return nullptr; }
ClientTransport* client_transport() override { return this; }
ServerTransport* server_transport() override { return nullptr; }
Expand All @@ -149,7 +146,8 @@ class InprocClientTransport final : public ClientTransport {
absl::UnavailableError("Client transport closed"));
}

const RefCountedPtr<InprocServerTransport> server_transport_;
RefCountedPtr<InprocServerTransport> server_transport_ =
MakeRefCounted<InprocServerTransport>();
};

bool UsePromiseBasedTransport() {
Expand All @@ -159,12 +157,6 @@ bool UsePromiseBasedTransport() {
return true;
}

OrphanablePtr<InprocClientTransport>
InprocServerTransport::MakeClientTransport() {
return MakeOrphanable<InprocClientTransport>(
RefAsSubclass<InprocServerTransport>());
}

OrphanablePtr<Channel> MakeLameChannel(absl::string_view why,
absl::Status error) {
gpr_log(GPR_ERROR, "%s: %s", std::string(why).c_str(),
Expand Down Expand Up @@ -206,8 +198,8 @@ OrphanablePtr<Channel> MakeInprocChannel(Server* server,

std::pair<OrphanablePtr<Transport>, OrphanablePtr<Transport>>
MakeInProcessTransportPair() {
auto server_transport = MakeOrphanable<InprocServerTransport>();
auto client_transport = server_transport->MakeClientTransport();
auto client_transport = MakeOrphanable<InprocClientTransport>();
auto server_transport = client_transport->GetServerTransport();
return std::make_pair(std::move(client_transport),
std::move(server_transport));
}
Expand Down
2 changes: 1 addition & 1 deletion src/core/lib/transport/transport.h
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,7 @@ class FilterStackTransport;
class ClientTransport;
class ServerTransport;

class Transport : public InternallyRefCounted<Transport> {
class Transport : public Orphanable {
public:
struct RawPointerChannelArgTag {};
static absl::string_view ChannelArgName() { return GRPC_ARG_TRANSPORT; }
Expand Down

0 comments on commit a7461bc

Please sign in to comment.