Reading from a socket with a dead peer returns the `ECONNRESET` error
code in some cases on Linux. This commit changes LibIPC to gracefully
shut down the socket if that happens, fixing an occasional crash in
Ladybird.
FixesSerenityOS/ladybird#116
These lambdas were marked mutable as they captured a Ptr wrapper
class by value, which then only returned const-qualified references
to the value they point from the previous const pointer operators.
Nothing is actually mutating in the lambdas state here, and now
that the Ptr operators don't add extra const qualifiers these
can be removed.
In order to avoid the base encode/decode methods from being used (and
failing a static assertion), we must be sure to declare/define the
custom type implementations as template specializations.
After this, LibIPC is no longer sensitive to include order.
For the most part, we try to provide specializations of these functions
in various headers by including "LibIPC/Forward.h" and then declaring
encode() and decode() specializations. However, without any forward
declaration of these types, we aren't actually specializing anything.
Rather, we are just declaring overloads, which trips up the base encode
and decode template definitions.
The result is that LibIPC is very sensitive to include order, and the
DependentFalse<> static assertion would fail if the includes weren't
perfectly ordered.
By properly forward declaring these templates, we can make sure the
specializations receive precedence over the base templates.
Otherwise, we end up propagating those dependencies into targets that
link against that library, which creates unnecessary link-time
dependencies.
Also included are changes to readd now missing dependencies to tools
that actually need them.
Even though the toolchain implicitly links against -lc, it does not know
where it should get LibC from except for the sysroot. In the case of
Clang this causes it to pick up the LibC stub instead, which might be
slightly outdated and feature missing symbols.
This is currently not an issue that manifests because we pass through
the dependency on LibC and other libraries by accident, which causes
CMake to link against the LibC target (instead of just the library),
and thus points the linker at the build output directory.
Since we are looking to fix that in the upcoming commits, let's make
sure that everything will still be able to find the proper LibC first.
This will allow Ladybird to use IPC::Connection without having an
actively running Core::EventLoop.
The abstraction here is not great, and we should think of something
nicer, but we have to start somewhere.
Our IPC protocol currently relies on the behavior of recvfd() and
sendfd() on SerenityOS, which provide an out-of-band queue that can be
accessed independently of the in-band data stream.
To make LibIPC usable on other platforms, this patch adds a mechanism
where IPC::Connection can be given a dedicated socket for FD passing.
This gives us the same behavior as the syscalls on SerenityOS, without
having to change the protocol implementation.
This commit does three things atomically:
- switch over Core::Account+SystemServer+LoginServer to sid based socket
names.
- change socket names with %uid to %sid.
- add/update necessary pledges and unveils.
Userland: Switch over servers to sid based sockets
Userland: Properly pledge and unveil for sid based sockets
This remained undetected for a long time as HeaderCheck is disabled by
default. This commit makes the following file compile again:
// file: compile_me.cpp
#include <LibDNS/Question.h>
// That's it, this was enough to cause a compilation error.
Likewise for most other files touched by this commit.
This patch allows to insert "%uid" in `IPC_CLIENT_CONNECTION`
declaration and in SystemServer's ini files. This pattern is replaced
then replaced by the UID of the owner of the service. It opens a path
for seamlessly managed, per-user portal.
This is a quick and dirty fix to at least get the Inspector working
again. This is a very bad solution long term, because it will spin on
the EventLoop. That is why there is a message reminding everyone this
problem still needs to be fixed, every time this is actually done.
Error::from_string_literal now takes direct char const*s, while
Error::from_string_view does what Error::from_string_literal used to do:
taking StringViews. This change will remove the need to insert `sv`
after error strings when returning string literal errors once
StringView(char const*) is removed.
No functional changes.
This commit moves the length calculations out to be directly on the
StringView users. This is an important step towards the goal of removing
StringView(char const*), as it moves the responsibility of calculating
the size of the string to the user of the StringView (which will prevent
naive uses causing OOB access).
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.
This functionality is required by Core::LocalServer and LibIPC depends
on LibCore.
take_over_accepted_socket_from_system_server has also been renamed to
take_over_socket_from_system_server as the socket need not be accepted
before taking it over. :^)
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 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. :^)
This encapsulates what our multi-client IPC servers typically do on
startup:
1. Create a Core::LocalServer
2. Take over a listening socket file descriptor from SystemServer
3. Set up an accept handler for incoming connections
IPC::MultiServer does all this for you! All you have to do is provide
the relevant client connection type as a template argument.
Before this, we only had ClientConnection::did_misbehave() to report an
error and shut the connection down. But it's not fair to say that *all*
errors are the client misbehaving! A typical non-misbehavior is resource
allocation failure on the server side.
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.
