Changeset 232888 in webkit


Ignore:
Timestamp:
Jun 15, 2018 12:36:00 PM (6 years ago)
Author:
Basuke Suzuki
Message:

[WinCairo] Move unrelated features of WorkQueueWin into IPC::Connection
​https://bugs.webkit.org/show_bug.cgi?id=186582

Source/WebKit:

Add EventListener private class to handle signaled tasks for I/O.
Originally they were in WTF::WorkQueueWin, but those features were not related
to WorkQueue and only used in IPC::ConnectionWin. Moved logic is more specialized
than old generalized logic. That was unneeded generalization.

Reviewed by Brent Fulgham.

  • Platform/IPC/Connection.h:

(IPC::Connection::EventListener::state):

  • Platform/IPC/win/ConnectionWin.cpp:

(IPC::Connection::platformInitialize):
(IPC::Connection::platformInvalidate):
(IPC::Connection::readEventHandler):
(IPC::Connection::writeEventHandler):
(IPC::Connection::invokeReadEventHandler):
(IPC::Connection::invokeWriteEventHandler):
(IPC::Connection::open):
(IPC::Connection::sendOutgoingMessage):
(IPC::Connection::EventListener::open):
(IPC::Connection::EventListener::callback):
(IPC::Connection::EventListener::close):

Source/WTF:

Remove unrelated feature from WorkQueueWin.

Reviewed by Brent Fulgham.

  • wtf/PlatformWin.cmake: Remove WorkItemContext.*
  • wtf/WorkQueue.cpp:
  • wtf/WorkQueue.h:
  • wtf/win/Win32Handle.h:
  • wtf/win/WorkItemContext.cpp: Removed.
  • wtf/win/WorkItemContext.h: Removed.
  • wtf/win/WorkQueueWin.cpp:

(WTF::WorkQueue::handleCallback): Deleted.
(WTF::WorkQueue::registerHandle): Deleted.
(WTF::WorkQueue::unregisterAndCloseHandle): Deleted.
(WTF::WorkQueue::unregisterWaitAndDestroyItemSoon): Deleted.
(WTF::WorkQueue::unregisterWaitAndDestroyItemCallback): Deleted.

