[Libreoffice-commits] online.git: 2 commits - kit/Kit.cpp wsd/DocumentBroker.cpp wsd/TileCache.cpp wsd/TileCache.hpp
Michael Meeks
michael.meeks at collabora.com
Thu Jun 22 17:41:34 UTC 2017
kit/Kit.cpp | 33 +++++++++++++++------------------
wsd/DocumentBroker.cpp | 1 +
wsd/TileCache.cpp | 25 +++++++++++++++++--------
wsd/TileCache.hpp | 11 +++++------
4 files changed, 38 insertions(+), 32 deletions(-)
New commits:
commit d43589c34355694b0ffbc3b129fcaba22bcdbb22
Author: Michael Meeks <michael.meeks at collabora.com>
Date: Thu Jun 22 13:58:50 2017 +0100
Replace now un-used locking with thread affinity assertions.
Change-Id: I8c08d1618404740e9dc1d5ff2cb7d9d460ca2be5
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 2e37ccf7..f1785de5 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -558,6 +558,7 @@ bool DocumentBroker::load(const std::shared_ptr<ClientSession>& session, const s
// Use the local temp file's timestamp.
_lastFileModifiedTime = Poco::File(_storage->getRootFilePath()).getLastModified();
_tileCache.reset(new TileCache(_storage->getUri(), _lastFileModifiedTime, _cacheRoot));
+ _tileCache->setThreadOwner(std::this_thread::get_id());
}
LOOLWSD::dumpNewSessionTrace(getJailId(), sessionId, _uriOrig, _storage->getRootFilePath());
diff --git a/wsd/TileCache.cpp b/wsd/TileCache.cpp
index ff30710f..ab3070d8 100644
--- a/wsd/TileCache.cpp
+++ b/wsd/TileCache.cpp
@@ -17,7 +17,6 @@
#include <fstream>
#include <iostream>
#include <memory>
-#include <mutex>
#include <sstream>
#include <string>
#include <vector>
@@ -73,6 +72,7 @@ TileCache::TileCache(const std::string& docURL,
TileCache::~TileCache()
{
+ _owner = std::thread::id(0);
LOG_INF("~TileCache dtor for uri [" << _docURL << "].");
}
@@ -114,7 +114,7 @@ std::shared_ptr<TileCache::TileBeingRendered> TileCache::findTileBeingRendered(c
{
const std::string cachedName = cacheFileName(tileDesc);
- Util::assertIsLocked(_tilesBeingRenderedMutex);
+ assertCorrectThread();
const auto tile = _tilesBeingRendered.find(cachedName);
return tile != _tilesBeingRendered.end() ? tile->second : nullptr;
@@ -124,7 +124,7 @@ void TileCache::forgetTileBeingRendered(const TileDesc& tile)
{
const std::string cachedName = cacheFileName(tile);
- Util::assertIsLocked(_tilesBeingRenderedMutex);
+ assertCorrectThread();
_tilesBeingRendered.erase(cachedName);
}
@@ -149,7 +149,7 @@ std::unique_ptr<std::fstream> TileCache::lookupTile(const TileDesc& tile)
void TileCache::saveTileAndNotify(const TileDesc& tile, const char *data, const size_t size)
{
- std::unique_lock<std::mutex> lock(_tilesBeingRenderedMutex);
+ assertCorrectThread();
std::shared_ptr<TileBeingRendered> tileBeingRendered = findTileBeingRendered(tile);
@@ -321,8 +321,7 @@ void TileCache::invalidateTiles(int part, int x, int y, int width, int height)
File dir(_cacheDir);
- std::unique_lock<std::mutex> lock(_cacheMutex);
- std::unique_lock<std::mutex> lockSubscribers(_tilesBeingRenderedMutex);
+ assertCorrectThread();
if (dir.exists() && dir.isDirectory())
{
@@ -446,7 +445,7 @@ void TileCache::subscribeToTileRendering(const TileDesc& tile, const std::shared
oss << '(' << tile.getPart() << ',' << tile.getTilePosX() << ',' << tile.getTilePosY() << ')';
const auto name = oss.str();
- std::unique_lock<std::mutex> lock(_tilesBeingRenderedMutex);
+ assertCorrectThread();
std::shared_ptr<TileBeingRendered> tileBeingRendered = findTileBeingRendered(tile);
@@ -493,7 +492,7 @@ std::string TileCache::cancelTiles(const std::shared_ptr<ClientSession> &subscri
assert(subscriber && "cancelTiles expects valid subscriber");
LOG_TRC("Cancelling tiles for " << subscriber->getName());
- std::unique_lock<std::mutex> lock(_tilesBeingRenderedMutex);
+ assertCorrectThread();
const auto sub = subscriber.get();
@@ -534,4 +533,14 @@ std::string TileCache::cancelTiles(const std::shared_ptr<ClientSession> &subscri
return canceltiles.empty() ? canceltiles : "canceltiles " + canceltiles;
}
+void TileCache::assertCorrectThread()
+{
+ const bool correctThread = _owner == std::thread::id(0) || std::this_thread::get_id() == _owner;
+ if (!correctThread)
+ LOG_ERR("TileCache method invoked from foreign thread. Expected: 0x" << std::hex <<
+ _owner << " but called from 0x" << std::this_thread::get_id() << " (" <<
+ std::dec << Util::getThreadId() << ").");
+ assert (correctThread);
+}
+
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/wsd/TileCache.hpp b/wsd/TileCache.hpp
index 9f86cc72..5f8e71cf 100644
--- a/wsd/TileCache.hpp
+++ b/wsd/TileCache.hpp
@@ -13,7 +13,7 @@
#include <iosfwd>
#include <map>
#include <memory>
-#include <mutex>
+#include <thread>
#include <string>
#include <Poco/Timestamp.h>
@@ -75,10 +75,11 @@ public:
/// Store the timestamp to modtime.txt.
void saveLastModified(const Poco::Timestamp& timestamp);
- std::unique_lock<std::mutex> getTilesBeingRenderedLock() { return std::unique_lock<std::mutex>(_tilesBeingRenderedMutex); }
-
void forgetTileBeingRendered(const TileDesc& tile);
+ void setThreadOwner(const std::thread::id &id) { _owner = id; }
+ void assertCorrectThread();
+
private:
void invalidateTiles(int part, int x, int y, int width, int height);
@@ -98,9 +99,7 @@ private:
const std::string _cacheDir;
- std::mutex _cacheMutex;
-
- mutable std::mutex _tilesBeingRenderedMutex;
+ std::thread::id _owner;
std::map<std::string, std::shared_ptr<TileBeingRendered> > _tilesBeingRendered;
};
commit 3f59438f629aee8eacef48058ce16ad8a5787868
Author: Michael Meeks <michael.meeks at collabora.com>
Date: Wed Jun 21 15:03:54 2017 +0100
Simplify renderid generation, and re-order.
Change-Id: I3f934fdf16b1a9cf4ea00571616d9b5568535707
diff --git a/kit/Kit.cpp b/kit/Kit.cpp
index be07bbb8..f811be5d 100644
--- a/kit/Kit.cpp
+++ b/kit/Kit.cpp
@@ -105,6 +105,12 @@ class Document;
static std::shared_ptr<Document> document;
static LokHookFunction2* initFunction = nullptr;
+#if ENABLE_DEBUG
+# define ADD_DEBUG_RENDERID(s) ((s)+ " renderid=" + Util::UniqueId())
+#else
+# define ADD_DEBUG_RENDERID(s) (s)
+#endif
+
namespace
{
#ifndef BUILDING_TESTS
@@ -685,26 +691,21 @@ public:
tile.setWireId(wid);
+ if (hash != 0 && tile.getOldWireId() == wid)
+ {
+ // The tile content is identical to what the client already has, so skip it
+ LOG_TRC("Match oldWireId==wid (" << wid << " for hash " << hash << "); unchanged");
+ return;
+ }
+
// Send back the request with all optional parameters given in the request.
- const auto tileMsg = tile.serialize("tile:");
-#if ENABLE_DEBUG
- const std::string response = tileMsg + " renderid=" + Util::UniqueId() + "\n";
-#else
- const std::string response = tileMsg + "\n";
-#endif
+ std::string response = ADD_DEBUG_RENDERID(tile.serialize("tile:")) + "\n";
std::vector<char> output;
output.reserve(response.size() + pixmapDataSize);
output.resize(response.size());
std::memcpy(output.data(), response.data(), response.size());
- if (hash != 0 && tile.getOldWireId() == wid)
- {
- // The tile content is identical to what the client already has, so skip it
- LOG_TRC("Match oldWireId==wid (" << wid << " for hash " << hash << "), skipping");
- return;
- }
-
if (!_pngCache.encodeBufferToPNG(pixmap.data(), tile.getWidth(), tile.getHeight(), output, mode, hash, wid))
{
//FIXME: Return error.
@@ -826,11 +827,7 @@ public:
renderArea.getWidth() << ", " << renderArea.getHeight() << ") " <<
" took " << (elapsed/1000.) << " ms (including the paintTile).");
-#if ENABLE_DEBUG
- const auto tileMsg = tileCombined.serialize("tilecombine:") + " renderid=" + Util::UniqueId() + "\n";
-#else
- const auto tileMsg = tileCombined.serialize("tilecombine:") + "\n";
-#endif
+ const auto tileMsg = ADD_DEBUG_RENDERID(tileCombined.serialize("tilecombine:")) + "\n";
LOG_TRC("Sending back painted tiles for " << tileMsg);
std::vector<char> response;
More information about the Libreoffice-commits
mailing list