[Webkit-unassigned] [Bug 92365] New: Refactor cross thread communication

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=92365

           Summary: Refactor cross thread communication
           Product: WebKit
           Version: 528+ (Nightly build)
          Platform: Unspecified
        OS/Version: Unspecified
            Status: UNCONFIRMED
          Severity: Normal
          Priority: P2
         Component: New Bugs
        AssignedTo: webkit-unassigned at lists.webkit.org
        ReportedBy: gram at company100.net


1. Introduction

In WebCore, there are several implementations of threads, tasks and cross thread communication. For example, FileThread, DatabaseThread and StorageThread perform blocking IO asynchronously and invoke callbacks using ScriptExecutionContext.  Unfortunately, there is no code sharing at all though each of these implementation is pretty similar. Every thread has its own slightly different cross thread communication mechanism.

We will be better off if we could unify the cross thread communication mechanism and reuse it. Levin’s sample proposal looks promising, but I think the very first step is to refactor common code into reusable components and combine various task classes into one.

Levin filed a bug on Bug 51857 and wrote a wiki on this issue:
https://trac.webkit.org/wiki/ThreadCommunication


2. Requirements

I analyzed the current task threads code and extracted basic requirements:

- Add a task to the end of the task queue (append)
- Add a task to the front of the task queue (prepend)
- Cancel a group of tasks that are pending in the task queue
- Specify an action that will performed on thread termination



3. API Proposal

3.1 Task

Instead of creating a task type for each use case, WTF::Function is used everywhere. CrossThreadTask is merged into WTF::Function because they play essentially the same role. 

However, there is one problem with WTF::Function. When binding a non-static method with WTF::Function, if the receiver object’s class has ref/deref methods, the object’s reference count is incremented and decremented on WTF::Function construction and destruction respectively. If the object’s class is not a subclass of ThreadSafeRefCounted, this behavior can cause a race condition. We must modify WTF::Function to perform ref/deref only for ThreadSafeRefCounted.

3.2 TaskGroup

>From the requirement above, we must be able to cancel a group of task. TaskGroup is introduced to specify the group of a task. When posting a task, we can specify the task group where the task belongs. See TaskThread::postTask(const Function<void()>& task, PassRefPtr<TaskGroup> group) method below.

class TaskGroup : public ThreadSafeRefCounted<TaskGroup> { };


3.3 Thread

TaskThread is our new abstraction for task thread. FileThread, DatabaseThread and other task threads reuse the code by inheriting from TaskThread.

class TaskThread : public ThreadSafeRefCounted<TaskThread> {
public:
    ~TaskThread();
    void start();
    void stop();

    // Schedule a task to be executed by the task thread.
    void postTask(const Function<void()>& task);
    // Schedule a task to be executed by the task thread and also specify the task group.
    // Tasks scheduled with this method can be cancelled by removeTasks() method.
    void postTask(const Function<void()>& task, PassRefPtr<TaskGroup> group);

    // Schedule a task to be immediately executed by the task thread.
    void postImmediateTask(PassOwnPtr<TaskThreadTask> task);
    // Schedule a task to be executed by the task thread and also specify the task group.
    // Tasks scheduled with this method can be cancelled by removeTasks() method.
    void postImmediateTask(const Function<void()>& task, PassRefPtr<TaskGroup> group);

    // Cancel pending tasks that belong to the given group.
    void removeTasks(PassRefPtr<TaskGroup> group);

protected:
    TaskThread();
    // Subclasses can override this method to specify an action to be executed
    // on thread termination
    virtual void onRunLoopTerminated();
};


3.4 Callback to ScriptExecutionContext

ScriptExecutionContext::postTask() now takes a WTF::Function as an argument because WTF::Function replaced CrossThreadTask.

class ScriptExecutionContext :
   public SecurityContext, public Supplementable<ScriptExecutionContext> {
public:
…
   virtual void postTask(const Function<void()>&) = 0;
...
};

There is a small change in usage. CrossThreadTask does not bind the pointer to ScriptExecutionContext because it is automatically passed as the first argument when the task is invoked. However, since WTF::Function does not support partial application, the pointer to ScriptExecutionContext must be bound as the first argument when binding parameters to a task. This is possible because task threads already know the pointer. See the example below.

void AsyncFileStream::openForReadOnFileThread(const String& path, long long offset, long long length)
{
   bool success = m_stream->openForRead(path, offset, length);
   m_context->postTask(createCallbackTask(&didOpen, AllowCrossThreadAccess(this), success));
}

=>

void AsyncFileStream::openForReadOnFileThread(const String& path, long long offset, long long length)
{
   bool success = m_stream->openForRead(path, offset, length);
   m_context->postTask(bind(&didOpen, m_context, this, success));
}


3.5 CrossThreadCopyable

If the parameter type is a subclass of CrossThreadCopyable, the parameter is copied by calling crossThreadCopy().

template<typename Orig, New>
class CrossThreadCopyable<Orig, New> {
public:
    virtual New crossThreadCopy();
}

AllowCrossThreadAccess is no longer needed because WTF::Function ref counts only the first parameter.


3.6 deepCopy

If a class can’t be a subclass of CrossThreadCopyable, deepCopy() can be used to pass thread-unsafe instances by calling custom copy methods. For example, we can pass String and KURL instances thread-safely by wrapping them with deepCopy. For a string s, deepCopy(s) returns s.isolatedCopy().

void LocalFileSystem::readFileSystem(ScriptExecutionContext* context, FileSystemType, PassOwnPtr<AsyncFileSystemCallbacks> callbacks, FileSystemSynchronousType)
{
    context->postTask(bind(&openFileSystem, context, deepCopy(fileSystemBasePath()), deepCopy(fcontext->securityOrigin()->databaseIdentifier()), false, callbacks));
}


4. Plan

This proposal does not add any new mechanism except for a few minor changes in WTF::Function. It simply removes duplicated code and extracts common code into reusable components. Once we have this refactoring done successfully, we can move on to fancier cross thread communication mechanism.

Before I take an initiative to work hard on this proposal, I want to hear opinions from reviewers and committers who understand WebKit threads very well. Also if you have any concerns on implementation level details, please let me know before I struggle with C++ template hacks :)

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.