1168337 - TabChild::PreloadSlowThings() is too slow on emulator mochitest

Status: RESOLVED INVALID

(Core :: DOM: Content Processes, defect)

Description

[Cervantes Yu [:cyu] [:cervantes]]

When running the Nuwa test cases locally, I sometimes ran into a crash. The test case that is reported as crashing is random. It turns out that the preallocated process could be still running PreloadSlowThings() when we shut down the whole test suite. An example is NS_RUNTIMEABORT() in mozilla::net::PNeckoChild::SendPCookieServiceConstructor() because the IPC channel is closed.

Comment 1

[Cervantes Yu [:cyu] [:cervantes]]

Attachment: dont_preallocate_emu.patch

This should make test cases on emulator more predictable and stable.

Comment 2

[Cervantes Yu [:cyu] [:cervantes]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3faeeb5e4329

Comment 3

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

Does this mean we won't get automated test coverage for the code that handles preallocated processes?

Comment 4

[Cervantes Yu [:cyu] [:cervantes]]

(In reply to Jed Davis [:jld] from comment #3)
> Does this mean we won't get automated test coverage for the code that
> handles preallocated processes?

We can still have test of preallocated process. The plan is we have dedicated test cases of preallocated process, controlled by some test-only pref. In other test cases, we just don't preallocate and create the content process by request (with processed forked from Nuwa, this is not too slow).

Comment 5

[Cervantes Yu [:cyu] [:cervantes]]

The log excerpt:

> 07:23:31     INFO -  09-23 14:14:15.436 I/Gecko   (  865): [Child 865] WARNING: Exiting child process early!: file /builds/slave/b2g_try_emu-d_dep-000000000000/build/gecko/xpcom/build/XPCOMInit.cpp, line 1060
> 07:23:31     INFO -  09-23 14:14:15.496 W/Nuwa    (  865): Threads remaining at exit:
> 07:23:31     INFO -  09-23 14:14:15.496 W/Nuwa    (  865):   Chrome_ChildThr (origNativeThreadID=809 recreatedNativeThreadID=866)
> 07:23:31     INFO -  09-23 14:14:15.496 W/Nuwa    (  865):   ImageBridgeChil (origNativeThreadID=840 recreatedNativeThreadID=879)
> 07:23:31     INFO -  09-23 14:14:15.496 W/Nuwa    (  865):   BufferMgrChild (origNativeThreadID=841 recreatedNativeThreadID=880)
> 07:23:31     INFO -  09-23 14:14:15.496 W/Nuwa    (  865): total: 3 outstanding threads. Please fix them so they're destroyed before this point!
> 07:23:31     INFO -  09-23 14:14:15.506 W/Nuwa    (  865): note: sThreadCount=3, sThreadFreezeCount=14
> 07:23:31     INFO -  09-23 14:14:15.566 I/Gecko   (  753): [Parent 753] WARNING: NS_ENSURE_TRUE(context) failed: file /builds/slave/b2g_try_emu-d_dep-000000000000/build/gecko/xpcom/threads/nsThread.cpp, line 765
> 07:23:31     INFO -  09-23 14:14:15.635 I/Gecko   (  753): [Parent 753] ###!!! ASSERTION: Using observer service after XPCOM shutdown!: 'Error', file /builds/slave/b2g_try_emu-d_dep-000000000000/build/gecko/xpcom/ds/nsObserverService.cpp, line 268

Shows that we are shutting down a process when its ImageBridgeChild is still alive as in https://bugzilla.mozilla.org/show_bug.cgi?id=1204837#c14

Comment 6

[Gregor Wagner [:gwagner]]

I don't know if this changed but isn't ENABLE_TESTS always true?

Comment 7

[Cervantes Yu [:cyu] [:cervantes]]

Attachment: The fix for abort in the preallocated process

The new patch that doesn't disable process preallocation. The patch shuts down the preallocated and Nuwa processes with ContentParent::KillHard() instead of ContentParent::Close() for the problem that preloading runs too slow in the preallocated process. I am doing this because even we partially preload in the Nuwa process, the abort still happens locally when the preallocated process runs the part of preload that must run after fork. With this patch (and some shutdown hang patches, which are unrelated to Nuwa), I can run the test:
./mach mochitest -f plain dom/ipc/tests
overnight > 400 times without failure. If this also runs well on try, I'll start the review process to land this patch.

Comment 8

[Cervantes Yu [:cyu] [:cervantes]]

(In reply to Gregor Wagner [:gwagner] from comment #6)
> I don't know if this changed but isn't ENABLE_TESTS always true?

Yes. The #ifdef looks useless. I'll try the approach without disabling process preallocation in the new patch and leave disabling preallocation as plan B if the new patch doesn't work.

Comment 9

[Cervantes Yu [:cyu] [:cervantes]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c1cb762f118a

Comment 10

[Cervantes Yu [:cyu] [:cervantes]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7a4baae5a1ea

Comment 11

[Cervantes Yu [:cyu] [:cervantes]]

Attachment: The fix for abort in the preallocated process

Given:
1. local overnight run of the test cases under dom/ipc/tests/ passed 400+ times without a single failure.
2. https://treeherder.mozilla.org/#/jobs?repo=try&revision=92b40a3e7a9c passed 89 times (3 reds are WebRTC intermittent failures).
3. https://treeherder.mozilla.org/#/jobs?repo=try&revision=377109d30d4f passed on all platforms,
we should add reenable test_NuwaProcessCreation.html

Comment 12

[Cervantes Yu [:cyu] [:cervantes]]

https://hg.mozilla.org/integration/b2g-inbound/rev/3a17ffae98051c8e963c10260a7528d4c1045311
Bug 1168337: Kill the processes on startup of the Nuwa test case fix the abort in the preallocated process. r=khuey

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3a17ffae9805

Comment 14

[Jens Stutte [:jstutte]]

There is no more TabChild::PreloadSlowThings and also the functions added by https://hg.mozilla.org/mozilla-central/rev/3a17ffae9805 like SendTestOnlyNotification do not exist anymore. I assume there is nothing more to do here such that related to our today's code base this is INVALID.