1397194 - Follow up Bug 1347519, polling manually in macOS gamepad API backend

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement, P2)

Description

[Michael Leu[:Lenzak](UTC+8)]

In current design, we use CFRunLoopRun as a blocking eventloop for receiving gamepad events and call CFRunLoopStop in a different thread to shut it down.

Although Apple's dev docs claim that it's "generally" thread-safe, it leads to
number of crashes in libsystem_pthread which is reported in Bug 1347519.

To avoid this, we should manage eventloops ourselves.

Comment 1

[Michael Leu[:Lenzak](UTC+8)]

Attachment: Bug 1397194 - Polling manually in macOS gamepad API backend

Review commit: https://reviewboard.mozilla.org/r/176748/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/176748/

Comment 2

[Michael Leu[:Lenzak](UTC+8)]

Comment on attachment 8904936 [details]
Bug 1397194 - Polling manually in macOS gamepad API backend

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/176748/diff/1-2/

Comment 3

[Michael Leu[:Lenzak](UTC+8)]

Now try looks good.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7be0a5faff4e&selectedJob=128850003

Comment 4

[Michael Leu[:Lenzak](UTC+8)]

Comment on attachment 8904936 [details]
Bug 1397194 - Polling manually in macOS gamepad API backend

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/176748/diff/2-3/

Comment 5

[Michael Leu[:Lenzak](UTC+8)]

Hi Kyle,

I once wanted to mitigate the crash inside pthread by using macos' native callback functions in bug 1347519, but it does not work properly, so I tried to rewrite the eventloop by gecko's nsITimer just like our Windows XInput backend.

Actually this patch is finished months ago, but I had concern landing it that time because this change is hard to test by mochitest and the crash rate is low, however, I encounter a 100% crash case in bug 1422560, I think it's time to give up using macos' native event loop.

BTW, I remembered that you were developing WebMIDI backend in mac and encounter issue about the blocking CFRunLoop for USB device detecting, I think maybe this solution apply to your situation as well.

Comment 6

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

Wow, thanks! The timing on this is great too, as I'm now reworking the patches for WebMIDI and was going to be hopefully shipping with a macOS implementation. :D

Comment 7

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

Comment on attachment 8904936 [details]
Bug 1397194 - Polling manually in macOS gamepad API backend

https://reviewboard.mozilla.org/r/176748/#review211544

::: dom/gamepad/cocoa/CocoaGamepad.cpp:515
(Diff revision 3)
>  
>    return dict;
>  }
>  
> -DarwinGamepadService::DarwinGamepadService() : mManager(nullptr) {}
> +DarwinGamepadService::DarwinGamepadService()
> +  : mManager(nullptr), mIsRunning(false) {}

Nit: {} on separate line.

Comment 8

[Michael Leu[:Lenzak](UTC+8)]

Comment on attachment 8904936 [details]
Bug 1397194 - Polling manually in macOS gamepad API backend

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/176748/diff/3-4/

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/43f6a70b2f39
Polling manually in macOS gamepad API backend r=qdot

Comment 10

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/43f6a70b2f39