I saw what looked like a UAF of this URL in a RequestServer crash,
and it seems reasonable to make a copy here since we end up passing
them to Core::deferred_invoke().
`it.is_end()` could be updated to return false for a previously-invalid
iterator after we append a new socket, copy its value out to a local
variable to not hit this behaviour.
There's a possible window where the notifications are disabled, and any
request coming at that time will never get any data if it relies on
socket notifications.
This commit converts TLS::TLSv12 to a Core::Stream object, and in the
process allows TLS to now wrap other Core::Stream::Socket objects.
As a large part of LibHTTP and LibGemini depend on LibTLS's interface,
this also converts those to support Core::Stream, which leads to a
simplification of LibHTTP (as there's no need to care about the
underlying socket type anymore).
Note that RequestServer now controls the TLS socket options, which is a
better place anyway, as RS is the first receiver of the user-requested
options (though this is currently not particularly useful).
We previously only replaced disconnected sockets on the queued-request
path, leading to attempts to send requests on a disconnected socket if
the disconnection happened in the deletion grace period.
This reverts most of commit ede5c9548e.
The one change not reverted is ClockWidget.h, so that the taskbar clock
can continue to notice time zone changes.
In most applications, we invoke tzset once at startup for now. Most of
these are short lived and don't need to know about time zone changes.
The exception is the ClockWidget in the taskbar. Here, we invoke tzset
each time we update the system time. This way, any time zone changes can
take effect immediately.
Apologies for the enormous commit, but I don't see a way to split this
up nicely. In the vast majority of cases it's a simple change. A few
extra places can use TRY instead of manual error checking though. :^)
vdbgln() was responsible for ~10% of samples on pv's flamegraph for
RequestServer (under request_did_finish) when loading github.com in
Browser and recording a whole-system profile. This makes that almost
completely disappear.
This change unfortunately cannot be atomically made without a single
commit changing everything.
Most of the important changes are in LibIPC/Connection.cpp,
LibIPC/ServerConnection.cpp and LibCore/LocalServer.cpp.
The notable changes are:
- IPCCompiler now generates the decode and decode_message functions such
that they take a Core::Stream::LocalSocket instead of the socket fd.
- IPC::Decoder now uses the receive_fd method of LocalSocket instead of
doing system calls directly on the fd.
- IPC::ConnectionBase and related classes now use the Stream API
functions.
- IPC::ServerConnection no longer constructs the socket itself; instead,
a convenience macro, IPC_CLIENT_CONNECTION, is used in place of
C_OBJECT and will generate a static try_create factory function for
the ServerConnection subclass. The subclass is now responsible for
passing the socket constructed in this function to its
ServerConnection base; the socket is passed as the first argument to
the constructor (as a NonnullOwnPtr<Core::Stream::LocalServer>) before
any other arguments.
- The functionality regarding taking over sockets from SystemServer has
been moved to LibIPC/SystemServerTakeover.cpp. The Core::LocalSocket
implementation of this functionality hasn't been deleted due to my
intention of removing this class in the near future and to reduce
noise on this (already quite noisy) PR.
This is an encapsulation of the common work done by all of our
single-client IPC servers on startup:
1. Create a Core::LocalSocket, taking over an accepted fd.
2. Create an application-specific ClientConnection object,
wrapping the socket.
It's not a huge change in terms of lines saved, but I do feel that it
improves expressiveness. :^)
These ones all manage their storage internally, whereas the WebContent
and ImageDecoder ones require the caller to manage their lifetime. This
distinction is not obvious to the user without looking through the code,
so an API that makes this clearer would be nice.
With this change, System::foo() becomes Core::System::foo().
Since LibCore builds on other systems than SerenityOS, we now have to
make sure that wrappers work with just a standard C library underneath.
Derivatives of Core::Object should be constructed through
ClassName::construct(), to avoid handling ref-counted objects with
refcount zero. Fixing the visibility means that misuses like this are
more difficult.
Until we're confident that RequestServer doesn't need this runtime debug
dump helper, it's much nicer if everyone has it built in, so they can
simply send a SIGINFO if they see it acting up.
Unused sockets created by EnsureConnection should not keep the socket
around (storing a strong reference will create a reference cycle).
This fixes a whole bunch more RS spins.
Just as removing individual connections can cause the vector entries to
change positions, adding or removing connections to the cache can also
move the connections around, which would make it possible for a
connection to avoid being deleted (and make the RS spin on the Notifier
for that connection).
This commit makes it so that no connection cache is left when it's
supposed to be deleted.
Fixes a few more RS spins.
Otherwise we'd end up trying to delete the wrong connection if a
connection made before us is deleted.
Fixes _some_ RequestServer spins (though not all...).
This commit also adds a small debug mechanism to RequestServer (which
can be enabled by turning REQUEST_SERVER_DEBUG on), that can dump all
the current active connections in the cache, what they're doing, and how
long they've been doing that by sending it a SIGINFO.
We need to set the root certificates, and tell the connection cache that
the preconnect job finished (otherwise it would spin forever, waiting
for us to tell it that).
This makes connections (particularly TLS-based ones) do the handshaking
stuff only once.
Currently the cache is configured to keep at most two connections evenly
balanced in queue size, and with a grace period of 10s after the last
queued job has finished (after which the connection will be dropped).
I broke this when I made the protocol objects be wrapped by smart
pointers to appease static analysis.
The Protocol base class currently VERIFY's that it's never called.
So to have the best of both worlds until someone actually fixes
the code to do proper de-registration, just call `exit(..)` so the
smart pointers never go out of scope.
