⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 284476 in webkit


Ignore:
Timestamp:
Oct 19, 2021, 12:09:41 PM (5 years ago)
Author:
Wenson Hsieh
Message:

REGRESSION (r284079): fast/canvas/gradient-with-clip.html and fast/canvas/gradient-text-with-shadow.html are flaky failures
​https://bugs.webkit.org/show_bug.cgi?id=231681
rdar://84202478

Reviewed by Kimmo Kinnunen.

Source/WebKit:

These tests occasionally fail when we time out while waiting for the pixel buffer to be populated underneath
RemoteImageBufferProxy::getPixelBuffer; this happens because the GPU process sometimes halts (and never
resumes) while processing stream messages, and consequently never ends up processing the GetPixelBuffer IPC
stream message that's necessary to populate the shared memory buffer for GetPixelBuffer.

This, in turn, happens when the IPC stream buffer is about to exhaust all available capacity (i.e. the write
cursor is nearing the end of the 2MB buffer), and as a result, we send messages that would normally be encoded
in the IPC stream as out-of-line IPC messages instead, which are received in the GPU process on the IPC thread
and appended to the receive queue corresponding to the destination ID in Connection::processIncomingMessage().

If we happen to have sent RemoteRenderingBackend::CreateImageBuffer(a) right before exhausting capacity and
sending out-of-line messages targeting the RemoteDisplayListRecorder corresponding to the newly created image
buffer a, we'll end up with a race condition where the IPC thread may receive the out-of-line messages meant
for the new remote display list destination *before* the new remote display list has added itself as an IPC
receive queue by calling into StreamServerConnection::startReceivingMessages().

In this particular scenario (where we "lose" the race), Connection::processIncomingMessage will skip past the
early return where it would normally find its corresponding receive queue via m_receiveQueues.get(*message),
and instead falls through to Connection::SyncMessageState::processIncomingMessage(), which will attempt to
dispatch the incoming message to the Connection's client (GPUConnectionToWebProcess). Of course, the
GPUConnectionToWebProcess isn't aware of how to process this incoming message, so it simply gets dropped. By the
time the receive queue is added for the new RemoteDisplayListRecorder, we'll observe a ProcessOutOfStreamMessage
and end up waiting forever for this out-of-line message to be added to the receive queue, but this never ends up
happening because it has already been received and dropped by GPUConnectionToWebProcess.

To address the flaky tests in the short term, we work around this issue by making CreateImageBuffer a sync
message, so we guarantee that any subsequent messages sent to the image buffer's RemoteDisplayListRecorder will
not be dropped. In the (slightly) longer term, we should turn this back into an async stream message, and add a
mechanism to redirect all receiver-less RemoteDisplayListRecorder messages to a separate queue.

  • GPUProcess/graphics/RemoteRenderingBackend.cpp:

(WebKit::RemoteRenderingBackend::createImageBuffer):

  • GPUProcess/graphics/RemoteRenderingBackend.h:
  • GPUProcess/graphics/RemoteRenderingBackend.messages.in:

Make CreateImageBuffer a synchronous stream message for now.

  • WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp:

(WebKit::RemoteRenderingBackendProxy::createRemoteImageBuffer):

LayoutTests:

Remove the flaky failure entries in test expectations. See WebKit2 ChangeLog for more details.

  • platform/ios-wk2/TestExpectations:
  • platform/mac-wk2/TestExpectations:
