From d3de9c548d1121e7c4787a4b81fd66be714abc04 Mon Sep 17 00:00:00 2001 From: Magnus Jedvert Date: Thu, 20 Aug 2015 16:03:52 +0200 Subject: [PATCH] rtc::Bind: Capture method objects as scoped_refptr if they are ref counted R=tommi@webrtc.org Review URL: https://codereview.webrtc.org/1300523004 . Cr-Commit-Position: refs/heads/master@{#9744} --- webrtc/base/bind.h | 213 ++++++++++++++++++++++++++++++++--- webrtc/base/bind.h.pump | 101 +++++++++++++++-- webrtc/base/bind_unittest.cc | 102 +++++++++++++++++ 3 files changed, 396 insertions(+), 20 deletions(-) diff --git a/webrtc/base/bind.h b/webrtc/base/bind.h index ebd395979c..2d8114052d 100644 --- a/webrtc/base/bind.h +++ b/webrtc/base/bind.h @@ -16,12 +16,13 @@ // /home/build/google3/third_party/gtest/scripts/pump.py bind.h.pump // Bind() is an overloaded function that converts method calls into function -// objects (aka functors). It captures any arguments to the method by value -// when Bind is called, producing a stateful, nullary function object. Care -// should be taken about the lifetime of objects captured by Bind(); the -// returned functor knows nothing about the lifetime of the method's object or -// any arguments passed by pointer, and calling the functor with a destroyed -// object will surely do bad things. +// objects (aka functors). The method object is captured as a scoped_refptr<> if +// possible, and as a raw pointer otherwise. Any arguments to the method are +// captured by value. The return value of Bind is a stateful, nullary function +// object. Care should be taken about the lifetime of objects captured by +// Bind(); the returned functor knows nothing about the lifetime of a non +// ref-counted method object or any arguments passed by pointer, and calling the +// functor with a destroyed object will surely do bad things. // // Example usage: // struct Foo { @@ -38,10 +39,33 @@ // cout << rtc::Bind(&Foo::Test3, &foo, 3)() << endl; // cout << rtc::Bind(&Foo::Test4, &foo, 7, 8.5f)() << endl; // } +// +// Example usage of ref counted objects: +// struct Bar { +// int AddRef(); +// int Release(); +// +// void Test() {} +// void BindThis() { +// // The functor passed to AsyncInvoke() will keep this object alive. +// invoker.AsyncInvoke(rtc::Bind(&Bar::Test, this)); +// } +// }; +// +// int main() { +// rtc::scoped_refptr bar = new rtc::RefCountedObject(); +// auto functor = rtc::Bind(&Bar::Test, bar); +// bar = nullptr; +// // The functor stores an internal scoped_refptr, so this is safe. +// functor(); +// } +// #ifndef WEBRTC_BASE_BIND_H_ #define WEBRTC_BASE_BIND_H_ +#include "webrtc/base/scoped_ref_ptr.h" + #define NONAME namespace rtc { @@ -53,6 +77,57 @@ namespace detail { // references stripped. This trick allows the compiler to dictate the Bind // parameter types rather than deduce them. template struct identity { typedef T type; }; + +// IsRefCounted::value will be true for types that can be used in +// rtc::scoped_refptr, i.e. types that implements nullary functions AddRef() +// and Release(), regardless of their return types. AddRef() and Release() can +// be defined in T or any superclass of T. +template +class IsRefCounted { + // This is a complex implementation detail done with SFINAE. + + // Define types such that sizeof(Yes) != sizeof(No). + struct Yes { char dummy[1]; }; + struct No { char dummy[2]; }; + // Define two overloaded template functions with return types of different + // size. This way, we can use sizeof() on the return type to determine which + // function the compiler would have chosen. One function will be preferred + // over the other if it is possible to create it without compiler errors, + // otherwise the compiler will simply remove it, and default to the less + // preferred function. + template + static Yes test(R* r, decltype(r->AddRef(), r->Release(), 42)); + template static No test(...); + +public: + // Trick the compiler to tell if it's possible to call AddRef() and Release(). + static const bool value = sizeof(test((T*)nullptr, 42)) == sizeof(Yes); +}; + +// TernaryTypeOperator is a helper class to select a type based on a static bool +// value. +template +struct TernaryTypeOperator {}; + +template +struct TernaryTypeOperator { + typedef IfTrueT type; +}; + +template +struct TernaryTypeOperator { + typedef IfFalseT type; +}; + +// PointerType::type will be scoped_refptr for ref counted types, and T* +// otherwise. +template +struct PointerType { + typedef typename TernaryTypeOperator::value, + scoped_refptr, + T*>::type type; +}; + } // namespace detail template @@ -64,7 +139,7 @@ class MethodFunctor0 { return (object_->*method_)(); } private: MethodT method_; - ObjectT* object_; + typename detail::PointerType::type object_; }; template @@ -98,6 +173,16 @@ Bind(FP_T(method), const ObjectT* object) { method, object); } +#undef FP_T +#define FP_T(x) R (ObjectT::*x)() + +template +MethodFunctor0 +Bind(FP_T(method), const scoped_refptr& object) { + return MethodFunctor0( + method, object.get()); +} + #undef FP_T #define FP_T(x) R (*x)() @@ -122,7 +207,7 @@ class MethodFunctor1 { return (object_->*method_)(p1_); } private: MethodT method_; - ObjectT* object_; + typename detail::PointerType::type object_; P1 p1_; }; @@ -164,6 +249,18 @@ Bind(FP_T(method), const ObjectT* object, method, object, p1); } +#undef FP_T +#define FP_T(x) R (ObjectT::*x)(P1) + +template +MethodFunctor1 +Bind(FP_T(method), const scoped_refptr& object, + typename detail::identity::type p1) { + return MethodFunctor1( + method, object.get(), p1); +} + #undef FP_T #define FP_T(x) R (*x)(P1) @@ -193,7 +290,7 @@ class MethodFunctor2 { return (object_->*method_)(p1_, p2_); } private: MethodT method_; - ObjectT* object_; + typename detail::PointerType::type object_; P1 p1_; P2 p2_; }; @@ -243,6 +340,20 @@ Bind(FP_T(method), const ObjectT* object, method, object, p1, p2); } +#undef FP_T +#define FP_T(x) R (ObjectT::*x)(P1, P2) + +template +MethodFunctor2 +Bind(FP_T(method), const scoped_refptr& object, + typename detail::identity::type p1, + typename detail::identity::type p2) { + return MethodFunctor2( + method, object.get(), p1, p2); +} + #undef FP_T #define FP_T(x) R (*x)(P1, P2) @@ -277,7 +388,7 @@ class MethodFunctor3 { return (object_->*method_)(p1_, p2_, p3_); } private: MethodT method_; - ObjectT* object_; + typename detail::PointerType::type object_; P1 p1_; P2 p2_; P3 p3_; @@ -335,6 +446,22 @@ Bind(FP_T(method), const ObjectT* object, method, object, p1, p2, p3); } +#undef FP_T +#define FP_T(x) R (ObjectT::*x)(P1, P2, P3) + +template +MethodFunctor3 +Bind(FP_T(method), const scoped_refptr& object, + typename detail::identity::type p1, + typename detail::identity::type p2, + typename detail::identity::type p3) { + return MethodFunctor3( + method, object.get(), p1, p2, p3); +} + #undef FP_T #define FP_T(x) R (*x)(P1, P2, P3) @@ -374,7 +501,7 @@ class MethodFunctor4 { return (object_->*method_)(p1_, p2_, p3_, p4_); } private: MethodT method_; - ObjectT* object_; + typename detail::PointerType::type object_; P1 p1_; P2 p2_; P3 p3_; @@ -440,6 +567,24 @@ Bind(FP_T(method), const ObjectT* object, method, object, p1, p2, p3, p4); } +#undef FP_T +#define FP_T(x) R (ObjectT::*x)(P1, P2, P3, P4) + +template +MethodFunctor4 +Bind(FP_T(method), const scoped_refptr& object, + typename detail::identity::type p1, + typename detail::identity::type p2, + typename detail::identity::type p3, + typename detail::identity::type p4) { + return MethodFunctor4( + method, object.get(), p1, p2, p3, p4); +} + #undef FP_T #define FP_T(x) R (*x)(P1, P2, P3, P4) @@ -484,7 +629,7 @@ class MethodFunctor5 { return (object_->*method_)(p1_, p2_, p3_, p4_, p5_); } private: MethodT method_; - ObjectT* object_; + typename detail::PointerType::type object_; P1 p1_; P2 p2_; P3 p3_; @@ -558,6 +703,26 @@ Bind(FP_T(method), const ObjectT* object, method, object, p1, p2, p3, p4, p5); } +#undef FP_T +#define FP_T(x) R (ObjectT::*x)(P1, P2, P3, P4, P5) + +template +MethodFunctor5 +Bind(FP_T(method), const scoped_refptr& object, + typename detail::identity::type p1, + typename detail::identity::type p2, + typename detail::identity::type p3, + typename detail::identity::type p4, + typename detail::identity::type p5) { + return MethodFunctor5( + method, object.get(), p1, p2, p3, p4, p5); +} + #undef FP_T #define FP_T(x) R (*x)(P1, P2, P3, P4, P5) @@ -607,7 +772,7 @@ class MethodFunctor6 { return (object_->*method_)(p1_, p2_, p3_, p4_, p5_, p6_); } private: MethodT method_; - ObjectT* object_; + typename detail::PointerType::type object_; P1 p1_; P2 p2_; P3 p3_; @@ -689,6 +854,28 @@ Bind(FP_T(method), const ObjectT* object, method, object, p1, p2, p3, p4, p5, p6); } +#undef FP_T +#define FP_T(x) R (ObjectT::*x)(P1, P2, P3, P4, P5, P6) + +template +MethodFunctor6 +Bind(FP_T(method), const scoped_refptr& object, + typename detail::identity::type p1, + typename detail::identity::type p2, + typename detail::identity::type p3, + typename detail::identity::type p4, + typename detail::identity::type p5, + typename detail::identity::type p6) { + return MethodFunctor6( + method, object.get(), p1, p2, p3, p4, p5, p6); +} + #undef FP_T #define FP_T(x) R (*x)(P1, P2, P3, P4, P5, P6) diff --git a/webrtc/base/bind.h.pump b/webrtc/base/bind.h.pump index 11767abe50..9a4bc664c3 100644 --- a/webrtc/base/bind.h.pump +++ b/webrtc/base/bind.h.pump @@ -12,12 +12,13 @@ // /home/build/google3/third_party/gtest/scripts/pump.py bind.h.pump // Bind() is an overloaded function that converts method calls into function -// objects (aka functors). It captures any arguments to the method by value -// when Bind is called, producing a stateful, nullary function object. Care -// should be taken about the lifetime of objects captured by Bind(); the -// returned functor knows nothing about the lifetime of the method's object or -// any arguments passed by pointer, and calling the functor with a destroyed -// object will surely do bad things. +// objects (aka functors). The method object is captured as a scoped_refptr<> if +// possible, and as a raw pointer otherwise. Any arguments to the method are +// captured by value. The return value of Bind is a stateful, nullary function +// object. Care should be taken about the lifetime of objects captured by +// Bind(); the returned functor knows nothing about the lifetime of a non +// ref-counted method object or any arguments passed by pointer, and calling the +// functor with a destroyed object will surely do bad things. // // Example usage: // struct Foo { @@ -34,10 +35,33 @@ // cout << rtc::Bind(&Foo::Test3, &foo, 3)() << endl; // cout << rtc::Bind(&Foo::Test4, &foo, 7, 8.5f)() << endl; // } +// +// Example usage of ref counted objects: +// struct Bar { +// int AddRef(); +// int Release(); +// +// void Test() {} +// void BindThis() { +// // The functor passed to AsyncInvoke() will keep this object alive. +// invoker.AsyncInvoke(rtc::Bind(&Bar::Test, this)); +// } +// }; +// +// int main() { +// rtc::scoped_refptr bar = new rtc::RefCountedObject(); +// auto functor = rtc::Bind(&Bar::Test, bar); +// bar = nullptr; +// // The functor stores an internal scoped_refptr, so this is safe. +// functor(); +// } +// #ifndef WEBRTC_BASE_BIND_H_ #define WEBRTC_BASE_BIND_H_ +#include "webrtc/base/scoped_ref_ptr.h" + #define NONAME namespace rtc { @@ -49,6 +73,57 @@ namespace detail { // references stripped. This trick allows the compiler to dictate the Bind // parameter types rather than deduce them. template struct identity { typedef T type; }; + +// IsRefCounted::value will be true for types that can be used in +// rtc::scoped_refptr, i.e. types that implements nullary functions AddRef() +// and Release(), regardless of their return types. AddRef() and Release() can +// be defined in T or any superclass of T. +template +class IsRefCounted { + // This is a complex implementation detail done with SFINAE. + + // Define types such that sizeof(Yes) != sizeof(No). + struct Yes { char dummy[1]; }; + struct No { char dummy[2]; }; + // Define two overloaded template functions with return types of different + // size. This way, we can use sizeof() on the return type to determine which + // function the compiler would have chosen. One function will be preferred + // over the other if it is possible to create it without compiler errors, + // otherwise the compiler will simply remove it, and default to the less + // preferred function. + template + static Yes test(R* r, decltype(r->AddRef(), r->Release(), 42)); + template static No test(...); + +public: + // Trick the compiler to tell if it's possible to call AddRef() and Release(). + static const bool value = sizeof(test((T*)nullptr, 42)) == sizeof(Yes); +}; + +// TernaryTypeOperator is a helper class to select a type based on a static bool +// value. +template +struct TernaryTypeOperator {}; + +template +struct TernaryTypeOperator { + typedef IfTrueT type; +}; + +template +struct TernaryTypeOperator { + typedef IfFalseT type; +}; + +// PointerType::type will be scoped_refptr for ref counted types, and T* +// otherwise. +template +struct PointerType { + typedef typename TernaryTypeOperator::value, + scoped_refptr, + T*>::type type; +}; + } // namespace detail $var n = 6 @@ -68,7 +143,7 @@ class MethodFunctor$i { return (object_->*method_)($for j , [[p$(j)_]]); } private: MethodT method_; - ObjectT* object_;$for j [[ + typename detail::PointerType::type object_;$for j [[ P$j p$(j)_;]] @@ -115,6 +190,18 @@ Bind(FP_T(method), const ObjectT* object$for j [[, method, object$for j [[, p$j]]); } +#undef FP_T +#define FP_T(x) R (ObjectT::*x)($for j , [[P$j]]) + +template +MethodFunctor$i +Bind(FP_T(method), const scoped_refptr& object$for j [[, + typename detail::identity::type p$j]]) { + return MethodFunctor$i( + method, object.get()$for j [[, p$j]]); +} + #undef FP_T #define FP_T(x) R (*x)($for j , [[P$j]]) diff --git a/webrtc/base/bind_unittest.cc b/webrtc/base/bind_unittest.cc index ed8dd5cf2d..7a621dce2c 100644 --- a/webrtc/base/bind_unittest.cc +++ b/webrtc/base/bind_unittest.cc @@ -11,6 +11,8 @@ #include "webrtc/base/bind.h" #include "webrtc/base/gunit.h" +#include "webrtc/base/refcount.h" + namespace rtc { namespace { @@ -26,12 +28,67 @@ struct MethodBindTester { mutable int call_count; }; +struct A { int dummy; }; +struct B: public RefCountInterface { int dummy; }; +struct C: public A, B {}; +struct D { + int AddRef(); +}; +struct E: public D { + int Release(); +}; +struct F { + void AddRef(); + void Release(); +}; + +class LifeTimeCheck : public RefCountInterface { + public: + LifeTimeCheck(bool* has_died) : has_died_(has_died), is_ok_to_die_(false) {} + ~LifeTimeCheck() { + EXPECT_TRUE(is_ok_to_die_); + *has_died_ = true; + } + void PrepareToDie() { is_ok_to_die_ = true; } + void NullaryVoid() {} + + private: + bool* const has_died_; + bool is_ok_to_die_; +}; + int Return42() { return 42; } int Negate(int a) { return -a; } int Multiply(int a, int b) { return a * b; } } // namespace +// Try to catch any problem with scoped_refptr type deduction in rtc::Bind at +// compile time. +#define EXPECT_IS_CAPTURED_AS_PTR(T) \ + static_assert(is_same::type, T*>::value, \ + "PointerType") +#define EXPECT_IS_CAPTURED_AS_SCOPED_REFPTR(T) \ + static_assert( \ + is_same::type, scoped_refptr>::value, \ + "PointerType") + +EXPECT_IS_CAPTURED_AS_PTR(void); +EXPECT_IS_CAPTURED_AS_PTR(int); +EXPECT_IS_CAPTURED_AS_PTR(double); +EXPECT_IS_CAPTURED_AS_PTR(A); +EXPECT_IS_CAPTURED_AS_PTR(D); +EXPECT_IS_CAPTURED_AS_PTR(RefCountInterface*); + +EXPECT_IS_CAPTURED_AS_SCOPED_REFPTR(RefCountInterface); +EXPECT_IS_CAPTURED_AS_SCOPED_REFPTR(B); +EXPECT_IS_CAPTURED_AS_SCOPED_REFPTR(C); +EXPECT_IS_CAPTURED_AS_SCOPED_REFPTR(E); +EXPECT_IS_CAPTURED_AS_SCOPED_REFPTR(F); +EXPECT_IS_CAPTURED_AS_SCOPED_REFPTR(RefCountedObject); +EXPECT_IS_CAPTURED_AS_SCOPED_REFPTR(RefCountedObject); +EXPECT_IS_CAPTURED_AS_SCOPED_REFPTR(RefCountedObject); + TEST(BindTest, BindToMethod) { MethodBindTester object = {0}; EXPECT_EQ(0, object.call_count); @@ -64,4 +121,49 @@ TEST(BindTest, BindToFunction) { EXPECT_EQ(56, Bind(&Multiply, 8, 7)()); } +// Test Bind where method object implements RefCountInterface and is passed as a +// pointer. +TEST(BindTest, CapturePointerAsScopedRefPtr) { + bool object_has_died = false; + scoped_refptr object = + new RefCountedObject(&object_has_died); + { + auto functor = Bind(&LifeTimeCheck::PrepareToDie, object.get()); + object = nullptr; + EXPECT_FALSE(object_has_died); + // Run prepare to die via functor. + functor(); + } + EXPECT_TRUE(object_has_died); +} + +// Test Bind where method object implements RefCountInterface and is passed as a +// scoped_refptr<>. +TEST(BindTest, CaptureScopedRefPtrAsScopedRefPtr) { + bool object_has_died = false; + scoped_refptr object = + new RefCountedObject(&object_has_died); + { + auto functor = Bind(&LifeTimeCheck::PrepareToDie, object); + object = nullptr; + EXPECT_FALSE(object_has_died); + // Run prepare to die via functor. + functor(); + } + EXPECT_TRUE(object_has_died); +} + +// Test Bind where method object is captured as scoped_refptr<> and the functor +// dies while there are references left. +TEST(BindTest, FunctorReleasesObjectOnDestruction) { + bool object_has_died = false; + scoped_refptr object = + new RefCountedObject(&object_has_died); + Bind(&LifeTimeCheck::NullaryVoid, object.get())(); + EXPECT_FALSE(object_has_died); + object->PrepareToDie(); + object = nullptr; + EXPECT_TRUE(object_has_died); +} + } // namespace rtc