Provide a never-reused PID alternative for all Gecko processes
Categories
(Core :: IPC, enhancement)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox131 | --- | fixed |
People
(Reporter: nika, Assigned: nika)
References
(Blocks 1 open bug)
Details
Attachments
(5 files)
Currently Gecko frequently uses the native OS-level PID for many different checks, including comparing process identities for IPC. This is largely because it is relatively easily available both for the current process and for other processes (thanks to the [NeedsOtherPid] attribute on IPDL actors), and because we don't provide a simple ID-based alternative value.
This bug is to introduce a new GeckoChildID value which can be used for this purpose. This is inspired by the existing ChildID value on ContentParent and ContentChild which can be used to uniquely and in a non-reused way identify content processes.
The approach of using PIDs has 2 major pitfalls which motivated this:
- The pid value can be re-used if a process exits or crashes, meaning that any stale references to the pid could be confused.
- In a situation where pid-namespacing is in use, the pid value could be incorrect or inconsistent (due to multiple namespaces) between processes, meaning that the values would not compare equal when they should and vice-versa.
This manual approach should avoid both issues.
|
Assignee | |
Comment 1•2 months ago
|
||
The new GeckoChildID type introduced in this patch is inspired by the existing
ContentParentID type used by ContentParent, but is currently distinct. It is
supported by all process types at the GeckoChildProcessHost level and can be
read for the current process from anywhere.
As this type is similar in many ways to the process type, and should be
available as early as possible within child processes, this was added alongside
the GeckoProcessType value within mozglue to make that easier to do.
The type was chosen to be an int32_t to make it feel similar to a PID, which we
currently use for process identity comparisons across the codebase. The
intention is for GeckoChildID to be preferred for these within-gecko checks, as
these IDs will not be re-used and can be known earlier during child process
creation.
|
|
Assignee | |
Comment 2•2 months ago
|
||
This requires quite a bit of piping to get the ChildID passed everywhere where
we currently pass the pid in IPC. This is done by adding a new struct type
(EndpointProcInfo), which is passed around instead of OtherPid in these places,
and contains the full pid.
In most cases, it was a fairly painless change to move over, however in some
cases, more complex changes were required, as the pid was being stored
previously in something like an Atomic<...>, and needed to be switched to using
a mutex-protected value.
In the future, it may be possible to remove OtherPid from IPDL actors once
everything is migrated to ChildID, but we're still a long way off from that, so
for now we unfortunately need to pass both around.
Depends on D217117
|
|
Assignee | |
Comment 3•2 months ago
|
||
The type for ContentParentId is uint64_t, however only at most 22 bits
are allowed to be used (as it is used in the high bits of nsContentUtils
process-specific IDs, which need to fit within a JS double). Switching to use
the same existing ChildID reduces the risk of confusion, but this patch doesn't
attempt to update the types used for ContentParentId/ChildID to match
GeckoChildID.
In the future, we probably will want to align these types more closely, and
perhaps de-duplicate some code which currently passes around both.
Depends on D217118
|
|
Assignee | |
Comment 4•2 months ago
|
||
There are many other uses of OtherPid which could be switched over to
OtherChildID, but these were a couple of obvious low-hanging fruit use-cases
which will work better when using OtherChildID.
Depends on D217119
|
||
Updated•2 months ago
|
|
|
Assignee | |
Comment 5•2 months ago
|
||
This patch adjusts the various places where we initialize content
processes to call SetGeckoProcessType as early as possible, and be more
consistent. After this change we should only ever set GeckoProcessType
and GeckoChildID once per-process (with the exception of the fork server
process).
In addition to this validation, some more checks around the fork server
were added, such as to prevent forking another forkserver, or forking a
non-content process.
As part of this change, there was some refactoring/cleanup done, such as
removing plugin-container.cpp and content_process_main, as compared to
the other duplicated code between the two call-sites, the duplication
was relatively small, and inlining it helped make things more readable.
Depends on D217120
https://hg.mozilla.org/mozilla-central/rev/32a47adad8f7
https://hg.mozilla.org/mozilla-central/rev/b29d26e1f91b
https://hg.mozilla.org/mozilla-central/rev/ec8589c662ea
https://hg.mozilla.org/mozilla-central/rev/f531a0c30736
https://hg.mozilla.org/mozilla-central/rev/43335f6cff94
Description
•