Makefile.am | 3 ++- common/Log.cpp | 21 ++++++++++++++++----- common/Log.hpp | 4 ++-- common/Session.cpp | 4 ++-- common/Util.cpp | 21 +++++++++++++++++++++ common/Util.hpp | 4 ++++ kit/ForKit.cpp | 2 ++ net/ServerSocket.hpp | 6 +++--- net/Socket.hpp | 6 +++--- net/SslSocket.hpp | 8 ++------ test/Makefile.am | 4 ++-- test/UnitPrefork.cpp | 5 +++-- wsd/DocumentBroker.cpp | 4 ++-- wsd/LOOLWSD.cpp | 12 ++++++------ wsd/LOOLWSD.hpp | 1 + wsd/README | 3 +++ wsd/SenderQueue.hpp | 8 ++++---- wsd/Storage.cpp | 6 ++++-- wsd/TileCache.cpp | 6 +++--- 19 files changed, 85 insertions(+), 43 deletions(-)
New commits: commit 4986ac9027a00a2b87f3bebacfb53d0086ae93a9 Author: Michael Meeks <michael.me...@collabora.com> Date: Thu Mar 30 21:55:33 2017 +0100 Re-enable UnitPrefork. diff --git a/test/Makefile.am b/test/Makefile.am index c55c306f..9008efa1 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -74,7 +74,7 @@ check-local: ./run_unit.sh --log-file test.log --trs-file test.trs # FIXME 2: unit-oob.la fails with symbol undefined: # UnitWSD::testHandleRequest(UnitWSD::TestRequest, UnitHTTPServerRequest&, UnitHTTPServerResponse&) , -TESTS = # unit-prefork.la # unit-tilecache.la unit-storage.la unit-timeout.la unit-admin.la unit-minsocketbuffersize.la +TESTS = unit-prefork.la # unit-tilecache.la unit-storage.la unit-timeout.la unit-admin.la unit-minsocketbuffersize.la else TESTS = ${top_builddir}/test/test endif diff --git a/test/UnitPrefork.cpp b/test/UnitPrefork.cpp index 841fbd37..90852a69 100644 --- a/test/UnitPrefork.cpp +++ b/test/UnitPrefork.cpp @@ -27,9 +27,10 @@ public: setTimeout(60 * 1000); } - virtual void configure(Poco::Util::LayeredConfiguration& /* config */) override + virtual void configure(Poco::Util::LayeredConfiguration& config) override { - LOOLWSD::NumPreSpawnedChildren = NumToPrefork; + config.setInt("num_prespawn_children", NumToPrefork); + UnitWSD::configure(config); } virtual void newChild(WebSocketHandler &) override commit f4d5559b9844d0f97815bf36cfad48b1fb2bc293 Author: Michael Meeks <michael.me...@collabora.com> Date: Thu Mar 30 21:55:17 2017 +0100 Improve debugging information. diff --git a/kit/ForKit.cpp b/kit/ForKit.cpp index 62c34c57..59da5d64 100644 --- a/kit/ForKit.cpp +++ b/kit/ForKit.cpp @@ -341,6 +341,8 @@ int main(int argc, char** argv) SigUtil::setTerminationSignals(); #endif + Util::setThreadName("forkit"); + // Initialization const bool logToFile = std::getenv("LOOL_LOGFILE"); const char* logFilename = std::getenv("LOOL_LOGFILENAME"); diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp index f8c8c5d9..9a5920d2 100644 --- a/wsd/LOOLWSD.cpp +++ b/wsd/LOOLWSD.cpp @@ -347,6 +347,8 @@ static int rebalanceChildren(int balance) { Util::assertIsLocked(NewChildrenMutex); + LOG_WRN("rebalance children to " << balance); + // Do the cleanup first. const bool rebalance = cleanupChildren(); @@ -2193,7 +2195,8 @@ public: << " TerminationFlag: " << TerminationFlag << "\n" << " isShuttingDown: " << ShutdownRequestFlag << "\n" << " NewChildren: " << NewChildren.size() << "\n" - << " OutstandingForks: " << OutstandingForks << "\n"; + << " OutstandingForks: " << OutstandingForks << "\n" + << " NumPreSpawnedChildren: " << LOOLWSD::NumPreSpawnedChildren << "\n"; os << "Server poll:\n"; _acceptPoll.dumpState(os); commit d7172e3ee1241902055c0a9358b43ebbdcb0e888 Author: Michael Meeks <michael.me...@collabora.com> Date: Thu Mar 30 21:54:40 2017 +0100 Fix unit test delay time check. diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp index ff12b061..f8c8c5d9 100644 --- a/wsd/LOOLWSD.cpp +++ b/wsd/LOOLWSD.cpp @@ -2434,7 +2434,7 @@ int LOOLWSD::innerMain() // Unit test timeout if (std::chrono::duration_cast<std::chrono::milliseconds>( - std::chrono::steady_clock::now() - startStamp).count() < + std::chrono::steady_clock::now() - startStamp).count() > UnitWSD::get().getTimeoutMilliSeconds()) UnitWSD::get().timeout(); commit 35917b140f058dd651b64e3d9660ba903561ce25 Author: Michael Meeks <michael.me...@collabora.com> Date: Thu Mar 30 18:14:40 2017 +0100 Cleanup prctl / gettid system-call thrash on logging. Makes the strace look much prettier. diff --git a/Makefile.am b/Makefile.am index 80aed7c6..b7d066f4 100644 --- a/Makefile.am +++ b/Makefile.am @@ -131,7 +131,8 @@ looltool_SOURCES = tools/Tool.cpp loolstress_CPPFLAGS = -DTDOC=\"$(abs_top_srcdir)/test/data\" ${include_paths} loolstress_SOURCES = tools/Stress.cpp \ common/Protocol.cpp \ - common/Log.cpp + common/Log.cpp \ + common/Util.cpp wsd_headers = wsd/Admin.hpp \ wsd/AdminModel.hpp \ diff --git a/common/Log.cpp b/common/Log.cpp index eaf101f1..71b5a5b6 100644 --- a/common/Log.cpp +++ b/common/Log.cpp @@ -31,6 +31,7 @@ #include <Poco/Timestamp.h> #include "Log.hpp" +#include "Util.hpp" static char LogPrefix[256] = { '\0' }; @@ -77,12 +78,22 @@ namespace Log } } - char* prefix(char* buffer, const char* level, const long osTid) + char* prefix(char* buffer, const char* level, bool sigSafe) { + long osTid; char procName[32]; - if (prctl(PR_GET_NAME, reinterpret_cast<unsigned long>(procName), 0, 0, 0) != 0) + const char *threadName = procName; + if (sigSafe) { - strncpy(procName, "<noid>", sizeof(procName) - 1); + osTid = syscall(SYS_gettid); + + if (prctl(PR_GET_NAME, reinterpret_cast<unsigned long>(procName), 0, 0, 0) != 0) + strncpy(procName, "<noid>", sizeof(procName) - 1); + } + else + { + osTid = Util::getThreadId(); + threadName = Util::getThreadName(); } Poco::DateTime time; @@ -91,14 +102,14 @@ namespace Log osTid, time.hour(), time.minute(), time.second(), time.millisecond() * 1000 + time.microsecond(), - procName, level); + threadName, level); return buffer; } void signalLogPrefix() { char buffer[1024]; - prefix(buffer, "SIG", syscall(SYS_gettid)); + prefix(buffer, "SIG", true); signalLog(buffer); } diff --git a/common/Log.hpp b/common/Log.hpp index 2ad4e5dd..3f64303a 100644 --- a/common/Log.hpp +++ b/common/Log.hpp @@ -28,7 +28,7 @@ namespace Log std::map<std::string, std::string> config); Poco::Logger& logger(); - char* prefix(char* buffer, const char* level, const long osTid); + char* prefix(char* buffer, const char* level, bool sigSafe); void trace(const std::string& msg); void debug(const std::string& msg); @@ -175,7 +175,7 @@ namespace Log } } -#define LOG_BODY_(PRIO, LVL, X) Poco::Message m_(l_.name(), "", Poco::Message::PRIO_##PRIO); char b_[1024]; std::ostringstream oss_(Log::prefix(b_, LVL, syscall(SYS_gettid)), std::ostringstream::ate); oss_ << std::boolalpha << X << "| " << __FILE__ << ':' << __LINE__; m_.setText(oss_.str()); l_.log(m_); +#define LOG_BODY_(PRIO, LVL, X) Poco::Message m_(l_.name(), "", Poco::Message::PRIO_##PRIO); char b_[1024]; std::ostringstream oss_(Log::prefix(b_, LVL, false), std::ostringstream::ate); oss_ << std::boolalpha << X << "| " << __FILE__ << ':' << __LINE__; m_.setText(oss_.str()); l_.log(m_); #define LOG_TRC(X) do { auto& l_ = Log::logger(); if (l_.trace()) { LOG_BODY_(TRACE, "TRC", X); } } while (false) #define LOG_DBG(X) do { auto& l_ = Log::logger(); if (l_.debug()) { LOG_BODY_(DEBUG, "DBG", X); } } while (false) #define LOG_INF(X) do { auto& l_ = Log::logger(); if (l_.information()) { LOG_BODY_(INFORMATION, "INF", X); } } while (false) diff --git a/common/Util.cpp b/common/Util.cpp index 16335fe3..70987c3b 100644 --- a/common/Util.cpp +++ b/common/Util.cpp @@ -258,8 +258,12 @@ namespace Util return replace(r, "\n", " / "); } + static __thread char ThreadName[32]; + void setThreadName(const std::string& s) { + strncpy(ThreadName, s.c_str(), 31); + ThreadName[31] = '\0'; if (prctl(PR_SET_NAME, reinterpret_cast<unsigned long>(s.c_str()), 0, 0, 0) != 0) { LOG_SYS("Cannot set thread name to " << s << "."); @@ -270,6 +274,23 @@ namespace Util } } + const char *getThreadName() + { + // Avoid so many redundant system calls + return ThreadName; + } + + static __thread pid_t ThreadTid; + + pid_t getThreadId() + { + // Avoid so many redundant system calls + if (!ThreadTid) + ThreadTid = syscall(SYS_gettid); + return ThreadTid; + } + + void getVersionInfo(std::string& version, std::string& hash) { version = std::string(LOOLWSD_VERSION); diff --git a/common/Util.hpp b/common/Util.hpp index 57dc6237..b0e29024 100644 --- a/common/Util.hpp +++ b/common/Util.hpp @@ -101,6 +101,10 @@ namespace Util void setThreadName(const std::string& s); + const char *getThreadName(); + + pid_t getThreadId(); + /// Get version information void getVersionInfo(std::string& version, std::string& hash); diff --git a/test/Makefile.am b/test/Makefile.am index bb9377e3..c55c306f 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -35,12 +35,12 @@ wsd_sources = \ ../common/Log.cpp \ ../common/Protocol.cpp \ ../common/Session.cpp \ + ../common/Util.cpp \ ../common/MessageQueue.cpp \ ../kit/Kit.cpp \ ../wsd/TileCache.cpp \ ../wsd/TestStubs.cpp \ ../common/Unit.cpp \ - ../common/Util.cpp \ ../net/Socket.cpp unittest_CPPFLAGS = -I$(top_srcdir) -DBUILDING_TESTS commit 0f5915b1a40d155770c66017161675cc013ec09c Author: Michael Meeks <michael.me...@collabora.com> Date: Thu Mar 30 10:15:28 2017 +0100 Cleanup whitespace, return is not a function. diff --git a/common/Session.cpp b/common/Session.cpp index 4c30f30f..d6a4eb8a 100644 --- a/common/Session.cpp +++ b/common/Session.cpp @@ -64,13 +64,13 @@ Session::~Session() bool Session::sendTextFrame(const char* buffer, const int length) { LOG_TRC(getName() << ": Send: " << getAbbreviatedMessage(buffer, length)); - return (sendMessage(buffer, length, WSOpCode::Text) >= length); + return sendMessage(buffer, length, WSOpCode::Text) >= length; } bool Session::sendBinaryFrame(const char *buffer, int length) { LOG_TRC(getName() << ": Send: " << std::to_string(length) << " bytes."); - return (sendMessage(buffer, length, WSOpCode::Binary) >= length); + return sendMessage(buffer, length, WSOpCode::Binary) >= length; } void Session::parseDocOptions(const std::vector<std::string>& tokens, int& part, std::string& timestamp) diff --git a/net/ServerSocket.hpp b/net/ServerSocket.hpp index a0604e0b..805430ea 100644 --- a/net/ServerSocket.hpp +++ b/net/ServerSocket.hpp @@ -46,7 +46,7 @@ public: ::setsockopt(getFD(), SOL_SOCKET, SO_REUSEADDR, &reuseAddress, len); const int rc = ::bind(getFD(), address.addr(), address.length()); - return (rc == 0); + return rc == 0; } /// Listen to incoming connections (Servers only). @@ -55,7 +55,7 @@ public: bool listen(const int backlog = 64) { const int rc = ::listen(getFD(), backlog); - return (rc == 0); + return rc == 0; } /// Accepts an incoming connection (Servers only). @@ -70,7 +70,7 @@ public: try { // Create a socket object using the factory. - return (rc != -1 ? _sockFactory->create(rc) : std::shared_ptr<Socket>(nullptr)); + return rc != -1 ? _sockFactory->create(rc) : std::shared_ptr<Socket>(nullptr); } catch (const std::exception& ex) { diff --git a/net/Socket.hpp b/net/Socket.hpp index 111b32df..33e1091c 100644 --- a/net/Socket.hpp +++ b/net/Socket.hpp @@ -129,7 +129,7 @@ public: int size; unsigned int len = sizeof(size); const int rc = ::getsockopt(_fd, SOL_SOCKET, SO_SNDBUF, &size, &len); - return (rc == 0 ? size : -1); + return rc == 0 ? size : -1; } /// Gets our fast cache of the socket buffer size @@ -149,7 +149,7 @@ public: { constexpr unsigned int len = sizeof(size); const int rc = ::setsockopt(_fd, SOL_SOCKET, SO_RCVBUF, &size, len); - return (rc == 0); + return rc == 0; } /// Gets the actual receive buffer size in bytes, -1 on error. @@ -158,7 +158,7 @@ public: int size; unsigned int len = sizeof(size); const int rc = ::getsockopt(_fd, SOL_SOCKET, SO_RCVBUF, &size, &len); - return (rc == 0 ? size : -1); + return rc == 0 ? size : -1; } /// Gets the error code. diff --git a/net/SslSocket.hpp b/net/SslSocket.hpp index ec732c1a..52292c58 100644 --- a/net/SslSocket.hpp +++ b/net/SslSocket.hpp @@ -81,9 +81,7 @@ public: const int rc = doHandshake(); if (rc <= 0) - { - return (rc != 0); - } + return rc != 0; // Default implementation. return StreamSocket::readIncomingData(); @@ -168,9 +166,7 @@ private: { rc = handleSslState(rc); if (rc <= 0) - { - return (rc != 0); - } + return rc != 0; } _doHandshake = false; diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp index a0cf1067..ee4bc052 100644 --- a/wsd/DocumentBroker.cpp +++ b/wsd/DocumentBroker.cpp @@ -58,8 +58,8 @@ std::string getCachePath(const std::string& uri) digestEngine.update(uri.c_str(), uri.size()); - return (LOOLWSD::Cache + '/' + - Poco::DigestEngine::digestToHex(digestEngine.digest()).insert(3, "/").insert(2, "/").insert(1, "/")); + return LOOLWSD::Cache + '/' + + Poco::DigestEngine::digestToHex(digestEngine.digest()).insert(3, "/").insert(2, "/").insert(1, "/"); } } diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp index 455e03e5..ff12b061 100644 --- a/wsd/LOOLWSD.cpp +++ b/wsd/LOOLWSD.cpp @@ -1144,15 +1144,12 @@ void PrisonerPoll::wakeupHook() std::unique_lock<std::mutex> docBrokersLock(DocBrokersMutex, std::defer_lock); if (docBrokersLock.try_lock()) - { cleanupDocBrokers(); - } } void LOOLWSD::triggerChildAndDocHousekeeping() { PrisonerPoll.wakeup(); - } bool LOOLWSD::createForKit() @@ -1220,7 +1217,7 @@ bool LOOLWSD::createForKit() // Init the Admin manager Admin::instance().setForKitPid(ForKitProcId); - return (ForKitProcId != -1); + return ForKitProcId != -1; #endif } diff --git a/wsd/LOOLWSD.hpp b/wsd/LOOLWSD.hpp index d20314c1..b86b23ac 100644 --- a/wsd/LOOLWSD.hpp +++ b/wsd/LOOLWSD.hpp @@ -23,6 +23,7 @@ class ChildProcess; class TraceFileWriter; +class DocumentBroker; std::shared_ptr<ChildProcess> getNewChild_Blocks(); diff --git a/wsd/README b/wsd/README index ab8ea060..2768d859 100644 --- a/wsd/README +++ b/wsd/README @@ -387,3 +387,6 @@ The style is roughly as follows, in rough order of importance: CamelCaseWithInitialUpperCase. - [ No kind of Hungarian prefixes. ] + +- return - is not a function; but a statement - it doesn't need extra () + diff --git a/wsd/SenderQueue.hpp b/wsd/SenderQueue.hpp index 2b48d70b..77f09770 100644 --- a/wsd/SenderQueue.hpp +++ b/wsd/SenderQueue.hpp @@ -102,8 +102,8 @@ private: const auto& pos = std::find_if(_queue.begin(), _queue.end(), [&newTile](const queue_item_t& cur) { - return (cur->firstToken() == "tile:" && - newTile == TileDesc::parse(cur->firstLine())); + return cur->firstToken() == "tile:" && + newTile == TileDesc::parse(cur->firstLine()); }); if (pos != _queue.end()) @@ -119,7 +119,7 @@ private: const auto& pos = std::find_if(_queue.begin(), _queue.end(), [&command](const queue_item_t& cur) { - return (cur->firstToken() == command); + return cur->firstToken() == command; }); if (pos != _queue.end()) @@ -145,7 +145,7 @@ private: Poco::JSON::Parser parser; const auto result = parser.parse(msg); const auto& json = result.extract<Poco::JSON::Object::Ptr>(); - return (viewId == json->get("viewId").toString()); + return viewId == json->get("viewId").toString(); } return false; diff --git a/wsd/Storage.cpp b/wsd/Storage.cpp index 639d057d..94a01c93 100644 --- a/wsd/Storage.cpp +++ b/wsd/Storage.cpp @@ -280,8 +280,10 @@ namespace { inline Poco::Net::HTTPClientSession* getHTTPClientSession(const Poco::URI& uri) { - return (LOOLWSD::isSSLEnabled() || LOOLWSD::isSSLTermination()) ? new Poco::Net::HTTPSClientSession(uri.getHost(), uri.getPort(), Poco::Net::SSLManager::instance().defaultClientContext()) - : new Poco::Net::HTTPClientSession(uri.getHost(), uri.getPort()); + return (LOOLWSD::isSSLEnabled() || LOOLWSD::isSSLTermination()) + ? new Poco::Net::HTTPSClientSession(uri.getHost(), uri.getPort(), + Poco::Net::SSLManager::instance().defaultClientContext()) + : new Poco::Net::HTTPClientSession(uri.getHost(), uri.getPort()); } int getLevenshteinDist(const std::string& string1, const std::string& string2) { diff --git a/wsd/TileCache.cpp b/wsd/TileCache.cpp index 54ff8e75..4c1d7572 100644 --- a/wsd/TileCache.cpp +++ b/wsd/TileCache.cpp @@ -111,7 +111,7 @@ std::shared_ptr<TileCache::TileBeingRendered> TileCache::findTileBeingRendered(c Util::assertIsLocked(_tilesBeingRenderedMutex); const auto tile = _tilesBeingRendered.find(cachedName); - return (tile != _tilesBeingRendered.end() ? tile->second : nullptr); + return tile != _tilesBeingRendered.end() ? tile->second : nullptr; } void TileCache::forgetTileBeingRendered(const TileDesc& tile) @@ -389,7 +389,7 @@ std::string TileCache::cacheFileName(const TileDesc& tile) bool TileCache::parseCacheFileName(const std::string& fileName, int& part, int& width, int& height, int& tilePosX, int& tilePosY, int& tileWidth, int& tileHeight) { - return (std::sscanf(fileName.c_str(), "%d_%dx%d.%d,%d.%dx%d.png", &part, &width, &height, &tilePosX, &tilePosY, &tileWidth, &tileHeight) == 7); + return std::sscanf(fileName.c_str(), "%d_%dx%d.%d,%d.%dx%d.png", &part, &width, &height, &tilePosX, &tilePosY, &tileWidth, &tileHeight) == 7; } bool TileCache::intersectsTile(const std::string& fileName, int part, int x, int y, int width, int height) @@ -525,7 +525,7 @@ std::string TileCache::cancelTiles(const std::shared_ptr<ClientSession> &subscri } const auto canceltiles = oss.str(); - return (canceltiles.empty() ? canceltiles : "canceltiles " + canceltiles); + return canceltiles.empty() ? canceltiles : "canceltiles " + canceltiles; } /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits