In part 1, a fallback was added to allow message buffers which would be sent as
shmem to be sent inline if shmem allocation or mapping failed. This could
potentially lead to an increase in message size too large crashes, as these
messages are now being sent inline again.
This patch adds an extra crash annotaion such that failures of this kind can be
identified in socorro.
Depends on D209880
Differential Revision: https://phabricator.services.mozilla.com/D209881
This changes comes with several different refactorings all rolled into one,
unfotunately I couldn't find a way to pull them apart:
- First of all annotations now can either recorded (that is, we copy the value
and have the crash reporting code own the copy) or registered. Several
annotations are changed to use this functionality so that we don't need to
update them as their value change.
- The code in the exception handler is modified to read the annotations from
the mozannotation_client crate. This has the unfortunate side-effect that
we need three different bits of code to serialize them: one for annotations
read from a child process, one for reading annotations from the main process
outside of the exception handler and one for reading annotations from the
main process within the exception handler. As we move to fully
out-of-process crash reporting the last two methods will go away.
- The mozannotation_client crate now doesn't record annotation types anymore.
I realized as I was working on this that storing types at runtime has two
issues: the first one is that buggy code might change the type of an
annotation (that is record it under two different types at two different
moments), the second issue is that types might become corrupt during a
crash, so better enforce them at annotation-writing time. The end result is
that the mozannotation_* crates now only store byte buffers, track the
format the data is stored in (null-terminated string, fixed size buffer,
etc...) but not the type of data each annotation is supposed to contain.
- Which brings us to the next change: concrete types for annotations are now
enforced when they're written out. If an annotation doesn't match the
expected type it's skipped. Storing an annotation with the wrong type will
also trigger an assertion in debug builds.
Differential Revision: https://phabricator.services.mozilla.com/D195248
This refactoring cleans up some dead code, and makes some semantic
changes to how the MessageChannel lifecycle is handled.
These changes ensure that messages which were sent by a peer before the
GOODBYE message will be delivered, without allowing messages sent after
the GOODBYE message (e.g. by a misbehaving process) to be delivered.
The lifecycle and shutdown states were simplified, and moved to be
entirely in MessageChannel, rather than split between MessageChannel and
MessageLink.
The dead-code ChannelTimeout error state was removed, along with the
corresponding CloseWithTimeout method.
The CloseWithError method was updated to behave more consistently with
the normal Close method, synchronously triggering a connection error,
and closing the MessageLink. This method is currently unused, but will
useful in the future for handling processing errors.
Differential Revision: https://phabricator.services.mozilla.com/D178382
This refactoring cleans up some dead code, and makes some semantic
changes to how the MessageChannel lifecycle is handled.
These changes ensure that messages which were sent by a peer before the
GOODBYE message will be delivered, without allowing messages sent after
the GOODBYE message (e.g. by a misbehaving process) to be delivered.
The lifecycle and shutdown states were simplified, and moved to be
entirely in MessageChannel, rather than split between MessageChannel and
MessageLink.
The dead-code ChannelTimeout error state was removed, along with the
corresponding CloseWithTimeout method.
The CloseWithError method was updated to behave more consistently with
the normal Close method, synchronously triggering a connection error,
and closing the MessageLink. This method is currently unused, but will
useful in the future for handling processing errors.
Differential Revision: https://phabricator.services.mozilla.com/D178382
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
This removes the last form of unique link between two MessageChannels so that
all MessageChannels communicate using PortLink, as it is fairly straightforward
to use PortLink to communicate between two threads in-process.
Differential Revision: https://phabricator.services.mozilla.com/D116672
The NodeController and NodeChannel types act as the backbone connecting the
existing IPC logic and driving the ports routing code. Individual NodeChannel
objects wrap and respond to messages from IPC::Channel, and the NodeController
orchestrates all messaging for a process.
The design of these types are inspired by the types with the same names from
Mojo but have been simplified and streamlined to only support features used by
Gecko.
Support for attaching ports or handles to messages hasn't been added yet, but
can be added in follow-up patches.
Differential Revision: https://phabricator.services.mozilla.com/D112775
This removes the last form of unique link between two MessageChannels so that
all MessageChannels communicate using PortLink, as it is fairly straightforward
to use PortLink to communicate between two threads in-process.
Differential Revision: https://phabricator.services.mozilla.com/D116672
The NodeController and NodeChannel types act as the backbone connecting the
existing IPC logic and driving the ports routing code. Individual NodeChannel
objects wrap and respond to messages from IPC::Channel, and the NodeController
orchestrates all messaging for a process.
The design of these types are inspired by the types with the same names from
Mojo but have been simplified and streamlined to only support features used by
Gecko.
Support for attaching ports or handles to messages hasn't been added yet, but
can be added in follow-up patches.
Differential Revision: https://phabricator.services.mozilla.com/D112775
The change to MessageChannel::Clear() makes mLink get cleared before
we call ~ThreadLink. This causes a race because Clear() is not
holding the monitor. To work around this, I introduced a new method
PrepareToDestroy() that handles the ThreadLink splitting. Once the
ThreadLinks are split, MessageChannel can clear mLink without a
race.
An alternative approach would be to hold the monitor in Clear()
before mLink is cleared, but then we'd end up acquiring the lock
when we didn't need to in the case where mLink is a ProcessLink.
Differential Revision: https://phabricator.services.mozilla.com/D79185
We are seeing crashes on aarch64 Fenix devices that appear to be related
to zero-sized messages. But we're seeing the crashes when we're trying
to send the messages on the IO thread, and not where we're dispatching
them from. Add some asserts so we get errors closer to the source, and
add some asserts for other things that we believe to be true and would
be useful to know aren't actually true.
Differential Revision: https://phabricator.services.mozilla.com/D76496
- modify line wrap up to 80 chars; (tw=80)
- modify size of tab to 2 chars everywhere; (sts=2, sw=2)
--HG--
extra : rebase_source : 7eedce0311b340c9a5a1265dc42d3121cc0f32a0
extra : amend_source : 9cb4ffdd5005f5c4c14172390dd00b04b2066cd7
This introduces the machinery needed to generate crash annotations from a YAML
file. The relevant C++ functions are updated to take a typed enum. JavaScript
calls are unaffected but they will throw if the string argument does not
correspond to one of the known entries in the C++ enum. The existing whitelists
and blacklists of annotations are also generated from the YAML file and all
duplicate code related to them has been consolidated. Once written out to the
.extra file the annotations are converted in string form and are no different
than the existing ones.
All existing annotations have been included in the list (and some obsolete ones
have been removed) and all call sites have been updated including tests where
appropriate.
--HG--
extra : source : 4f6c43f2830701ec5552e08e3f1b06fe6d045860
This was done automatically replacing:
s/mozilla::Move/std::move/
s/ Move(/ std::move(/
s/(Move(/(std::move(/
Removing the 'using mozilla::Move;' lines.
And then with a few manual fixups, see the bug for the split series..
MozReview-Commit-ID: Jxze3adipUh
Nika Layzell
Gabriele Svelto
Jim Blandy
Stanca Serban
Noemi Erli
Randell Jesup
Butkovits Atila
Jean-Yves Avenard
Narcis Beleuzu
Andrew McCreight
Nathan Froyd
Jan Varga
Simon Giesecke
Sylvestre Ledru
Tooru Fujisawa
Benjamin Bouvier
Emilio Cobos Álvarez
Chris Peterson
shindli
Bill McCloskey
David Anderson
Carsten "Tomcat" Book