Bug 1204584 broke the Gecko Profiler

RESOLVED FIXED in Firefox 43

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: vladan, Assigned: fitzgen)

Tracking

({regression})

43 Branch
mozilla44
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox42 unaffected, firefox43+ fixed, firefox44 fixed)

Details

Attachments

(1 attachment)

Reporter

Description

4 years ago
Bug 1204584 broke the Gecko Profiler (it never returns from retrieving the profile). Nightly 44 and Aurora 43 are affected

I used mozRegression to find the offending changeset:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f33142322571

STR:
1. Set xpinstall.signatures.required to false (needed for next step)
2. Install the unsigned Gecko Profiler extension from https://github.com/bgirard/Gecko-Profiler-Addon/raw/master/src/geckoprofiler.xpi
3. Click Analyze to show a profile

Expected: Profile opens
Actual: Profiler never returns from "Retrieving profile"
Reporter

Updated

4 years ago
Blocks: 1204584
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression
I get the following error when using Gecko Profiler:

TypeError: types is undefined src/main.js:866:9

Looks like when inflating the optimizations in the frame, the value in the optimizations slot is a number, not an object containing sites and attempts, etc., not sure how that patch could've caused that, or if it's from something else and not evident in the devtools performance tool (even in browser toolbox)
Flags: needinfo?(nfitzgerald)
I'll look into this.
Assignee: nobody → nfitzgerald
Status: NEW → ASSIGNED
Flags: needinfo?(nfitzgerald)
Was able to reproduce the type error:

> TypeError: types is undefined main.js:866:9
If I revert the patch from bug 1204854 locally, I start hitting assertion failures:

Assertion failure: entry->isMarkedFromAnyThread(rt), at /Users/fitzgen/src/mozilla-central/js/src/jit/JitcodeMap.cpp:484
[Child 73545] ###!!! ABORT: Aborting on channel error.: file /Users/fitzgen/src/mozilla-central/ipc/glue/MessageChannel.cpp, line 1768
#01: mozilla::ipc::MessageChannel::OnChannelErrorFromLink()[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x892ca4]
#02: mozilla::ipc::ProcessLink::OnChannelError()[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x8946a1]
#03: non-virtual thunk to mozilla::ipc::ProcessLink::OnChannelError()[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x8946cc]
#04: IPC::Channel::ChannelImpl::OnFileCanReadWithoutBlocking(int)[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x812e3c]
#05: base::MessagePumpLibevent::OnLibeventNotification(int, short, void*)[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x7c51fe]
#06: event_persist_closure[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x873efa]
#07: event_process_active_single_queue[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x873750]
#08: event_process_active[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x857fdc]
#09: event_base_loop[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x8570d7]
#10: base::MessagePumpLibevent::Run(base::MessagePump::Delegate*)[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x7c563c]
#11: MessageLoop::RunInternal()[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x7c3125]
#12: MessageLoop::RunHandler()[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x7c3035]
#13: MessageLoop::Run()[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x7c2fdd]
#14: base::Thread::ThreadMain()[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x7e8436]
#15: ThreadFunc(void*)[/Users/fitzgen/src/mozilla-central/obj.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x7e960e]
#16: _pthread_body[/usr/lib/system/libsystem_pthread.dylib +0x3268]
#17: _pthread_body[/usr/lib/system/libsystem_pthread.dylib +0x31e5]
[Child 73545] ###!!! ABORT: Aborting on channel error.: file /Users/fitzgen/src/mozilla-central/ipc/glue/MessageChannel.cpp, line 1768
Hit MOZ_CRASH() at /Users/fitzgen/src/mozilla-central/memory/mozalloc/mozalloc_abort.cpp:33
I reverted the patch locally as well to use the profiler and haven't seen this assertion/crash.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #5)
> I reverted the patch locally as well to use the profiler and haven't seen
> this assertion/crash.

It's a sanity assertion, probably not a crash.  Are you running a debug build?  If not, you probably won't see this.
Yup, running a debug build. I didn't see the assertion stack either but it's possibly it flew by in the console output while I wasn't looking. I was getting a lot of other spew on the console so I might have missed it. It's certainly not hitting frequently though.
Interesting, in debug builds it should crash the browser.
Turned out to be a latent bug exposed by my patch.

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=83333509c891
Not sure if this goes without saying, but I verified that the GP addon works with this patch applied.
Comment on attachment 8668159 [details] [diff] [review]
Ensure that all null elements are written when streaming profiler JSON

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

Sorry about this, thanks.
Attachment #8668159 - Flags: review?(shu) → review+
Duplicate of this bug: 1207651
Duplicate of this bug: 1210521
https://hg.mozilla.org/mozilla-central/rev/ce8c63fd5fec
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Reporter

Comment 17

4 years ago
Comment on attachment 8668159 [details] [diff] [review]
Ensure that all null elements are written when streaming profiler JSON

Approval Request Comment
[Feature/regressing bug #]: 1204584
[User impact if declined]: Impossible to profile performance problems on Firefox 43
[Describe test coverage new/current, TreeHerder]:
[Risks and why]: None, tiny fix, confined to profiler code
[String/UUID change made/needed]: None
Attachment #8668159 - Flags: approval-mozilla-aurora?
The patch that landed was empty. ni? cbook so he knows whatever process he used to land it (comment 13) doesn't work. I relanded it on inbound.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
^
Flags: needinfo?(cbook)
Duplicate of this bug: 1210521
https://hg.mozilla.org/mozilla-central/rev/39983a1913cf
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 years ago
Resolution: --- → FIXED
Comment on attachment 8668159 [details] [diff] [review]
Ensure that all null elements are written when streaming profiler JSON

Minor fix for profiler regression; let's uplift to aurora.
Attachment #8668159 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
seems this time it worked
Flags: needinfo?(cbook)
You need to log in before you can comment on or make changes to this bug.