Skip to content

Commit

Permalink
Merge pull request grpc#20982 from veblush/memory-legacy-2
Browse files Browse the repository at this point in the history
Fix the memory new/delete mismatch for unique_ptr<char>
  • Loading branch information
veblush authored Nov 8, 2019
2 parents a8450e7 + e45b60d commit da3a1d3
Show file tree
Hide file tree
Showing 131 changed files with 392 additions and 339 deletions.
39 changes: 21 additions & 18 deletions src/core/ext/filters/client_channel/client_channel.cc
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ class ChannelData {
void ProcessLbPolicy(
const Resolver::Result& resolver_result,
const internal::ClientChannelGlobalParsedConfig* parsed_service_config,
std::unique_ptr<char>* lb_policy_name,
grpc_core::UniquePtr<char>* lb_policy_name,
RefCountedPtr<LoadBalancingPolicy::Config>* lb_policy_config);

//
Expand All @@ -265,8 +265,8 @@ class ChannelData {
ClientChannelFactory* client_channel_factory_;
const grpc_channel_args* channel_args_;
RefCountedPtr<ServiceConfig> default_service_config_;
std::unique_ptr<char> server_name_;
std::unique_ptr<char> target_uri_;
grpc_core::UniquePtr<char> server_name_;
grpc_core::UniquePtr<char> target_uri_;
channelz::ChannelNode* channelz_node_;

//
Expand All @@ -288,7 +288,7 @@ class ChannelData {
RefCountedPtr<SubchannelPoolInterface> subchannel_pool_;
OrphanablePtr<ResolvingLoadBalancingPolicy> resolving_lb_policy_;
ConnectivityStateTracker state_tracker_;
std::unique_ptr<char> health_check_service_name_;
grpc_core::UniquePtr<char> health_check_service_name_;
RefCountedPtr<ServiceConfig> saved_service_config_;
bool received_first_resolver_result_ = false;
// The number of SubchannelWrapper instances referencing a given Subchannel.
Expand All @@ -314,8 +314,8 @@ class ChannelData {
// synchronously via get_channel_info().
//
gpr_mu info_mu_;
std::unique_ptr<char> info_lb_policy_name_;
std::unique_ptr<char> info_service_config_json_;
grpc_core::UniquePtr<char> info_lb_policy_name_;
grpc_core::UniquePtr<char> info_service_config_json_;

//
// Fields guarded by a mutex, since they need to be accessed
Expand Down Expand Up @@ -843,7 +843,7 @@ class CallData {
class ChannelData::SubchannelWrapper : public SubchannelInterface {
public:
SubchannelWrapper(ChannelData* chand, Subchannel* subchannel,
std::unique_ptr<char> health_check_service_name)
grpc_core::UniquePtr<char> health_check_service_name)
: SubchannelInterface(&grpc_client_channel_routing_trace),
chand_(chand),
subchannel_(subchannel),
Expand Down Expand Up @@ -906,7 +906,8 @@ class ChannelData::SubchannelWrapper : public SubchannelInterface {
initial_state);
subchannel_->WatchConnectivityState(
initial_state,
std::unique_ptr<char>(gpr_strdup(health_check_service_name_.get())),
grpc_core::UniquePtr<char>(
gpr_strdup(health_check_service_name_.get())),
OrphanablePtr<Subchannel::ConnectivityStateWatcherInterface>(
watcher_wrapper));
}
Expand All @@ -929,7 +930,7 @@ class ChannelData::SubchannelWrapper : public SubchannelInterface {
}

void UpdateHealthCheckServiceName(
std::unique_ptr<char> health_check_service_name) {
grpc_core::UniquePtr<char> health_check_service_name) {
if (GRPC_TRACE_FLAG_ENABLED(grpc_client_channel_routing_trace)) {
gpr_log(GPR_INFO,
"chand=%p: subchannel wrapper %p: updating health check service "
Expand All @@ -955,7 +956,8 @@ class ChannelData::SubchannelWrapper : public SubchannelInterface {
watcher_wrapper = replacement;
subchannel_->WatchConnectivityState(
replacement->last_seen_state(),
std::unique_ptr<char>(gpr_strdup(health_check_service_name.get())),
grpc_core::UniquePtr<char>(
gpr_strdup(health_check_service_name.get())),
OrphanablePtr<Subchannel::ConnectivityStateWatcherInterface>(
replacement));
}
Expand Down Expand Up @@ -1114,7 +1116,7 @@ class ChannelData::SubchannelWrapper : public SubchannelInterface {

ChannelData* chand_;
Subchannel* subchannel_;
std::unique_ptr<char> health_check_service_name_;
grpc_core::UniquePtr<char> health_check_service_name_;
// Maps from the address of the watcher passed to us by the LB policy
// to the address of the WrapperWatcher that we passed to the underlying
// subchannel. This is needed so that when the LB policy calls
Expand Down Expand Up @@ -1300,7 +1302,7 @@ class ChannelData::ClientChannelControlHelper
const grpc_channel_args& args) override {
bool inhibit_health_checking = grpc_channel_arg_get_bool(
grpc_channel_args_find(&args, GRPC_ARG_INHIBIT_HEALTH_CHECKING), false);
std::unique_ptr<char> health_check_service_name;
grpc_core::UniquePtr<char> health_check_service_name;
if (!inhibit_health_checking) {
health_check_service_name.reset(
gpr_strdup(chand_->health_check_service_name_.get()));
Expand Down Expand Up @@ -1596,7 +1598,7 @@ void ChannelData::CreateResolvingLoadBalancingPolicyLocked() {
lb_args.combiner = combiner_;
lb_args.channel_control_helper = MakeUnique<ClientChannelControlHelper>(this);
lb_args.args = channel_args_;
std::unique_ptr<char> target_uri(gpr_strdup(target_uri_.get()));
grpc_core::UniquePtr<char> target_uri(gpr_strdup(target_uri_.get()));
resolving_lb_policy_.reset(new ResolvingLoadBalancingPolicy(
std::move(lb_args), &grpc_client_channel_routing_trace,
std::move(target_uri), ProcessResolverResultLocked, this));
Expand All @@ -1619,7 +1621,7 @@ void ChannelData::DestroyResolvingLoadBalancingPolicyLocked() {
void ChannelData::ProcessLbPolicy(
const Resolver::Result& resolver_result,
const internal::ClientChannelGlobalParsedConfig* parsed_service_config,
std::unique_ptr<char>* lb_policy_name,
grpc_core::UniquePtr<char>* lb_policy_name,
RefCountedPtr<LoadBalancingPolicy::Config>* lb_policy_config) {
// Prefer the LB policy name found in the service config.
if (parsed_service_config != nullptr &&
Expand Down Expand Up @@ -1714,7 +1716,7 @@ bool ChannelData::ProcessResolverResultLocked(
return false;
}
// Process service config.
std::unique_ptr<char> service_config_json;
grpc_core::UniquePtr<char> service_config_json;
const internal::ClientChannelGlobalParsedConfig* parsed_service_config =
nullptr;
if (service_config != nullptr) {
Expand Down Expand Up @@ -1748,8 +1750,9 @@ bool ChannelData::ProcessResolverResultLocked(
}
// Update health check service name used by existing subchannel wrappers.
for (auto* subchannel_wrapper : chand->subchannel_wrappers_) {
subchannel_wrapper->UpdateHealthCheckServiceName(std::unique_ptr<char>(
gpr_strdup(chand->health_check_service_name_.get())));
subchannel_wrapper->UpdateHealthCheckServiceName(
grpc_core::UniquePtr<char>(
gpr_strdup(chand->health_check_service_name_.get())));
}
// Save service config.
chand->saved_service_config_ = std::move(service_config);
Expand All @@ -1774,7 +1777,7 @@ bool ChannelData::ProcessResolverResultLocked(
chand->UpdateServiceConfigLocked(std::move(retry_throttle_data),
chand->saved_service_config_);
}
std::unique_ptr<char> processed_lb_policy_name;
grpc_core::UniquePtr<char> processed_lb_policy_name;
chand->ProcessLbPolicy(result, parsed_service_config,
&processed_lb_policy_name, lb_policy_config);
// Swap out the data used by GetChannelInfo().
Expand Down
4 changes: 2 additions & 2 deletions src/core/ext/filters/client_channel/http_proxy.cc
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,8 @@ static bool proxy_mapper_map_name(grpc_proxy_mapper* /*mapper*/,
if (no_proxy_str != nullptr) {
static const char* NO_PROXY_SEPARATOR = ",";
bool use_proxy = true;
std::unique_ptr<char> server_host;
std::unique_ptr<char> server_port;
grpc_core::UniquePtr<char> server_host;
grpc_core::UniquePtr<char> server_port;
if (!grpc_core::SplitHostPort(
uri->path[0] == '/' ? uri->path + 1 : uri->path, &server_host,
&server_port)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ class GrpcLb : public LoadBalancingPolicy {
const grpc_grpclb_serverlist* serverlist() const { return serverlist_; }

// Returns a text representation suitable for logging.
std::unique_ptr<char> AsText() const;
grpc_core::UniquePtr<char> AsText() const;

// Extracts all non-drop entries into a ServerAddressList.
ServerAddressList GetServerAddressList(
Expand Down Expand Up @@ -430,7 +430,7 @@ void ParseServer(const grpc_grpclb_server* server,
}
}

std::unique_ptr<char> GrpcLb::Serverlist::AsText() const {
grpc_core::UniquePtr<char> GrpcLb::Serverlist::AsText() const {
gpr_strvec entries;
gpr_strvec_init(&entries);
for (size_t i = 0; i < serverlist_->num_servers; ++i) {
Expand All @@ -449,7 +449,7 @@ std::unique_ptr<char> GrpcLb::Serverlist::AsText() const {
gpr_free(ipport);
gpr_strvec_add(&entries, entry);
}
std::unique_ptr<char> result(gpr_strvec_flatten(&entries, nullptr));
grpc_core::UniquePtr<char> result(gpr_strvec_flatten(&entries, nullptr));
gpr_strvec_destroy(&entries);
return result;
}
Expand Down Expand Up @@ -1099,7 +1099,7 @@ void GrpcLb::BalancerCallState::OnBalancerMessageReceivedLocked(
GPR_ASSERT(lb_calld->lb_call_ != nullptr);
auto serverlist_wrapper = MakeRefCounted<Serverlist>(serverlist);
if (GRPC_TRACE_FLAG_ENABLED(grpc_lb_glb_trace)) {
std::unique_ptr<char> serverlist_text = serverlist_wrapper->AsText();
grpc_core::UniquePtr<char> serverlist_text = serverlist_wrapper->AsText();
gpr_log(GPR_INFO,
"[grpclb %p] lb_calld=%p: Serverlist with %" PRIuPTR
" servers received:\n%s",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ namespace grpc_core {

namespace {

int BalancerNameCmp(const std::unique_ptr<char>& a,
const std::unique_ptr<char>& b) {
int BalancerNameCmp(const grpc_core::UniquePtr<char>& a,
const grpc_core::UniquePtr<char>& b) {
return strcmp(a.get(), b.get());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ void GrpcLbClientStats::AddCallDropped(const char* token) {
}
}
// Not found, so add a new entry.
drop_token_counts_->emplace_back(std::unique_ptr<char>(gpr_strdup(token)), 1);
drop_token_counts_->emplace_back(
grpc_core::UniquePtr<char>(gpr_strdup(token)), 1);
}

namespace {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@ namespace grpc_core {
class GrpcLbClientStats : public RefCounted<GrpcLbClientStats> {
public:
struct DropTokenCount {
std::unique_ptr<char> token;
grpc_core::UniquePtr<char> token;
int64_t count;

DropTokenCount(std::unique_ptr<char> token, int64_t count)
DropTokenCount(grpc_core::UniquePtr<char> token, int64_t count)
: token(std::move(token)), count(count) {}
};

Expand Down
8 changes: 4 additions & 4 deletions src/core/ext/filters/client_channel/lb_policy/xds/cds.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,13 @@ constexpr char kCds[] = "cds_experimental";
// Parsed config for this LB policy.
class ParsedCdsConfig : public LoadBalancingPolicy::Config {
public:
explicit ParsedCdsConfig(std::unique_ptr<char> cluster)
explicit ParsedCdsConfig(grpc_core::UniquePtr<char> cluster)
: cluster_(std::move(cluster)) {}
const char* cluster() const { return cluster_.get(); }
const char* name() const override { return kCds; }

private:
std::unique_ptr<char> cluster_;
grpc_core::UniquePtr<char> cluster_;
};

// CDS LB policy.
Expand Down Expand Up @@ -136,7 +136,7 @@ void CdsLb::ClusterWatcher::OnClusterChanged(CdsUpdate cluster_data) {
? parent_->config_->cluster()
: cluster_data.eds_service_name.get()));
gpr_free(lrs_str);
std::unique_ptr<char> json_str_deleter(json_str);
grpc_core::UniquePtr<char> json_str_deleter(json_str);
if (GRPC_TRACE_FLAG_ENABLED(grpc_cds_lb_trace)) {
gpr_log(GPR_INFO, "[cdslb %p] generated config for child policy: %s",
parent_.get(), json_str);
Expand Down Expand Up @@ -343,7 +343,7 @@ class CdsFactory : public LoadBalancingPolicyFactory {
}
if (error_list.empty()) {
return MakeRefCounted<ParsedCdsConfig>(
std::unique_ptr<char>(gpr_strdup(cluster)));
grpc_core::UniquePtr<char>(gpr_strdup(cluster)));
} else {
*error = GRPC_ERROR_CREATE_FROM_VECTOR("Cds Parser", &error_list);
return nullptr;
Expand Down
17 changes: 9 additions & 8 deletions src/core/ext/filters/client_channel/lb_policy/xds/xds.cc
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ class ParsedXdsConfig : public LoadBalancingPolicy::Config {
public:
ParsedXdsConfig(RefCountedPtr<LoadBalancingPolicy::Config> child_policy,
RefCountedPtr<LoadBalancingPolicy::Config> fallback_policy,
std::unique_ptr<char> eds_service_name,
std::unique_ptr<char> lrs_load_reporting_server_name)
grpc_core::UniquePtr<char> eds_service_name,
grpc_core::UniquePtr<char> lrs_load_reporting_server_name)
: child_policy_(std::move(child_policy)),
fallback_policy_(std::move(fallback_policy)),
eds_service_name_(std::move(eds_service_name)),
Expand All @@ -105,8 +105,8 @@ class ParsedXdsConfig : public LoadBalancingPolicy::Config {
private:
RefCountedPtr<LoadBalancingPolicy::Config> child_policy_;
RefCountedPtr<LoadBalancingPolicy::Config> fallback_policy_;
std::unique_ptr<char> eds_service_name_;
std::unique_ptr<char> lrs_load_reporting_server_name_;
grpc_core::UniquePtr<char> eds_service_name_;
grpc_core::UniquePtr<char> lrs_load_reporting_server_name_;
};

class XdsLb : public LoadBalancingPolicy {
Expand Down Expand Up @@ -406,7 +406,7 @@ class XdsLb : public LoadBalancingPolicy {
}

// Server name from target URI.
std::unique_ptr<char> server_name_;
grpc_core::UniquePtr<char> server_name_;

// Current channel args and config from the resolver.
const grpc_channel_args* args_ = nullptr;
Expand Down Expand Up @@ -495,7 +495,7 @@ LoadBalancingPolicy::PickResult XdsLb::EndpointPickerWrapper::Pick(

XdsLb::PickResult XdsLb::LocalityPicker::Pick(PickArgs args) {
// Handle drop.
const std::unique_ptr<char>* drop_category;
const grpc_core::UniquePtr<char>* drop_category;
if (drop_config_->ShouldDrop(&drop_category)) {
xds_policy_->client_stats_.AddCallDropped(*drop_category);
PickResult result;
Expand Down Expand Up @@ -1871,8 +1871,9 @@ class XdsFactory : public LoadBalancingPolicyFactory {
if (error_list.empty()) {
return MakeRefCounted<ParsedXdsConfig>(
std::move(child_policy), std::move(fallback_policy),
std::unique_ptr<char>(gpr_strdup(eds_service_name)),
std::unique_ptr<char>(gpr_strdup(lrs_load_reporting_server_name)));
grpc_core::UniquePtr<char>(gpr_strdup(eds_service_name)),
grpc_core::UniquePtr<char>(
gpr_strdup(lrs_load_reporting_server_name)));
} else {
*error = GRPC_ERROR_CREATE_FROM_VECTOR("Xds Parser", &error_list);
return nullptr;
Expand Down
8 changes: 4 additions & 4 deletions src/core/ext/filters/client_channel/parse_address.cc
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,8 @@ bool grpc_parse_ipv4_hostport(const char* hostport, grpc_resolved_address* addr,
bool log_errors) {
bool success = false;
// Split host and port.
std::unique_ptr<char> host;
std::unique_ptr<char> port;
grpc_core::UniquePtr<char> host;
grpc_core::UniquePtr<char> port;
if (!grpc_core::SplitHostPort(hostport, &host, &port)) {
if (log_errors) {
gpr_log(GPR_ERROR, "Failed gpr_split_host_port(%s, ...)", hostport);
Expand Down Expand Up @@ -125,8 +125,8 @@ bool grpc_parse_ipv6_hostport(const char* hostport, grpc_resolved_address* addr,
bool log_errors) {
bool success = false;
// Split host and port.
std::unique_ptr<char> host;
std::unique_ptr<char> port;
grpc_core::UniquePtr<char> host;
grpc_core::UniquePtr<char> port;
if (!grpc_core::SplitHostPort(hostport, &host, &port)) {
if (log_errors) {
gpr_log(GPR_ERROR, "Failed gpr_split_host_port(%s, ...)", hostport);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,8 @@ static bool should_use_ares(const char* resolver_env) {
static bool g_use_ares_dns_resolver;

void grpc_resolver_dns_ares_init() {
std::unique_ptr<char> resolver = GPR_GLOBAL_CONFIG_GET(grpc_dns_resolver);
grpc_core::UniquePtr<char> resolver =
GPR_GLOBAL_CONFIG_GET(grpc_dns_resolver);
if (should_use_ares(resolver.get())) {
g_use_ares_dns_resolver = true;
gpr_log(GPR_DEBUG, "Using ares dns resolver");
Expand Down
Loading

0 comments on commit da3a1d3

Please sign in to comment.