Merge pull request #23799 from vjpai/call_registration

Fix issues around lifetime of c_str() results used in call registration maps
pull/23804/head
Vijay Pai 5 years ago committed by GitHub
commit 412e987f67
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 48
      src/core/lib/surface/channel.cc
  2. 11
      src/core/lib/surface/channel.h

@ -415,26 +415,33 @@ grpc_call* grpc_channel_create_pollset_set_call(
namespace grpc_core {
RegisteredCall::RegisteredCall(const char* method, const char* host) {
path = grpc_mdelem_from_slices(GRPC_MDSTR_PATH,
grpc_core::ExternallyManagedSlice(method));
authority =
host ? grpc_mdelem_from_slices(GRPC_MDSTR_AUTHORITY,
grpc_core::ExternallyManagedSlice(host))
: GRPC_MDNULL;
}
RegisteredCall::RegisteredCall(const char* method_arg, const char* host_arg)
: method(method_arg != nullptr ? method_arg : ""),
host(host_arg != nullptr ? host_arg : ""),
path(grpc_mdelem_from_slices(
GRPC_MDSTR_PATH, grpc_core::ExternallyManagedSlice(method.c_str()))),
authority(!host.empty()
? grpc_mdelem_from_slices(
GRPC_MDSTR_AUTHORITY,
grpc_core::ExternallyManagedSlice(host.c_str()))
: GRPC_MDNULL) {}
// TODO(vjpai): Delete copy-constructor when allowed by all supported compilers.
RegisteredCall::RegisteredCall(const RegisteredCall& other) {
path = other.path;
authority = other.authority;
GRPC_MDELEM_REF(path);
GRPC_MDELEM_REF(authority);
}
RegisteredCall::RegisteredCall(RegisteredCall&& other) noexcept {
path = other.path;
authority = other.authority;
RegisteredCall::RegisteredCall(const RegisteredCall& other)
: RegisteredCall(other.method.c_str(), other.host.c_str()) {}
RegisteredCall::RegisteredCall(RegisteredCall&& other) noexcept
: method(std::move(other.method)),
host(std::move(other.host)),
path(grpc_mdelem_from_slices(
GRPC_MDSTR_PATH, grpc_core::ExternallyManagedSlice(method.c_str()))),
authority(!host.empty()
? grpc_mdelem_from_slices(
GRPC_MDSTR_AUTHORITY,
grpc_core::ExternallyManagedSlice(host.c_str()))
: GRPC_MDNULL) {
GRPC_MDELEM_UNREF(other.path);
GRPC_MDELEM_UNREF(other.authority);
other.path = GRPC_MDNULL;
other.authority = GRPC_MDNULL;
}
@ -457,13 +464,14 @@ void* grpc_channel_register_call(grpc_channel* channel, const char* method,
grpc_core::MutexLock lock(&channel->registration_table->mu);
channel->registration_table->method_registration_attempts++;
auto key = std::make_pair(host, method);
auto key = std::make_pair(std::string(host != nullptr ? host : ""),
std::string(method != nullptr ? method : ""));
auto rc_posn = channel->registration_table->map.find(key);
if (rc_posn != channel->registration_table->map.end()) {
return &rc_posn->second;
}
auto insertion_result = channel->registration_table->map.insert(
{key, grpc_core::RegisteredCall(method, host)});
{std::move(key), grpc_core::RegisteredCall(method, host)});
return &insertion_result.first->second;
}

@ -69,21 +69,28 @@ void grpc_channel_update_call_size_estimate(grpc_channel* channel, size_t size);
namespace grpc_core {
struct RegisteredCall {
// The method and host are kept as part of this struct just to manage their
// lifetime since they must outlive the mdelem contents.
std::string method;
std::string host;
grpc_mdelem path;
grpc_mdelem authority;
explicit RegisteredCall(const char* method, const char* host);
explicit RegisteredCall(const char* method_arg, const char* host_arg);
// TODO(vjpai): delete copy constructor once all supported compilers allow
// std::map value_type to be MoveConstructible.
RegisteredCall(const RegisteredCall& other);
RegisteredCall(RegisteredCall&& other) noexcept;
RegisteredCall& operator=(const RegisteredCall&) = delete;
RegisteredCall& operator=(RegisteredCall&&) = delete;
~RegisteredCall();
};
struct CallRegistrationTable {
grpc_core::Mutex mu;
std::map<std::pair<const char*, const char*>, RegisteredCall>
std::map<std::pair<std::string, std::string>, RegisteredCall>
map /* GUARDED_BY(mu) */;
int method_registration_attempts /* GUARDED_BY(mu) */ = 0;
};

Loading…
Cancel
Save