802288 - [b2g-bluetooth] BluetoothOppManager does not destruct correctly on shutdown

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Not that we ever shut the phone down this way, but...

On b2g desktop, we get XPCOM errors on shutdown due to BluetoothOppManager not being destructed at time of xpcom-shutdown event. We just need the same listeners that BluetoothHfp/ScoManager have already.

Comment 1

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Failing shutdown like this shouldn't cause crashes. Marking blocking+.

Comment 2

[Andrew Overholt [:overholt]]

We don't think we'll be doing shutdowns in v1 and instead just killing the process.

Comment 3

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Attachment: Patch 1 (v1) - Make BluetoothOppManager shutdown at xpcom-shutdown

This is pretty much copy/paste from HfpManager, and I have little to no idea what happens in OppManager, so let me know if bringing things down this way is bad (i.e. if we need to "gracefully" disconnect or something on shutdown). But, while not sending a file, this clears up our XPCOM bitching on shutdown.

Comment 4

[Eric Chou [:ericchou] [:echou]]

Comment on attachment 674925 [details] [diff] [review]
Patch 1 (v1) - Make BluetoothOppManager shutdown at xpcom-shutdown

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

Even if we are sending or receiving data via OPP, CloseSocket() should be a way to tell remote side that we are going to close this session.

Comment 5

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/71e97a4400ea

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/71e97a4400ea

Comment 7

[Gina Yeh [:gyeh] [:ginayeh]]

Comment on attachment 674925 [details] [diff] [review]
Patch 1 (v1) - Make BluetoothOppManager shutdown at xpcom-shutdown

Since patches in Bug 819832(bb+) and Bug 817930(bb+, P1, C3) have conflicts on mozilla-beta and mozilla-aurora, ask for approval of mozilla-beta and mozilla-aurora.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 792823 (BluetoothOppManager)
User impact if declined: BluetoothOppManager won't be destroyed properly when we shut down our phone
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): fairly low, just make BluetoothOppManager as an observer of xpcom-shutdown, and reset everything when it is notified.
String or UUID changes made by this patch: none

Comment 8

[Alex Keybl [:akeybl]]

Comment on attachment 674925 [details] [diff] [review]
Patch 1 (v1) - Make BluetoothOppManager shutdown at xpcom-shutdown

please make sure to land on mozilla-b2g18, and not mozilla-beta.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/3c3b15f19b9d

Note that this landed on m-c when it was still Mozilla19 and is therefore already on Aurora. Also, this had some conflicts that I had to work around, so you should probably take a look to make sure you're happy with the end result.

Comment 10

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Looks ok to me.

Comment 11

[Gina Yeh [:gyeh] [:ginayeh]]

Thanks for your help, Ryan. I've confirmed the changeset of 3c3b15f19b9d and it's perfect :)