1175898 - crash in mozilla::CrossProcessMutex::ShareToProcess(int)

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[James Willcox (:snorp) ([e-mail]) (he/him)]

This bug was filed from the Socorro interface and is 
report bp-a9524f91-ba27-4982-9c3f-44b272150618.
=============================================================

I get this crash at startup with APZ enabled on 6/18 Mac nightly

Comment 1

[James Willcox (:snorp) ([e-mail]) (he/him)]

This looks to be fallout from bug 1161166:

[26376] WARNING: request for ports for pid 26376, but we're the chrome process
: file /builds/slave/m-cen-m64-d-000000000000000000/build/src/ipc/glue/SharedMemoryBasic_mach.cpp, line 225
[26376] WARNING: Unable to get ports for process.
: file /builds/slave/m-cen-m64-d-000000000000000000/build/src/ipc/glue/SharedMemoryBasic_mach.cpp, line 572
[26376] WARNING: request for ports for pid 26376, but we're the chrome process
: file /builds/slave/m-cen-m64-d-000000000000000000/build/src/ipc/glue/SharedMemoryBasic_mach.cpp, line 225
[26376] WARNING: Unable to get ports for process.
: file /builds/slave/m-cen-m64-d-000000000000000000/build/src/ipc/glue/SharedMemoryBasic_mach.cpp, line 572
Hit MOZ_CRASH() at /builds/slave/m-cen-m64-d-000000000000000000/build/src/ipc/glue/CrossProcessMutex_posix.cpp:146

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Note that this only happens if layers.progressive-paint is set to true. The default value is false on OS X.

Comment 3

[Paul Rouget [:paul]]

Happening on B2G Desktop as well (OS X + apzc). Crash at startup.

Comment 4

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: share_to_self.patch

I didn't consider a process sharing memory with itself. Snorp tried this and said it works under the conditions (progressive tiles and apz enabled) that he saw it crash without this patch.

Comment 5

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 8624450 [details] [diff] [review]
share_to_self.patch

Review of attachment 8624450 [details] [diff] [review]:
-----------------------------------------------------------------

Canceling for now because I think we need a test for this.

Could you please test this using the IPDL unit tests? Unfortunately they're not run on tinderbox, but they're useful when modifying IPDL. You need to build with --enable-ipdl-tests in your mozconfig. Then, from $OBJDIR/dist/bin, you can do:

./run-mozilla.sh ./ipdlunittest TestShmem

The test should run in both threaded and cross-process mode.

If the test doesn't fail without the mod_refs thing I mentioned below, can you change it so that it does?

::: ipc/glue/SharedMemoryBasic_mach.cpp
@@ +565,5 @@
>  SharedMemoryBasic::ShareToProcess(base::ProcessId pid,
>                                    Handle* aNewHandle)
>  {
> +  if (pid == getpid()) {
> +    *aNewHandle = mPort;

I'm worried about ownership here. We're going to end up with two SharedMemoryBasics here that use the same port. When either one dies, we'll call mach_port_deallocate and the port will be gone. That doesn't sound like what we want. I think maybe we want to call mach_port_mod_refs here with a delta of +1.

Comment 6

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: share_to_self.patch

you're right about ownership, good catch. And mod_refs is the right thing to do here. The tests, however, don't run (at least on my machine). I also tried running them on Aurora, so this isn't a recent regression.

[23984] WARNING: parent WaitForMessage() failed: 0x10004003 (ipc/rcv) timed out: file /Users/blassey/src/mozilla-aurora/ipc/glue/GeckoChildProcessHost.cpp, line 756
TEST-UNEXPECTED-FAIL | TestShmem | problem launching subprocess
[23984] ###!!! ABORT: failed test: file ../../../../dist/include/mozilla/_ipdltest/IPDLUnitTests.h, line 50
[23984] ###!!! ABORT: failed test: file ../../../../dist/include/mozilla/_ipdltest/IPDLUnitTests.h, line 50
Segmentation fault: 11

That failure is the timeout receiving the bootstrapped mach port from the child.

Comment 7

[Brad Lassey [:blassey] (use needinfo?)]

update, commenting out this line http://mxr.mozilla.org/mozilla-central/source/toolkit/xre/nsXREDirProvider.cpp#108 lets the ipdl tests run and the Shmem tests pass

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/e6c3f42f0534

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e6c3f42f0534