[Libreoffice-commits] online.git: net/Socket.hpp net/SslSocket.hpp net/WebSocketHandler.hpp

[Ashod Nakashian]

 net/Socket.hpp   |2 ++
 net/SslSocket.hpp|4 ++--
 net/WebSocketHandler.hpp |3 +++
 3 files changed, 7 insertions(+), 2 deletions(-)

New commits:
commit 11cdfb7688dea76d2647758af767187903a0ec72
Author: Ashod Nakashian 
Date:   Sun Jan 21 20:34:38 2018 -0500

wsd: avoid misleading socket error logs

Change-Id: Ie70d8eb1ecc0442643f4b2a4757b95688b4cf1f7
Reviewed-on: https://gerrit.libreoffice.org/48284
Reviewed-by: Ashod Nakashian 
Tested-by: Ashod Nakashian 

diff --git a/net/Socket.hpp b/net/Socket.hpp
index 01759953..da34b5e3 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -730,6 +730,8 @@ public:
 }
 }
 
+bool isClosed() const { return _closed; }
+
 /// Just trigger the async shutdown.
 virtual void shutdown() override
 {
diff --git a/net/SslSocket.hpp b/net/SslSocket.hpp
index 4b5d1323..2f8d45cb 100644
--- a/net/SslSocket.hpp
+++ b/net/SslSocket.hpp
@@ -251,8 +251,8 @@ private:
 {
 if (rc == 0)
 {
-// Socket closed.
-LOG_ERR("Socket #" << getFD() << " SSL BIO error: 
closed (0).");
+// Socket closed. Not an error.
+LOG_INF("Socket #" << getFD() << " SSL BIO error: 
closed (0).");
 return 0;
 }
 else if (rc == -1)
diff --git a/net/WebSocketHandler.hpp b/net/WebSocketHandler.hpp
index 1b37389d..161272e9 100644
--- a/net/WebSocketHandler.hpp
+++ b/net/WebSocketHandler.hpp
@@ -337,6 +337,9 @@ protected:
 if (!socket || data == nullptr || len == 0)
 return -1;
 
+if (socket->isClosed())
+return 0;
+
 socket->assertCorrectThread();
 std::vector& out = socket->_outBuffer;
 const size_t oldSize = out.size();
___
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits

[Libreoffice-commits] online.git: net/Socket.hpp net/SslSocket.hpp net/WebSocketHandler.hpp wsd/AdminModel.cpp wsd/AdminModel.hpp wsd/ClientSession.cpp wsd/ClientSession.hpp wsd/DocumentBroker.cpp wsd

[Jan Holesovsky]

 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 
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)
 {
 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 ) 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, 

[Libreoffice-commits] online.git: net/Socket.hpp net/SslSocket.hpp net/WebSocketHandler.hpp

[Ashod Nakashian]

 net/Socket.hpp   |1 +
 net/SslSocket.hpp|2 +-
 net/WebSocketHandler.hpp |   15 ++-
 3 files changed, 12 insertions(+), 6 deletions(-)

New commits:
commit 3895897213930c45c63b9ab3049c6bc101ebc114
Author: Ashod Nakashian 
Date:   Sat Mar 25 21:50:24 2017 -0400

wsd: improved socket logging

Change-Id: Ib4751a5a73b7ec0c7ca319f552d5e0aaff06ffea
Reviewed-on: https://gerrit.libreoffice.org/35707
Reviewed-by: Ashod Nakashian 
Tested-by: Ashod Nakashian 

diff --git a/net/Socket.hpp b/net/Socket.hpp
index e0ecc807..56a33de6 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -69,6 +69,7 @@ public:
 /// TODO: Support separate read/write shutdown.
 virtual void shutdown()
 {
+LOG_TRC("#" << _fd << ": socket shutdown RDWR.");
 ::shutdown(_fd, SHUT_RDWR);
 }
 
diff --git a/net/SslSocket.hpp b/net/SslSocket.hpp
index 3654c4b7..ec732c1a 100644
--- a/net/SslSocket.hpp
+++ b/net/SslSocket.hpp
@@ -64,7 +64,7 @@ public:
 /// Shutdown the TLS/SSL connection properly.
 void closeConnection() override
 {
-LOG_DBG("SslStreamSocket::performShutdown() #" << getFD());
+LOG_DBG("SslStreamSocket::closeConnection() #" << getFD());
 if (SSL_shutdown(_ssl) == 0)
 {
 // Complete the bidirectional shutdown.
diff --git a/net/WebSocketHandler.hpp b/net/WebSocketHandler.hpp
index 64b057b9..08500bf2 100644
--- a/net/WebSocketHandler.hpp
+++ b/net/WebSocketHandler.hpp
@@ -107,7 +107,9 @@ public:
 if (socket == nullptr)
 return;
 
-LOG_TRC("#" << socket->getFD() << ": Shutdown websocket.");
+LOG_TRC("#" << socket->getFD() << ": Shutdown websocket, code: " <<
+static_cast(statusCode) << ", message: " << 
statusMessage);
+_shuttingDown = true;
 
 const size_t len = statusMessage.size();
 std::vector buf(2 + len);
@@ -119,7 +121,6 @@ public:
 
 auto lock = socket->getWriteLock();
 sendFrame(socket, buf.data(), buf.size(), flags);
-_shuttingDown = true;
 }
 
 /// Implementation of the SocketHandlerInterface.
@@ -197,16 +198,16 @@ public:
 socket->_inBuffer.erase(socket->_inBuffer.begin(), 
socket->_inBuffer.begin() + headerLen + payloadLen);
 
 // FIXME: fin, aggregating payloads into _wsPayload etc.
-LOG_TRC("#" << socket->getFD() << ": Incoming WebSocket message code " 
<< code << " fin? " << fin << " payload length " << _wsPayload.size());
+LOG_TRC("#" << socket->getFD() << ": Incoming WebSocket message code " 
<< code << " fin? " << fin << ", payload length: " << _wsPayload.size());
 
 switch (code)
 {
 case WSOpCode::Pong:
 _pingTimeUs = 
std::chrono::duration_cast(std::chrono::steady_clock::now()
 - _pingSent).count();
-LOG_TRC("Pong received: " << _pingTimeUs << " microseconds");
+LOG_TRC("#" << socket->getFD() << ": Pong received: " << 
_pingTimeUs << " microseconds");
 break;
 case WSOpCode::Ping:
-LOG_ERR("Clients should not send pings, only servers");
+LOG_ERR("#" << socket->getFD() << ": Clients should not send 
pings, only servers");
 // drop through
 case WSOpCode::Close:
 if (!_shuttingDown)
@@ -224,6 +225,10 @@ public:
 shutdown(statusCode);
 }
 }
+else
+{
+LOG_TRC("#" << socket->getFD() << ": Client responded to our 
shutdown.");
+}
 
 // TCP Close.
 socket->closeConnection();
___
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits