Given that libevent's signal handling code is known to have race
conditions, and there are fundamental issues that make it hard to fix
upstream, and previous patches have removed our last usage of it, we
should assert that it's no longer used.
Differential Revision: https://phabricator.services.mozilla.com/D141312
The function DidProcessCrash is now dead code. Before the ProcessWatcher
rewrite, its return value (i.e., whether the process crashed) was never
used, so effectively its only purpose was to make it harder to understand
where the waitpid calls were happening.
Differential Revision: https://phabricator.services.mozilla.com/D141310
This patch rewrites the Unix backend of ProcessWatcher for two reasons:
1. To remove the use of libevent's signal handling, which has concurrency
bugs that can't be easily fixed upstream (see Bugzilla for details)
2. To simplify the code in general; in particular, the new version has one
place where the process and its exit status are consumed from the OS
The new implementation uses the same pipe-to-self technique as libevent
(and which we use elsewhere) to deal with async signal safety. Unlike
the previous version, there is a single object which manages all
monitored child processes rather than one each. (Previously, this
multiplexing was done inside libevent.)
Differential Revision: https://phabricator.services.mozilla.com/D141309
This type is also used in other places to start non-initial actors, and will
allow us to attach additional state more easily without needing to thread it
through every child process callsite manually.
Differential Revision: https://phabricator.services.mozilla.com/D153618
Currently, Subprocess.jsm on Unix uses js-ctypes to call `posix_spawn`.
This has some issues, primarily that file descriptors are inherited by
the child process unless explicitly opted-out, which unfortunately a lot
of code doesn't do. This patch changes it to use IPC process launching,
where fd inheritance is opt-in, by:
1. Extending `base::LaunchApp` to handle a few features that Subprocess
needs (setting the process's working directory, specifying the full
environment, and the macOS `disclaim` flag)
2. Adding a WebIDL method to `IOUtils` to expose that function to JS
(currently Unix-only; Windows could also be supported but it would
probably want to use a different IDL type for strings, given that the
OS APIs use 16-bit code units).
3. Replacing the part of Subprocess that invokes `posix_spawn` (and
related functions) by calling that method; the rest of Subprocess's
machinery to manage pipes and I/O is unchanged. (The Windows backend
is also unchanged; I'm not aware of any functional issues there.)
This results in some dead code, which is removed.
Differential Revision: https://phabricator.services.mozilla.com/D152336
gcc -std=c++20 (but not clang -std=c++20) complains about template class definitions that specify the template parameter on the class constructor. For example:
template <typename T>
class C {
C(int x) {}
// OK in clang and gcc for both -std=c++17 and -std=c++20.
C<T>(int x, int y) {}
// OK in clang for both -std=c++17 and -std=c++20.
// OK in gcc -std=c++17 but a build error with -std=c++20.
};
The ipc/chromium build error because the `DISALLOW_COPY_AND_ASSIGN` macro expands to such a template class definition:
ipc/chromium/src/base/basictypes.h:53:12: error: expected unqualified-id before 'const'
| TypeName(const TypeName&) = delete
| ^~~~~
ipc/chromium/src/base/threading/thread_local.h:95:3: note: in expansion of macro 'DISALLOW_COPY_AND_ASSIGN'
| DISALLOW_COPY_AND_ASSIGN(ThreadLocalPointer<T>);
| ^~~~~~~~~~~~~~~~~~~~~~~~
Google replaced chromium/base's DISALLOW_COPY_AND_ASSIGN macros with explicitly deleted constructors, so let's apply their upstream fix:
7319bbdb7d
Differential Revision: https://phabricator.services.mozilla.com/D150216
There are two parts to this patch; both affect only Linux:
1. The GMP sandbox policy is adjusted to allow certain syscalls used in
shared memory creation (ftruncate and fallocate). However, the file
broker is not used; the process still has no access to files in /dev/shm.
2. The profiler is not initialized for GMP processes unless memfd_create
is available (so the process can create shared memory to send
profiling data back, without filesystem access), or the GMP sandbox
is disabled (either at runtime or build time).
As of this patch, profiling GMP processes on Linux should succeed on
distros with kernel >=3.17 (Oct. 2014), but native stack frames won't
have symbols (and may be incorrectly unwound, not that it matters much
without symbols); see the bug for more info. Pseudo-stack frames and
markers should work, however.
Differential Revision: https://phabricator.services.mozilla.com/D148470
Before this change, we wouldn't re-try sending fds if the first attempt
to send them failed, meaning that some fds wouldn't arrive if there was
any error sending (e.g. because the send buffer was full, which
is more common on macOS).
This new approach ensures we don't record that we've sent the fds until
the message is marked as successful, and should avoid the macOS errors.
Depends on D145392
Differential Revision: https://phabricator.services.mozilla.com/D146621
This is made possible by part 1, which made it possible to send more messages
using IPC::Channel. A limit is still in place, however it is now substantially
higher, hopefully making it effectively unlimited for practical purposes.
Differential Revision: https://phabricator.services.mozilla.com/D145392
This is done by splitting messages with large numbers of handles into multiple
`sendmsg` calls, each of which contains less than the maximum number of
transferred handles per-message, and stitching the message back together on the
receiving side. Most of the work on the receiving side was already handled by
the IPC::Channel code, so the work required was only to ensure we could split
the handle list up when sending.
Differential Revision: https://phabricator.services.mozilla.com/D145391
This makes passing around the type more consistent, and hopefully will make
changes to IPC::Message easier to work with in the future.
In addition, this should save us a few copies as we move the message type into
and out of UniquePtr, however I expect this won't make much of a difference.
Differential Revision: https://phabricator.services.mozilla.com/D145885
This makes passing around the type more consistent, and hopefully will make
changes to IPC::Message easier to work with in the future.
In addition, this should save us a few copies as we move the message type into
and out of UniquePtr, however I expect this won't make much of a difference.
Differential Revision: https://phabricator.services.mozilla.com/D145885
dom/media/ipc/RDDProcessManager.cpp(320,21): error: comparison of integers of different signs: 'base::ProcessId' (aka 'unsigned long') and 'int' [-Werror,-Wsign-compare]
gpuProcessPid != -1 ? gpuProcessPid : base::GetCurrentProcId();
~~~~~~~~~~~~~ ^ ~~
dom/media/ipc/RDDProcessManager.cpp(332,21): error: comparison of integers of different signs: 'base::ProcessId' (aka 'unsigned long') and 'int' [-Werror,-Wsign-compare]
if (gpuProcessPid != -1) {
~~~~~~~~~~~~~ ^ ~~
gfx/layers/ipc/SharedSurfacesParent.cpp(360,38): error: comparison of integers of different signs: 'base::ProcessId' (aka 'unsigned long') and 'int' [-Werror,-Wsign-compare]
if (!gpm || gpm->GPUProcessPid() != -1) {
~~~~~~~~~~~~~~~~~~~~ ^ ~~
ipc/glue/MessageChannel.cpp(2145,13): error: comparison of integers of different signs: 'int32_t' (aka 'int') and 'const base::ProcessId' (aka 'const unsigned long') [-Werror,-Wsign-compare]
if (pid != base::kInvalidProcessId &&
~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~
Differential Revision: https://phabricator.services.mozilla.com/D144688
dom/system/PathUtils.cpp(77,10): error: 'return' will never be executed [-Werror,-Wunreachable-code-return]
return false;
^~~~~
ipc/chromium/src/chrome/common/ipc_channel_win.cc(479,10): error: 'return' will never be executed [-Werror,-Wunreachable-code-return]
return true;
^~~~
mozglue/misc/PreXULSkeletonUI.cpp(1263,10): error: 'return' will never be executed [-Werror,-Wunreachable-code-return]
return 0;
^
mozglue/tests/TestPEExportSection.cpp(348,12): error: 'return' will never be executed [-Werror,-Wunreachable-code-return]
return 0;
^
security/manager/ssl/OSReauthenticator.cpp(428,10): error: 'return' will never be executed [-Werror,-Wunreachable-code-return]
return NS_OK;
^~~~~
toolkit/components/maintenanceservice/maintenanceservice.cpp(214,10): error: 'return' will never be executed [-Werror,-Wunreachable-code-return]
return 0;
^
widget/windows/WindowsUIUtils.cpp(383,10): error: 'return' will never be executed [-Werror,-Wunreachable-code-return]
return false;
^~~~~
Differential Revision: https://phabricator.services.mozilla.com/D144661
Due to the way that ports are locked (occasionally in large groups in
address order) these mutexes seem to occasionally overwhelm the deadlock
detector with very long chains of lock dependencies, sometimes leading
to stack overflow issues.
As the imported code from chromium is already quite careful about
avoiding deadlocks with these locks, this patch switches them to use a
custom mutex which is not registered with the deadlock detector to avoid
these issues.
Differential Revision: https://phabricator.services.mozilla.com/D142130
Unfortunately the locking behaviour in ports around the Port type is a bit too
complex to easily express in a way which satisfies the thread safety analysis,
so it was not annotated.
Differential Revision: https://phabricator.services.mozilla.com/D141536
Unfortunately the locking behaviour in ports around the Port type is a bit too
complex to easily express in a way which satisfies the thread safety analysis,
so it was not annotated.
Differential Revision: https://phabricator.services.mozilla.com/D141536
And in one case, #include "mozilla/ProfilerThreadState.h" where only `AUTO_PROFILER_THREAD_WAKE` is used.
Depends on D140172
Differential Revision: https://phabricator.services.mozilla.com/D140173
Norisz Fay
Jed Davis
Nika Layzell
Mike Hommey
Fabrice Desré
Andreea Pavel
Chris Peterson
serge-sans-paille
Noemi Erli
Christian Holler
Ryan VanderMeulen
Florian Queze
Cristian Tuns
Randell Jesup
Nazım Can Altınova
Butkovits Atila
Gerald Squelart