net/Socket.hpp | 53 ++++++++++++++++++++---------------------------
net/SslSocket.hpp | 14 ++++++------
net/WebSocketHandler.hpp | 2 -
wsd/AdminModel.cpp | 48 +++++++++++++++++++-----------------------
wsd/AdminModel.hpp | 3 +-
wsd/ClientSession.cpp | 2 -
wsd/ClientSession.hpp | 3 +-
wsd/DocumentBroker.cpp | 38 ++++++++++++++++-----------------
wsd/DocumentBroker.hpp | 3 +-
wsd/LOOLWSD.cpp | 2 -
wsd/TestStubs.cpp | 2 -
11 files changed, 81 insertions(+), 89 deletions(-)
New commits:
commit cb2b788cc77912ce943bb891eebb2299950a0fc2
Author: Jan Holesovsky <ke...@collabora.com>
Date: Wed Apr 5 14:48:49 2017 +0200
assert(isCorrectThread()) -> assertCorrectThread().
assert()'s are no-op in the release builds, but we still want to see
threading
problems in the log at least.
Change-Id: Idb02bb018e8f2d628a57ab570249613ad00bcff2
diff --git a/net/Socket.hpp b/net/Socket.hpp
index a96c5c60..706c8ef2 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -195,20 +195,17 @@ public:
#endif
}
- virtual bool isCorrectThread()
+ /// Asserts in the debug builds, otherwise just logs.
+ virtual void assertCorrectThread()
{
-#if ENABLE_DEBUG
// 0 owner means detached and can be invoked by any thread.
const bool sameThread = (_owner == std::thread::id(0) ||
std::this_thread::get_id() == _owner);
if (!sameThread)
- LOG_WRN("#" << _fd << " Invoked from foreign thread. Expected: 0x"
<< std::hex <<
+ LOG_ERR("#" << _fd << " Invoked from foreign thread. Expected: 0x"
<< std::hex <<
_owner << " but called from 0x" <<
std::this_thread::get_id() << " (" <<
std::dec << Util::getThreadId() << ").");
- return sameThread;
-#else
- return true;
-#endif
+ assert(sameThread);
}
protected:
@@ -293,25 +290,23 @@ public:
}
/// Are we running in either shutdown, or the polling thread.
- bool isCorrectThread() const
+ /// Asserts in the debug builds, otherwise just logs.
+ void assertCorrectThread() const
{
-#if ENABLE_DEBUG
// 0 owner means detached and can be invoked by any thread.
- if (_owner != std::thread::id(0) && std::this_thread::get_id() !=
_owner)
- LOG_WRN("Incorrect thread affinity for " << _name << ". Expected:
0x" << std::hex <<
+ const bool sameThread = (_owner == std::thread::id(0) ||
std::this_thread::get_id() == _owner);
+ if (!sameThread)
+ LOG_ERR("Incorrect thread affinity for " << _name << ". Expected:
0x" << std::hex <<
_owner << " (" << std::dec << Util::getThreadId() << ")
but called from 0x" <<
std::hex << std::this_thread::get_id() << std::dec << ",
stop: " << _stop);
- return _stop || _owner == std::thread::id(0) ||
std::this_thread::get_id() == _owner;
-#else
- return true;
-#endif
+ assert(_stop || sameThread);
}
/// Poll the sockets for available data to read or buffer to write.
void poll(int timeoutMaxMs)
{
- assert(isCorrectThread());
+ assertCorrectThread();
std::chrono::steady_clock::time_point now =
std::chrono::steady_clock::now();
@@ -460,8 +455,8 @@ public:
void releaseSocket(const std::shared_ptr<Socket>& socket)
{
assert(socket);
- assert(isCorrectThread());
- assert(socket->isCorrectThread());
+ assertCorrectThread();
+ socket->assertCorrectThread();
auto it = std::find(_pollSockets.begin(), _pollSockets.end(), socket);
assert(it != _pollSockets.end());
@@ -472,7 +467,7 @@ public:
size_t getSocketCount() const
{
- assert(isCorrectThread());
+ assertCorrectThread();
return _pollSockets.size();
}
@@ -621,10 +616,8 @@ public:
if (!_closed)
{
- if (isCorrectThread())
- _socketHandler->onDisconnect();
- else
- LOG_WRN("#" << getFD() << " not properly shutdown.
onDisconnect not called.");
+ assertCorrectThread();
+ _socketHandler->onDisconnect();
}
if (!_shutdownSignalled)
@@ -651,7 +644,7 @@ public:
int &timeoutMaxMs) override
{
// cf. SslSocket::getPollEvents
- assert(isCorrectThread());
+ assertCorrectThread();
int events = _socketHandler->getPollEvents(now, timeoutMaxMs);
if (!_outBuffer.empty() || _shutdownSignalled)
events |= POLLOUT;
@@ -661,7 +654,7 @@ public:
/// Send data to the socket peer.
void send(const char* data, const int len, const bool flush = true)
{
- assert(isCorrectThread());
+ assertCorrectThread();
if (data != nullptr && len > 0)
{
_outBuffer.insert(_outBuffer.end(), data, data + len);
@@ -689,7 +682,7 @@ public:
/// Return false iff the socket is closed.
virtual bool readIncomingData()
{
- assert(isCorrectThread());
+ assertCorrectThread();
// SSL decodes blocks of 16Kb, so for efficiency we use the same.
char buf[16 * 1024];
@@ -743,7 +736,7 @@ protected:
HandleResult handlePoll(std::chrono::steady_clock::time_point now,
const int events) override
{
- assert(isCorrectThread());
+ assertCorrectThread();
_socketHandler->checkTimeout(now);
@@ -811,7 +804,7 @@ protected:
/// Override to write data out to socket.
virtual void writeOutgoingData()
{
- assert(isCorrectThread());
+ assertCorrectThread();
assert(!_outBuffer.empty());
do
{
@@ -849,14 +842,14 @@ protected:
/// Override to handle reading of socket data differently.
virtual int readData(char* buf, int len)
{
- assert(isCorrectThread());
+ assertCorrectThread();
return ::read(getFD(), buf, len);
}
/// Override to handle writing data to socket differently.
virtual int writeData(const char* buf, const int len)
{
- assert(isCorrectThread());
+ assertCorrectThread();
return ::write(getFD(), buf, len);
}
diff --git a/net/SslSocket.hpp b/net/SslSocket.hpp
index 52292c58..2e495883 100644
--- a/net/SslSocket.hpp
+++ b/net/SslSocket.hpp
@@ -77,7 +77,7 @@ public:
bool readIncomingData() override
{
- assert(isCorrectThread());
+ assertCorrectThread();
const int rc = doHandshake();
if (rc <= 0)
@@ -89,7 +89,7 @@ public:
void writeOutgoingData() override
{
- assert(isCorrectThread());
+ assertCorrectThread();
const int rc = doHandshake();
if (rc <= 0)
@@ -103,14 +103,14 @@ public:
virtual int readData(char* buf, int len) override
{
- assert(isCorrectThread());
+ assertCorrectThread();
return handleSslState(SSL_read(_ssl, buf, len));
}
virtual int writeData(const char* buf, const int len) override
{
- assert(isCorrectThread());
+ assertCorrectThread();
assert (len > 0); // Never write 0 bytes.
return handleSslState(SSL_write(_ssl, buf, len));
@@ -119,7 +119,7 @@ public:
int getPollEvents(std::chrono::steady_clock::time_point now,
int & timeoutMaxMs) override
{
- assert(isCorrectThread());
+ assertCorrectThread();
int events = _socketHandler->getPollEvents(now, timeoutMaxMs);
if (_sslWantsTo == SslWantsTo::Read)
@@ -151,7 +151,7 @@ private:
int doHandshake()
{
- assert(isCorrectThread());
+ assertCorrectThread();
if (_doHandshake)
{
@@ -179,7 +179,7 @@ private:
/// Handles the state of SSL after read or write.
int handleSslState(const int rc)
{
- assert(isCorrectThread());
+ assertCorrectThread();
if (rc > 0)
{
diff --git a/net/WebSocketHandler.hpp b/net/WebSocketHandler.hpp
index 8d689de3..66adbc0a 100644
--- a/net/WebSocketHandler.hpp
+++ b/net/WebSocketHandler.hpp
@@ -330,7 +330,7 @@ protected:
if (!socket || data == nullptr || len == 0)
return -1;
- assert(socket->isCorrectThread());
+ socket->assertCorrectThread();
std::vector<char>& out = socket->_outBuffer;
out.push_back(flags);
diff --git a/wsd/AdminModel.cpp b/wsd/AdminModel.cpp
index 1f3e6dcc..a838cbf3 100644
--- a/wsd/AdminModel.cpp
+++ b/wsd/AdminModel.cpp
@@ -94,25 +94,21 @@ void Subscriber::unsubscribe(const std::string& command)
_subscriptions.erase(command);
}
-bool AdminModel::isCorrectThread() const
+void AdminModel::assertCorrectThread() const
{
-#if ENABLE_DEBUG
// FIXME: share this code [!]
const bool sameThread = std::this_thread::get_id() == _owner;
if (!sameThread)
- LOG_WRN("Admin command invoked from foreign thread. Expected: 0x" <<
std::hex <<
+ LOG_ERR("Admin command invoked from foreign thread. Expected: 0x" <<
std::hex <<
_owner << " but called from 0x" << std::this_thread::get_id() << " ("
<<
std::dec << Util::getThreadId() << ").");
- return sameThread;
-#else
- return true;
-#endif
+ assert(sameThread);
}
std::string AdminModel::query(const std::string& command)
{
- assert (isCorrectThread());
+ assertCorrectThread();
const auto token = LOOLProtocol::getFirstToken(command);
if (token == "documents")
@@ -150,7 +146,7 @@ std::string AdminModel::query(const std::string& command)
/// Returns memory consumed by all active loolkit processes
unsigned AdminModel::getKitsMemoryUsage()
{
- assert (isCorrectThread());
+ assertCorrectThread();
unsigned totalMem = 0;
unsigned docs = 0;
@@ -178,7 +174,7 @@ unsigned AdminModel::getKitsMemoryUsage()
void AdminModel::subscribe(int sessionId, const
std::weak_ptr<WebSocketHandler>& ws)
{
- assert (isCorrectThread());
+ assertCorrectThread();
const auto ret = _subscribers.emplace(sessionId, Subscriber(sessionId,
ws));
if (!ret.second)
@@ -189,7 +185,7 @@ void AdminModel::subscribe(int sessionId, const
std::weak_ptr<WebSocketHandler>&
void AdminModel::subscribe(int sessionId, const std::string& command)
{
- assert (isCorrectThread());
+ assertCorrectThread();
auto subscriber = _subscribers.find(sessionId);
if (subscriber != _subscribers.end())
@@ -200,7 +196,7 @@ void AdminModel::subscribe(int sessionId, const
std::string& command)
void AdminModel::unsubscribe(int sessionId, const std::string& command)
{
- assert (isCorrectThread());
+ assertCorrectThread();
auto subscriber = _subscribers.find(sessionId);
if (subscriber != _subscribers.end())
@@ -209,7 +205,7 @@ void AdminModel::unsubscribe(int sessionId, const
std::string& command)
void AdminModel::addMemStats(unsigned memUsage)
{
- assert (isCorrectThread());
+ assertCorrectThread();
_memStats.push_back(memUsage);
if (_memStats.size() > _memStatsSize)
@@ -220,7 +216,7 @@ void AdminModel::addMemStats(unsigned memUsage)
void AdminModel::addCpuStats(unsigned cpuUsage)
{
- assert (isCorrectThread());
+ assertCorrectThread();
_cpuStats.push_back(cpuUsage);
if (_cpuStats.size() > _cpuStatsSize)
@@ -231,7 +227,7 @@ void AdminModel::addCpuStats(unsigned cpuUsage)
void AdminModel::setCpuStatsSize(unsigned size)
{
- assert (isCorrectThread());
+ assertCorrectThread();
int wasteValuesLen = _cpuStats.size() - size;
while (wasteValuesLen-- > 0)
@@ -245,7 +241,7 @@ void AdminModel::setCpuStatsSize(unsigned size)
void AdminModel::setMemStatsSize(unsigned size)
{
- assert (isCorrectThread());
+ assertCorrectThread();
int wasteValuesLen = _memStats.size() - size;
while (wasteValuesLen-- > 0)
@@ -259,7 +255,7 @@ void AdminModel::setMemStatsSize(unsigned size)
void AdminModel::notify(const std::string& message)
{
- assert (isCorrectThread());
+ assertCorrectThread();
if (!_subscribers.empty())
{
@@ -281,7 +277,7 @@ void AdminModel::notify(const std::string& message)
void AdminModel::addDocument(const std::string& docKey, Poco::Process::PID pid,
const std::string& filename, const std::string&
sessionId)
{
- assert (isCorrectThread());
+ assertCorrectThread();
const auto ret = _documents.emplace(docKey, Document(docKey, pid,
filename));
ret.first->second.addView(sessionId);
@@ -321,7 +317,7 @@ void AdminModel::addDocument(const std::string& docKey,
Poco::Process::PID pid,
void AdminModel::removeDocument(const std::string& docKey, const std::string&
sessionId)
{
- assert (isCorrectThread());
+ assertCorrectThread();
auto docIt = _documents.find(docKey);
if (docIt != _documents.end() && !docIt->second.isExpired())
@@ -345,7 +341,7 @@ void AdminModel::removeDocument(const std::string& docKey,
const std::string& se
void AdminModel::removeDocument(const std::string& docKey)
{
- assert (isCorrectThread());
+ assertCorrectThread();
auto docIt = _documents.find(docKey);
if (docIt != _documents.end())
@@ -368,7 +364,7 @@ void AdminModel::removeDocument(const std::string& docKey)
std::string AdminModel::getMemStats()
{
- assert (isCorrectThread());
+ assertCorrectThread();
std::ostringstream oss;
for (const auto& i: _memStats)
@@ -381,7 +377,7 @@ std::string AdminModel::getMemStats()
std::string AdminModel::getCpuStats()
{
- assert (isCorrectThread());
+ assertCorrectThread();
std::ostringstream oss;
for (const auto& i: _cpuStats)
@@ -394,7 +390,7 @@ std::string AdminModel::getCpuStats()
unsigned AdminModel::getTotalActiveViews()
{
- assert (isCorrectThread());
+ assertCorrectThread();
unsigned numTotalViews = 0;
for (const auto& it: _documents)
@@ -410,7 +406,7 @@ unsigned AdminModel::getTotalActiveViews()
std::string AdminModel::getDocuments() const
{
- assert (isCorrectThread());
+ assertCorrectThread();
std::ostringstream oss;
for (const auto& it: _documents)
@@ -433,7 +429,7 @@ std::string AdminModel::getDocuments() const
void AdminModel::updateLastActivityTime(const std::string& docKey)
{
- assert (isCorrectThread());
+ assertCorrectThread();
auto docIt = _documents.find(docKey);
if (docIt != _documents.end())
@@ -456,7 +452,7 @@ bool Document::updateMemoryDirty(int dirty)
void AdminModel::updateMemoryDirty(const std::string& docKey, int dirty)
{
- assert (isCorrectThread());
+ assertCorrectThread();
auto docIt = _documents.find(docKey);
if (docIt != _documents.end() &&
diff --git a/wsd/AdminModel.hpp b/wsd/AdminModel.hpp
index 57bd702b..9a7ee8c9 100644
--- a/wsd/AdminModel.hpp
+++ b/wsd/AdminModel.hpp
@@ -153,7 +153,8 @@ public:
void setThreadOwner(const std::thread::id &id) { _owner = id; }
/// In debug mode check that code is running in the correct thread.
- bool isCorrectThread() const;
+ /// Asserts in the debug builds, otherwise just logs.
+ void assertCorrectThread() const;
std::string query(const std::string& command);
diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index af5a653b..8eca9d80 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -730,7 +730,7 @@ void ClientSession::onDisconnect()
const auto docBroker = getDocumentBroker();
LOG_CHECK_RET(docBroker && "Null DocumentBroker instance", );
- assert(docBroker->isCorrectThread());
+ docBroker->assertCorrectThread();
const auto docKey = docBroker->getDocKey();
try
diff --git a/wsd/ClientSession.hpp b/wsd/ClientSession.hpp
index f3cecee9..3181b0c8 100644
--- a/wsd/ClientSession.hpp
+++ b/wsd/ClientSession.hpp
@@ -68,7 +68,8 @@ public:
{
const auto docBroker = _docBroker.lock();
// If in the correct thread - no need for wakeups.
- assert (!docBroker || docBroker->isCorrectThread());
+ if (docBroker)
+ docBroker->assertCorrectThread();
LOG_TRC(getName() << " enqueueing client message " << data->id());
_senderQueue.enqueue(data);
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index cafbd424..5a4de9e2 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -172,9 +172,9 @@ void DocumentBroker::startThread()
_poll->startThread();
}
-bool DocumentBroker::isCorrectThread()
+void DocumentBroker::assertCorrectThread()
{
- return _poll->isCorrectThread();
+ _poll->assertCorrectThread();
}
// The inner heart of the DocumentBroker - our poll loop.
@@ -281,7 +281,7 @@ bool DocumentBroker::isAlive() const
DocumentBroker::~DocumentBroker()
{
- assert(isCorrectThread());
+ assertCorrectThread();
Admin::instance().rmDoc(_docKey);
@@ -308,7 +308,7 @@ void DocumentBroker::joinThread()
bool DocumentBroker::load(const std::shared_ptr<ClientSession>& session, const
std::string& jailId)
{
- assert(isCorrectThread());
+ assertCorrectThread();
const std::string sessionId = session->getId();
@@ -503,7 +503,7 @@ bool DocumentBroker::load(const
std::shared_ptr<ClientSession>& session, const s
bool DocumentBroker::saveToStorage(const std::string& sessionId,
bool success, const std::string& result)
{
- assert(isCorrectThread());
+ assertCorrectThread();
const bool res = saveToStorageInternal(sessionId, success, result);
@@ -527,7 +527,7 @@ bool DocumentBroker::saveToStorage(const std::string&
sessionId,
bool DocumentBroker::saveToStorageInternal(const std::string& sessionId,
bool success, const std::string&
result)
{
- assert(isCorrectThread());
+ assertCorrectThread();
// If save requested, but core didn't save because document was unmodified
// notify the waiting thread, if any.
@@ -636,7 +636,7 @@ void DocumentBroker::setLoaded()
bool DocumentBroker::autoSave(const bool force)
{
- assert(isCorrectThread());
+ assertCorrectThread();
if (_sessions.empty() || _storage == nullptr || !_isLoaded ||
!_childProcess->isAlive() || (!_isModified && !force))
@@ -677,7 +677,7 @@ bool DocumentBroker::autoSave(const bool force)
bool DocumentBroker::sendUnoSave(const bool dontSaveIfUnmodified)
{
- assert(isCorrectThread());
+ assertCorrectThread();
LOG_INF("Autosave triggered for doc [" << _docKey << "].");
@@ -750,7 +750,7 @@ std::string DocumentBroker::getJailRoot() const
size_t DocumentBroker::addSession(const std::shared_ptr<ClientSession>&
session)
{
- assert(isCorrectThread());
+ assertCorrectThread();
try
{
@@ -803,7 +803,7 @@ size_t DocumentBroker::addSession(const
std::shared_ptr<ClientSession>& session)
size_t DocumentBroker::removeSession(const std::string& id, bool destroyIfLast)
{
- assert(isCorrectThread());
+ assertCorrectThread();
if (destroyIfLast)
destroyIfLastEditor(id);
@@ -826,7 +826,7 @@ size_t DocumentBroker::removeSession(const std::string& id,
bool destroyIfLast)
size_t DocumentBroker::removeSessionInternal(const std::string& id)
{
- assert(isCorrectThread());
+ assertCorrectThread();
try
{
Admin::instance().rmDoc(_docKey, id);
@@ -880,7 +880,7 @@ void DocumentBroker::addSocketToPoll(const
std::shared_ptr<Socket>& socket)
void DocumentBroker::alertAllUsers(const std::string& msg)
{
- assert(isCorrectThread());
+ assertCorrectThread();
auto payload = std::make_shared<Message>(msg, Message::Dir::Out);
@@ -952,7 +952,7 @@ void DocumentBroker::invalidateTiles(const std::string&
tiles)
void DocumentBroker::handleTileRequest(TileDesc& tile,
const std::shared_ptr<ClientSession>&
session)
{
- assert(isCorrectThread());
+ assertCorrectThread();
std::unique_lock<std::mutex> lock(_mutex);
tile.setVersion(++_tileVersion);
@@ -1142,7 +1142,7 @@ void DocumentBroker::handleTileCombinedResponse(const
std::vector<char>& payload
void DocumentBroker::destroyIfLastEditor(const std::string& id)
{
- assert(isCorrectThread());
+ assertCorrectThread();
const auto currentSession = _sessions.find(id);
if (currentSession == _sessions.end())
@@ -1182,7 +1182,7 @@ void DocumentBroker::setModified(const bool value)
bool DocumentBroker::forwardToChild(const std::string& viewId, const
std::string& message)
{
- assert(isCorrectThread());
+ assertCorrectThread();
LOG_TRC("Forwarding payload to child [" << viewId << "]: " << message);
@@ -1214,7 +1214,7 @@ bool DocumentBroker::forwardToChild(const std::string&
viewId, const std::string
bool DocumentBroker::forwardToClient(const std::shared_ptr<Message>& payload)
{
- assert(isCorrectThread());
+ assertCorrectThread();
const std::string& msg = payload->abbr();
const std::string& prefix = payload->forwardToken();
@@ -1258,7 +1258,7 @@ bool DocumentBroker::forwardToClient(const
std::shared_ptr<Message>& payload)
void DocumentBroker::childSocketTerminated()
{
- assert(isCorrectThread());
+ assertCorrectThread();
if (!_childProcess->isAlive())
{
@@ -1282,7 +1282,7 @@ void DocumentBroker::childSocketTerminated()
void DocumentBroker::terminateChild(const std::string& closeReason, const bool
rude)
{
- assert(isCorrectThread());
+ assertCorrectThread();
LOG_INF("Terminating doc [" << _docKey << "].");
@@ -1323,7 +1323,7 @@ void DocumentBroker::terminateChild(const std::string&
closeReason, const bool r
void DocumentBroker::closeDocument(const std::string& reason)
{
- assert(isCorrectThread());
+ assertCorrectThread();
LOG_DBG("Closing DocumentBroker for docKey [" << _docKey << "] with
reason: " << reason);
terminateChild(reason, true);
diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp
index 168734cc..0af441ff 100644
--- a/wsd/DocumentBroker.hpp
+++ b/wsd/DocumentBroker.hpp
@@ -259,7 +259,8 @@ public:
}
/// Are we running in either shutdown, or the polling thread.
- bool isCorrectThread();
+ /// Asserts in the debug builds, otherwise just logs.
+ void assertCorrectThread();
/// Pretty print internal state to a stream.
void dumpState(std::ostream& os);
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 763faa1c..196a1c5d 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -1374,7 +1374,7 @@ private:
if (docBroker)
{
auto lock = docBroker->getLock();
- assert(docBroker->isCorrectThread());
+ docBroker->assertCorrectThread();
docBroker->stop();
}
}
diff --git a/wsd/TestStubs.cpp b/wsd/TestStubs.cpp
index 7972aec5..b9ff5a58 100644
--- a/wsd/TestStubs.cpp
+++ b/wsd/TestStubs.cpp
@@ -15,6 +15,6 @@
#include "DocumentBroker.hpp"
-bool DocumentBroker::isCorrectThread() { return true; }
+void DocumentBroker::assertCorrectThread() {}
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
