[Libreoffice-commits] online.git: net/Socket.hpp net/WebSocketHandler.hpp tools/WebSocketDump.cpp wsd/LOOLWSD.cpp

2018-10-25 Thread Libreoffice Gerrit user 
 net/Socket.hpp   |   20 
 net/WebSocketHandler.hpp |   24 
 tools/WebSocketDump.cpp  |2 +-
 wsd/LOOLWSD.cpp  |   12 ++--
 4 files changed, 31 insertions(+), 27 deletions(-)

New commits:
commit 4724aa0cf78f3aedf7f7f4222d251ef8395d5523
Author: Miklos Vajna 
AuthorDate: Thu Oct 25 16:38:54 2018 +0200
Commit: Miklos Vajna 
CommitDate: Thu Oct 25 16:39:02 2018 +0200

Socket: do what the TODO says

Drop the list of friends and just add getters for the relevant
members.

diff --git a/net/Socket.hpp b/net/Socket.hpp
index 2d8a2f26a..698edd328 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -938,6 +938,16 @@ public:
 return _clientAddress;
 }
 
+std::vector& getInBuffer()
+{
+return _inBuffer;
+}
+
+std::vector& getOutBuffer()
+{
+return _outBuffer;
+}
+
 protected:
 
 /// Called when a polling event is received.
@@ -1012,6 +1022,7 @@ protected:
 disposition.setClosed();
 }
 
+public:
 /// Override to write data out to socket.
 virtual void writeOutgoingData()
 {
@@ -1051,6 +1062,7 @@ protected:
 while (!_outBuffer.empty());
 }
 
+protected:
 /// Override to handle reading of socket data differently.
 virtual int readData(char* buf, int len)
 {
@@ -1096,14 +1108,6 @@ protected:
 
 /// True when shutdown was requested via shutdown().
 bool _shutdownSignalled;
-
-// To be able to access _inBuffer and _outBuffer.
-// TODO we probably need accessors to the _inBuffer & _outBuffer
-// instead of this many friends...
-friend class WebSocketHandler;
-friend class ClientRequestDispatcher;
-friend class PrisonerRequestDispatcher;
-friend class SimpleResponseClient;
 };
 
 enum class WSOpCode : unsigned char {
diff --git a/net/WebSocketHandler.hpp b/net/WebSocketHandler.hpp
index fcb9d3660..3b60fe9b8 100644
--- a/net/WebSocketHandler.hpp
+++ b/net/WebSocketHandler.hpp
@@ -131,7 +131,7 @@ public:
 assert(socket && "Expected a valid socket instance.");
 
 // websocket fun !
-const size_t len = socket->_inBuffer.size();
+const size_t len = socket->getInBuffer().size();
 
 if (len == 0)
 return false; // avoid logging.
@@ -143,7 +143,7 @@ public:
 return false;
 }
 
-unsigned char *p = reinterpret_cast(&socket->_inBuffer[0]);
+unsigned char *p = reinterpret_cast(&socket->getInBuffer()[0]);
 const bool fin = p[0] & 0x80;
 const WSOpCode code = static_cast(p[0] & 0x0f);
 const bool hasMask = p[1] & 0x80;
@@ -191,7 +191,7 @@ public:
 return false;
 }
 
-LOG_TRC("#" << socket->getFD() << ": Incoming WebSocket data of " << 
len << " bytes: " << Util::stringifyHexLine(socket->_inBuffer, 0, 
std::min((size_t)32, len)));
+LOG_TRC("#" << socket->getFD() << ": Incoming WebSocket data of " << 
len << " bytes: " << Util::stringifyHexLine(socket->getInBuffer(), 0, 
std::min((size_t)32, len)));
 
 data = p + headerLen;
 
@@ -214,14 +214,14 @@ public:
 
 assert(_wsPayload.size() >= payloadLen);
 
-socket->_inBuffer.erase(socket->_inBuffer.begin(), 
socket->_inBuffer.begin() + headerLen + payloadLen);
+socket->getInBuffer().erase(socket->getInBuffer().begin(), 
socket->getInBuffer().begin() + headerLen + payloadLen);
 
 #ifndef MOBILEAPP
 
 // FIXME: fin, aggregating payloads into _wsPayload etc.
 LOG_TRC("#" << socket->getFD() << ": Incoming WebSocket message code " 
<< static_cast(code) <<
 ", fin? " << fin << ", mask? " << hasMask << ", payload 
length: " << _wsPayload.size() <<
-", residual socket data: " << socket->_inBuffer.size() << " 
bytes.");
+", residual socket data: " << socket->getInBuffer().size() << 
" bytes.");
 
 bool doClose = false;
 
@@ -444,7 +444,7 @@ private:
 return 0;
 
 socket->assertCorrectThread();
-std::vector& out = socket->_outBuffer;
+std::vector& out = socket->getOutBuffer();
 const size_t oldSize = out.size();
 
 #ifndef MOBILEAPP
