Relax Shmem DEBUG behaviour around actor destruction
Categories
(Core :: IPC, enhancement)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox124 | --- | fixed |
People
(Reporter: nika, Assigned: nika)
Details
Attachments
(2 files)
Currently when in a DEBUG build, Shmem has a bunch of extra behaviour and checks, which can occasionally make it more difficult to use. This bug is to track removing some of the unnecessary checks being performed which can make the type more complex to use safely.
Specifically, when the Shmem is destroyed at the actor level using IPrococol::DeallocShmem or because the actor has died, in DEBUG builds the memory mapping used by the Shmem is immediately mapped as non-readable and non-writable. As this can happen at any time, this can make using one of these Shmems from a different thread than the actor it was originally bound to quite difficult. As an example, in bug 1812498, special logic was added to VideoBridgeParent::OnChannelError to block the actor's thread until off-thread uses of the Shmem were complete.
This behaviour was always debug-only, and is actually unnecessary. The actual Shmem mapping is not managed by the actor, only the ability to send it between processes, so it should be reasonable for one of these objects to be used after the actor it was sent over is destroyed. This patch does not change the behaviour of mapping the shared memory pages as inaccessible for non-unsafe shmems while they are "owned" by another process.
We may want to change the name "DeallocShmem" to be something more representative of the true behaviour of the function (which is to prevent further sending of the Shmem object over IPDL, freeing IPDL's reference to the underlying shared memory segment).
|
Assignee | |
Comment 1•1 year ago
|
||
Currently when in a DEBUG build, Shmem has a bunch of extra behaviour
and checks, which can occasionally make it more difficult to use. This
bug is to track removing some of the unnecessary checks being performed
which can make the type more complex to use safely.
Specifically, when the Shmem is destroyed at the actor level using
IPrococol::DeallocShmem or because the actor has died, in DEBUG builds
the memory mapping used by the Shmem is immediately mapped as
non-readable and non-writable. As this can happen at any time, this can
make using one of these Shmems from a different thread than the actor it
was originally bound to quite difficult. As an example, in bug 1812498,
special logic was added to VideoBridgeParent::OnChannelError to block
the actor's thread until off-thread uses of the Shmem were complete.
This behaviour was always debug-only, and is actually unnecessary. The
actual Shmem mapping is not managed by the actor, only the ability to
send it between processes, so it should be reasonable for one of these
objects to be used after the actor it was sent over is destroyed. This
patch does not change the behaviour of mapping the shared memory pages
as inaccessible for non-unsafe shmems while they are "owned" by another
process.
In the future, we may want to change the name "DeallocShmem" to be
something more representative of the true behaviour of the function
(which is to prevent further sending of the Shmem object over IPDL,
freeing IPDL's reference to the underlying shared memory segment).
|
||
Updated•1 year ago
|
|
|
Assignee | |
Comment 2•1 year ago
|
||
This was added in bug 1812498, but should no longer be necessary after the
changes in part 1, which avoids unnecessary read-protecting Shmem data in debug
builds during IPDL actor destruction.
Depends on D197828
|
||
Comment 4•1 year ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/e067e965bf17
https://hg.mozilla.org/mozilla-central/rev/9559ddacce7e
|
|
||
Comment 5•1 year ago
|
||
| bugherder | ||
Description
•