Closed
Bug 1347519
Opened 7 years ago
Closed 4 years ago
Crash in libsystem_pthread.dylib@0x14fc
Categories
(Core :: DOM: Device Interfaces, defect, P2)
Tracking
()
RESOLVED
WORKSFORME
Tracking | Status | |
---|---|---|
firefox55 | --- | affected |
People
(Reporter: jujjyl, Assigned: cleu)
References
Details
(Keywords: crash)
Crash Data
Attachments
(1 file, 1 obsolete file)
This bug was filed from the Socorro interface and is report bp-2d3c0d66-3b7f-4858-8576-101aa2170315. ============================================================= Crashed when visiting http://mzl.la/webassemblydemo briefly after loading up.
Assignee | ||
Comment 2•7 years ago
|
||
It crashes when we try to stop the Gamepad monitoring thread by CFRunLoopStop which is quite weird and I have never seen it before. But I cannot reproduce it on my Macbook pro, is it 100% crash or happens intermittently?
Flags: needinfo?(cleu) → needinfo?(jujjyl)
Reporter | ||
Comment 3•7 years ago
|
||
It happens intermittently, less than one out of ten times perhaps. I have gotten a crash twice on that page.
Flags: needinfo?(jujjyl)
Reporter | ||
Comment 4•7 years ago
|
||
Got the crash again today: https://crash-stats.mozilla.com/report/index/5b453c33-f4e0-4ad9-a5f6-58bde2170320 FF Nightly 55.0a1 (2017-03-15) (64-bit), MacBook Air (13-inch, Mid 2011) OS X El Capitan 10.11.6
Assignee | ||
Comment 5•7 years ago
|
||
I suspect it is caused by buggy macOS c++ API, do you run this page with any game controller attached? Frankly speaking, I don't quite like the gamepad backend on macOS because I can only utilize blocking eventloop API (CFRunLoopRun()), which I have no choice but controlling the eventloop thread by another thread and it is not a good practice. Apple does have newer API which is higher-level and seems to be easier to use, but it's written in Objctive-C. https://developer.apple.com/reference/gamecontroller?language=objc Although it's ok to mix C++ and Objective-c, I'm not sure whether we can do it in Gecko or not.
Flags: needinfo?(jujjyl)
Reporter | ||
Comment 6•7 years ago
|
||
Oh right, yeah, there were no game controllers attached. The engine that the page uses does support game controllers, but the page itself does not use them. So there's a couple of functions that do query for Gamepad API functions. A local copy of the demo page is available at https://s3.amazonaws.com/mozilla-games/ZenGarden/2017-03-16-ZenGarden.zip if that helps.
Flags: needinfo?(jujjyl)
Comment 7•7 years ago
|
||
Did things progress here? I still see a few crashes per day.
Flags: needinfo?(cleu)
Priority: P1 → P2
Assignee | ||
Comment 8•7 years ago
|
||
I found CFRunloopSource sounds promising, I think it worth a try. https://developer.apple.com/documentation/corefoundation/cfrunloopsource?language=objc
Flags: needinfo?(cleu)
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → cleu
Assignee | ||
Comment 9•7 years ago
|
||
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8900456 -
Attachment is obsolete: true
Assignee | ||
Updated•7 years ago
|
Attachment #8903480 -
Flags: review?(kyle)
Assignee | ||
Comment 11•7 years ago
|
||
Hi Kyle Can you review this patch for me? Although Daosheng have done quite a few gamepad work before, he have not dealt with platform backend.
Comment 12•7 years ago
|
||
mozreview-review |
Comment on attachment 8903480 [details] Bug 1347519 - Use CFRunLoopSource for passing shutdown signal https://reviewboard.mozilla.org/r/175316/#review181454 LGTM. ::: dom/gamepad/cocoa/CocoaGamepad.cpp:265 (Diff revision 1) > + // in background thread. > + explicit DarwinGamepadServiceShutdownRunnable() > + : Runnable("DarwinGamepadServiceStartupRunnable") {} > + NS_IMETHOD Run() override > + { > + MOZ_ASSERT(gService); nit: Might also assert that we're on the background thread here, just makes things a little more readable.
Attachment #8903480 -
Flags: review?(kyle) → review+
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Comment 14•7 years ago
|
||
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/e98549c5dd9d Use CFRunLoopSource for passing shutdown signal r=qdot
Keywords: checkin-needed
Comment 15•7 years ago
|
||
Backed out in https://hg.mozilla.org/integration/autoland/rev/867b17852af8 RunWatchdog watches shutdown, and when it decides it is taking too long, crashes the browser to provide what it hopes will be a useful stack showing what was hanging. That tends not to work terribly well in automated testing, since what triggered the shutdown hang could be anything that happens between browser startup and shutdown, but it blames the failure on the last test which ran. Usually that just gives us garbage intermittents, but in the case of this patch, Mac debug consistently fails mochitest-1, https://treeherder.mozilla.org/logviewer.html#?job_id=128772406&repo=autoland
Assignee | ||
Comment 16•7 years ago
|
||
It seems that CFRunLoopStop does not work properly. I think it is not a good choice to use CFRunLoopRun API since it is not documented very well. Maybe reimplement it as a manually polling backend like Windows one is better.
Comment 17•6 years ago
|
||
Isn't the actual problem here that the CFRunLoop isn't kept alive? You call CFRunLoopGetCurrent() [1] and the docs say "Ownership follows the The Get Rule." So as long as you don't call CFRetain() it's not guaranteed to stay alive. Bug 1418018 fixed the same problem in our U2F library. [1] https://hg.mozilla.org/mozilla-central/annotate/58753259bfeb/dom/gamepad/cocoa/CocoaGamepad.cpp#l534
Comment 18•4 years ago
|
||
Closing because no crashes reported for 12 weeks.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → WORKSFORME
You need to log in
before you can comment on or make changes to this bug.
Description
•