diff --git a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc index a87dfda7321..2f3516066da 100644 --- a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc +++ b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc @@ -161,9 +161,7 @@ class GrpcLb : public LoadBalancingPolicy { bool seen_serverlist() const { return seen_serverlist_; } private: - // So Delete() can access our private dtor. - template - friend void grpc_core::Delete(T*); + GRPC_ALLOW_CLASS_TO_USE_NON_PUBLIC_DELETE ~BalancerCallState(); diff --git a/src/core/ext/filters/client_channel/lb_policy/xds/xds.cc b/src/core/ext/filters/client_channel/lb_policy/xds/xds.cc index e790ec25520..e5c27fe67a4 100644 --- a/src/core/ext/filters/client_channel/lb_policy/xds/xds.cc +++ b/src/core/ext/filters/client_channel/lb_policy/xds/xds.cc @@ -185,9 +185,7 @@ class XdsLb : public LoadBalancingPolicy { bool seen_initial_response() const { return seen_initial_response_; } private: - // So Delete() can access our private dtor. - template - friend void grpc_core::Delete(T*); + GRPC_ALLOW_CLASS_TO_USE_NON_PUBLIC_DELETE ~BalancerCallState(); diff --git a/src/core/ext/filters/client_channel/retry_throttle.h b/src/core/ext/filters/client_channel/retry_throttle.h index fddafcd903e..9e2fff56d24 100644 --- a/src/core/ext/filters/client_channel/retry_throttle.h +++ b/src/core/ext/filters/client_channel/retry_throttle.h @@ -21,6 +21,7 @@ #include +#include "src/core/lib/gprpp/memory.h" #include "src/core/lib/gprpp/ref_counted.h" namespace grpc_core { @@ -42,9 +43,7 @@ class ServerRetryThrottleData : public RefCounted { intptr_t milli_token_ratio() const { return milli_token_ratio_; } private: - // So Delete() can call our private dtor. - template - friend void grpc_core::Delete(T*); + GRPC_ALLOW_CLASS_TO_USE_NON_PUBLIC_DELETE ~ServerRetryThrottleData(); diff --git a/src/core/lib/gprpp/memory.h b/src/core/lib/gprpp/memory.h index 70ad430c51d..2e7658e4866 100644 --- a/src/core/lib/gprpp/memory.h +++ b/src/core/lib/gprpp/memory.h @@ -22,6 +22,7 @@ #include #include +#include #include #include @@ -29,14 +30,17 @@ // Add this to a class that want to use Delete(), but has a private or // protected destructor. -#define GRPC_ALLOW_CLASS_TO_USE_NON_PUBLIC_DELETE \ - template \ - friend void grpc_core::Delete(T*); +#define GRPC_ALLOW_CLASS_TO_USE_NON_PUBLIC_DELETE \ + template \ + friend void ::grpc_core::Delete(_Delete_T*); \ + template \ + friend void ::grpc_core::Delete(_Delete_T*); + // Add this to a class that want to use New(), but has a private or // protected constructor. -#define GRPC_ALLOW_CLASS_TO_USE_NON_PUBLIC_NEW \ - template \ - friend T* grpc_core::New(Args&&...); +#define GRPC_ALLOW_CLASS_TO_USE_NON_PUBLIC_NEW \ + template \ + friend _New_T* grpc_core::New(_New_Args&&...); namespace grpc_core { @@ -48,17 +52,30 @@ inline T* New(Args&&... args) { } // Alternative to delete, since we cannot use it (for fear of libstdc++) -template +// We cannot add a default value for can_be_null, because they are used as +// as friend template methods where we cannot define a default value. +// Instead we simply define two variants, one with and one without the boolean +// argument. +template inline void Delete(T* p) { - if (p == nullptr) return; + GPR_DEBUG_ASSERT(can_be_null || p != nullptr); + if (can_be_null && p == nullptr) return; p->~T(); gpr_free(p); } +template +inline void Delete(T* p) { + Delete(p); +} template class DefaultDelete { public: - void operator()(T* p) { Delete(p); } + void operator()(T* p) { + // std::unique_ptr is gauranteed not to call the deleter + // if the pointer is nullptr. + Delete(p); + } }; template > diff --git a/src/core/lib/slice/slice_hash_table.h b/src/core/lib/slice/slice_hash_table.h index c20eca9db77..bd003c2c5b4 100644 --- a/src/core/lib/slice/slice_hash_table.h +++ b/src/core/lib/slice/slice_hash_table.h @@ -25,6 +25,7 @@ #include #include "src/core/lib/gpr/useful.h" +#include "src/core/lib/gprpp/memory.h" #include "src/core/lib/gprpp/ref_counted.h" #include "src/core/lib/gprpp/ref_counted_ptr.h" #include "src/core/lib/slice/slice_internal.h" @@ -77,13 +78,8 @@ class SliceHashTable : public RefCounted> { static int Cmp(const SliceHashTable& a, const SliceHashTable& b); private: - // So New() can call our private ctor. - template - friend T2* New(Args&&... args); - - // So Delete() can call our private dtor. - template - friend void Delete(T2*); + GRPC_ALLOW_CLASS_TO_USE_NON_PUBLIC_DELETE + GRPC_ALLOW_CLASS_TO_USE_NON_PUBLIC_NEW SliceHashTable(size_t num_entries, Entry* entries, ValueCmp value_cmp); virtual ~SliceHashTable(); diff --git a/src/core/lib/slice/slice_weak_hash_table.h b/src/core/lib/slice/slice_weak_hash_table.h index 8c5562bf196..bd59a4c2e48 100644 --- a/src/core/lib/slice/slice_weak_hash_table.h +++ b/src/core/lib/slice/slice_weak_hash_table.h @@ -61,13 +61,8 @@ class SliceWeakHashTable : public RefCounted> { } private: - // So New() can call our private ctor. - template - friend T2* New(Args&&... args); - - // So Delete() can call our private dtor. - template - friend void Delete(T2*); + GRPC_ALLOW_CLASS_TO_USE_NON_PUBLIC_NEW + GRPC_ALLOW_CLASS_TO_USE_NON_PUBLIC_DELETE SliceWeakHashTable() = default; ~SliceWeakHashTable() = default; diff --git a/src/core/tsi/ssl/session_cache/ssl_session_cache.h b/src/core/tsi/ssl/session_cache/ssl_session_cache.h index 37fa2d124c9..16ab97714a7 100644 --- a/src/core/tsi/ssl/session_cache/ssl_session_cache.h +++ b/src/core/tsi/ssl/session_cache/ssl_session_cache.h @@ -67,13 +67,8 @@ class SslSessionLRUCache : public grpc_core::RefCounted { SslSessionPtr Get(const char* key); private: - // So New() can call our private ctor. - template - friend T* grpc_core::New(Args&&... args); - - // So Delete() can call our private dtor. - template - friend void grpc_core::Delete(T*); + GRPC_ALLOW_CLASS_TO_USE_NON_PUBLIC_NEW + GRPC_ALLOW_CLASS_TO_USE_NON_PUBLIC_DELETE class Node;