Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (201463 => 201464)
--- trunk/Source/_javascript_Core/ChangeLog 2016-05-27 20:14:16 UTC (rev 201463)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-05-27 20:22:12 UTC (rev 201464)
@@ -1,3 +1,21 @@
+2016-05-27 Chris Dumez <[email protected]>
+
+ WorkQueue::dispatch() / RunLoop::dispatch() should not copy captured lambda variables
+ https://bugs.webkit.org/show_bug.cgi?id=158111
+
+ Reviewed by Darin Adler.
+
+ WorkQueue::dispatch() / RunLoop::dispatch() should not copy captured lambda variables.
+ These are often used cross-thread and copying the captured lambda variables can be
+ dangerous (e.g. we do not want to copy a String after calling isolatedCopy() upon
+ capture).
+
+ * runtime/Watchdog.cpp:
+ (JSC::Watchdog::startTimer):
+ (JSC::Watchdog::Watchdog): Deleted.
+ (JSC::Watchdog::setTimeLimit): Deleted.
+ * runtime/Watchdog.h:
+
2016-05-27 Konstantin Tokarev <[email protected]>
Removed unused headers from ExecutableAllocatorFixedVMPool.cpp.
Modified: trunk/Source/_javascript_Core/runtime/Watchdog.cpp (201463 => 201464)
--- trunk/Source/_javascript_Core/runtime/Watchdog.cpp 2016-05-27 20:14:16 UTC (rev 201463)
+++ trunk/Source/_javascript_Core/runtime/Watchdog.cpp 2016-05-27 20:22:12 UTC (rev 201464)
@@ -50,13 +50,6 @@
, m_callbackData2(0)
, m_timerQueue(WorkQueue::create("jsc.watchdog.queue", WorkQueue::Type::Serial, WorkQueue::QOS::Utility))
{
- m_timerHandler = [this] {
- {
- LockHolder locker(m_lock);
- this->m_timerDidFire = true;
- }
- this->deref();
- };
}
void Watchdog::setTimeLimit(std::chrono::microseconds limit,
@@ -177,7 +170,13 @@
this->ref(); // m_timerHandler will deref to match later.
m_wallClockDeadline = wallClockDeadline;
- m_timerQueue->dispatchAfter(std::chrono::nanoseconds(timeLimit), m_timerHandler);
+ m_timerQueue->dispatchAfter(std::chrono::nanoseconds(timeLimit), [this] {
+ {
+ LockHolder locker(m_lock);
+ m_timerDidFire = true;
+ }
+ deref();
+ });
}
void Watchdog::stopTimer(LockHolder&)
Modified: trunk/Source/_javascript_Core/runtime/Watchdog.h (201463 => 201464)
--- trunk/Source/_javascript_Core/runtime/Watchdog.h 2016-05-27 20:14:16 UTC (rev 201463)
+++ trunk/Source/_javascript_Core/runtime/Watchdog.h 2016-05-27 20:22:12 UTC (rev 201464)
@@ -91,7 +91,6 @@
void* m_callbackData2;
Ref<WorkQueue> m_timerQueue;
- std::function<void ()> m_timerHandler;
friend class LLIntOffsetsExtractor;
};
Modified: trunk/Source/WTF/ChangeLog (201463 => 201464)
--- trunk/Source/WTF/ChangeLog 2016-05-27 20:14:16 UTC (rev 201463)
+++ trunk/Source/WTF/ChangeLog 2016-05-27 20:22:12 UTC (rev 201464)
@@ -1,3 +1,70 @@
+2016-05-27 Chris Dumez <[email protected]>
+
+ WorkQueue::dispatch() / RunLoop::dispatch() should not copy captured lambda variables
+ https://bugs.webkit.org/show_bug.cgi?id=158111
+
+ Reviewed by Darin Adler.
+
+ WorkQueue::dispatch() / RunLoop::dispatch() should not copy captured lambda variables.
+ These are often used cross-thread and copying the captured lambda variables can be
+ dangerous (e.g. we do not want to copy a String after calling isolatedCopy() upon
+ capture).
+
+ This patch introduces a new NoncopyableFunction type that behaves similarly to
+ std::function but guarantees that the passed-in lambda (and its captured variables)
+ cannot be copied. This new NoncopyableFunction type is now used for
+ WorkQueue / RunLoop's dispatch() / dispatchAfter() which are commonly used
+ cross-thread. This should now allow us to call WorkQueue::dispatch() with a lambda
+ that captures a String like so:
+ [str = str.isolatedCopy()]() { }
+
+ Also note that even though this is not leveraged in this patch, NoncopyableFunction
+ would allow us to capture move-only types such as std::unique_ptr as so:
+ [p = WTFMove(p)]() { }
+ This does not work if we convert the lambda into an std::function because
+ std::function requires the lambda to be copyable, NoncopyableFunction does not.
+
+ * wtf/FunctionDispatcher.h:
+ (WTF::CallableWrapperBase::~CallableWrapperBase):
+ (WTF::NoncopyableFunction::NoncopyableFunction):
+ (WTF::NoncopyableFunction::operator()):
+ (WTF::NoncopyableFunction::operator bool):
+ (WTF::NoncopyableFunction::operator=):
+ * wtf/RunLoop.cpp:
+ (WTF::RunLoop::performWork):
+ (WTF::RunLoop::dispatch):
+ * wtf/RunLoop.h:
+ * wtf/WorkQueue.h:
+ * wtf/cocoa/WorkQueueCocoa.cpp:
+ (WTF::WorkQueue::dispatch):
+ (WTF::WorkQueue::dispatchAfter):
+ * wtf/efl/DispatchQueueWorkItemEfl.h:
+ (WorkItem::WorkItem):
+ (TimerWorkItem::create):
+ (TimerWorkItem::TimerWorkItem):
+ * wtf/efl/WorkQueueEfl.cpp:
+ (WTF::WorkQueue::dispatch):
+ (WTF::WorkQueue::dispatchAfter):
+ * wtf/generic/RunLoopGeneric.cpp:
+ (WTF::RunLoop::dispatchAfter):
+ * wtf/generic/WorkQueueGeneric.cpp:
+ (WorkQueue::dispatch):
+ (WorkQueue::dispatchAfter):
+ * wtf/glib/RunLoopGLib.cpp:
+ (WTF::DispatchAfterContext::DispatchAfterContext):
+ (WTF::RunLoop::dispatchAfter):
+ * wtf/win/WorkItemWin.cpp:
+ (WTF::WorkItemWin::WorkItemWin):
+ (WTF::WorkItemWin::create):
+ (WTF::HandleWorkItem::HandleWorkItem):
+ (WTF::HandleWorkItem::createByAdoptingHandle):
+ * wtf/win/WorkItemWin.h:
+ (WTF::WorkItemWin::function):
+ * wtf/win/WorkQueueWin.cpp:
+ (WTF::WorkQueue::dispatch):
+ (WTF::WorkQueue::timerCallback):
+ (WTF::WorkQueue::dispatchAfter):
+
2016-05-26 Filip Pizlo <[email protected]>
ScopedLambda should have a lifetime story that makes sense to the compiler
Modified: trunk/Source/WTF/wtf/FunctionDispatcher.h (201463 => 201464)
--- trunk/Source/WTF/wtf/FunctionDispatcher.h 2016-05-27 20:14:16 UTC (rev 201463)
+++ trunk/Source/WTF/wtf/FunctionDispatcher.h 2016-05-27 20:22:12 UTC (rev 201464)
@@ -31,6 +31,62 @@
namespace WTF {
+// FIXME: Move this to its own header (e.g. Functional.h).
+// FIXME: We could make this templated to support other lambdas than void() and make this more reusable.
+class NoncopyableFunction {
+public:
+ NoncopyableFunction() = default;
+
+ template<typename CallableType, class = typename std::enable_if<std::is_rvalue_reference<CallableType&&>::value>::type>
+ NoncopyableFunction(CallableType&& callable)
+ : m_callableWrapper(std::make_unique<CallableWrapper<CallableType>>(WTFMove(callable)))
+ {
+ }
+
+ void operator()() const
+ {
+ if (m_callableWrapper)
+ m_callableWrapper->call();
+ }
+
+ explicit operator bool() const { return !!m_callableWrapper; }
+
+ template<typename CallableType, class = typename std::enable_if<std::is_rvalue_reference<CallableType&&>::value>::type>
+ NoncopyableFunction& operator=(CallableType&& callable)
+ {
+ m_callableWrapper = std::make_unique<CallableWrapper<CallableType>>(WTFMove(callable));
+ return *this;
+ }
+
+private:
+ class CallableWrapperBase {
+ WTF_MAKE_FAST_ALLOCATED;
+ public:
+ virtual ~CallableWrapperBase() { }
+
+ virtual void call() = 0;
+ };
+
+ template<typename CallableType>
+ class CallableWrapper final : public CallableWrapperBase {
+ public:
+ explicit CallableWrapper(CallableType&& callable)
+ : m_callable(WTFMove(callable))
+ {
+ }
+
+ CallableWrapper(const CallableWrapper&) = delete;
+ CallableWrapper& operator=(const CallableWrapper&) = delete;
+
+ void call() final { m_callable(); }
+
+ private:
+ CallableType m_callable;
+ };
+
+ std::unique_ptr<CallableWrapperBase> m_callableWrapper;
+};
+
// FunctionDispatcher is an abstract representation of something that functions can be
// dispatched to. This can for example be a run loop or a work queue.
@@ -38,7 +94,7 @@
public:
WTF_EXPORT_PRIVATE virtual ~FunctionDispatcher();
- virtual void dispatch(std::function<void ()>) = 0;
+ virtual void dispatch(NoncopyableFunction&&) = 0;
protected:
WTF_EXPORT_PRIVATE FunctionDispatcher();
@@ -47,5 +103,6 @@
} // namespace WTF
using WTF::FunctionDispatcher;
+using WTF::NoncopyableFunction;
#endif // FunctionDispatcher_h
Modified: trunk/Source/WTF/wtf/RunLoop.cpp (201463 => 201464)
--- trunk/Source/WTF/wtf/RunLoop.cpp 2016-05-27 20:14:16 UTC (rev 201463)
+++ trunk/Source/WTF/wtf/RunLoop.cpp 2016-05-27 20:22:12 UTC (rev 201464)
@@ -91,7 +91,7 @@
size_t functionsToHandle = 0;
{
- std::function<void()> function;
+ NoncopyableFunction function;
{
MutexLocker locker(m_functionQueueLock);
functionsToHandle = m_functionQueue.size();
@@ -106,7 +106,7 @@
}
for (size_t functionsHandled = 1; functionsHandled < functionsToHandle; ++functionsHandled) {
- std::function<void()> function;
+ NoncopyableFunction function;
{
MutexLocker locker(m_functionQueueLock);
@@ -123,7 +123,7 @@
}
}
-void RunLoop::dispatch(std::function<void ()> function)
+void RunLoop::dispatch(NoncopyableFunction&& function)
{
{
MutexLocker locker(m_functionQueueLock);
Modified: trunk/Source/WTF/wtf/RunLoop.h (201463 => 201464)
--- trunk/Source/WTF/wtf/RunLoop.h 2016-05-27 20:14:16 UTC (rev 201463)
+++ trunk/Source/WTF/wtf/RunLoop.h 2016-05-27 20:22:12 UTC (rev 201464)
@@ -59,7 +59,7 @@
WTF_EXPORT_PRIVATE static bool isMain();
~RunLoop();
- void dispatch(std::function<void()>) override;
+ void dispatch(NoncopyableFunction&&) override;
WTF_EXPORT_PRIVATE static void run();
WTF_EXPORT_PRIVATE void stop();
@@ -79,7 +79,7 @@
#endif
#if USE(GLIB_EVENT_LOOP) || USE(GENERIC_EVENT_LOOP)
- WTF_EXPORT_PRIVATE void dispatchAfter(std::chrono::nanoseconds, std::function<void()>);
+ WTF_EXPORT_PRIVATE void dispatchAfter(std::chrono::nanoseconds, NoncopyableFunction&&);
#endif
class TimerBase {
@@ -155,7 +155,7 @@
void performWork();
Mutex m_functionQueueLock;
- Deque<std::function<void ()>> m_functionQueue;
+ Deque<NoncopyableFunction> m_functionQueue;
#if USE(WINDOWS_EVENT_LOOP)
static bool registerRunLoopMessageWindowClass();
Modified: trunk/Source/WTF/wtf/WorkQueue.h (201463 => 201464)
--- trunk/Source/WTF/wtf/WorkQueue.h 2016-05-27 20:14:16 UTC (rev 201463)
+++ trunk/Source/WTF/wtf/WorkQueue.h 2016-05-27 20:22:12 UTC (rev 201464)
@@ -68,12 +68,12 @@
Utility,
Background
};
-
+
WTF_EXPORT_PRIVATE static Ref<WorkQueue> create(const char* name, Type = Type::Serial, QOS = QOS::Default);
virtual ~WorkQueue();
- WTF_EXPORT_PRIVATE void dispatch(std::function<void ()>) override;
- WTF_EXPORT_PRIVATE void dispatchAfter(std::chrono::nanoseconds, std::function<void ()>);
+ WTF_EXPORT_PRIVATE void dispatch(NoncopyableFunction&&) override;
+ WTF_EXPORT_PRIVATE void dispatchAfter(std::chrono::nanoseconds, NoncopyableFunction&&);
WTF_EXPORT_PRIVATE static void concurrentApply(size_t iterations, const std::function<void (size_t index)>&);
Modified: trunk/Source/WTF/wtf/cocoa/WorkQueueCocoa.cpp (201463 => 201464)
--- trunk/Source/WTF/wtf/cocoa/WorkQueueCocoa.cpp 2016-05-27 20:14:16 UTC (rev 201463)
+++ trunk/Source/WTF/wtf/cocoa/WorkQueueCocoa.cpp 2016-05-27 20:22:12 UTC (rev 201464)
@@ -28,20 +28,24 @@
namespace WTF {
-void WorkQueue::dispatch(std::function<void ()> function)
+void WorkQueue::dispatch(NoncopyableFunction&& function)
{
ref();
+ auto* functionPtr = new NoncopyableFunction(WTFMove(function));
dispatch_async(m_dispatchQueue, ^{
- function();
+ (*functionPtr)();
+ delete functionPtr;
deref();
});
}
-void WorkQueue::dispatchAfter(std::chrono::nanoseconds duration, std::function<void ()> function)
+void WorkQueue::dispatchAfter(std::chrono::nanoseconds duration, NoncopyableFunction&& function)
{
ref();
+ auto* functionPtr = new NoncopyableFunction(WTFMove(function));
dispatch_after(dispatch_time(DISPATCH_TIME_NOW, duration.count()), m_dispatchQueue, ^{
- function();
+ (*functionPtr)();
+ delete functionPtr;
deref();
});
}
Modified: trunk/Source/WTF/wtf/efl/DispatchQueueWorkItemEfl.h (201463 => 201464)
--- trunk/Source/WTF/wtf/efl/DispatchQueueWorkItemEfl.h 2016-05-27 20:14:16 UTC (rev 201463)
+++ trunk/Source/WTF/wtf/efl/DispatchQueueWorkItemEfl.h 2016-05-27 20:22:12 UTC (rev 201464)
@@ -28,13 +28,14 @@
#include <wtf/Assertions.h>
#include <wtf/CurrentTime.h>
+#include <wtf/FunctionDispatcher.h>
#include <wtf/RefCounted.h>
#include <wtf/WorkQueue.h>
class WorkItem {
public:
- WorkItem(PassRefPtr<WorkQueue> workQueue, std::function<void ()> function)
- : m_workQueue(workQueue)
+ WorkItem(Ref<WorkQueue>&& workQueue, NoncopyableFunction&& function)
+ : m_workQueue(WTFMove(workQueue))
, m_function(WTFMove(function))
{
}
@@ -42,23 +43,23 @@
void dispatch() { m_function(); }
private:
- RefPtr<WorkQueue> m_workQueue;
- std::function<void ()> m_function;
+ Ref<WorkQueue> m_workQueue;
+ NoncopyableFunction m_function;
};
class TimerWorkItem : public WorkItem {
public:
- static std::unique_ptr<TimerWorkItem> create(PassRefPtr<WorkQueue> workQueue, std::function<void ()> function, std::chrono::nanoseconds delayNanoSeconds)
+ static std::unique_ptr<TimerWorkItem> create(Ref<WorkQueue>&& workQueue, NoncopyableFunction&& function, std::chrono::nanoseconds delayNanoSeconds)
{
ASSERT(delayNanoSeconds.count() >= 0);
- return std::unique_ptr<TimerWorkItem>(new TimerWorkItem(workQueue, WTFMove(function), monotonicallyIncreasingTime() * 1000000000.0 + delayNanoSeconds.count()));
+ return std::unique_ptr<TimerWorkItem>(new TimerWorkItem(WTFMove(workQueue), WTFMove(function), monotonicallyIncreasingTime() * 1000000000.0 + delayNanoSeconds.count()));
}
double expirationTimeNanoSeconds() const { return m_expirationTimeNanoSeconds; }
bool hasExpired(double currentTimeNanoSeconds) const { return currentTimeNanoSeconds >= m_expirationTimeNanoSeconds; }
protected:
- TimerWorkItem(PassRefPtr<WorkQueue> workQueue, std::function<void ()> function, double expirationTimeNanoSeconds)
- : WorkItem(workQueue, WTFMove(function))
+ TimerWorkItem(Ref<WorkQueue>&& workQueue, NoncopyableFunction&& function, double expirationTimeNanoSeconds)
+ : WorkItem(WTFMove(workQueue), WTFMove(function))
, m_expirationTimeNanoSeconds(expirationTimeNanoSeconds)
{
}
Modified: trunk/Source/WTF/wtf/efl/WorkQueueEfl.cpp (201463 => 201464)
--- trunk/Source/WTF/wtf/efl/WorkQueueEfl.cpp 2016-05-27 20:14:16 UTC (rev 201463)
+++ trunk/Source/WTF/wtf/efl/WorkQueueEfl.cpp 2016-05-27 20:22:12 UTC (rev 201464)
@@ -54,20 +54,20 @@
m_dispatchQueue->clearSocketEventHandler();
}
-void WorkQueue::dispatch(std::function<void ()> function)
+void WorkQueue::dispatch(NoncopyableFunction&& function)
{
if (!m_dispatchQueue)
return;
- m_dispatchQueue->dispatch(std::make_unique<WorkItem>(this, WTFMove(function)));
+ m_dispatchQueue->dispatch(std::make_unique<WorkItem>(*this, WTFMove(function)));
}
-void WorkQueue::dispatchAfter(std::chrono::nanoseconds duration, std::function<void ()> function)
+void WorkQueue::dispatchAfter(std::chrono::nanoseconds duration, NoncopyableFunction&& function)
{
if (!m_dispatchQueue)
return;
- m_dispatchQueue->dispatch(TimerWorkItem::create(this, WTFMove(function), duration));
+ m_dispatchQueue->dispatch(TimerWorkItem::create(*this, WTFMove(function), duration));
}
}
Modified: trunk/Source/WTF/wtf/generic/RunLoopGeneric.cpp (201463 => 201464)
--- trunk/Source/WTF/wtf/generic/RunLoopGeneric.cpp 2016-05-27 20:14:16 UTC (rev 201463)
+++ trunk/Source/WTF/wtf/generic/RunLoopGeneric.cpp 2016-05-27 20:22:12 UTC (rev 201464)
@@ -239,11 +239,11 @@
wakeUp(locker);
}
-void RunLoop::dispatchAfter(std::chrono::nanoseconds delay, std::function<void()> function)
+void RunLoop::dispatchAfter(std::chrono::nanoseconds delay, NoncopyableFunction&& function)
{
LockHolder locker(m_loopLock);
bool repeating = false;
- schedule(locker, TimerBase::ScheduledTask::create(function, delay.count() / 1000.0 / 1000.0 / 1000.0, repeating));
+ schedule(locker, TimerBase::ScheduledTask::create(WTFMove(function), delay.count() / 1000.0 / 1000.0 / 1000.0, repeating));
wakeUp(locker);
}
Modified: trunk/Source/WTF/wtf/generic/WorkQueueGeneric.cpp (201463 => 201464)
--- trunk/Source/WTF/wtf/generic/WorkQueueGeneric.cpp 2016-05-27 20:14:16 UTC (rev 201463)
+++ trunk/Source/WTF/wtf/generic/WorkQueueGeneric.cpp 2016-05-27 20:22:12 UTC (rev 201464)
@@ -81,18 +81,18 @@
}
}
-void WorkQueue::dispatch(std::function<void()> function)
+void WorkQueue::dispatch(NoncopyableFunction&& function)
{
RefPtr<WorkQueue> protect(this);
- m_runLoop->dispatch([protect, function] {
+ m_runLoop->dispatch([protect, function = WTFMove(function)] {
function();
});
}
-void WorkQueue::dispatchAfter(std::chrono::nanoseconds delay, std::function<void()> function)
+void WorkQueue::dispatchAfter(std::chrono::nanoseconds delay, NoncopyableFunction&& function)
{
RefPtr<WorkQueue> protect(this);
- m_runLoop->dispatchAfter(delay, [protect, function] {
+ m_runLoop->dispatchAfter(delay, [protect, function = WTFMove(function)] {
function();
});
}
Modified: trunk/Source/WTF/wtf/glib/RunLoopGLib.cpp (201463 => 201464)
--- trunk/Source/WTF/wtf/glib/RunLoopGLib.cpp 2016-05-27 20:14:16 UTC (rev 201463)
+++ trunk/Source/WTF/wtf/glib/RunLoopGLib.cpp 2016-05-27 20:22:12 UTC (rev 201464)
@@ -124,7 +124,7 @@
class DispatchAfterContext {
WTF_MAKE_FAST_ALLOCATED;
public:
- DispatchAfterContext(std::function<void()>&& function)
+ DispatchAfterContext(NoncopyableFunction&& function)
: m_function(WTFMove(function))
{
}
@@ -135,10 +135,10 @@
}
private:
- std::function<void()> m_function;
+ NoncopyableFunction m_function;
};
-void RunLoop::dispatchAfter(std::chrono::nanoseconds duration, std::function<void()> function)
+void RunLoop::dispatchAfter(std::chrono::nanoseconds duration, NoncopyableFunction&& function)
{
GRefPtr<GSource> source = adoptGRef(g_timeout_source_new(std::chrono::duration_cast<std::chrono::milliseconds>(duration).count()));
g_source_set_name(source.get(), "[WebKit] RunLoop dispatchAfter");
Modified: trunk/Source/WTF/wtf/win/WorkItemWin.cpp (201463 => 201464)
--- trunk/Source/WTF/wtf/win/WorkItemWin.cpp 2016-05-27 20:14:16 UTC (rev 201463)
+++ trunk/Source/WTF/wtf/win/WorkItemWin.cpp 2016-05-27 20:22:12 UTC (rev 201464)
@@ -32,32 +32,32 @@
namespace WTF {
-WorkItemWin::WorkItemWin(std::function<void()> function, WorkQueue* queue)
- : m_function(function)
+WorkItemWin::WorkItemWin(NoncopyableFunction&& function, WorkQueue* queue)
+ : m_function(WTFMove(function))
, m_queue(queue)
{
}
-RefPtr<WorkItemWin> WorkItemWin::create(std::function<void()> function, WorkQueue* queue)
+RefPtr<WorkItemWin> WorkItemWin::create(NoncopyableFunction&& function, WorkQueue* queue)
{
- return adoptRef(new WorkItemWin(function, queue));
+ return adoptRef(new WorkItemWin(WTFMove(function), queue));
}
WorkItemWin::~WorkItemWin()
{
}
-HandleWorkItem::HandleWorkItem(HANDLE handle, const std::function<void()>& function, WorkQueue* queue)
- : WorkItemWin(function, queue)
+HandleWorkItem::HandleWorkItem(HANDLE handle, NoncopyableFunction&& function, WorkQueue* queue)
+ : WorkItemWin(WTFMove(function), queue)
, m_handle(handle)
, m_waitHandle(0)
{
ASSERT_ARG(handle, handle);
}
-RefPtr<HandleWorkItem> HandleWorkItem::createByAdoptingHandle(HANDLE handle, const std::function<void()>& function, WorkQueue* queue)
+RefPtr<HandleWorkItem> HandleWorkItem::createByAdoptingHandle(HANDLE handle, NoncopyableFunction&& function, WorkQueue* queue)
{
- return adoptRef(new HandleWorkItem(handle, function, queue));
+ return adoptRef(new HandleWorkItem(handle, WTFMove(function), queue));
}
HandleWorkItem::~HandleWorkItem()
Modified: trunk/Source/WTF/wtf/win/WorkItemWin.h (201463 => 201464)
--- trunk/Source/WTF/wtf/win/WorkItemWin.h 2016-05-27 20:14:16 UTC (rev 201463)
+++ trunk/Source/WTF/wtf/win/WorkItemWin.h 2016-05-27 20:22:12 UTC (rev 201464)
@@ -29,6 +29,7 @@
#include <Windows.h>
#include <functional>
+#include <wtf/FunctionDispatcher.h>
#include <wtf/RefPtr.h>
#include <wtf/ThreadSafeRefCounted.h>
@@ -38,30 +39,30 @@
class WorkItemWin : public ThreadSafeRefCounted<WorkItemWin> {
public:
- static RefPtr<WorkItemWin> create(std::function<void()>, WorkQueue*);
+ static RefPtr<WorkItemWin> create(NoncopyableFunction&&, WorkQueue*);
virtual ~WorkItemWin();
- std::function<void()>& function() { return m_function; }
+ NoncopyableFunction& function() { return m_function; }
WorkQueue* queue() const { return m_queue.get(); }
protected:
- WorkItemWin(std::function<void()>, WorkQueue*);
+ WorkItemWin(NoncopyableFunction&&, WorkQueue*);
private:
- std::function<void()> m_function;
+ NoncopyableFunction m_function;
RefPtr<WorkQueue> m_queue;
};
class HandleWorkItem : public WorkItemWin {
public:
- static RefPtr<HandleWorkItem> createByAdoptingHandle(HANDLE, const std::function<void()>&, WorkQueue*);
+ static RefPtr<HandleWorkItem> createByAdoptingHandle(HANDLE, NoncopyableFunction&&, WorkQueue*);
virtual ~HandleWorkItem();
void setWaitHandle(HANDLE waitHandle) { m_waitHandle = waitHandle; }
HANDLE waitHandle() const { return m_waitHandle; }
private:
- HandleWorkItem(HANDLE, const std::function<void()>&, WorkQueue*);
+ HandleWorkItem(HANDLE, NoncopyableFunction&&, WorkQueue*);
HANDLE m_handle;
HANDLE m_waitHandle;
Modified: trunk/Source/WTF/wtf/win/WorkQueueWin.cpp (201463 => 201464)
--- trunk/Source/WTF/wtf/win/WorkQueueWin.cpp 2016-05-27 20:14:16 UTC (rev 201463)
+++ trunk/Source/WTF/wtf/win/WorkQueueWin.cpp 2016-05-27 20:22:12 UTC (rev 201464)
@@ -129,11 +129,11 @@
::DeleteTimerQueueEx(m_timerQueue, 0);
}
-void WorkQueue::dispatch(std::function<void()> function)
+void WorkQueue::dispatch(NoncopyableFunction&& function)
{
MutexLocker locker(m_workItemQueueLock);
ref();
- m_workItemQueue.append(WorkItemWin::create(function, this));
+ m_workItemQueue.append(WorkItemWin::create(WTFMove(function), this));
// Spawn a work thread to perform the work we just added. As an optimization, we avoid
// spawning the thread if a work thread is already registered. This prevents multiple work
@@ -149,7 +149,7 @@
static RefPtr<TimerContext> create() { return adoptRef(new TimerContext); }
WorkQueue* queue;
- std::function<void()> function;
+ NoncopyableFunction function;
Mutex timerMutex;
HANDLE timer;
@@ -169,7 +169,7 @@
// Balanced by leakRef in scheduleWorkAfterDelay.
RefPtr<TimerContext> timerContext = adoptRef(static_cast<TimerContext*>(context));
- timerContext->queue->dispatch(timerContext->function);
+ timerContext->queue->dispatch(WTFMove(timerContext->function));
MutexLocker lock(timerContext->timerMutex);
ASSERT(timerContext->timer);
@@ -180,14 +180,14 @@
}
}
-void WorkQueue::dispatchAfter(std::chrono::nanoseconds duration, std::function<void()> function)
+void WorkQueue::dispatchAfter(std::chrono::nanoseconds duration, NoncopyableFunction&& function)
{
ASSERT(m_timerQueue);
ref();
RefPtr<TimerContext> context = TimerContext::create();
context->queue = this;
- context->function = function;
+ context->function = WTFMove(function);
{
// The timer callback could fire before ::CreateTimerQueueTimer even returns, so we protect
Modified: trunk/Source/WebKit2/ChangeLog (201463 => 201464)
--- trunk/Source/WebKit2/ChangeLog 2016-05-27 20:14:16 UTC (rev 201463)
+++ trunk/Source/WebKit2/ChangeLog 2016-05-27 20:22:12 UTC (rev 201464)
@@ -1,3 +1,33 @@
+2016-05-27 Chris Dumez <[email protected]>
+
+ WorkQueue::dispatch() / RunLoop::dispatch() should not copy captured lambda variables
+ https://bugs.webkit.org/show_bug.cgi?id=158111
+
+ Reviewed by Darin Adler.
+
+ WorkQueue::dispatch() / RunLoop::dispatch() should not copy captured lambda variables.
+ These are often used cross-thread and copying the captured lambda variables can be
+ dangerous (e.g. we do not want to copy a String after calling isolatedCopy() upon
+ capture).
+
+ * NetworkProcess/NetworkProcess.cpp:
+ (WebKit::clearDiskCacheEntries):
+ * NetworkProcess/cache/NetworkCache.cpp:
+ (WebKit::NetworkCache::Cache::clear):
+ * NetworkProcess/cache/NetworkCacheIOChannelSoup.cpp:
+ (WebKit::NetworkCache::runTaskInQueue):
+ * Platform/IPC/Connection.cpp:
+ (IPC::Connection::processIncomingMessage):
+ * UIProcess/Storage/StorageManager.cpp:
+ (WebKit::StorageManager::getSessionStorageOrigins):
+ (WebKit::StorageManager::deleteSessionStorageOrigins):
+ (WebKit::StorageManager::deleteSessionStorageEntriesForOrigins):
+ (WebKit::StorageManager::getLocalStorageOrigins):
+ (WebKit::StorageManager::getLocalStorageOriginDetails):
+ (WebKit::StorageManager::deleteLocalStorageOriginsModifiedSince):
+ (WebKit::StorageManager::deleteLocalStorageEntriesForOrigins):
+ * UIProcess/Storage/StorageManager.h:
+
2016-05-27 Alex Christensen <[email protected]>
Expose content extension failure error codes in SPI
Modified: trunk/Source/WebKit2/NetworkProcess/NetworkProcess.cpp (201463 => 201464)
--- trunk/Source/WebKit2/NetworkProcess/NetworkProcess.cpp 2016-05-27 20:14:16 UTC (rev 201463)
+++ trunk/Source/WebKit2/NetworkProcess/NetworkProcess.cpp 2016-05-27 20:22:12 UTC (rev 201464)
@@ -421,7 +421,7 @@
completionHandler();
}
-static void clearDiskCacheEntries(const Vector<SecurityOriginData>& origins, std::function<void ()> completionHandler)
+static void clearDiskCacheEntries(const Vector<SecurityOriginData>& origins, std::function<void ()>&& completionHandler)
{
#if ENABLE(NETWORK_CACHE)
if (NetworkCache::singleton().isEnabled()) {
@@ -432,7 +432,7 @@
auto* cacheKeysToDelete = new Vector<NetworkCache::Key>;
- NetworkCache::singleton().traverse([completionHandler, originsToDelete, cacheKeysToDelete](auto* traversalEntry) {
+ NetworkCache::singleton().traverse([completionHandler = WTFMove(completionHandler), originsToDelete, cacheKeysToDelete](auto* traversalEntry) mutable {
if (traversalEntry) {
if (originsToDelete->contains(SecurityOrigin::create(traversalEntry->entry.response().url())))
cacheKeysToDelete->append(traversalEntry->entry.key());
@@ -446,7 +446,7 @@
delete cacheKeysToDelete;
- RunLoop::main().dispatch(completionHandler);
+ RunLoop::main().dispatch(WTFMove(completionHandler));
return;
});
Modified: trunk/Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp (201463 => 201464)
--- trunk/Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp 2016-05-27 20:14:16 UTC (rev 201463)
+++ trunk/Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp 2016-05-27 20:22:12 UTC (rev 201464)
@@ -632,7 +632,7 @@
m_statistics->clear();
if (!m_storage) {
- RunLoop::main().dispatch(completionHandler);
+ RunLoop::main().dispatch(WTFMove(completionHandler));
return;
}
String anyType;
Modified: trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheIOChannelSoup.cpp (201463 => 201464)
--- trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheIOChannelSoup.cpp 2016-05-27 20:14:16 UTC (rev 201463)
+++ trunk/Source/WebKit2/NetworkProcess/cache/NetworkCacheIOChannelSoup.cpp 2016-05-27 20:22:12 UTC (rev 201464)
@@ -69,10 +69,10 @@
return adoptRef(*new IOChannel(filePath, type));
}
-static inline void runTaskInQueue(std::function<void ()> task, WorkQueue* queue)
+static inline void runTaskInQueue(NoncopyableFunction&& task, WorkQueue* queue)
{
if (queue) {
- queue->dispatch(task);
+ queue->dispatch(WTFMove(task));
return;
}
Modified: trunk/Source/WebKit2/Platform/IPC/Connection.cpp (201463 => 201464)
--- trunk/Source/WebKit2/Platform/IPC/Connection.cpp 2016-05-27 20:14:16 UTC (rev 201463)
+++ trunk/Source/WebKit2/Platform/IPC/Connection.cpp 2016-05-27 20:22:12 UTC (rev 201464)
@@ -681,7 +681,7 @@
std::lock_guard<Lock> lock(m_incomingSyncMessageCallbackMutex);
for (auto& callback : m_incomingSyncMessageCallbacks.values())
- m_incomingSyncMessageCallbackQueue->dispatch(callback);
+ m_incomingSyncMessageCallbackQueue->dispatch(WTFMove(callback));
m_incomingSyncMessageCallbacks.clear();
}
Modified: trunk/Source/WebKit2/UIProcess/Storage/StorageManager.cpp (201463 => 201464)
--- trunk/Source/WebKit2/UIProcess/Storage/StorageManager.cpp 2016-05-27 20:14:16 UTC (rev 201463)
+++ trunk/Source/WebKit2/UIProcess/Storage/StorageManager.cpp 2016-05-27 20:22:12 UTC (rev 201464)
@@ -553,11 +553,11 @@
});
}
-void StorageManager::getSessionStorageOrigins(std::function<void (HashSet<RefPtr<WebCore::SecurityOrigin>>&&)> completionHandler)
+void StorageManager::getSessionStorageOrigins(std::function<void (HashSet<RefPtr<WebCore::SecurityOrigin>>&&)>&& completionHandler)
{
RefPtr<StorageManager> storageManager(this);
- m_queue->dispatch([storageManager, completionHandler] {
+ m_queue->dispatch([storageManager, completionHandler = WTFMove(completionHandler)]() mutable {
HashSet<RefPtr<SecurityOrigin>> origins;
for (const auto& sessionStorageNamespace : storageManager->m_sessionStorageNamespaces.values()) {
@@ -565,25 +565,25 @@
origins.add(WTFMove(origin));
}
- RunLoop::main().dispatch([origins, completionHandler]() mutable {
+ RunLoop::main().dispatch([origins, completionHandler = WTFMove(completionHandler)]() mutable {
completionHandler(WTFMove(origins));
});
});
}
-void StorageManager::deleteSessionStorageOrigins(std::function<void ()> completionHandler)
+void StorageManager::deleteSessionStorageOrigins(std::function<void ()>&& completionHandler)
{
RefPtr<StorageManager> storageManager(this);
- m_queue->dispatch([storageManager, completionHandler] {
+ m_queue->dispatch([storageManager, completionHandler = WTFMove(completionHandler)]() mutable {
for (auto& sessionStorageNamespace : storageManager->m_sessionStorageNamespaces.values())
sessionStorageNamespace->clearAllStorageAreas();
- RunLoop::main().dispatch(completionHandler);
+ RunLoop::main().dispatch(WTFMove(completionHandler));
});
}
-void StorageManager::deleteSessionStorageEntriesForOrigins(const Vector<RefPtr<WebCore::SecurityOrigin>>& origins, std::function<void ()> completionHandler)
+void StorageManager::deleteSessionStorageEntriesForOrigins(const Vector<RefPtr<WebCore::SecurityOrigin>>& origins, std::function<void ()>&& completionHandler)
{
Vector<RefPtr<WebCore::SecurityOrigin>> copiedOrigins;
copiedOrigins.reserveInitialCapacity(origins.size());
@@ -592,21 +592,21 @@
copiedOrigins.uncheckedAppend(origin->isolatedCopy());
RefPtr<StorageManager> storageManager(this);
- m_queue->dispatch([storageManager, copiedOrigins, completionHandler] {
+ m_queue->dispatch([storageManager, copiedOrigins, completionHandler = WTFMove(completionHandler)]() mutable {
for (auto& origin : copiedOrigins) {
for (auto& sessionStorageNamespace : storageManager->m_sessionStorageNamespaces.values())
sessionStorageNamespace->clearStorageAreasMatchingOrigin(*origin);
}
- RunLoop::main().dispatch(completionHandler);
+ RunLoop::main().dispatch(WTFMove(completionHandler));
});
}
-void StorageManager::getLocalStorageOrigins(std::function<void (HashSet<RefPtr<WebCore::SecurityOrigin>>&&)> completionHandler)
+void StorageManager::getLocalStorageOrigins(std::function<void (HashSet<RefPtr<WebCore::SecurityOrigin>>&&)>&& completionHandler)
{
RefPtr<StorageManager> storageManager(this);
- m_queue->dispatch([storageManager, completionHandler] {
+ m_queue->dispatch([storageManager, completionHandler = WTFMove(completionHandler)]() mutable {
HashSet<RefPtr<SecurityOrigin>> origins;
for (auto& origin : storageManager->m_localStorageDatabaseTracker->origins())
@@ -617,20 +617,20 @@
origins.add(WTFMove(origin));
}
- RunLoop::main().dispatch([origins, completionHandler]() mutable {
+ RunLoop::main().dispatch([origins, completionHandler = WTFMove(completionHandler)]() mutable {
completionHandler(WTFMove(origins));
});
});
}
-void StorageManager::getLocalStorageOriginDetails(std::function<void (Vector<LocalStorageDatabaseTracker::OriginDetails>)> completionHandler)
+void StorageManager::getLocalStorageOriginDetails(std::function<void (Vector<LocalStorageDatabaseTracker::OriginDetails>)>&& completionHandler)
{
RefPtr<StorageManager> storageManager(this);
- m_queue->dispatch([storageManager, completionHandler] {
+ m_queue->dispatch([storageManager, completionHandler = WTFMove(completionHandler)]() mutable {
auto originDetails = storageManager->m_localStorageDatabaseTracker->originDetails();
- RunLoop::main().dispatch([originDetails, completionHandler]() mutable {
+ RunLoop::main().dispatch([originDetails, completionHandler = WTFMove(completionHandler)]() mutable {
completionHandler(WTFMove(originDetails));
});
});
@@ -652,11 +652,11 @@
});
}
-void StorageManager::deleteLocalStorageOriginsModifiedSince(std::chrono::system_clock::time_point time, std::function<void ()> completionHandler)
+void StorageManager::deleteLocalStorageOriginsModifiedSince(std::chrono::system_clock::time_point time, std::function<void ()>&& completionHandler)
{
RefPtr<StorageManager> storageManager(this);
- m_queue->dispatch([storageManager, time, completionHandler] {
+ m_queue->dispatch([storageManager, time, completionHandler = WTFMove(completionHandler)]() mutable {
auto deletedOrigins = storageManager->m_localStorageDatabaseTracker->deleteDatabasesModifiedSince(time);
for (const auto& origin : deletedOrigins) {
@@ -667,11 +667,11 @@
for (auto& transientLocalStorageNamespace : storageManager->m_transientLocalStorageNamespaces.values())
transientLocalStorageNamespace->clearAllStorageAreas();
- RunLoop::main().dispatch(completionHandler);
+ RunLoop::main().dispatch(WTFMove(completionHandler));
});
}
-void StorageManager::deleteLocalStorageEntriesForOrigins(const Vector<RefPtr<WebCore::SecurityOrigin>>& origins, std::function<void ()> completionHandler)
+void StorageManager::deleteLocalStorageEntriesForOrigins(const Vector<RefPtr<WebCore::SecurityOrigin>>& origins, std::function<void ()>&& completionHandler)
{
Vector<RefPtr<WebCore::SecurityOrigin>> copiedOrigins;
copiedOrigins.reserveInitialCapacity(origins.size());
@@ -680,7 +680,7 @@
copiedOrigins.uncheckedAppend(origin->isolatedCopy());
RefPtr<StorageManager> storageManager(this);
- m_queue->dispatch([storageManager, copiedOrigins, completionHandler] {
+ m_queue->dispatch([storageManager, copiedOrigins, completionHandler = WTFMove(completionHandler)]() mutable {
for (auto& origin : copiedOrigins) {
for (auto& localStorageNamespace : storageManager->m_localStorageNamespaces.values())
localStorageNamespace->clearStorageAreasMatchingOrigin(*origin);
@@ -691,7 +691,7 @@
storageManager->m_localStorageDatabaseTracker->deleteDatabaseWithOrigin(origin.get());
}
- RunLoop::main().dispatch(completionHandler);
+ RunLoop::main().dispatch(WTFMove(completionHandler));
});
}
Modified: trunk/Source/WebKit2/UIProcess/Storage/StorageManager.h (201463 => 201464)
--- trunk/Source/WebKit2/UIProcess/Storage/StorageManager.h 2016-05-27 20:14:16 UTC (rev 201463)
+++ trunk/Source/WebKit2/UIProcess/Storage/StorageManager.h 2016-05-27 20:22:12 UTC (rev 201464)
@@ -59,17 +59,17 @@
void processDidCloseConnection(WebProcessProxy&, IPC::Connection&);
void applicationWillTerminate();
- void getSessionStorageOrigins(std::function<void (HashSet<RefPtr<WebCore::SecurityOrigin>>&&)> completionHandler);
- void deleteSessionStorageOrigins(std::function<void ()> completionHandler);
- void deleteSessionStorageEntriesForOrigins(const Vector<RefPtr<WebCore::SecurityOrigin>>&, std::function<void ()> completionHandler);
+ void getSessionStorageOrigins(std::function<void (HashSet<RefPtr<WebCore::SecurityOrigin>>&&)>&& completionHandler);
+ void deleteSessionStorageOrigins(std::function<void ()>&& completionHandler);
+ void deleteSessionStorageEntriesForOrigins(const Vector<RefPtr<WebCore::SecurityOrigin>>&, std::function<void ()>&& completionHandler);
- void getLocalStorageOrigins(std::function<void (HashSet<RefPtr<WebCore::SecurityOrigin>>&&)> completionHandler);
+ void getLocalStorageOrigins(std::function<void (HashSet<RefPtr<WebCore::SecurityOrigin>>&&)>&& completionHandler);
void deleteLocalStorageEntriesForOrigin(const WebCore::SecurityOrigin&);
- void deleteLocalStorageOriginsModifiedSince(std::chrono::system_clock::time_point, std::function<void ()> completionHandler);
- void deleteLocalStorageEntriesForOrigins(const Vector<RefPtr<WebCore::SecurityOrigin>>&, std::function<void ()> completionHandler);
+ void deleteLocalStorageOriginsModifiedSince(std::chrono::system_clock::time_point, std::function<void ()>&& completionHandler);
+ void deleteLocalStorageEntriesForOrigins(const Vector<RefPtr<WebCore::SecurityOrigin>>&, std::function<void ()>&& completionHandler);
- void getLocalStorageOriginDetails(std::function<void (Vector<LocalStorageDatabaseTracker::OriginDetails>)> completionHandler);
+ void getLocalStorageOriginDetails(std::function<void (Vector<LocalStorageDatabaseTracker::OriginDetails>)>&& completionHandler);
private:
explicit StorageManager(const String& localStorageDirectory);
Modified: trunk/Tools/ChangeLog (201463 => 201464)
--- trunk/Tools/ChangeLog 2016-05-27 20:14:16 UTC (rev 201463)
+++ trunk/Tools/ChangeLog 2016-05-27 20:22:12 UTC (rev 201464)
@@ -1,3 +1,18 @@
+2016-05-27 Chris Dumez <[email protected]>
+
+ WorkQueue::dispatch() / RunLoop::dispatch() should not copy captured lambda variables
+ https://bugs.webkit.org/show_bug.cgi?id=158111
+
+ Reviewed by Darin Adler.
+
+ WorkQueue::dispatch() / RunLoop::dispatch() should not copy captured lambda variables.
+ These are often used cross-thread and copying the captured lambda variables can be
+ dangerous (e.g. we do not want to copy a String after calling isolatedCopy() upon
+ capture).
+
+ * WebKitTestRunner/TestController.cpp:
+ (WTR::TestController::decidePolicyForNavigationAction):
+
2016-05-27 Brady Eidson <[email protected]>
Modern IDB: After closing a Netflix video, trying to watch it again fails.
Modified: trunk/Tools/WebKitTestRunner/TestController.cpp (201463 => 201464)
--- trunk/Tools/WebKitTestRunner/TestController.cpp 2016-05-27 20:14:16 UTC (rev 201463)
+++ trunk/Tools/WebKitTestRunner/TestController.cpp 2016-05-27 20:22:12 UTC (rev 201464)
@@ -2022,7 +2022,7 @@
{
WKRetainPtr<WKFramePolicyListenerRef> retainedListener { listener };
const bool shouldIgnore { m_policyDelegateEnabled && !m_policyDelegatePermissive };
- std::function<void()> decisionFunction = [shouldIgnore, retainedListener]() {
+ auto decisionFunction = [shouldIgnore, retainedListener]() {
if (shouldIgnore)
WKFramePolicyListenerIgnore(retainedListener.get());
else
@@ -2030,7 +2030,7 @@
};
if (m_shouldDecideNavigationPolicyAfterDelay)
- RunLoop::main().dispatch(decisionFunction);
+ RunLoop::main().dispatch(WTFMove(decisionFunction));
else
decisionFunction();
}
