[Libreoffice-commits] online.git: loolwsd/LOOLKit.cpp

[Ashod Nakashian]

 loolwsd/LOOLKit.cpp |    2 --
 1 file changed, 2 deletions(-)

New commits:
commit 335611b0ff2bcbe2341d9718f49c8c9139c5c4b5
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Sun May 29 18:31:56 2016 -0400

    loolwsd: don't unlock too soon
    
    Since rendering moved to centralized WSD<->Kit
    processing, it runs on a different thread than
    those processing the communication between
    ClientSession and ChildSession. This introduces
    a new race between events and tile rendering.
    
    The shared ChildSession lock prevents this race
    such that no events are processed while a tile
    is rendered and, more importantly, response
    is prepared and sent back. That preparation
    could be lengthy due to png compression.
    
    The typical race happens when two keystrokes
    are entered in quick succession such that the
    same tile is invalidated while it's rendered.
    If the invalidation is processed in parallel
    to rendering, it will find no cached image to
    remove. It will reach the client, who will
    request a new tile. Meanwhile, the first tile
    is rendered and cached. By the time the second
    request arrives we have a cache hit, albeit on
    an outdated tile, missing the second character.
    
    By locking until the tile response is sent we
    ensure that the invalidate event will follow
    it, and by then the image will have been cached.
    The invalidation then removes the cached image
    and the second tile request is forced to place
    a new tile render request.
    
    There is some inefficiency, it would seem, but
    that is not really true, as Core is really
    sequential anyway and we shouldn't process
    events in parallel in the first place.
    
    Change-Id: Id8170a41a7e69bca6ac8b520029b7cdb2d96c880
    Reviewed-on: https://gerrit.libreoffice.org/25662
    Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
    Tested-by: Ashod Nakashian <ashnakash at gmail.com>

diff --git a/loolwsd/LOOLKit.cpp b/loolwsd/LOOLKit.cpp
index 626435a..b462076 100644
--- a/loolwsd/LOOLKit.cpp
+++ b/loolwsd/LOOLKit.cpp
@@ -597,7 +597,6 @@ public:
         Log::trace() << "paintTile at (" << tile.getPart() << ',' << tile.getTilePosX() << ',' << tile.getTilePosY()
                      << ") rendered in " << (timestamp.elapsed()/1000.) << " ms" << Log::end;
         const auto mode = static_cast<LibreOfficeKitTileMode>(_loKitDocument->getTileMode());
-        lock.unlock();
 
         if (!png::encodeBufferToPNG(pixmap.data(), tile.getWidth(), tile.getHeight(), output, mode))
         {
@@ -667,7 +666,6 @@ public:
                      << " (" << renderArea.getWidth() << ", " << renderArea.getHeight() << ") rendered in "
                      << double(timestamp.elapsed())/1000 <<  " ms." << Log::end;
         const auto mode = static_cast<LibreOfficeKitTileMode>(_loKitDocument->getTileMode());
-        lock.unlock();
 
         std::vector<char> output;
         output.reserve(pixmapWidth * pixmapHeight * 4);