Location:
trunk
Files:
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r284475 r284476  
     12021-10-19  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        REGRESSION (r284079): fast/canvas/gradient-with-clip.html and fast/canvas/gradient-text-with-shadow.html are flaky failures
     4        https://bugs.webkit.org/show_bug.cgi?id=231681
     5        rdar://84202478
     6
     7        Reviewed by Kimmo Kinnunen.
     8
     9        Remove the flaky failure entries in test expectations. See WebKit2 ChangeLog for more details.
     10
     11        * platform/ios-wk2/TestExpectations:
     12        * platform/mac-wk2/TestExpectations:
     13
    1142021-10-19  Arcady Goldmints-Orlov  <agoldmints@igalia.com>
    215

  • trunk/LayoutTests/platform/ios-wk2/TestExpectations

    r284469 r284476  
    20332033webkit.org/b/230413 [ Debug ] fast/canvas/canvas-drawImage-detached-leak.html [ Pass Failure ]
    20342034
    2035 webkit.org/b/231681 [ Release ] fast/canvas/gradient-text-with-shadow.html [ Pass Failure ]
    2036 webkit.org/b/231681 [ Release ] fast/canvas/gradient-with-clip.html [ Pass Failure ]
    2037 
    20382035webanimations/multiple-transform-properties-and-multiple-transform-properties-animation-with-delay-on-forced-layer.html [ Pass ]
    20392036webanimations/rotate-property-and-rotate-animation-with-delay-on-forced-layer.html [ Pass ]

  • trunk/LayoutTests/platform/mac-wk2/TestExpectations

    r284473 r284476  
    15091509webkit.org/b/230366 [ Release ] fast/canvas/canvas-composite-text-alpha.html [ Pass Failure ]
    15101510
    1511 webkit.org/b/231681 [ Release ] fast/canvas/gradient-text-with-shadow.html [ Pass Failure ]
    1512 webkit.org/b/231681 [ Release ] fast/canvas/gradient-with-clip.html [ Pass Failure ]
    1513 
    15141511webkit.org/b/230413 fast/canvas/canvas-drawImage-detached-leak.html [ Pass Failure ]
    15151512

  • trunk/Source/WebKit/ChangeLog

    r284472 r284476  
     12021-10-19  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        REGRESSION (r284079): fast/canvas/gradient-with-clip.html and fast/canvas/gradient-text-with-shadow.html are flaky failures
     4        https://bugs.webkit.org/show_bug.cgi?id=231681
     5        rdar://84202478
     6
     7        Reviewed by Kimmo Kinnunen.
     8
     9        These tests occasionally fail when we time out while waiting for the pixel buffer to be populated underneath
     10        `RemoteImageBufferProxy::getPixelBuffer`; this happens because the GPU process sometimes halts (and never
     11        resumes) while processing stream messages, and consequently never ends up processing the GetPixelBuffer IPC
     12        stream message that's necessary to populate the shared memory buffer for GetPixelBuffer.
     13
     14        This, in turn, happens when the IPC stream buffer is about to exhaust all available capacity (i.e. the write
     15        cursor is nearing the end of the 2MB buffer), and as a result, we send messages that would normally be encoded
     16        in the IPC stream as out-of-line IPC messages instead, which are received in the GPU process on the IPC thread
     17        and appended to the receive queue corresponding to the destination ID in `Connection::processIncomingMessage()`.
     18
     19        If we happen to have sent `RemoteRenderingBackend::CreateImageBuffer(a)` right before exhausting capacity and
     20        sending out-of-line messages targeting the RemoteDisplayListRecorder corresponding to the newly created image
     21        buffer `a`, we'll end up with a race condition where the IPC thread may receive the out-of-line messages meant
     22        for the new remote display list destination *before* the new remote display list has added itself as an IPC
     23        receive queue by calling into `StreamServerConnection::startReceivingMessages()`.
     24
     25        In this particular scenario (where we "lose" the race), `Connection::processIncomingMessage` will skip past the
     26        early return where it would normally find its corresponding receive queue via `m_receiveQueues.get(*message)`,
     27        and instead falls through to `Connection::SyncMessageState::processIncomingMessage()`, which will attempt to
     28        dispatch the incoming message to the Connection's client (GPUConnectionToWebProcess). Of course, the
     29        GPUConnectionToWebProcess isn't aware of how to process this incoming message, so it simply gets dropped. By the
     30        time the receive queue is added for the new RemoteDisplayListRecorder, we'll observe a ProcessOutOfStreamMessage
     31        and end up waiting forever for this out-of-line message to be added to the receive queue, but this never ends up
     32        happening because it has already been received and dropped by GPUConnectionToWebProcess.
     33
     34        To address the flaky tests in the short term, we work around this issue by making `CreateImageBuffer` a sync
     35        message, so we guarantee that any subsequent messages sent to the image buffer's RemoteDisplayListRecorder will
     36        not be dropped. In the (slightly) longer term, we should turn this back into an async stream message, and add a
     37        mechanism to redirect all receiver-less RemoteDisplayListRecorder messages to a separate queue.
     38
     39        * GPUProcess/graphics/RemoteRenderingBackend.cpp:
     40        (WebKit::RemoteRenderingBackend::createImageBuffer):
     41        * GPUProcess/graphics/RemoteRenderingBackend.h:
     42        * GPUProcess/graphics/RemoteRenderingBackend.messages.in:
     43
     44        Make `CreateImageBuffer` a synchronous stream message for now.
     45
     46        * WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp:
     47        (WebKit::RemoteRenderingBackendProxy::createRemoteImageBuffer):
     48
    1492021-10-19  Youenn Fablet  <youenn@apple.com>
    250

  • trunk/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp

    r284427 r284476  
    139139}
    140140
    141 void RemoteRenderingBackend::createImageBuffer(const FloatSize& logicalSize, RenderingMode renderingMode, float resolutionScale, const DestinationColorSpace& colorSpace, PixelFormat pixelFormat, RenderingResourceIdentifier imageBufferResourceIdentifier)
     141void RemoteRenderingBackend::createImageBuffer(const FloatSize& logicalSize, RenderingMode renderingMode, float resolutionScale, const DestinationColorSpace& colorSpace, PixelFormat pixelFormat, RenderingResourceIdentifier imageBufferResourceIdentifier, CompletionHandler<void()>&& completionHandler)
    142142{
    143143    // Immediately turn the RenderingResourceIdentifier (which is error-prone) to a QualifiedRenderingResourceIdentifier,
    144144    // and use a helper function to make sure that don't accidentally use the RenderingResourceIdentifier (because the helper function can't see it).
    145145    createImageBufferWithQualifiedIdentifier(logicalSize, renderingMode, resolutionScale, colorSpace, pixelFormat, { imageBufferResourceIdentifier, m_gpuConnectionToWebProcess->webProcessIdentifier() });
     146    completionHandler();
    146147}
    147148

  • trunk/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.h

    r284427 r284476  
    102102
    103103    // Messages to be received.
    104     void createImageBuffer(const WebCore::FloatSize& logicalSize, WebCore::RenderingMode, float resolutionScale, const WebCore::DestinationColorSpace&, WebCore::PixelFormat, WebCore::RenderingResourceIdentifier);
     104    void createImageBuffer(const WebCore::FloatSize& logicalSize, WebCore::RenderingMode, float resolutionScale, const WebCore::DestinationColorSpace&, WebCore::PixelFormat, WebCore::RenderingResourceIdentifier, CompletionHandler<void()>&&);
    105105    void updateSharedMemoryForGetPixelBuffer(uint32_t byteCount, CompletionHandler<void(const SharedMemory::IPCHandle&)>&&);
    106106    void semaphoreForGetPixelBuffer(CompletionHandler<void(const IPC::Semaphore&)>&&);

  • trunk/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.messages.in

    r284079 r284476  
    2424
    2525messages -> RemoteRenderingBackend NotRefCounted Stream {
    26     CreateImageBuffer(WebCore::FloatSize logicalSize, WebCore::RenderingMode renderingMode, float resolutionScale, WebCore::DestinationColorSpace colorSpace, enum:uint8_t WebCore::PixelFormat pixelFormat, WebCore::RenderingResourceIdentifier renderingResourceIdentifier)
     26    CreateImageBuffer(WebCore::FloatSize logicalSize, WebCore::RenderingMode renderingMode, float resolutionScale, WebCore::DestinationColorSpace colorSpace, enum:uint8_t WebCore::PixelFormat pixelFormat, WebCore::RenderingResourceIdentifier renderingResourceIdentifier) -> () Synchronous
    2727    UpdateSharedMemoryForGetPixelBuffer(uint32_t byteCount) -> (WebKit::SharedMemory::IPCHandle handle) Synchronous NotStreamEncodableReply
    2828    SemaphoreForGetPixelBuffer() -> (IPC::Semaphore semaphore) Synchronous NotStreamEncodableReply

  • trunk/Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp

    r284142 r284476  
    128128void RemoteRenderingBackendProxy::createRemoteImageBuffer(ImageBuffer& imageBuffer)
    129129{
    130     sendToStream(Messages::RemoteRenderingBackend::CreateImageBuffer(imageBuffer.logicalSize(), imageBuffer.renderingMode(), imageBuffer.resolutionScale(), imageBuffer.colorSpace(), imageBuffer.pixelFormat(), imageBuffer.renderingResourceIdentifier()));
     130    // FIXME: This should be an normal (async) stream message. However, doing so may cause subsequent out-of-stream messages sent to the newly created image
     131    // buffer to be dropped by the IPC connection. For the time being, work around this by using sync IPC (see <https://webkit.org/b/231681> for more details).
     132    sendSyncToStream(Messages::RemoteRenderingBackend::CreateImageBuffer(imageBuffer.logicalSize(), imageBuffer.renderingMode(), imageBuffer.resolutionScale(), imageBuffer.colorSpace(), imageBuffer.pixelFormat(), imageBuffer.renderingResourceIdentifier()), Messages::RemoteRenderingBackend::CreateImageBuffer::Reply(), 3_s);
    131133}
    132134
Note: See TracChangeset for help on using the changeset viewer.