Changeset 245024 in webkit

Timestamp:

May 7, 2019 12:54:00 PM (5 years ago)

Author:

achristensen@apple.com

Message:

Add a release assertion that Functions can only be constructed from non-null CompletionHandlers
​https://bugs.webkit.org/show_bug.cgi?id=197641

Reviewed by Chris Dumez.

This will help us find the cause of rdar://problem/48679972 by seeing the crash when the Function is dispatched,
not when it's called with no interesting stack trace. I manually verified this assertion is hit in such a case.
We should also have no legitimate use of creating a Function out of a null CompletionHandler then never calling it.

• wtf/CompletionHandler.h:

(WTF::Detail::CallableWrapper<CompletionHandler<Out):

• wtf/Function.h:

(WTF::Detail::CallableWrapperBase::~CallableWrapperBase):
(WTF::Detail::CallableWrapper::CallableWrapper):
(WTF::Function<Out):

Location:

trunk/Source/WTF

Files:

• ChangeLog (modified) (1 diff)
• wtf/CompletionHandler.h (modified) (1 diff)
• wtf/Function.h (modified) (5 diffs)

trunk/Source/WTF/ChangeLog

@@ +1 @@
+2019-05-07  Alex Christensen  <achristensen@webkit.org>
+
+        Add a release assertion that Functions can only be constructed from non-null CompletionHandlers
+        https://bugs.webkit.org/show_bug.cgi?id=197641
+
+        Reviewed by Chris Dumez.
+
+        This will help us find the cause of rdar://problem/48679972 by seeing the crash when the Function is dispatched,
+        not when it's called with no interesting stack trace.  I manually verified this assertion is hit in such a case.
+        We should also have no legitimate use of creating a Function out of a null CompletionHandler then never calling it.
+
+        * wtf/CompletionHandler.h:
+        (WTF::Detail::CallableWrapper<CompletionHandler<Out):
+        * wtf/Function.h:
+        (WTF::Detail::CallableWrapperBase::~CallableWrapperBase):
+        (WTF::Detail::CallableWrapper::CallableWrapper):
+        (WTF::Function<Out):
+
 2019-05-06  Christopher Reid  <chris.reid@sony.com>
 

trunk/Source/WTF/wtf/CompletionHandler.h

@@ -64 +64 @@
 };
 
+namespace Detail {
+
+template<typename Out, typename... In>
+class CallableWrapper<CompletionHandler<Out(In...)>, Out, In...> : public CallableWrapperBase<Out, In...> {
+public:
+    explicit CallableWrapper(CompletionHandler<Out(In...)>&& completionHandler)
+        : m_completionHandler(WTFMove(completionHandler))
+    {
+        RELEASE_ASSERT(m_completionHandler);
+    }
+    Out call(In... in) final { return m_completionHandler(std::forward<In>(in)...); }
+private:
+    CompletionHandler<Out(In...)> m_completionHandler;
+};
+
+} // namespace Detail
+
 class CompletionHandlerCallingScope {
 public:

trunk/Source/WTF/wtf/Function.h

@@ -31 +31 @@
 namespace WTF {
 
+namespace Detail {
+
+template<typename Out, typename... In>
+class CallableWrapperBase {
+    WTF_MAKE_FAST_ALLOCATED;
+public:
+    virtual ~CallableWrapperBase() { }
+    virtual Out call(In...) = 0;
+};
+
+template<typename, typename, typename...> class CallableWrapper;
+
+template<typename CallableType, typename Out, typename... In>
+class CallableWrapper : public CallableWrapperBase<Out, In...> {
+public:
+    explicit CallableWrapper(CallableType&& callable)
+        : m_callable(WTFMove(callable)) { }
+    CallableWrapper(const CallableWrapper&) = delete;
+    CallableWrapper& operator=(const CallableWrapper&) = delete;
+    Out call(In... in) final { return m_callable(std::forward<In>(in)...); }
+private:
+    CallableType m_callable;
+};
+
+} // namespace Detail
+
 template<typename> class Function;
 
@@ -42 +68 @@
     template<typename CallableType, class = typename std::enable_if<!(std::is_pointer<CallableType>::value && std::is_function<typename std::remove_pointer<CallableType>::type>::value) && std::is_rvalue_reference<CallableType&&>::value>::type>
     Function(CallableType&& callable)
-        : m_callableWrapper(std::make_unique<CallableWrapper<CallableType>>(WTFMove(callable)))
-    {
-    }
+        : m_callableWrapper(std::make_unique<Detail::CallableWrapper<CallableType, Out, In...>>(WTFMove(callable))) { }
 
     template<typename FunctionType, class = typename std::enable_if<std::is_pointer<FunctionType>::value && std::is_function<typename std::remove_pointer<FunctionType>::type>::value>::type>
     Function(FunctionType f)
-        : m_callableWrapper(std::make_unique<CallableWrapper<FunctionType>>(WTFMove(f)))
-    {
-    }
+        : m_callableWrapper(std::make_unique<Detail::CallableWrapper<FunctionType, Out, In...>>(WTFMove(f))) { }
 
     Out operator()(In... in) const
@@ -63 +85 @@
     Function& operator=(CallableType&& callable)
     {
-        m_callableWrapper = std::make_unique<CallableWrapper<CallableType>>(WTFMove(callable));
+        m_callableWrapper = std::make_unique<Detail::CallableWrapper<CallableType, Out, In...>>(WTFMove(callable));
         return *this;
     }
@@ -70 +92 @@
     Function& operator=(FunctionType f)
     {
-        m_callableWrapper = std::make_unique<CallableWrapper<FunctionType>>(WTFMove(f));
+        m_callableWrapper = std::make_unique<Detail::CallableWrapper<FunctionType, Out, In...>>(WTFMove(f));
         return *this;
     }
@@ -81 +103 @@
 
 private:
-    class CallableWrapperBase {
-        WTF_MAKE_FAST_ALLOCATED;
-    public:
-        virtual ~CallableWrapperBase() { }
-
-        virtual Out call(In...) = 0;
-    };
-
-    template<typename CallableType>
-    class CallableWrapper : public CallableWrapperBase {
-    public:
-        explicit CallableWrapper(CallableType&& callable)
-            : m_callable(WTFMove(callable))
-        {
-        }
-
-        CallableWrapper(const CallableWrapper&) = delete;
-        CallableWrapper& operator=(const CallableWrapper&) = delete;
-
-        Out call(In... in) final { return m_callable(std::forward<In>(in)...); }
-
-    private:
-        CallableType m_callable;
-    };
-
-    std::unique_ptr<CallableWrapperBase> m_callableWrapper;
+    std::unique_ptr<Detail::CallableWrapperBase<Out, In...>> m_callableWrapper;
 };