Thread-affinity checks must be inhibited
not just on Socket, but on the SocketPoll as well,
before destroying DocumentBroker instances.
Also, properly initialize the inhibit statics.
Change-Id: I2ced1554d477f0c3faf09bda74034cbae99e4ce1
Reviewed-on: https://gerrit.libreoffice.org/37608
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Dung out overlapping return enumerations. Move more work into 'move'
callbacks at a safer time, etc.
Change-Id: I62ba5a35f12073b7b9c8de4674be9dae519a8aca
HTTP HEAD verb requires sending back NO content.
Currently we don't support HEAD and that harms
the browser from using its cache where possible.
Change-Id: I3c67dc106df95312c73f6ae786b7b1657a4167fb
Reviewed-on: https://gerrit.libreoffice.org/36871
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
This slows things down terribly, particularly the setting on the websocket
made tiles appearing one by one. Let's keep the possibility to zero the buffer
sizes for debugging, but hide that behind an env. variable (and in debug
builds only anyway).
Change-Id: Ie4d2cdb3c0ec3c50f1a2b4f9941a462ac4f2d196
The server correctly saves all documents
and waits to upload them before exiting.
Change-Id: I04dc9ce588bc0fa39a9deb298d0a5efa61a03f1a
Reviewed-on: https://gerrit.libreoffice.org/36654
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Without an explicit WS message, the client
does not get this message and the handler
is not invoked at all.
Change-Id: I71e210a9958965cff35dd4d0f1d99985429b82f4
Reviewed-on: https://gerrit.libreoffice.org/36593
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Apparently pinging was enabled only when
_not_ WebSocket upgraded, which is wrong.
Removed sending ping immediately after
upgrading to WS as it's superfluous.
Change-Id: Ic8103bab063d87f58d371f0eab49f7b7530e2374
Reviewed-on: https://gerrit.libreoffice.org/36322
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Don't think it is necessary/useful to have this header at other places.
This is the most important and perhaps the only where presence of this
header is required and seems sensible to prevent potential attacks.
Change-Id: Iad318e4b83264ac83620b86a40a49e7384e4015e
None of the subclasses override it, and if they would, it would be
problematic, since e.g. the StreamSocket dtor calls it (and virtual
calls during dtors are a problem).
Change-Id: Ie0891349808a81539078fd1f2d95a55a4ce5107a
Plenty of time to do that next time around the cleanup.
We should still, really be doing the majority of the timeout work
inside the DocumentBroker poll itself.
First reason is that compression is very slow, and we re-compress the
files again and again.
Another reason is that IE/Edge doesn't work well with deflate turned on.
Related: https://connect.microsoft.com/IE/feedbackdetail/view/950689
The documents are not loaded at all with current code snapshot modulo
this patch.
Change-Id: I1fdd85856f448dc4ce02e1ab79e9c7474c3bb7f3
Only during shutdown do we expect a callback
to invalidate the pollSockets, but if it happens
we shouldn't invoke the poll handlers.
Change-Id: I2f56da19aec2f04cc871bd4eae1f93da110e0eb9
Reviewed-on: https://gerrit.libreoffice.org/36189
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
When shutting down accept_poll from
main, we can't remove sockets or cleanup.
That work needs to be done fro within accept_poll's
thread. This is different from when DocBroker's
poll needs to cleanup its own sockets before
it exists.
So we split the stop and removeSockets so they
can each be called in the proper way.
For accept_poll and others that joinThread
we queue a callback to cleanup before stopping.
Change-Id: If780d6a97ac0fc6da6897f895d5b4dda443f9e73
Reviewed-on: https://gerrit.libreoffice.org/36186
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
And assume correct thread if poll thread is
not running (i.e. no race).
Change-Id: I17958e682aba434ebb47fe0de199b9f530b54dee
Reviewed-on: https://gerrit.libreoffice.org/36183
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
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
The DocBroker might not get a chance to
take ownership of a socket (which is done
via callbacks that are invoked in the poll loop)
if it (or WSD) is flagged for termination.
In that case, DocBroker doesn't take ownership
but ultimately needs to disconnect the socket.
By detaching ownership we signal that any thread
can rightly take ownership and thus avoid spurious
warning or assertions.
Change-Id: Idb192bfaac05c5c86809cb21876f3926a080b775
Reviewed-on: https://gerrit.libreoffice.org/36117
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
And insert sockets after starting the
thread so we poll the socket immediately.
Change-Id: Id336e1838f2f624ebfe59c4c2caf33eaa1a638c9
Reviewed-on: https://gerrit.libreoffice.org/36110
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
...and warn if we are in the wrong thread.
This can happen when the socket is not properly
closed from the poll thread and is being destroyed.
Change-Id: I749c09b15d04b49038f7cee6a7a13e8f0145acff
Reviewed-on: https://gerrit.libreoffice.org/36057
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Valgrind found a number of erroneous data access
during the construction and destruction of SslContext.
Change-Id: Ie5072798a3660ed8acc707ba32ac196fa2d0f8af
Reviewed-on: https://gerrit.libreoffice.org/36055
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
This was a workaround to Poco's limitation
of requiring socket receiveFrame be given
preallocated buffer, which couldn't be
exceeded by a larger payload. This meant
the receiver had to know the maximum
payload in advance.
Since only the Kit uses Poco sockets,
and the Kit never receives large payloads,
this preamble is now obsolete.
100% (94/94) of old-style tests PASS.
Change-Id: I76776f89497409e5755e335a3e25553e91cf0876
Reviewed-on: https://gerrit.libreoffice.org/36037
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Callbacks are used to initialize handlers,
as is the case with addSession on DocumentBroker.
If the socket gets data before the callback is
invoked, the handler will fail since the expected
initialization hasn't happened yet.
This race indeed happens (rarely) with addSession.
100% (94/94) of old-style tests PASS.
Change-Id: Id9b4f63b45c5564add252e1671b7b0b08aff8150
Reviewed-on: https://gerrit.libreoffice.org/36035
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
==20033== Invalid read of size 4
==20033== at 0x466504: ChildProcess::close(bool) (DocumentBroker.hpp:111)
==20033== by 0x44EA28: DocumentBroker::terminateChild(std::string const&, bool) (DocumentBroker.cpp:1313)
==20033== by 0x45F70E: DocumentBroker::pollThread() (DocumentBroker.cpp:264)
==20033== by 0x504B2F: SocketPoll::pollingThreadEntry() (Socket.hpp:486)
==20033== by 0x7310E6F: execute_native_thread_routine (thread.cc:84)
==20033== by 0x7AF60A3: start_thread (pthread_create.c:309)
==20033== by 0x7DF002C: clone (clone.S:111)
==20033== Address 0x0 is not stack'd, malloc'd or (recently) free'd
When not sending ping the ping time is not set
which results in the setting the poll timeout to
a negative value, forcing it to return immediately.
This happens when sending ping before upgrading
to WebSocket, which isn't common. One way to
reproduce it, however, is to connect to the
admin console with an unauthenticated socket.
Change-Id: I9f3db1a02b8f8e2781d23d843e848068ad434958
This prevents a race where the thread is started
a second time before the first gets a chance to
set the flag.
Change-Id: Ib106aa0626cdfa403b321822180b0545d3aa9139
Once a socket has changed ownership to a new
poll it will assert thread affinity with said
new poll. So we cannot do any IO on the old
poll's thread at that point and on.
Change-Id: I662f188dea7c377a18f3e546839ec43f2875dc7b
This reverts commit 388d7b1dbf.
It is vital to have clean control of thread start. By starting
a thread during init. of a member (or base-clase) we loose lots of
control, some examples:
Any members initialized after a member that auto-starts a
thread, is effectively un-defined, and cannot be accessed
reliably.
This is particularly problematic for sub-classes.
I've seen threads started before the base-class has
finished constructing in the original creating thread -
such that the vtable was not yet updated to the sub-class
causing the transient parent vtable used during construction
to be used in-error.
Now that DocumentBroker has SocketPoll thread,
it's isAlive() must be defined by the lifetime of
both the SocketPoll thread and the ChildProcess,
which it previously did.
Change-Id: I093f8774cf4374d01729a383f6c535de4143fec6
Reviewed-on: https://gerrit.libreoffice.org/35122
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
We only move sockets in response to input on the socket, which
must happen in the thread that we're processing that input on.
Ergo - no need for this complexity.
The situation made more problematic since the std::thread is only
created in startThread - so getting ownership right before then in
Socket is problematic.
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.
As there isn't support (yet) to send files
asynchronously, when the socket native buffer
is small, asynchronous writes naturally return
EWOULDBLOCK. As a temp solution, we send files
synchronously, so there is no need to poll.
This should be replaced witha file-server
polling/serving thread that is dedicated to
sending files only (which closes the connection
when done).
Change-Id: I062fea44bfe54ab8d147b745da97bd499bf00657
SSL only requests what to poll for next.
So it's more accurate to rename ReadOrWrite
to Neither, since in that case SSL really
isn't blocked on either read or write.
Change-Id: I62dd4f94730d51666a7661b10a9d582d69fbf45e
No longer necessary to have them enabled
permanently, especially that they affect
performance significantly negatively.
Change-Id: I02ef99da00ba4ecb8e24647ee372a03d79d07fe7
There are other types of frames than application
data.
Also a message can be composed of multiple frames.
Change-Id: Ia97349553b61ae05fa78854222808eaa43386c0e
These are really GET requests that aren't
WebSocket upgrade. Should rename to something
less misleading.
Re-enabled testSlideShow which depended on this.
Change-Id: I52b7f67b650fcdcbae7c2bff020b756099263141
It's necessary to do the SSL handshake
and to get the request (if we get there)
on accepting so by the time we poll we know
what SSL needs to do next. No reason to
read on first poll when we should be
expecting a request upon connection anyway.
Change-Id: I8eecaad5f8450075d45e487702972418cad125bc
Since a socket client can push data into
the socket in a different thread than the one
polling (indeed that's the only possible scenario),
the write buffer must be protected by a lock.
On the other hand, the read buffer is always
invoked from a single thread, the polling. So
it is perfectly safe without locks.
Change-Id: Id0b6a01f8e96124a299810f0aacab9cecd1ff979
Because the socket can be freed while a separate
thread is sending data via the handler, we must
have a locked reference to the socket instance
in the handler.
Change-Id: Iefad3fc2b147f96b8d538d9edd7cac3fce25b5bf
We need to flush writes to socket as soon as
ready to either send or, if buffers are full,
to poll for write.
With WebSocket we do this after writing a frame.
Change-Id: I1bc276e678375a84079e69624414a16271f25351
Requests need to be introspected and
dispatched to the appropriate handler.
File serving, admin, POST request, etc.
are all valid types that we need to support.
But of course the primary one is the WS request
to load to and interact with a document.
Change-Id: Id2c3214deb6b54b06b2735ec3370f09ed7a1ae51
We don't need a special "WebSocket" class, as websocket itself is just an
upgrade of an existing socket / connection, and trying to come up with a
concept where a Socket class magically upgrades to a WebSocket class would be
messy.
So let's have just a WebSocketHandler, that communicates over a StreamSocket
or SslStreamSocket, and be done with that :-)
Change-Id: I449e4d662fbe2c5b1789e308053e4a71376fe481
Introduce the appropriate interface instead of the template, so that we can
de-couple the ResponseClient from the Socket itself.
Change-Id: I21892bbce5d83a18ca580a25cfc2efbb05b08705
SSL's decryption frame is 16kb, so it
makes sense to read by that multiple
for efficiency reasons.
Change-Id: Ib5d7b5bfc18fa1b27bcbc9aac93350c4e1292874
Reviewed-on: https://gerrit.libreoffice.org/34451
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
We now always read new data, because
SSL might need to process internal buffers,
and call the app message handler as long
as it keeps consuming data.
Finally, we write when we have data
to flush, even if we didn't get POLLOUT
event, because SSL might need to do
handshake, which is handled by the write
handler.
Change-Id: I909333c3a0492b0830044d51ae478ac5981b1d90
Reviewed-on: https://gerrit.libreoffice.org/34448
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
We expect to have more data if the buffer was
filled completely. Otherwise, there is no
reason to suspect there is more data, unless
we hope to have received time in that short
interval, which is not very likely.
Change-Id: I9a3dcb6378e8dcfa0d49b6c7ea92d7cec6690787
Reviewed-on: https://gerrit.libreoffice.org/34446
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
The new test sends data of of 1 byte to
N bytes and expects the exact same data
returned in response.
This tests both buffering sizes and
websocket frames.
Change-Id: Ic6232b4e899d82d90a0ee7c96e4852ffaaf8e958
Reviewed-on: https://gerrit.libreoffice.org/34441
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
SSL expects the write buffer to be the same on
retry. This requirement cannot be guaranteed
when using standard containers as the internal
memory might be re-allocated between calls.
Luckily SSL can relax this requirement when asked.
Change-Id: I26771609a8cc0f9de0e554f9fa255d998f6c7b14
Reviewed-on: https://gerrit.libreoffice.org/34440
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Draining the buffer is the correct appraoch
to avoid accumulating data in the kernel
unnecessarily. But it also reduces the overhead
of reparsing the incoming data for message
boundaries.
The client that is to parse the data should
parse as many messages as possible and remove
them from the socket buffer.
Finally, we probably need to cap the maximum
buffer size to avoid bloating. However this
heavily depends on the application's max
message size, assuming there is no separate
buffer beyond that of the socket to accumulate
the messages in.
Change-Id: I49c4eccebd474cd07ca84f3f4eae33bc717ed1f2
Reviewed-on: https://gerrit.libreoffice.org/34411
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Depending on the base class, which is passed as
template parameter, SimpleResponseClient can
abstract away the underlying socket type.
Change-Id: I59a403357512f329aa2565c1ef55094704e3b4ad
Reviewed-on: https://gerrit.libreoffice.org/34393
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