Location:
trunk/Source
Files:
2 deleted
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WTF/ChangeLog

    r232801 r232888  
     12018-06-15  Basuke Suzuki  <Basuke.Suzuki@sony.com>
     2
     3        [WinCairo] Move unrelated features of WorkQueueWin into IPC::Connection
     4        https://bugs.webkit.org/show_bug.cgi?id=186582
     5
     6        Remove unrelated feature from WorkQueueWin.
     7
     8        Reviewed by Brent Fulgham.
     9
     10        * wtf/PlatformWin.cmake: Remove WorkItemContext.*
     11        * wtf/WorkQueue.cpp:
     12        * wtf/WorkQueue.h:
     13        * wtf/win/Win32Handle.h:
     14        * wtf/win/WorkItemContext.cpp: Removed.
     15        * wtf/win/WorkItemContext.h: Removed.
     16        * wtf/win/WorkQueueWin.cpp:
     17        (WTF::WorkQueue::handleCallback): Deleted.
     18        (WTF::WorkQueue::registerHandle): Deleted.
     19        (WTF::WorkQueue::unregisterAndCloseHandle): Deleted.
     20        (WTF::WorkQueue::unregisterWaitAndDestroyItemSoon): Deleted.
     21        (WTF::WorkQueue::unregisterWaitAndDestroyItemCallback): Deleted.
     22
    1232018-06-13  Keith Miller  <keith_miller@apple.com>
    224

  • trunk/Source/WTF/wtf/PlatformWin.cmake

    r229572 r232888  
    55    win/SoftLinking.h
    66    win/Win32Handle.h
    7     win/WorkItemContext.h
    87)
    98
    … …  
    1716    win/MemoryPressureHandlerWin.cpp
    1817    win/RunLoopWin.cpp
    19     win/WorkItemContext.cpp
    2018    win/WorkQueueWin.cpp
    2119)

  • trunk/Source/WTF/wtf/WorkQueue.cpp

    r225778 r232888  
    3838#include <wtf/Threading.h>
    3939#include <wtf/text/WTFString.h>
    40 
    41 #if USE(WINDOWS_EVENT_LOOP)
    42 #include <wtf/win/WorkItemContext.h>
    43 #endif
    4440
    4541namespace WTF {

  • trunk/Source/WTF/wtf/WorkQueue.h

    r225684 r232888  
    5151namespace WTF {
    5252
    53 #if USE(WINDOWS_EVENT_LOOP)
    54 class WorkItemContext;
    55 #endif
    56 
    5753class WorkQueue final : public FunctionDispatcher {
    5854
    … …  
    8278#elif USE(GLIB_EVENT_LOOP) || USE(GENERIC_EVENT_LOOP)
    8379    RunLoop& runLoop() const { return *m_runLoop; }
    84 #elif USE(WINDOWS_EVENT_LOOP)
    85     WTF_EXPORT_PRIVATE void registerHandle(HANDLE, Function<void()>&&);
    86     WTF_EXPORT_PRIVATE void unregisterAndCloseHandle(HANDLE);
    8780#endif
    8881
    … …  
    9487
    9588#if USE(WINDOWS_EVENT_LOOP)
    96     static void CALLBACK handleCallback(void* context, BOOLEAN timerOrWaitFired);
    9789    static void CALLBACK timerCallback(void* context, BOOLEAN timerOrWaitFired);
    9890    static DWORD WINAPI workThreadCallback(void* context);
    … …  
    10193    void unregisterAsWorkThread();
    10294    void performWorkOnRegisteredWorkThread();
    103 
    104     static void unregisterWaitAndDestroyItemSoon(Ref<WorkItemContext>&&);
    105     static DWORD WINAPI unregisterWaitAndDestroyItemCallback(void* context);
    10695#endif
    10796
    … …  
    113102
    114103    Lock m_functionQueueLock;
    115     Lock m_itemsMapLock;
    116104    Vector<Function<void()>> m_functionQueue;
    117     HashMap<HANDLE, Ref<WorkItemContext>> m_itemsMap;
    118105
    119106    HANDLE m_timerQueue;

  • trunk/Source/WTF/wtf/win/Win32Handle.h

    r224137 r232888  
    3434    WTF_MAKE_NONCOPYABLE(Win32Handle);
    3535
    36     friend class WorkQueue;
    37 
    3836public:
    3937    Win32Handle() : m_handle(INVALID_HANDLE_VALUE) { }

  • trunk/Source/WTF/wtf/win/WorkQueueWin.cpp

    r230116 r232888  
    3030#include <wtf/MathExtras.h>
    3131#include <wtf/Threading.h>
    32 #include <wtf/win/WorkItemContext.h>
    3332
    3433namespace WTF {
    35 
    36 void WorkQueue::handleCallback(void* data, BOOLEAN timerOrWaitFired)
    37 {
    38     ASSERT_ARG(data, data);
    39     ASSERT_ARG(timerOrWaitFired, !timerOrWaitFired);
    40 
    41     WorkItemContext* context = static_cast<WorkItemContext*>(data);
    42     WorkQueue* queue = context->queue();
    43 
    44     RefPtr<WorkItemContext> protector(context);
    45     queue->dispatch([protector] {
    46         protector->function()();
    47     });
    48 }
    49 
    50 void WorkQueue::registerHandle(HANDLE handle, Function<void()>&& function)
    51 {
    52     Ref<WorkItemContext> context = WorkItemContext::create(handle, nullptr, WTFMove(function), this);
    53 
    54     if (!::RegisterWaitForSingleObject(&context->waitHandle().m_handle, handle, handleCallback, context.ptr(), INFINITE, WT_EXECUTEDEFAULT))
    55         ASSERT_WITH_MESSAGE(m_timerQueue, "::RegisterWaitForSingleObject %lu", ::GetLastError());
    56 
    57     auto locker = holdLock(m_itemsMapLock);
    58     ASSERT_ARG(handle, !m_itemsMap.contains(handle));
    59     m_itemsMap.set(handle, WTFMove(context));
    60 }
    61 
    62 void WorkQueue::unregisterAndCloseHandle(HANDLE handle)
    63 {
    64     auto locker = holdLock(m_itemsMapLock);
    65     ASSERT_ARG(handle, m_itemsMap.contains(handle));
    66 
    67     unregisterWaitAndDestroyItemSoon(m_itemsMap.take(handle).value());
    68 }
    6934
    7035DWORD WorkQueue::workThreadCallback(void* context)
    … …  
    223188}
    224189
    225 void WorkQueue::unregisterWaitAndDestroyItemSoon(Ref<WorkItemContext>&& workItem)
    226 {
    227     // We're going to make a blocking call to ::UnregisterWaitEx before closing the handle. (The
    228     // blocking version of ::UnregisterWaitEx is much simpler than the non-blocking version.) If we
    229     // do this on the current thread, we'll deadlock if we're currently in a callback function for
    230     // the wait we're unregistering. So instead we do it asynchronously on some other worker thread.
    231     ::QueueUserWorkItem(unregisterWaitAndDestroyItemCallback, workItem.ptr(), WT_EXECUTEDEFAULT);
    232 }
    233 
    234 DWORD WINAPI WorkQueue::unregisterWaitAndDestroyItemCallback(void* data)
    235 {
    236     ASSERT_ARG(data, data);
    237     WorkItemContext* context = static_cast<WorkItemContext*>(data);
    238 
    239     // Now that we know we're not in a callback function for the wait we're unregistering, we can
    240     // make a blocking call to ::UnregisterWaitEx.
    241     if (!::UnregisterWaitEx(context->waitHandle().get(), INVALID_HANDLE_VALUE))
    242         ASSERT_WITH_MESSAGE(false, "::UnregisterWaitEx failed with '%s'", ::GetLastError());
    243 
    244     return 0;
    245 }
    246 
    247190} // namespace WTF

  • trunk/Source/WebKit/ChangeLog

    r232887 r232888  
     12018-06-15  Basuke Suzuki  <Basuke.Suzuki@sony.com>
     2
     3        [WinCairo] Move unrelated features of WorkQueueWin into IPC::Connection
     4        https://bugs.webkit.org/show_bug.cgi?id=186582
     5
     6        Add EventListener private class to handle signaled tasks for I/O.
     7        Originally they were in WTF::WorkQueueWin, but those features were not related
     8        to WorkQueue and only used in IPC::ConnectionWin. Moved logic is more specialized
     9        than old generalized logic. That was unneeded generalization.
     10
     11        Reviewed by Brent Fulgham.
     12
     13        * Platform/IPC/Connection.h:
     14        (IPC::Connection::EventListener::state):
     15        * Platform/IPC/win/ConnectionWin.cpp:
     16        (IPC::Connection::platformInitialize):
     17        (IPC::Connection::platformInvalidate):
     18        (IPC::Connection::readEventHandler):
     19        (IPC::Connection::writeEventHandler):
     20        (IPC::Connection::invokeReadEventHandler):
     21        (IPC::Connection::invokeWriteEventHandler):
     22        (IPC::Connection::open):
     23        (IPC::Connection::sendOutgoingMessage):
     24        (IPC::Connection::EventListener::open):
     25        (IPC::Connection::EventListener::callback):
     26        (IPC::Connection::EventListener::close):
     27
    1282018-06-15  Brady Eidson  <beidson@apple.com>
    229

  • trunk/Source/WebKit/Platform/IPC/Connection.h

    r230359 r232888  
    344344    void readEventHandler();
    345345    void writeEventHandler();
     346    void invokeReadEventHandler();
     347    void invokeWriteEventHandler();
     348
     349    class EventListener {
     350    public:
     351        void open(Function<void()>&&);
     352        void close();
     353
     354        OVERLAPPED& state() { return m_state; }
     355
     356    private:
     357        static void callback(void*, BOOLEAN);
     358
     359        OVERLAPPED m_state;
     360        HANDLE m_waitHandle { INVALID_HANDLE_VALUE };
     361        Function<void()> m_handler;
     362    };
    346363
    347364    Vector<uint8_t> m_readBuffer;
    348     OVERLAPPED m_readState;
     365    EventListener m_readListener;
    349366    std::unique_ptr<Encoder> m_pendingWriteEncoder;
    350     OVERLAPPED m_writeState;
    351     HANDLE m_connectionPipe;
     367    EventListener m_writeListener;
     368    HANDLE m_connectionPipe { INVALID_HANDLE_VALUE };
    352369#endif
    353370};

  • trunk/Source/WebKit/Platform/IPC/win/ConnectionWin.cpp

    r232860 r232888  
    7373void Connection::platformInitialize(Identifier identifier)
    7474{
    75     memset(&m_readState, 0, sizeof(m_readState));
    76     m_readState.hEvent = ::CreateEventW(0, FALSE, FALSE, 0);
    77 
    78     memset(&m_writeState, 0, sizeof(m_writeState));
    79     m_writeState.hEvent = ::CreateEventW(0, FALSE, FALSE, 0);
    80 
    8175    m_connectionPipe = identifier;
    8276}
    … …  
    8983    m_isConnected = false;
    9084
    91     m_connectionQueue->unregisterAndCloseHandle(m_readState.hEvent);
    92     m_readState.hEvent = 0;
    93 
    94     m_connectionQueue->unregisterAndCloseHandle(m_writeState.hEvent);
    95     m_writeState.hEvent = 0;
     85    m_readListener.close();
     86    m_writeListener.close();
    9687
    9788    ::CloseHandle(m_connectionPipe);
    … …  
    10798        // Check if we got some data.
    10899        DWORD numberOfBytesRead = 0;
    109         if (!::GetOverlappedResult(m_connectionPipe, &m_readState, &numberOfBytesRead, FALSE)) {
     100        if (!::GetOverlappedResult(m_connectionPipe, &m_readListener.state(), &numberOfBytesRead, FALSE)) {
    110101            DWORD error = ::GetLastError();
    111102            switch (error) {
    … …  
    134125
    135126                m_readBuffer.grow(m_readBuffer.size() + bytesToRead);
    136                 if (!::ReadFile(m_connectionPipe, m_readBuffer.data() + numberOfBytesRead, bytesToRead, 0, &m_readState)) {
     127                if (!::ReadFile(m_connectionPipe, m_readBuffer.data() + numberOfBytesRead, bytesToRead, 0, &m_readListener.state())) {
    137128                    DWORD error = ::GetLastError();
    138129                    ASSERT_NOT_REACHED();
    … …  
    183174        // Either read the next available message (which should occur synchronously), or start an
    184175        // asynchronous read of the next message that becomes available.
    185         BOOL result = ::ReadFile(m_connectionPipe, m_readBuffer.data(), m_readBuffer.size(), 0, &m_readState);
     176        BOOL result = ::ReadFile(m_connectionPipe, m_readBuffer.data(), m_readBuffer.size(), 0, &m_readListener.state());
    186177        if (result) {
    187178            // There was already a message waiting in the pipe, and we read it synchronously.
    … …  
    219210
    220211    DWORD numberOfBytesWritten = 0;
    221     if (!::GetOverlappedResult(m_connectionPipe, &m_writeState, &numberOfBytesWritten, FALSE)) {
     212    if (!::GetOverlappedResult(m_connectionPipe, &m_writeListener.state(), &numberOfBytesWritten, FALSE)) {
    222213        DWORD error = ::GetLastError();
    223214
    … …  
    241232}
    242233
     234void Connection::invokeReadEventHandler()
     235{
     236    m_connectionQueue->dispatch([this, protectedThis = makeRef(*this)] {
     237        readEventHandler();
     238    });
     239}
     240
     241void Connection::invokeWriteEventHandler()
     242{
     243    m_connectionQueue->dispatch([this, protectedThis = makeRef(*this)] {
     244        writeEventHandler();
     245    });
     246}
     247
    243248bool Connection::open()
    244249{
    … …  
    246251    m_isConnected = true;
    247252
    248     RefPtr<Connection> protectedThis(this);
    249 
    250253    // Start listening for read and write state events.
    251     m_connectionQueue->registerHandle(m_readState.hEvent, [protectedThis] {
    252         protectedThis->readEventHandler();
     254    m_readListener.open([this] {
     255        invokeReadEventHandler();
    253256    });
    254257
    255     m_connectionQueue->registerHandle(m_writeState.hEvent, [protectedThis] {
    256         protectedThis->writeEventHandler();
     258    m_writeListener.open([this] {
     259        invokeWriteEventHandler();
    257260    });
    258261
    259262    // Schedule a read.
    260     m_connectionQueue->dispatch([protectedThis] {
    261         protectedThis->readEventHandler();
    262     });
     263    invokeReadEventHandler();
    263264    return true;
    264265}
    … …  
    285286    // Write the outgoing message.
    286287
    287     if (::WriteFile(m_connectionPipe, encoder->buffer(), encoder->bufferSize(), 0, &m_writeState)) {
     288    if (::WriteFile(m_connectionPipe, encoder->buffer(), encoder->bufferSize(), 0, &m_writeListener.state())) {
    288289        // We successfully sent this message.
    289290        return true;
    … …  
    319320}
    320321
     322void Connection::EventListener::open(Function<void()>&& handler)
     323{
     324    m_handler = WTFMove(handler);
     325
     326    memset(&m_state, 0, sizeof(m_state));
     327    m_state.hEvent = ::CreateEventW(0, FALSE, FALSE, 0);
     328
     329    BOOL result;
     330    result = ::RegisterWaitForSingleObject(&m_waitHandle, m_state.hEvent, callback, this, INFINITE, WT_EXECUTEDEFAULT);
     331    ASSERT(result);
     332}
     333
     334void Connection::EventListener::callback(void* data, BOOLEAN timerOrWaitFired)
     335{
     336    ASSERT_ARG(data, data);
     337    ASSERT_ARG(timerOrWaitFired, !timerOrWaitFired);
     338
     339    auto* listener = static_cast<Connection::EventListener*>(data);
     340    listener->m_handler();
     341}
     342
     343void Connection::EventListener::close()
     344{
     345    // We call ::UnregisterWaitEx directly here. Since ::UnregisterWaitEx drains all the remaining tasks here,
     346    // it would cause deadlock if this function itself is executed in Windows callback functions. But this call
     347    // is safe since our callbacks immediately dispatch a task to WorkQueue. And no Windows callbacks call this
     348    // Connection::EventListener::close().
     349    // https://msdn.microsoft.com/en-us/library/windows/desktop/ms685061(v=vs.85).aspx
     350    //
     351    // And do not ::CloseHandle(m_waitHandle).
     352    // > Note that a wait handle cannot be used in functions that require an object handle, such as CloseHandle.
     353    // https://msdn.microsoft.com/en-us/library/windows/desktop/ms685061(v=vs.85).aspx
     354    ::UnregisterWaitEx(m_waitHandle, INVALID_HANDLE_VALUE);
     355    m_waitHandle = INVALID_HANDLE_VALUE;
     356
     357    ::CloseHandle(m_state.hEvent);
     358    m_state.hEvent = 0;
     359
     360    m_handler = Function<void()>();
     361}
     362
    321363} // namespace IPC
Note: See TracChangeset for help on using the changeset viewer.