1516967 - Frequent Win7 Opt dt8 shutdown hangs with same-compartment realms

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Description

[Jan de Mooij [:jandem]]

With the patches for bug 1514210 I get frequent dt8 shutdown hangs on Win7 Opt.

I think what's happening is that in ParentImpl::ShutdownBackgroundThread we check sLiveActorCount and if that's > 0 we spin the event loop and schedule a timer to kill remaining actors [0].

However in this case we have a same-process actor and those are not included in sLiveActorsForBackgroundThread (that array is empty) so we never kill it. It seems to me that sLiveActorCount should be sLiveBackgroundActorCount or something, and we should only increment it for actors that actually use the background thread.

Jan, WDYT?

[0] https://searchfox.org/mozilla-central/rev/fc229ed2c78648e402a9bbd50d99b69d0e227844/ipc/glue/BackgroundImpl.cpp#952-961

Comment 1

[Jan de Mooij [:jandem]]

Or maybe we need to call actor->SetLiveActorArray(..) for actors created by CreateActorForSameProcess? For the other-process case we do that in ConnectActorRunnable...

Comment 2

[Kris Maglione [:kmag]]

Do you know why the same-process actor isn't being closed after your changes, when it seems like it was being closed before them?

Also, do you happen to know which thread the actor belongs to?

Comment 3

[Jan de Mooij [:jandem]]

(In reply to Kris Maglione [:kmag] from comment #2)
> Do you know why the same-process actor isn't being closed after your
> changes, when it seems like it was being closed before them?

It might be GC/CC timing related - it only repros intermittently on Win32 Opt and when I try to narrow it down by disabling some tests in the directory where it triggers the problem usually disappears. Pretty frustrating.

> Also, do you happen to know which thread the actor belongs to?

I'll try to add some more instrumentation to answer this...

Comment 4

[Jan Varga [:janv]]

Last time I checked this, it was not so easy to add the same process actors to the array. I have to take a closer look again.

Comment 5

[Jan de Mooij [:jandem]]

(In reply to Jan de Mooij [:jandem] from comment #3)
> > Also, do you happen to know which thread the actor belongs to?
> 
> I'll try to add some more instrumentation to answer this...

The actor is created on a "DOM Worker" thread and on shutdown I do see a DOM Worker thread that's still waiting for events. What's supposed to terminate these threads?

Comment 6

[Jan de Mooij [:jandem]]

Oh, I think we will terminate these threads with the nsThreadManager::get().Shutdown() call at [0], but that comes after the observerService->NotifyObservers call where we end up hanging..

[0] https://searchfox.org/mozilla-central/rev/fc229ed2c78648e402a9bbd50d99b69d0e227844/xpcom/build/XPCOMInit.cpp#840

Comment 7

[Jan de Mooij [:jandem]]

OK I think I figured it out. We're OOM'ing in the JS engine when setting up a worker runtime because of JIT code limits; here we can be a bit more aggressive with CC/GC before we fail. Then we fail to properly terminate the worker thread, I'll file a separate bug for that.

Comment 8

[Jan de Mooij [:jandem]]

Filed bug 1517126 for the worker issue.

Comment 9

[Jan de Mooij [:jandem]]

Attachment: Bug 1516967 - Call the large allocation callback before reporting OOM in JSRuntime::createJitRuntime. r?luke

Comment 10

[Kris Maglione [:kmag]]

We should shut down Worker threads long before we shut down XPCOM threads. Workers that belong to windows should shut down when the window closes. System scope workers... I'm not sure off the top of my head.

Either way, if fixing the OOM fixes the problem, that seems fine.

Comment 11

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

It looks like this turned out to not be an IPC bug.  (But if there's a way PBackground could be have been more helpful, feature-request bugs are welcome.)

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/85054765af34
Call the large allocation callback before reporting OOM in JSRuntime::createJitRuntime. r=lhansen

Comment 13

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/85054765af34