1209779 - Bug 1204584 broke the Gecko Profiler

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Vladan Djeric (:vladan)]

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"

Comment 1

[Jordan Santell [:jsantell] [@jsantell]]

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)

Comment 2

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

I'll look into this.

Comment 3

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Was able to reproduce the type error:

> TypeError: types is undefined main.js:866:9

Comment 4

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

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

Comment 5

[Kartikaya Gupta (email:[e-mail])]

I reverted the patch locally as well to use the profiler and haven't seen this assertion/crash.

Comment 6

[Kannan Vijayan [:djvj]]

(In reply to Kartikaya Gupta (email:[e-mail]) 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.

Comment 7

[Kartikaya Gupta (email:[e-mail])]

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.

Comment 8

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Interesting, in debug builds it should crash the browser.

Comment 9

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Attachment: Ensure that all null elements are written when streaming profiler JSON

Comment 10

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Turned out to be a latent bug exposed by my patch.

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=83333509c891

Comment 11

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Not sure if this goes without saying, but I verified that the GP addon works with this patch applied.

Comment 12

[Shu-yu Guo [:shu]]

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.

Comment 13

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ce8c63fd5fec

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ce8c63fd5fec

Comment 17

[Vladan Djeric (:vladan)]

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

Comment 18

[Kartikaya Gupta (email:[e-mail])]

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.

Comment 19

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/39983a1913cf

Comment 20

[Kartikaya Gupta (email:[e-mail])]

^

Comment 22

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/39983a1913cf

Comment 23

[Liz Henry (:lizzard) (relman/hg->git project)]

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.

Comment 24

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/57c9a4a0fe4a

Comment 25

[Carsten Book [:Tomcat]]

seems this time it worked