@@ -586,26 +586,26 @@ protected:
 {
 std::shared_ptr socket = _socket.lock();
 
-LOG_TRC("Incoming client websocket upgrade response: " << 
std::string(&socket->_inBuffer[0], socket->_inBuffer.size()));
+LOG_TRC("Incoming client websocket upgrade response: " << 
std::string(&socket->getInBuffer()[0], socket->getInBuffer().size()));
 
 bool bOk = false;
 size_t responseSize = 0;
 
 try
 {
-Poco::MemoryInputStream message(&socket->_inBuffer[0], 
socket->_inBuffer.size());;
+Poco::MemoryInputStream message(&socket->getInBuffer()[0], 
socket->getInBuffer().size());;
 Poco::Net::HTTPResponse response;
 
 response.read(message);
 
 {

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

2017-05-21 Thread Ashod Nakashian 
 net/Socket.hpp   |   15 ---
 net/WebSocketHandler.hpp |8 ++--
 wsd/ClientSession.cpp|   13 +++--
 wsd/DocumentBroker.cpp   |   12 +++-
 4 files changed, 28 insertions(+), 20 deletions(-)

New commits:
commit bdd8b715ddfac90ea814ea87f58a2985a2bc66c0
Author: Ashod Nakashian 
Date:   Sat May 20 13:28:43 2017 -0400

wsd: include cleanup

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

diff --git a/net/Socket.hpp b/net/Socket.hpp
index 7c7b55ab..88e24f05 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -21,20 +21,21 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
 #include 
-#include 
 
-#include "Common.hpp"
-#include "Log.hpp"
-#include "Util.hpp"
-#include "SigUtil.hpp"
+#include "common/Common.hpp"
+#include "common/Log.hpp"
+#include "common/Util.hpp"
+#include "common/SigUtil.hpp"
 
 namespace Poco
 {
@@ -136,8 +137,8 @@ public:
 virtual void setNoDelay()
 {
 const int val = 1;
-setsockopt (_fd, IPPROTO_TCP, TCP_NODELAY,
-(char *) &val, sizeof(val));
+::setsockopt(_fd, IPPROTO_TCP, TCP_NODELAY,
+ (char *) &val, sizeof(val));
 }
 
 /// Sets the kernel socket send buffer in size bytes.
diff --git a/net/WebSocketHandler.hpp b/net/WebSocketHandler.hpp
index 4ff01c36..7c52cf58 100644
--- a/net/WebSocketHandler.hpp
+++ b/net/WebSocketHandler.hpp
@@ -10,8 +10,12 @@
 #ifndef INCLUDED_WEBSOCKETHANDLER_HPP
 #define INCLUDED_WEBSOCKETHANDLER_HPP
 
-#include "Common.hpp"
-#include "Log.hpp"
+#include 
+#include 
+#include 
+
+#include "common/Common.hpp"
+#include "common/Log.hpp"
 #include "Socket.hpp"
 
 #include 
diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index d21d925d..387f7629 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -14,16 +14,17 @@
 #include 
 
 #include 
+#include 
 #include 
 
-#include "Common.hpp"
+#include "common/Common.hpp"
 #include "DocumentBroker.hpp"
 #include "LOOLWSD.hpp"
-#include "Log.hpp"
-#include "Protocol.hpp"
-#include "Session.hpp"
-#include "Util.hpp"
-#include "Unit.hpp"
+#include "common/Log.hpp"
+#include "common/Protocol.hpp"
+#include "common/Session.hpp"
+#include "common/Util.hpp"
+#include "common/Unit.hpp"
 
 using namespace LOOLProtocol;
 
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 5b5f4ebd..956a2fe6 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -11,6 +11,7 @@
 
 #include "DocumentBroker.hpp"
 
+#include 
 #include 
 #include 
 #include 
@@ -27,14 +28,14 @@
 #include "Admin.hpp"
 #include "ClientSession.hpp"
 #include "Exceptions.hpp"
-#include "Message.hpp"
-#include "Protocol.hpp"
 #include "LOOLWSD.hpp"
-#include "Log.hpp"
+#include "SenderQueue.hpp"
 #include "Storage.hpp"
 #include "TileCache.hpp"
-#include "SenderQueue.hpp"
-#include "Unit.hpp"
+#include "common/Log.hpp"
+#include "common/Message.hpp"
+#include "common/Protocol.hpp"
+#include "common/Unit.hpp"
 
 using namespace LOOLProtocol;
 
@@ -409,6 +410,7 @@ bool DocumentBroker::load(const 
std::shared_ptr& session, const s
 LOG_ERR("Failed to create Storage instance for [" << _docKey << "] 
in " << jailPath.toString());
 return false;
 }
+
 firstInstance = true;
 }
 
___
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/WebSocketHandler.hpp

2017-04-19 Thread Jan Holesovsky 
 net/Socket.hpp   |4 ++--
 net/WebSocketHandler.hpp |1 -
 2 files changed, 2 insertions(+), 3 deletions(-)

New commits:
commit 47ca32beb5659a58655a1cef6cc61b6aaccd355a
Author: Jan Holesovsky 
Date:   Wed Apr 19 11:07:55 2017 +0200

We never call setNoDelay() with false.

Change-Id: I865027300559b185222f21faa48ccd32100bd02d

diff --git a/net/Socket.hpp b/net/Socket.hpp
index 70b229b3..8c07e38d 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -84,9 +84,9 @@ public:
 virtual HandleResult handlePoll(std::chrono::steady_clock::time_point now, 
int events) = 0;
 
 /// manage latency issues around packet aggregation
-void setNoDelay(bool noDelay = true)
+void setNoDelay()
 {
-int val = noDelay ? 1 : 0;
+const int val = 1;
 setsockopt (_fd, IPPROTO_TCP, TCP_NODELAY,
 (char *) &val, sizeof(val));
 }
diff --git a/net/WebSocketHandler.hpp b/net/WebSocketHandler.hpp
index a863afad..14d97f81 100644
--- a/net/WebSocketHandler.hpp
+++ b/net/WebSocketHandler.hpp
@@ -417,7 +417,6 @@ protected:
 LOG_INF("#" << socket->getFD() << ": WebSocket version: " << wsVersion 
<<
 ", key: [" << wsKey << "], protocol: [" << wsProtocol << "].");
 
-socket->setNoDelay();
 #if ENABLE_DEBUG
 if (std::getenv("LOOL_ZERO_BUFFER_SIZE"))
 socket->setSocketBufferSize(0);
___
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/WebSocketHandler.hpp

2017-04-02 Thread Ashod Nakashian 
 net/Socket.hpp   |   16 ++--
 net/WebSocketHandler.hpp |2 +-
 2 files changed, 7 insertions(+), 11 deletions(-)

New commits:
commit 8932a1e92e9dbe61841a92d970751ecc41f32a80
Author: Ashod Nakashian 
Date:   Mon Apr 3 01:02:35 2017 -0400

wsd: remove LOOL_CHECK_THREADS

isCorrectThread now always checks
with ENABLE_DEBUG.

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

diff --git a/net/Socket.hpp b/net/Socket.hpp
index 7d227732..9cf83c49 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -195,7 +195,7 @@ public:
 #endif
 }
 
-virtual bool isCorrectThread(bool hard = false)
+virtual bool isCorrectThread()
 {
 #if ENABLE_DEBUG
 const bool sameThread = std::this_thread::get_id() == _owner;
@@ -204,12 +204,8 @@ public:
 _owner << " but called from 0x" << 
std::this_thread::get_id() << " (" <<
 std::dec << Util::getThreadId() << ").");
 
-if (hard)
-return sameThread;
-else
-return !getenv("LOOL_CHECK_THREADS") || sameThread;
+return sameThread;
 #else
-(void)hard;
 return true;
 #endif
 }
@@ -455,7 +451,7 @@ public:
 {
 assert(socket);
 assert(isCorrectThread());
-assert(socket->isCorrectThread(true));
+assert(socket->isCorrectThread());
 auto it = std::find(_pollSockets.begin(), _pollSockets.end(), socket);
 assert(it != _pollSockets.end());
 
@@ -650,7 +646,7 @@ public:
 /// Send data to the socket peer.
 void send(const char* data, const int len, const bool flush = true)
 {
-assert(isCorrectThread(true));
+assert(isCorrectThread());
 if (data != nullptr && len > 0)
 {
 _outBuffer.insert(_outBuffer.end(), data, data + len);
@@ -732,7 +728,7 @@ protected:
 HandleResult handlePoll(std::chrono::steady_clock::time_point now,
 const int events) override
 {
-assert(isCorrectThread(true));
+assert(isCorrectThread());
 
 _socketHandler->checkTimeout(now);
 
@@ -800,7 +796,7 @@ protected:
 /// Override to write data out to socket.
 virtual void writeOutgoingData()
 {
-assert(isCorrectThread(true));
+assert(isCorrectThread());
 assert(!_outBuffer.empty());
 do
 {
diff --git a/net/WebSocketHandler.hpp b/net/WebSocketHandler.hpp
index 0cf63a36..8d689de3 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(true));
+assert(socket->isCorrectThread());
 std::vector& out = socket->_outBuffer;
 
 out.push_back(flags);
___
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/WebSocketHandler.hpp

2017-03-31 Thread Michael Meeks 
 net/Socket.hpp   |   23 +--
 net/WebSocketHandler.hpp |6 ++
 2 files changed, 7 insertions(+), 22 deletions(-)

New commits:
commit 194c169f6811d0a00e04108ef2f3e4061ca4fac8
Author: Michael Meeks 
Date:   Fri Mar 31 12:25:21 2017 +0100

Remove obsolete write-lock.

The lock was used incompletely & inconsistently, and we should
always and only ever write in the associated SocketPoll's thread.

diff --git a/net/Socket.hpp b/net/Socket.hpp
index 0b765731..c45caf77 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -617,10 +617,9 @@ public:
 /// Send data to the socket peer.
 void send(const char* data, const int len, const bool flush = true)
 {
-assert(isCorrectThread());
+assert(isCorrectThread(true));
 if (data != nullptr && len > 0)
 {
-auto lock = getWriteLock();
 _outBuffer.insert(_outBuffer.end(), data, data + len);
 if (flush)
 writeOutgoingData();
@@ -700,7 +699,7 @@ protected:
 HandleResult handlePoll(std::chrono::steady_clock::time_point now,
 const int events) override
 {
-assert(isCorrectThread());
+assert(isCorrectThread(true));
 
 _socketHandler->checkTimeout(now);
 
@@ -746,14 +745,9 @@ protected:
 oldSize = _outBuffer.size();
 
 // Write if we can and have data to write.
-if ((events & POLLOUT) || !_outBuffer.empty())
+if ((events & POLLOUT) && !_outBuffer.empty())
 {
-std::unique_lock lock(_writeMutex, 
std::defer_lock);
-
-// The buffer could have been flushed while we waited for the 
lock.
-if (lock.try_lock() && !_outBuffer.empty())
-writeOutgoingData();
-
+writeOutgoingData();
 closed = closed || (errno == EPIPE);
 }
 }
@@ -773,9 +767,7 @@ protected:
 /// Override to write data out to socket.
 virtual void writeOutgoingData()
 {
-assert(isCorrectThread());
-
-Util::assertIsLocked(_writeMutex);
+assert(isCorrectThread(true));
 assert(!_outBuffer.empty());
 do
 {
@@ -826,9 +818,6 @@ protected:
 
 void dumpState(std::ostream& os) override;
 
-/// Get the Write Lock.
-std::unique_lock getWriteLock() { return 
std::unique_lock(_writeMutex); }
-
 protected:
 /// Client handling the actual data.
 std::shared_ptr _socketHandler;
@@ -842,8 +831,6 @@ protected:
 std::vector< char > _inBuffer;
 std::vector< char > _outBuffer;
 
-std::mutex _writeMutex;
-
 // To be able to access _inBuffer and _outBuffer.
 // TODO we probably need accessors to the _inBuffer & _outBuffer
 // instead of this many friends...
diff --git a/net/WebSocketHandler.hpp b/net/WebSocketHandler.hpp
index ac04da53..a7cae8f4 100644
--- a/net/WebSocketHandler.hpp
+++ b/net/WebSocketHandler.hpp
@@ -123,7 +123,6 @@ public:
 const unsigned char flags = static_cast(WSFrameMask::Fin)
   | static_cast(WSOpCode::Close);
 
-auto lock = socket->getWriteLock();
 sendFrame(socket, buf.data(), buf.size(), flags);
 }
 
@@ -316,8 +315,7 @@ public:
 if (socket == nullptr)
 return -1; // no socket == error.
 
-assert(socket->isCorrectThread());
-auto lock = socket->getWriteLock();
+assert(socket->isCorrectThread(true));
 std::vector& out = socket->_outBuffer;
 
 //TODO: Support fragmented messages.
@@ -349,7 +347,7 @@ protected:
 if (!socket || data == nullptr || len == 0)
 return -1;
 
-assert(socket->isCorrectThread());
+assert(socket->isCorrectThread(true));
 std::vector& out = socket->_outBuffer;
 
 out.push_back(flags);
___
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/WebSocketHandler.hpp wsd/LOOLWSD.cpp

2017-03-26 Thread Ashod Nakashian 
 net/Socket.hpp   |2 +-
 net/WebSocketHandler.hpp |9 -
 wsd/LOOLWSD.cpp  |   20 +++-
 3 files changed, 24 insertions(+), 7 deletions(-)

New commits:
commit 5ae94cc7ecc023122dfa4b67fa22bb2cf75fb088
Author: Ashod Nakashian 
Date:   Sun Mar 26 23:06:44 2017 -0400

wsd: onConnect takes shared_ptr and better logging

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

diff --git a/net/Socket.hpp b/net/Socket.hpp
index 56a33de6..6a511744 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -516,7 +516,7 @@ public:
 /// Called when the socket is newly created to
 /// set the socket associated with this ResponseClient.
 /// Will be called exactly once.
-virtual void onConnect(const std::weak_ptr& socket) = 0;
+virtual void onConnect(const std::shared_ptr& socket) = 0;
 
 enum class SocketOwnership
 {
diff --git a/net/WebSocketHandler.hpp b/net/WebSocketHandler.hpp
index 6983fbae..ce0d13e9 100644
--- a/net/WebSocketHandler.hpp
+++ b/net/WebSocketHandler.hpp
@@ -62,9 +62,10 @@ public:
 }
 
 /// Implementation of the SocketHandlerInterface.
-void onConnect(const std::weak_ptr& socket) override
+void onConnect(const std::shared_ptr& socket) override
 {
 _socket = socket;
+LOG_TRC("#" << socket->getFD() << " Connected to WS Handler 0x" << 
std::hex << this << std::dec);
 }
 
 enum WSOpCode {
@@ -105,7 +106,10 @@ public:
 {
 auto socket = _socket.lock();
 if (socket == nullptr)
+{
+LOG_ERR("No socket associated with WebSocketHandler 0x" << 
std::hex << this << std::dec);
 return;
+}
 
 LOG_TRC("#" << socket->getFD() << ": Shutdown websocket, code: " <<
 static_cast(statusCode) << ", message: " << 
statusMessage);
@@ -128,7 +132,10 @@ public:
 {
 auto socket = _socket.lock();
 if (socket == nullptr)
+{
+LOG_ERR("No socket associated with WebSocketHandler 0x" << 
std::hex << this << std::dec);
 return false;
+}
 
 // websocket fun !
 const size_t len = socket->_inBuffer.size();
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index d9b1db56..05abba1d 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -1396,15 +1396,19 @@ public:
 
 private:
 /// Keep our socket around ...
-void onConnect(const std::weak_ptr& socket) override
+void onConnect(const std::shared_ptr& socket) override
 {
-LOG_TRC("Prisoner - new socket");
 _socket = socket;
+LOG_TRC("#" << socket->getFD() << " Prisoner connected.");
 }
 
 void onDisconnect() override
 {
-LOG_TRC("Prisoner connection disconnected");
+auto socket = _socket.lock();
+if (socket)
+LOG_TRC("#" << socket->getFD() << " Prisoner connection 
disconnected.");
+else
+LOG_WRN("Prisoner connection disconnected but without valid 
socket.");
 
 // Notify the broker that we're done.
 auto child = _childProcess.lock();
@@ -1525,7 +1529,11 @@ private:
 if (UnitWSD::get().filterChildMessage(data))
 return;
 
-LOG_TRC("Prisoner message [" << getAbbreviatedMessage(&data[0], 
data.size()) << "].");
+auto socket = _socket.lock();
+if (socket)
+LOG_TRC("#" << socket->getFD() << " Prisoner message [" << 
getAbbreviatedMessage(&data[0], data.size()) << "].");
+else
+LOG_WRN("Message handler called but without valid socket.");
 
 auto child = _childProcess.lock();
 auto docBroker = child ? child->getDocumentBroker() : nullptr;
@@ -1567,10 +1575,11 @@ public:
 private:
 
 /// Set the socket associated with this ResponseClient.
-void onConnect(const std::weak_ptr& socket) override
+void onConnect(const std::shared_ptr& socket) override
 {
 _id = LOOLWSD::GenSessionId();
 _socket = socket;
+LOG_TRC("#" << socket->getFD() << " Connected to 
ClientRequestDispatcher.");
 }
 
 /// Called after successful socket reads.
@@ -2108,6 +2117,7 @@ private:
 {
 // Set the ClientSession to handle Socket events.
 socket->setHandler(clientSession);
+LOG_DBG("Socket #" << socket->getFD() << " handler is " << 
clientSession->getName());
 
 // Move the socket into DocBroker.
 docBroker->addSocketToPoll(socket);
___
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/WebSocketHandler.hpp wsd/Admin.cpp

2017-03-17 Thread Michael Meeks 
 net/Socket.hpp   |9 +++--
 net/WebSocketHandler.hpp |   45 +++--
 wsd/Admin.cpp|4 ++--
 3 files changed, 48 insertions(+), 10 deletions(-)

New commits:
commit a6a4094e52c3a7fafac4a617ca3956dc6228eb29
Author: Michael Meeks 
Date:   Fri Mar 17 22:59:03 2017 +

Send ping message, handle pong & store ping-time on the Websocket.

diff --git a/net/Socket.hpp b/net/Socket.hpp
index 268994a7..37b9478c 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -304,7 +304,7 @@ public:
 int rc;
 do
 {
-rc = ::poll(&_pollFds[0], size + 1, timeoutMaxMs);
+rc = ::poll(&_pollFds[0], size + 1, std::max(timeoutMaxMs,0));
 }
 while (rc < 0 && errno == EINTR);
 LOG_TRC("Poll completed with " << rc << " live polls max (" << 
timeoutMaxMs << "ms)"
@@ -524,6 +524,9 @@ public:
 virtual int getPollEvents(std::chrono::steady_clock::time_point now,
   int &timeoutMaxMs) = 0;
 
+/// Do we need to handle a timeout ?
+virtual void checkTimeout(std::chrono::steady_clock::time_point /* now */) 
{}
+
 /// Do some of the queued writing.
 virtual void performWrites() = 0;
 
@@ -673,11 +676,13 @@ protected:
 
 /// Called when a polling event is received.
 /// @events is the mask of events that triggered the wake.
-HandleResult handlePoll(std::chrono::steady_clock::time_point /* now */,
+HandleResult handlePoll(std::chrono::steady_clock::time_point now,
 const int events) override
 {
 assert(isCorrectThread());
 
+_socketHandler->checkTimeout(now);
+
 if (!events)
 return Socket::HandleResult::CONTINUE;
 
diff --git a/net/WebSocketHandler.hpp b/net/WebSocketHandler.hpp
index 50c5d0e9..bb6af23b 100644
--- a/net/WebSocketHandler.hpp
+++ b/net/WebSocketHandler.hpp
@@ -23,6 +23,10 @@ protected:
 // The socket that owns us (we can't own it).
 std::weak_ptr _socket;
 
+const int PingFrequencyMs = 18 * 1000;
+std::chrono::steady_clock::time_point _pingSent;
+int _pingTimeUs;
+
 std::vector _wsPayload;
 bool _shuttingDown;
 enum class WSState { HTTP, WS } _wsState;
@@ -35,6 +39,8 @@ protected:
 
 public:
 WebSocketHandler() :
+_pingSent(std::chrono::steady_clock::now()),
+_pingTimeUs(0),
 _shuttingDown(false),
 _wsState(WSState::HTTP)
 {
@@ -44,6 +50,8 @@ public:
 WebSocketHandler(const std::weak_ptr& socket,
  const Poco::Net::HTTPRequest& request) :
 _socket(socket),
+_pingSent(std::chrono::steady_clock::now()),
+_pingTimeUs(0),
 _shuttingDown(false),
 _wsState(WSState::HTTP)
 {
@@ -187,8 +195,16 @@ public:
 // FIXME: fin, aggregating payloads into _wsPayload etc.
 LOG_TRC("#" << socket->getFD() << ": Incoming WebSocket message code " 
<< code << " fin? " << fin << " payload length " << _wsPayload.size());
 
-if (code & WSOpCode::Close)
+switch (code)
 {
+case WSOpCode::Pong:
+_pingTimeUs = 
std::chrono::duration_cast(std::chrono::steady_clock::now()
 - _pingSent).count();
+LOG_TRC("Pong received: " << _pingTimeUs << " microseconds");
+break;
+case WSOpCode::Ping:
+LOG_ERR("Clients should not send pings, only servers");
+// drop through
+case WSOpCode::Close:
 if (!_shuttingDown)
 {
 // Peer-initiated shutdown must be echoed.
@@ -207,10 +223,10 @@ public:
 
 // TCP Close.
 socket->shutdown();
-}
-else
-{
+break;
+default:
 handleMessage(fin, code, _wsPayload);
+break;
 }
 
 _wsPayload.clear();
@@ -225,12 +241,29 @@ public:
 ; // can have multiple msgs in one recv'd packet.
 }
 
-int getPollEvents(std::chrono::steady_clock::time_point /* now */,
-  int & /* timeoutMaxMs */) override
+int getPollEvents(std::chrono::steady_clock::time_point now,
+  int & timeoutMaxMs) override
 {
+int timeSincePingMs =
+std::chrono::duration_cast(now - 
_pingSent).count();
+timeoutMaxMs = std::min(timeoutMaxMs, PingFrequencyMs - 
timeSincePingMs);
 return POLLIN;
 }
 
+/// Do we need to handle a timeout ?
+void checkTimeout(std::chrono::steady_clock::time_point now) override
+{
+int timeSincePingMs =
+std::chrono::duration_cast(now - 
_pingSent).count();
+if (timeSincePingMs >= PingFrequencyMs)
+{
+LOG_TRC("Send ping message");
+// FIXME: allow an empty payload.
+sendMessage("", 1, WSOpCode::Ping, false);
+_pingSent = now;
+}
+}
+
 /// By default rely on the socket buffer.

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

2017-03-12 Thread Ashod Nakashian 
 net/Socket.hpp   |1 +
 net/WebSocketHandler.hpp |2 ++
 2 files changed, 3 insertions(+)

New commits:
commit c9a07a3087cced9c0ea8973a039b284bcb01a0e7
Author: Ashod Nakashian 
Date:   Sun Mar 12 19:04:52 2017 -0400

wsd: assert socket is in correct thread

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

diff --git a/net/Socket.hpp b/net/Socket.hpp
index 6929938..c4badea 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -555,6 +555,7 @@ public:
 
 int getPollEvents() override
 {
+assert(isCorrectThread());
 if (!_outBuffer.empty() || _socketHandler->hasQueuedWrites() || 
_shutdownSignalled)
 return POLLIN | POLLOUT;
 else
diff --git a/net/WebSocketHandler.hpp b/net/WebSocketHandler.hpp
index 7416586..d89f49b 100644
--- a/net/WebSocketHandler.hpp
+++ b/net/WebSocketHandler.hpp
@@ -255,6 +255,7 @@ public:
 if (socket == nullptr)
 return -1; // no socket == error.
 
+assert(socket->isCorrectThread());
 auto lock = socket->getWriteLock();
 std::vector& out = socket->_outBuffer;
 
@@ -287,6 +288,7 @@ protected:
 if (!socket || data == nullptr || len == 0)
 return -1;
 
+assert(socket->isCorrectThread());
 std::vector& out = socket->_outBuffer;
 
 out.push_back(flags);
___
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/WebSocketHandler.hpp wsd/DocumentBroker.cpp wsd/DocumentBroker.hpp wsd/LOOLWSD.cpp

2017-03-10 Thread Michael Meeks 
 net/Socket.hpp   |5 ++-
 net/WebSocketHandler.hpp |2 +
 wsd/DocumentBroker.cpp   |1 
 wsd/DocumentBroker.hpp   |1 
 wsd/LOOLWSD.cpp  |   64 ---
 5 files changed, 13 insertions(+), 60 deletions(-)

New commits:
commit 160446fd235c3d1bf25413fea18c72f1e3487cbe
Author: Michael Meeks 
Date:   Fri Mar 10 17:58:51 2017 +

Work on resurrecting dying DocumentBrokers if we can.

The hope is that they will close lingering session sockets at the
end and the client will re-connect.

diff --git a/net/Socket.hpp b/net/Socket.hpp
index c5e5ece..9259d88 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -52,7 +52,9 @@ public:
 
 virtual ~Socket()
 {
-//TODO: Should we shutdown here or up to the client?
+// TODO: Should we shutdown here or up to the client?
+
+LOG_TRC("#" << getFD() << " close socket.");
 
 // Doesn't block on sockets; no error handling needed.
 close(_fd);
@@ -551,6 +553,7 @@ public:
 virtual void shutdown() override
 {
 _shutdownSignalled = true;
+LOG_TRC("#" << getFD() << ": shutdown signalled");
 }
 
 /// Perform the real shutdown.
diff --git a/net/WebSocketHandler.hpp b/net/WebSocketHandler.hpp
index 5e987fb..7416586 100644
--- a/net/WebSocketHandler.hpp
+++ b/net/WebSocketHandler.hpp
@@ -97,6 +97,8 @@ public:
 if (socket == nullptr)
 return;
 
+LOG_TRC("#" << socket->getFD() << " shutdown websocket.");
+
 const size_t len = statusMessage.size();
 std::vector buf(2 + len);
 buf[0] = int)statusCode) >> 8) & 0xff);
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index f50d9a3..1369ba5 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -751,6 +751,7 @@ size_t 
DocumentBroker::addSession(std::shared_ptr& session)
 // document). It is safe to reset their values to their defaults whenever 
a new session is added.
 _lastEditableSession = false;
 _markToDestroy = false;
+_stop = false;
 
 const auto id = session->getId();
 if (!_sessions.emplace(id, session).second)
diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp
index 5594c70..b07b9b5 100644
--- a/wsd/DocumentBroker.hpp
+++ b/wsd/DocumentBroker.hpp
@@ -175,7 +175,6 @@ public:
 try
 {
 return _pid > 1 && _ws && kill(_pid, 0) == 0;
-// FIXME:!_ws->poll(Poco::Timespan(0), 
Poco::Net::Socket::SelectMode::SELECT_ERROR));
 }
 catch (const std::exception&)
 {
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 8d0a3a4e..7eafcdf 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -1335,59 +1335,12 @@ static std::shared_ptr 
findOrCreateDocBroker(WebSocketHandler& w
 // Get the DocumentBroker from the Cache.
 LOG_DBG("Found DocumentBroker with docKey [" << docKey << "].");
 docBroker = it->second;
-if (docBroker->isMarkedToDestroy())
-{
-// Let the waiting happen in parallel to new requests.
-docBrokersLock.unlock();
-
-// If this document is going out, wait.
-LOG_DBG("Document [" << docKey << "] is marked to destroy, waiting 
to reload.");
-
-// FIXME: - easiest to send a fast message to the
-//  client to wait & retry in a bit ...
-
-#if 0 // loolnb
-bool timedOut = true;
-for (size_t i = 0; i < COMMAND_TIMEOUT_MS / POLL_TIMEOUT_MS; ++i)
-{
-
-// FIXME: blocks !
-
std::this_thread::sleep_for(std::chrono::milliseconds(POLL_TIMEOUT_MS));
-
-docBrokersLock.lock();
-it = DocBrokers.find(docKey);
-if (it == DocBrokers.end())
-{
-// went away successfully
-docBroker.reset();
-docBrokersLock.unlock();
-timedOut = false;
-break;
-}
-else if (it->second && !it->second->isMarkedToDestroy())
-{
-// was actually replaced by a real document
-docBroker = it->second;
-docBrokersLock.unlock();
-timedOut = false;
-break;
-}
 
-docBrokersLock.unlock();
-if (TerminationFlag)
-{
-LOG_ERR("Termination flag set. Not loading new session [" 
<< id << "]");
-return nullptr;
-}
-}
-
-if (timedOut)
-{
-// Still here, but marked to destroy. Proceed and hope to 
recover.
-LOG_ERR("Timed out while waiting for document to unload before 
loading.");
-}
-#endif
-}
+// Avoid notifying the client - either we catch and stop the
+

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

2017-03-10 Thread Michael Meeks 
 net/Socket.hpp   |   67 ++-
 net/WebSocketHandler.hpp |4 ++
 2 files changed, 53 insertions(+), 18 deletions(-)

New commits:
commit a5a227e9a3327bee423df9de6c3faae112aaa4bd
Author: Michael Meeks 
Date:   Fri Mar 10 09:55:28 2017 +

Re-work socket buffer options, sizing and setting.

Only set nodelay and small socket buffers on WebSockets.
Avoid writing more data than can be absorbed by our socket buffer.
It is fine to set socket buffer sizes after bind/accept.

diff --git a/net/Socket.hpp b/net/Socket.hpp
index 7b3199b..583d580 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -40,10 +40,14 @@
 class Socket
 {
 public:
+static const int DefaultSendBufferSize = 16 * 1024;
+static const int MaximumSendBufferSize = 128 * 1024;
+
 Socket() :
-_fd(socket(AF_INET, SOCK_STREAM | SOCK_NONBLOCK, 0))
+_fd(socket(AF_INET, SOCK_STREAM | SOCK_NONBLOCK, 0)),
+_sendBufferSize(DefaultSendBufferSize)
 {
-setNoDelay();
+init();
 }
 
 virtual ~Socket()
@@ -82,23 +86,39 @@ public:
 (char *) &val, sizeof(val));
 }
 
-/// Sets the send buffer in size bytes.
-/// Must be called before accept or connect.
+/// Sets the kernel socket send buffer in size bytes.
 /// Note: TCP will allocate twice this size for admin purposes,
 /// so a subsequent call to getSendBufferSize will return
 /// the larger (actual) buffer size, if this succeeds.
 /// Note: the upper limit is set via /proc/sys/net/core/wmem_max,
 /// and there is an unconfigurable lower limit as well.
 /// Returns true on success only.
-bool setSendBufferSize(const int size)
+bool setSocketBufferSize(const int size)
 {
-constexpr unsigned int len = sizeof(size);
-const int rc = ::setsockopt(_fd, SOL_SOCKET, SO_SNDBUF, &size, len);
-return (rc == 0);
+int rc = ::setsockopt(_fd, SOL_SOCKET, SO_SNDBUF, &size, sizeof(size));
+
+_sendBufferSize = getSocketBufferSize();
+if (rc != 0 || _sendBufferSize < 0 )
+{
+LOG_ERR("Error getting socket buffer size " << errno);
+_sendBufferSize = DefaultSendBufferSize;
+return false;
+}
+else
+{
+if (_sendBufferSize > MaximumSendBufferSize * 2)
+{
+LOG_TRC("Clamped send buffer size to " << 
MaximumSendBufferSize << " from " << _sendBufferSize);
+_sendBufferSize = MaximumSendBufferSize;
+}
+else
+LOG_TRC("Set socket buffer size to " << _sendBufferSize);
+return true;
+}
 }
 
 /// Gets the actual send buffer size in bytes, -1 for failure.
-int getSendBufferSize() const
+int getSocketBufferSize() const
 {
 int size;
 unsigned int len = sizeof(size);
@@ -106,10 +126,15 @@ public:
 return (rc == 0 ? size : -1);
 }
 
+/// Gets our fast cache of the socket buffer size
+int getSendBufferSize() const
+{
+return _sendBufferSize;
+}
+
 /// Sets the receive buffer size in bytes.
-/// Must be called before accept or connect.
 /// Note: TCP will allocate twice this size for admin purposes,
-/// so a subsequent call to getSendBufferSize will return
+/// so a subsequent call to getReceieveBufferSize will return
 /// the larger (actual) buffer size, if this succeeds.
 /// Note: the upper limit is set via /proc/sys/net/core/rmem_max,
 /// and there is an unconfigurable lower limit as well.
@@ -185,8 +210,8 @@ protected:
 #if ENABLE_DEBUG
 _owner = std::this_thread::get_id();
 
-const int oldSize = getSendBufferSize();
-setSendBufferSize(0);
+const int oldSize = getSocketBufferSize();
+setSocketBufferSize(0);
 LOG_TRC("Socket #" << _fd << " buffer size: " << getSendBufferSize() 
<< " (was " << oldSize << ")");
 #endif
 
@@ -194,6 +219,7 @@ protected:
 
 private:
 const int _fd;
+int _sendBufferSize;
 // always enabled to avoid ABI change in debug mode ...
 std::thread::id _owner;
 };
@@ -699,7 +725,9 @@ protected:
 ssize_t len;
 do
 {
-len = writeData(&_outBuffer[0], _outBuffer.size());
+// Writing more than we can absorb in the kernel causes SSL 
wasteage.
+len = writeData(&_outBuffer[0], 
std::min((int)_outBuffer.size(),
+ getSendBufferSize()));
 
 auto& log = Log::logger();
 if (log.trace() && len > 0) {
@@ -781,9 +809,12 @@ namespace HttpHelper
 return;
 }
 
-const int socketBufferSize = 16 * 1024;
-if (st.st_size >= socketBufferSize)
-socket->setSendBufferSize(socketBufferSize);
+int bufferSize = std::min(st.st_size, 
(off_t)