[e10s] Hang detector browser stacks should be taken before we message the UI

RESOLVED FIXED in Firefox 41

Status

()

Core
DOM: Content Processes
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: jimm, Assigned: jimm)

Tracking

(Depends on: 1 bug)

Trunk
mozilla41
Unspecified
All
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(e10sm7+, firefox40 affected, firefox41 fixed)

Details

Attachments

(3 attachments, 7 obsolete attachments)

(Assignee)

Description

3 years ago
We take a browser minidump after the user has responded to the hang UI. We want to collect a browser stack right when the hang is first detected, so we can catch the browser in its true hang state.

Here is a typical browser stack which is taken after we've posted a message to the main thread in response to the hang detector.

BROWSER
-----------------------------------------------------------------------------------
0 google_breakpad::ExceptionHandler::WriteMinidump() src
1 google_breakpad::ExceptionHandler::WriteMinidump(std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> > const &,bool (*)(wchar_t const *,wchar_t const *,void *,_EXCEPTION_POINTERS *,MDRawAssertionInfo *,bool),void *) src
2 CrashReporter::CreatePairedMinidumps(void *,unsigned long,nsIFile * *) src
3 mozilla::dom::CrashReporterParent::GeneratePairedMinidump<mozilla::plugins::PluginModuleChromeParent>(mozilla::plugins::PluginModuleChromeParent *) src
4 mozilla::plugins::PluginModuleChromeParent::TerminateChildProcess(MessageLoop *) src
5 mozilla::plugins::TerminatePlugin(unsigned int) src
6 `anonymous namespace'::HangMonitoredProcess::TerminatePlugin() src
7 XPTC__InvokebyIndex src
8 unknown: offset=unknown function=unknown
9 XPC_WN_CallMethod(JSContext *,unsigned int,JS::Value *) src
10 js::Invoke(JSContext *,JS::CallArgs,js::MaybeConstruct) src
11 Interpret src
12 js::RunScript(JSContext *,js::RunState &) src
13 js::Invoke(JSContext *,JS::CallArgs,js::MaybeConstruct) src
14 Interpret src
15 js::RunScript(JSContext *,js::RunState &) src
16 js::Invoke(JSContext *,JS::CallArgs,js::MaybeConstruct) src
17 js::CrossCompartmentWrapper::call(JSContext *,JS::Handle<JSObject *>,JS::CallArgs const &) src
18 js::Invoke(JSContext *,JS::CallArgs,js::MaybeConstruct) src
19 Interpret src
20 js::RunScript(JSContext *,js::RunState &) src
21 js::Invoke(JSContext *,JS::CallArgs,js::MaybeConstruct) src
22 JS::Call(JSContext *,JS::Handle<JS::Value>,JS::Handle<JS::Value>,JS::HandleValueArray const &,JS::MutableHandle<JS::Value>) src
23 mozilla::dom::EventHandlerNonNull::Call(JSContext *,JS::Handle<JS::Value>,mozilla::dom::Event &,JS::MutableHandle<JS::Value>,mozilla::ErrorResult &) src
24 mozilla::dom::EventHandlerNonNull::Call<nsISupports *>(nsISupports * const &,mozilla::dom::Event &,JS::MutableHandle<JS::Value>,mozilla::ErrorResult &,char const *,mozilla::dom::CallbackObject::ExceptionHandling,JSCompartment *) src
25 mozilla::JSEventHandler::HandleEvent(nsIDOMEvent *) src
26 mozilla::EventListenerManager::HandleEventInternal(nsPresContext *,mozilla::WidgetEvent *,nsIDOMEvent * *,mozilla::dom::EventTarget *,nsEventStatus *) src
27 mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem> &,mozilla::EventChainPostVisitor &,mozilla::EventDispatchingCallback *,mozilla::ELMCreationDetector &) src
28 mozilla::EventDispatcher::Dispatch(nsISupports *,nsPresContext *,mozilla::WidgetEvent *,nsIDOMEvent *,nsEventStatus *,mozilla::EventDispatchingCallback *,nsTArray<mozilla::dom::EventTarget *> *) src
29 mozilla::EventDispatcher::DispatchDOMEvent(nsISupports *,mozilla::WidgetEvent *,nsIDOMEvent *,nsPresContext *,nsEventStatus *) src
30 PresShell::HandleDOMEventWithTarget(nsIContent *,nsIDOMEvent *,nsEventStatus *) src
31 nsContentUtils::DispatchXULCommand(nsIContent *,bool,nsIDOMEvent *,nsIPresShell *,bool,bool,bool,bool) src
32 nsXULMenuCommandEvent::Run() src
33 nsThread::ProcessNextEvent(bool,bool *) src
34 mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate *) src
35 MessageLoop::RunHandler() src
36 MessageLoop::Run() src
37 nsBaseAppShell::Run() src
38 nsAppShell::Run() src
39 nsAppStartup::Run() src
40 XREMain::XRE_mainRun() src
41 XREMain::XRE_main(int,char * * const,nsXREAppData const *) src
42 XRE_main src
43 do_main src
44 NS_internal_main(int,char * *) src
45 wmain src
46 __tmainCRTStartup
47 kernel32.dll: offset=0x4d41 function=unknown
48 fast_error_exit
tracking-e10s: --- → m7+
(Assignee)

Updated

3 years ago
Assignee: nobody → jmathies
(Assignee)

Comment 1

3 years ago
Created attachment 8600905 [details] [diff] [review]
wip

Bill does this look correct to you? AFAICT all this hang code is still active in the browser process, but probably shouldn't be.
Attachment #8600905 - Flags: feedback?(wmccloskey)
(Assignee)

Comment 2

3 years ago
Comment on attachment 8600905 [details] [diff] [review]
wip

That's not quite right, I think the entire method is supposed to be restricted to non-e10s.
Attachment #8600905 - Flags: feedback?(wmccloskey)
Comment on attachment 8600905 [details] [diff] [review]
wip

I don't think this code does any harm, and it might get us out of some hangs. For example, I think this code is what catches the ClearSiteData hang.
(Assignee)

Comment 4

3 years ago
thanks. I'll figure out if we use this or not and add some commenting maybe if need be.
(Assignee)

Comment 5

3 years ago
(In reply to Bill McCloskey (:billm) from comment #3)
> Comment on attachment 8600905 [details] [diff] [review]
> wip
> 
> I don't think this code does any harm, and it might get us out of some
> hangs. For example, I think this code is what catches the ClearSiteData hang.

We do use this, and amazingly it all still seems to work. You're right too, this is how we're catching those chrome clear site hangs. So I'm not going to touch this.

I've also started documenting up on the wiki all the prefs we have related to timeouts - 

https://wiki.mozilla.org/Electrolysis/plugins
(Assignee)

Updated

3 years ago
Depends on: 1153205
(Assignee)

Updated

3 years ago
Attachment #8600905 - Attachment is obsolete: true
Assignee: jmathies → wmccloskey
(Assignee)

Comment 6

3 years ago
I'm currently working on this, not sure why assignment switched.
Assignee: wmccloskey → jmathies
We talked about it in triage and you said you were pretty overloaded, so I volunteered to take it. Let me know if there's a different bug I can take instead.
(Assignee)

Comment 8

3 years ago
(In reply to Bill McCloskey (:billm) from comment #7)
> We talked about it in triage and you said you were pretty overloaded, so I
> volunteered to take it. Let me know if there's a different bug I can take
> instead.

Wasn't that the clear site one? Anyway, I'm mostly through this today so no need to hand it off.
(Assignee)

Comment 9

3 years ago
Created attachment 8603476 [details] [diff] [review]
wip

Mostly finished except there's a race here where the hang detector receives multiple notifications and ends up taking multiple dumps before the first dump gets processed. Need to dig into that a bit still.
(Assignee)

Comment 10

3 years ago
Created attachment 8605283 [details] [diff] [review]
new exception handler apis v.1

Adding a couple apis we need for taking an early dump of the browser and matching that up later with a full report.
Attachment #8603476 - Attachment is obsolete: true
Attachment #8605283 - Flags: review?(ted)
(Assignee)

Comment 11

3 years ago
Created attachment 8605284 [details] [diff] [review]
wip front end with debug
(Assignee)

Comment 12

3 years ago
Created attachment 8605285 [details] [diff] [review]
new content crash reporter api v.1

Needed a new api in the content class as well.
Attachment #8605284 - Attachment is obsolete: true
Attachment #8605285 - Flags: review?(ted)
(Assignee)

Comment 13

3 years ago
Created attachment 8605286 [details] [diff] [review]
wip front end with debug
(Assignee)

Comment 14

3 years ago
ted - review ping
Comment on attachment 8605283 [details] [diff] [review]
new exception handler apis v.1

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

Sorry for the delay. This looks mostly OK but I have some reservations about code duplication.

::: toolkit/crashreporter/nsExceptionHandler.cpp
@@ +3116,5 @@
> +#ifdef DEBUG
> +  nsAutoCString debugNewPath;
> +  minidump->GetNativePath(debugNewPath);
> +  printf_stderr("RenameAdditionalHangMinidump from:'%s' to:'%s' result: %X\n", debugOldPath.get(), debugNewPath.get(), rv);
> +#endif

Did you intend to leave this debug spew in here?

@@ +3255,5 @@
> +  return true;
> +}
> +
> +bool
> +TakeMinidumpAndPair(ProcessHandle aTargetPid,

I'm not wild about the duplication here between this and CreatePairedMinidumps. I get that you have a slightly different use case for this vs. the existing plugin hang detector code, but it would be nice if we could share code. Maybe you could rework CreatePairedMinidumps to take an optional aIncomingDump and only dump the current process if that's not set?

@@ +3275,5 @@
> +  xpstring dump_path;
> +#ifndef XP_LINUX
> +  dump_path = gExceptionHandler->dump_path();
> +#else
> +  dump_path = gExceptionHandler->minidump_descriptor().directory();

This could probably stand to be refactored into a function like GetDumpPath().
Attachment #8605283 - Flags: review?(ted)
Comment on attachment 8605285 [details] [diff] [review]
new content crash reporter api v.1

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

::: dom/ipc/CrashReporterParent.h
@@ +222,5 @@
> +  nsCOMPtr<nsIFile> targetDump;
> +  if (CrashReporter::TakeMinidumpAndPair(childHandle,
> +                                         mMainThread, // child thread id
> +                                         aMinidumpToPair,
> +                                         aPairName,

Is this ever going to be anything but "browser"?
Attachment #8605285 - Flags: review?(ted) → review+
(Assignee)

Comment 17

3 years ago
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #16)
> Comment on attachment 8605285 [details] [diff] [review]
> new content crash reporter api v.1
> 
> Review of attachment 8605285 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/ipc/CrashReporterParent.h
> @@ +222,5 @@
> > +  nsCOMPtr<nsIFile> targetDump;
> > +  if (CrashReporter::TakeMinidumpAndPair(childHandle,
> > +                                         mMainThread, // child thread id
> > +                                         aMinidumpToPair,
> > +                                         aPairName,
> 
> Is this ever going to be anything but "browser"?

I really don't like that we have these hardcoded strings down in crash reporter code. IMHO there's no harm in passing this name in and it seems to me to make better sense to let the caller define it.
(Assignee)

Comment 18

3 years ago
Created attachment 8616694 [details] [diff] [review]
new exception handler apis v.2

Updated per comments - blended the two apis into one.
Attachment #8605283 - Attachment is obsolete: true
Attachment #8616694 - Flags: review?(ted)
Comment on attachment 8616694 [details] [diff] [review]
new exception handler apis v.2

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

::: toolkit/crashreporter/nsExceptionHandler.cpp
@@ +3294,5 @@
> +        PairedDumpCallback,
> +        static_cast<void*>(&incomingDump))) {
> +
> +      //childMinidump->Remove(false);
> +      //childExtra->Remove(false);

Either keep these or don't, but don't leave them commented out.
Attachment #8616694 - Flags: review?(ted) → review+
(Assignee)

Comment 20

3 years ago
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #19)
> Comment on attachment 8616694 [details] [diff] [review]
> new exception handler apis v.2
> 
> Review of attachment 8616694 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: toolkit/crashreporter/nsExceptionHandler.cpp
> @@ +3294,5 @@
> > +        PairedDumpCallback,
> > +        static_cast<void*>(&incomingDump))) {
> > +
> > +      //childMinidump->Remove(false);
> > +      //childExtra->Remove(false);
> 
> Either keep these or don't, but don't leave them commented out.

Woops, sorry. That should definitely be in there.
(Assignee)

Comment 21

3 years ago
Created attachment 8616822 [details] [diff] [review]
process hang monitor changes v.1
Attachment #8605286 - Attachment is obsolete: true
(Assignee)

Comment 22

3 years ago
Created attachment 8616825 [details] [diff] [review]
process hang monitor changes v.1

review to bill who wrote most of the hang monitor code here.

This is the front end part of this work:

1) takes the early minidump of the browser
2) stashes the id of the minidump in the hang monitor
3) prevents additional browser dumps from being taken while we wait for the user to respond.
4) when the user responds pass the browser minidump id to the content process crash handling code so that it can be paired up with the hung content minidump.
5) if the notification is ignored or cancelled send a message back to the hang monitor telling it to delete the browser minidump and clear its tracking.
Attachment #8616822 - Attachment is obsolete: true
Attachment #8616825 - Flags: review?(wmccloskey)
Comment on attachment 8616825 [details] [diff] [review]
process hang monitor changes v.1

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

Thanks, looks nice!

::: dom/ipc/ProcessHangMonitor.cpp
@@ +151,5 @@
>    NS_IMETHOD BeginStartingDebugger() override;
>    NS_IMETHOD EndStartingDebugger() override;
>    NS_IMETHOD TerminatePlugin() override;
>    NS_IMETHOD TerminateProcess() override;
> +  NS_IMETHOD UserCancelled() override;

Canceled is the preferred spelling in the US, so let's go with that.

@@ +216,5 @@
>  
>    // Must be accessed with mMonitor held.
>    nsRefPtr<HangMonitoredProcess> mProcess;
>    bool mShutdownDone;
> +  // pluginid, crashid

Maybe say "Map from plugin ID to crash dump ID. Protected by mBrowserCrashDumpHashLock."

@@ +565,5 @@
>    if (IsDebuggerPresent()) {
>      return true;
>    }
>  #endif
> +  

Extra space.

@@ +584,5 @@
> +          mBrowserCrashDumpIds.Put(phd.pluginId(), crashId);
> +        }
> +      }
> +    } else {
> +      return true;

I don't think we want to return here. The hang detector relies on periodic notification or else the hang UI is hidden again. If we return here, the hang notification will hide after some number of seconds even if the hang persists.

@@ +806,5 @@
>  
>    uint32_t id = mHangData.get_PluginHangData().pluginId();
> +  plugins::TerminatePlugin(id, mBrowserDumpId);
> +
> +  ProcessHangMonitor::Get()->MonitorLoop()->PostTask(

Why post this to another thread? It already takes a lock before it touches the hashtable. Do you want the disk access to happen off the main thread?

@@ +823,5 @@
>    }
>  
> +  if (mHangData.type() == HangData::TPluginHangData) {
> +    uint32_t id = mHangData.get_PluginHangData().pluginId();
> +    ProcessHangMonitor::Get()->MonitorLoop()->PostTask(

Same question here.

@@ +861,5 @@
> +    return NS_OK;
> +  }
> +
> +  uint32_t id = mHangData.get_PluginHangData().pluginId();
> +  ProcessHangMonitor::Get()->MonitorLoop()->PostTask(

And here.

::: dom/plugins/ipc/PluginModuleParent.cpp
@@ +1210,5 @@
> +    nsCOMPtr<nsIFile> browserDumpFile;
> +    if (aBrowserDumpId && !aBrowserDumpId->IsEmpty() &&
> +        CrashReporter::GetMinidumpForID(*aBrowserDumpId, getter_AddRefs(browserDumpFile)) &&
> +        browserDumpFile &&
> +        NS_SUCCEEDED(browserDumpFile->Exists(&exists)) && exists) {

Do we really need to check if the file exists?

Also, in files with 4 space indent I like to put the brace on its own line for multi-line conditionals. It makes it clear when the condition ends and the code starts.

@@ +1212,5 @@
> +        CrashReporter::GetMinidumpForID(*aBrowserDumpId, getter_AddRefs(browserDumpFile)) &&
> +        browserDumpFile &&
> +        NS_SUCCEEDED(browserDumpFile->Exists(&exists)) && exists) {
> +        nsAutoCString debugPath;
> +        browserDumpFile->GetNativePath(debugPath);

This looks like it can be removed.

@@ +1213,5 @@
> +        browserDumpFile &&
> +        NS_SUCCEEDED(browserDumpFile->Exists(&exists)) && exists) {
> +        nsAutoCString debugPath;
> +        browserDumpFile->GetNativePath(debugPath);
> +        // We have a single browser report, generate a new plugin process pareant

pareant

::: dom/plugins/ipc/PluginModuleParent.h
@@ +379,5 @@
> +     * @param aBrowserDumpId (optional) previously taken browser dump id. If
> +     *   provided TerminateChildProcess will use this browser dump file in
> +     *   generating a multi-process crash report. If not provided a browser
> +     *   dump will be taken at the time of this call.
> +     */ 

Space at end.

@@ +380,5 @@
> +     *   provided TerminateChildProcess will use this browser dump file in
> +     *   generating a multi-process crash report. If not provided a browser
> +     *   dump will be taken at the time of this call.
> +     */ 
> +    void TerminateChildProcess(MessageLoop* aMsgLoop, nsAString* aBrowserDumpId = nullptr);

Can we just use an empty string to signify that we're not passing in a browser dump?
Attachment #8616825 - Flags: review?(wmccloskey) → review+
(Assignee)

Comment 24

3 years ago
Created attachment 8617863 [details] [diff] [review]
process hang monitor changes v.2
Attachment #8616825 - Attachment is obsolete: true
(Assignee)

Comment 25

3 years ago
(In reply to Bill McCloskey (:billm) from comment #23)
> Comment on attachment 8616825 [details] [diff] [review]
> process hang monitor changes v.1
> 
> Review of attachment 8616825 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thanks, looks nice!
> 
> ::: dom/ipc/ProcessHangMonitor.cpp
> @@ +151,5 @@
> >    NS_IMETHOD BeginStartingDebugger() override;
> >    NS_IMETHOD EndStartingDebugger() override;
> >    NS_IMETHOD TerminatePlugin() override;
> >    NS_IMETHOD TerminateProcess() override;
> > +  NS_IMETHOD UserCancelled() override;
> 
> Canceled is the preferred spelling in the US, so let's go with that.

updated.

> @@ +216,5 @@
> >  
> >    // Must be accessed with mMonitor held.
> >    nsRefPtr<HangMonitoredProcess> mProcess;
> >    bool mShutdownDone;
> > +  // pluginid, crashid
> 
> Maybe say "Map from plugin ID to crash dump ID. Protected by
> mBrowserCrashDumpHashLock."

updated.

> @@ +565,5 @@
> >    if (IsDebuggerPresent()) {
> >      return true;
> >    }
> >  #endif
> > +  
> 
> Extra space.

removed.

> @@ +584,5 @@
> > +          mBrowserCrashDumpIds.Put(phd.pluginId(), crashId);
> > +        }
> > +      }
> > +    } else {
> > +      return true;
> 
> I don't think we want to return here. The hang detector relies on periodic
> notification or else the hang UI is hidden again. If we return here, the
> hang notification will hide after some number of seconds even if the hang
> persists.

removed.

> @@ +806,5 @@
> >  
> >    uint32_t id = mHangData.get_PluginHangData().pluginId();
> > +  plugins::TerminatePlugin(id, mBrowserDumpId);
> > +
> > +  ProcessHangMonitor::Get()->MonitorLoop()->PostTask(
> 
> Why post this to another thread? It already takes a lock before it touches
> the hashtable. Do you want the disk access to happen off the main thread?

Honestly don't remember why I bounced this, might have been left over from some other code. I agree it doesn't seem useful so I've removed it. Will test to be sure things are getting cleaned up properly.

> ::: dom/plugins/ipc/PluginModuleParent.cpp
> @@ +1210,5 @@
> > +    nsCOMPtr<nsIFile> browserDumpFile;
> > +    if (aBrowserDumpId && !aBrowserDumpId->IsEmpty() &&
> > +        CrashReporter::GetMinidumpForID(*aBrowserDumpId, getter_AddRefs(browserDumpFile)) &&
> > +        browserDumpFile &&
> > +        NS_SUCCEEDED(browserDumpFile->Exists(&exists)) && exists) {
> 
> Do we really need to check if the file exists?

Yea I think so, if for some reason it isn't there due to a bug, we'll generate a new one below so we still get a browser stack.

> Also, in files with 4 space indent I like to put the brace on its own line
> for multi-line conditionals. It makes it clear when the condition ends and
> the code starts.

updated.

> @@ +1212,5 @@
> > +        CrashReporter::GetMinidumpForID(*aBrowserDumpId, getter_AddRefs(browserDumpFile)) &&
> > +        browserDumpFile &&
> > +        NS_SUCCEEDED(browserDumpFile->Exists(&exists)) && exists) {
> > +        nsAutoCString debugPath;
> > +        browserDumpFile->GetNativePath(debugPath);
> 
> This looks like it can be removed.

removed.

> @@ +1213,5 @@
> > +        browserDumpFile &&
> > +        NS_SUCCEEDED(browserDumpFile->Exists(&exists)) && exists) {
> > +        nsAutoCString debugPath;
> > +        browserDumpFile->GetNativePath(debugPath);
> > +        // We have a single browser report, generate a new plugin process pareant
> 
> pareant

fixed.

> ::: dom/plugins/ipc/PluginModuleParent.h
> @@ +379,5 @@
> > +     * @param aBrowserDumpId (optional) previously taken browser dump id. If
> > +     *   provided TerminateChildProcess will use this browser dump file in
> > +     *   generating a multi-process crash report. If not provided a browser
> > +     *   dump will be taken at the time of this call.
> > +     */ 
> 
> Space at end.

removed.

> @@ +380,5 @@
> > +     *   provided TerminateChildProcess will use this browser dump file in
> > +     *   generating a multi-process crash report. If not provided a browser
> > +     *   dump will be taken at the time of this call.
> > +     */ 
> > +    void TerminateChildProcess(MessageLoop* aMsgLoop, nsAString* aBrowserDumpId = nullptr);
> 
> Can we just use an empty string to signify that we're not passing in a
> browser dump?

Sure, more overhead in the call but I don't mind. updated.
(Assignee)

Comment 26

3 years ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=719cf96f2ff6
(Assignee)

Comment 27

3 years ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c48bc76937ae
(Assignee)

Comment 28

3 years ago
(In reply to Jim Mathies [:jimm] from comment #27)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=c48bc76937ae

That osx opt failure looks like infra but I think I'll push again to try just to be sure.
(Assignee)

Comment 29

3 years ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=66c5919fa2f9
(Assignee)

Comment 30

3 years ago
(In reply to Jim Mathies [:jimm] from comment #29)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=66c5919fa2f9

apparently not infra!
(Assignee)

Comment 31

3 years ago
(In reply to Jim Mathies [:jimm] from comment #30)
> (In reply to Jim Mathies [:jimm] from comment #29)
> > https://treeherder.mozilla.org/#/jobs?repo=try&revision=66c5919fa2f9
> 
> apparently not infra!

Hmm, sure looks like infra though - 

07:01:11     INFO -  Checking for orphan ssltunnel processes...
07:01:11     INFO -  Checking for orphan xpcshell processes...
07:01:11     INFO -  Traceback (most recent call last):
07:01:11     INFO -    File "/builds/slave/test/build/tests/mochitest/runtests.py", line 2636, in <module>
07:01:11     INFO -      sys.exit(cli())
07:01:11     INFO -    File "/builds/slave/test/build/tests/mochitest/runtests.py", line 2633, in cli
07:01:11     INFO -      return run_test_harness(options)
07:01:11     INFO -    File "/builds/slave/test/build/tests/mochitest/runtests.py", line 2610, in run_test_harness
07:01:11     INFO -      result = runner.runTests(options)
07:01:11     INFO -    File "/builds/slave/test/build/tests/mochitest/runtests.py", line 2109, in runTests
07:01:11     INFO -      testsToRun = self.getTestsToRun(options)
07:01:11     INFO -    File "/builds/slave/test/build/tests/mochitest/runtests.py", line 2020, in getTestsToRun
07:01:11     INFO -      tests = self.getActiveTests(options)
07:01:11     INFO -    File "/builds/slave/test/build/tests/mochitest/runtests.py", line 1943, in getActiveTests
07:01:11     INFO -      runtime_file = self.resolve_runtime_file(options, info)
07:01:11     INFO -    File "/builds/slave/test/build/tests/mochitest/runtests.py", line 1875, in resolve_runtime_file
07:01:11     INFO -      platform = info['platform_guess']
07:01:11     INFO -  KeyError: 'platform_guess'
07:01:11    ERROR - Return code: 1
07:01:11    ERROR - No tests run or test summary not found
07:01:11     INFO - TinderboxPrint: mochitest-browser-chrome-chunked<br/><em class="testfail">T-FAIL</em>
07:01:11  WARNING - # TBPL FAILURE #
(Assignee)

Comment 32

3 years ago
<RyanVM> Tomcat|sheriffduty: jimm: kmoir|mtg: update your m-c clone
<RyanVM> that's from a bad parent rev
<jimm> oh really?
<RyanVM> yup
(Assignee)

Comment 33

3 years ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1d8dc34fca75

Comment 34

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/0413e4515c7d
https://hg.mozilla.org/integration/mozilla-inbound/rev/bcfa99bc90fa
https://hg.mozilla.org/integration/mozilla-inbound/rev/e986f0bc71c5
https://hg.mozilla.org/mozilla-central/rev/0413e4515c7d
https://hg.mozilla.org/mozilla-central/rev/bcfa99bc90fa
https://hg.mozilla.org/mozilla-central/rev/e986f0bc71c5
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox41: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
(Assignee)

Comment 36

3 years ago
looks to be working as expected, this s a test hang I generated with a long wait in the test plugin using nightly:

***********************************************************************************
97bf0329-3fc6-45a3-a70a-777c72150613
Windows 7, 6.1.7601 Service Pack 1
nptest.dll
***********************************************************************************
PLUGIN
-----------------------------------------------------------------------------------
0 ZwDelayExecution
1 SleepEx
2 Sleep
3 nptest.dll@0xc379
4 nptest.dll@0xd6ec
5 nptest.dll@0x10918
6 mozilla::plugins::PluginInstanceChild::WinlessHandleEvent(_NPEvent &) src
7 mozilla::plugins::PluginInstanceChild::AnswerNPP_HandleEvent(mozilla::plugins::NPRemoteEvent const &,short *) src
8 mozilla::plugins::PPluginInstanceChild::OnCallReceived(IPC::Message const &,IPC::Message * &) src
9 mozilla::plugins::PPluginModuleChild::OnCallReceived(IPC::Message const &,IPC::Message * &) src
10 mozilla::ipc::MessageChannel::DispatchInterruptMessage(IPC::Message const &,unsigned int) src
11 mozilla::ipc::MessageChannel::OnMaybeDequeueOne() src
12 MessageLoop::DoWork() src
13 base::MessagePumpForUI::DoRunLoop() src
14 base::MessagePumpWin::RunWithDispatcher(base::MessagePump::Delegate *,base::MessagePumpWin::Dispatcher *) src
15 base::MessagePumpWin::Run(base::MessagePump::Delegate *) src
16 MessageLoop::RunInternal() src
17 MessageLoop::RunHandler() src
18 MessageLoop::Run() src
19 XRE_InitChildProcess src
20 content_process_main(int,char * * const) src
21 wmain src
22 __tmainCRTStartup
23 BaseThreadInitThunk
24 __RtlUserThreadStart
25 _RtlUserThreadStart

CONTENT
-----------------------------------------------------------------------------------
0 ZwWaitForMultipleObjects
1 WaitForMultipleObjectsEx
2 WaitForMultipleObjectsExImplementation
3 RealMsgWaitForMultipleObjectsEx
4 MsgWaitForMultipleObjects
5 mozilla::ipc::MessageChannel::WaitForInterruptNotify() src
6 mozilla::ipc::MessageChannel::Call(IPC::Message *,IPC::Message *) src
7 mozilla::plugins::PPluginInstanceParent::CallNPP_HandleEvent(mozilla::plugins::NPRemoteEvent const &,short *) src
8 mozilla::plugins::PluginInstanceParent::NPP_HandleEvent(void *) src
9 mozilla::plugins::PluginModuleParent::NPP_HandleEvent(_NPP *,void *) src
10 nsNPAPIPluginInstance::HandleEvent(void *,short *,NSPluginCallReentry) src
11 nsPluginInstanceOwner::ProcessEvent(mozilla::WidgetGUIEvent const &) src
12 nsPluginInstanceOwner::DispatchMouseToPlugin(nsIDOMEvent *,bool) src
13 nsPluginInstanceOwner::HandleEvent(nsIDOMEvent *) src
14 mozilla::EventListenerManager::HandleEventInternal(nsPresContext *,mozilla::WidgetEvent *,nsIDOMEvent * *,mozilla::dom::EventTarget *,nsEventStatus *) src
15 mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem> &,mozilla::EventChainPostVisitor &,mozilla::EventDispatchingCallback *,mozilla::ELMCreationDetector &) src
16 mozilla::EventDispatcher::Dispatch(nsISupports *,nsPresContext *,mozilla::WidgetEvent *,nsIDOMEvent *,nsEventStatus *,mozilla::EventDispatchingCallback *,nsTArray<mozilla::dom::EventTarget *> *) src
17 PresShell::DispatchEventToDOM(mozilla::WidgetEvent *,nsEventStatus *,nsPresShellEventCB *) src
18 PresShell::HandleEventInternal(mozilla::WidgetEvent *,nsEventStatus *) src
19 PresShell::HandlePositionedEvent(nsIFrame *,mozilla::WidgetGUIEvent *,nsEventStatus *) src
20 PresShell::HandleEvent(nsIFrame *,mozilla::WidgetGUIEvent *,bool,nsEventStatus *,nsIContent * *) src
21 nsViewManager::DispatchEvent(mozilla::WidgetGUIEvent *,nsView *,nsEventStatus *) src
22 nsView::HandleEvent(mozilla::WidgetGUIEvent *,bool) src
23 mozilla::widget::PuppetWidget::DispatchEvent(mozilla::WidgetGUIEvent *,nsEventStatus &) src
24 mozilla::layers::APZCCallbackHelper::DispatchWidgetEvent(mozilla::WidgetGUIEvent &) src
25 mozilla::dom::TabChild::RecvRealMouseButtonEvent(mozilla::WidgetMouseEvent const &) src
26 mozilla::dom::PBrowserChild::OnMessageReceived(IPC::Message const &) src
27 mozilla::dom::PContentChild::OnMessageReceived(IPC::Message const &) src
28 mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const &) src
29 mozilla::ipc::MessageChannel::OnMaybeDequeueOne() src
30 MessageLoop::DoWork() src
31 mozilla::ipc::DoWorkRunnable::Run() src
32 nsThread::ProcessNextEvent(bool,bool *) src
33 NS_ProcessNextEvent(nsIThread *,bool) src
34 mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate *) src
35 mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate *) src
36 MessageLoop::RunHandler() src
37 MessageLoop::Run() src
38 nsBaseAppShell::Run() src
39 nsAppShell::Run() src
40 XRE_RunAppShell src
41 mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate *) src
42 MessageLoop::RunHandler() src
43 MessageLoop::Run() src
44 XRE_InitChildProcess src
45 content_process_main(int,char * * const) src
46 wmain src
47 __tmainCRTStartup
48 BaseThreadInitThunk
49 __RtlUserThreadStart
50 _RtlUserThreadStart

BROWSER
-----------------------------------------------------------------------------------
0 ZwWaitForMultipleObjects
1 WaitForMultipleObjectsEx
2 WaitForMultipleObjectsExImplementation
3 RealMsgWaitForMultipleObjectsEx
4 mozilla::widget::WinUtils::WaitForMessage(unsigned long) src
5 nsAppShell::ProcessNextNativeEvent(bool) src
6 nsBaseAppShell::DoProcessNextNativeEvent(bool,unsigned int) src
7 nsBaseAppShell::OnProcessNextEvent(nsIThreadInternal *,bool,unsigned int) src
8 nsThread::ProcessNextEvent(bool,bool *) src
9 NS_ProcessNextEvent(nsIThread *,bool) src
10 mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate *) src
11 MessageLoop::RunHandler() src
12 MessageLoop::Run() src
13 nsBaseAppShell::Run() src
14 nsAppShell::Run() src
15 nsAppStartup::Run() src
16 XREMain::XRE_mainRun() src
17 XREMain::XRE_main(int,char * * const,nsXREAppData const *) src
18 XRE_main src
19 do_main src
20 NS_internal_main(int,char * *) src
21 wmain src
22 __tmainCRTStartup
23 BaseThreadInitThunk
24 __RtlUserThreadStart
25 _RtlUserThreadStart

Updated

3 years ago
Depends on: 1175608
You need to log in before you can comment on or make changes to this bug.