Previously, an IPC connection error could shut down the entire process
without giving a hint as to what's wrong. Now, we report that error to
the debug console.
Previously we would shut down an ipc connection regardless of if there
were still bytes that have been read and not been handed over to
processing, causing WindowServer not to receive
WindowServer::SetFlashFlush messages sent by `wsctl -f` except the first
one.
This patch fixes that behavior by still shutting the connection down due
to having reached EOF while also processing remaining bytes.
Resolves#12954
See also #8912 which fixes the same issue that this patch fixes but also
seems to have initially broken SettingsWindow cancel not actually
closing the window unless the cursor got moved as described in #12003.
Pull request #12547 fixing the SettingsWindow behavior broke `wsctl`
again by always shutting down.
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. :^)
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 lets us avoid using Core::deferred_invoke() which is not usable
during application teardown (as there is no event loop to push the
deferred invocation onto.)
(Not that there is an event loop to fire the processing timer during
teardown *either*, but at least we can exit gracefully with pending
timers, unlike deferred invocations, which hang the process. This is an
area where more improvements are definitely needed!)
This patch moves the templated message parsing code to a virtual
try_parse_messages() helper. By doing that, we can move the rest of the
socket draining code up to ConnectionBase and keep it out of line.
This patch splits IPC::Connection into Connection and ConnectionBase.
ConnectionBase moves into Connection.cpp so we don't have to inline it
for every single templated subclass.
