976537 - Crash in NFC shutdown.

Status: RESOLVED FIXED

(Firefox OS Graveyard :: NFC, defect)

Description

[Douglas Crosher [:dougc]]

Seeing crashes in NFC Shutdown due to sNfcConsumer being NULL.  This seems to occur if there are other problems in SystemWorkerManager initialization and it bails out and calls NfcConsumer::Shutdown in the exit path.

Program received signal SIGSEGV, Segmentation fault.
0xb4936096 in mozilla::ipc::NfcConsumer::Shutdown () at ../../../src/ipc/nfc/Nfc.cpp:314
314	    sNfcConsumer->mShutdown = true;
(gdb) back
#0  0xb4936096 in mozilla::ipc::NfcConsumer::Shutdown () at ../../../src/ipc/nfc/Nfc.cpp:314
#1  0xb4f7b35e in mozilla::dom::gonk::SystemWorkerManager::Shutdown (this=0x1669598) at ../../../../src/dom/system/gonk/SystemWorkerManager.cpp:132
#2  0xb4f7b6fa in mozilla::dom::gonk::SystemWorkerManager::FactoryCreate () at ../../../../src/dom/system/gonk/SystemWorkerManager.cpp:161
#3  0xb4d9eafc in SystemWorkerManagerConstructor (aResult=0xbe9109f8, aIID=..., aOuter=<optimized out>) at ../../../src/layout/build/nsLayoutModule.cpp:290
#4  SystemWorkerManagerConstructor (aOuter=<optimized out>, aIID=..., aResult=0xbe9109f8) at ../../../src/layout/build/nsLayoutModule.cpp:291

Comment 1

[Douglas Crosher [:dougc]]

Attachment: Guard against crash in NFC shutdown when the sNfcConsumer is NULL

Comment 2

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 8381384 [details] [diff] [review]
Guard against crash in NFC shutdown when the sNfcConsumer is NULL

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

Nice catch. Add r=me.

May I know which device you're testing on? Nexus 4?

Comment 3

[Douglas Crosher [:dougc]]

(In reply to Yoshi Huang[:allstars.chh] from comment #2)
> Comment on attachment 8381384 [details] [diff] [review]
> Guard against crash in NFC shutdown when the sNfcConsumer is NULL
> 
> Review of attachment 8381384 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Nice catch. Add r=me.
 
Thank you for the quick review.

> May I know which device you're testing on? Nexus 4?

Yes, the Nexus 4.

btw This path was taken when Wifi debugging was enabled, see bug 976455.

Comment 4

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Hi Douglas
Due to the new policy we cannot land patches right now.
Let's ask Ken for this.

Comment 5

[Ken Chang[:kenkai]]

After discussing with Yoshi, it causes regression.

Comment 6

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

After discussed with Ken, nominate this with 1.3+

Comment 7

[Ken Chang[:kenkai]]

It is 1.3+.

Comment 8

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

https://hg.mozilla.org/integration/b2g-inbound/rev/34dd23453855

Comment 9

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 8381384 [details] [diff] [review]
Guard against crash in NFC shutdown when the sNfcConsumer is NULL

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 933635
User impact if declined: Crash due to SIGSEGV
Testing completed: verified on Nexus-4
Risk to taking this patch (and alternatives if risky): Low. This patch just checks if the pointer is valid or not before accessing it.
String or UUID changes made by this patch: No

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/34dd23453855

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/7333d34e9c4e