Bug 217363 - REGRESSION(r267763): [GTK][WPE] Broken main thread assertion in MemoryPressureMonitor
Summary: REGRESSION(r267763): [GTK][WPE] Broken main thread assertion in MemoryPressur...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Lauro Moura
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-10-05 21:35 PDT by Lauro Moura
Modified: 2021-02-04 00:48 PST (History)
5 users (show)

See Also:


Attachments
js/throw-large-string-oom.html trace (16.02 KB, text/plain)
2020-10-05 21:35 PDT, Lauro Moura
no flags Details
Patch (2.26 KB, patch)
2021-02-03 19:42 PST, Lauro Moura
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Lauro Moura 2020-10-05 21:35:49 PDT
Created attachment 410609 [details]
js/throw-large-string-oom.html trace

Some tests affected:

imported/w3c/web-platform-tests/xhr/sync-no-timeout.any.html
js/throw-large-string-oom.html (all the time, running ~10 iterations to reproduce locally)
scrollbars/scrollbar-selectors.html (only once)

Sample trace (full attached):

Thread 1 (Thread 0x7fa8ba7ff700 (LWP 29320)):
#0  0x00007fa8cac8e9ab in WTFCrash () at /app/webkit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#1  0x0000000000428bba in  ()
#2  0x00007fa8cd3b6ea5 in _ZN6WebKit19NetworkProcessProxy19allNetworkProcessesEv () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#3  0x00007fa8cd2b657a in _ZN6WebKit14WebProcessPool23sendMemoryPressureEventEb () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#4  0x00007fa8cd40c483 in _ZZN6WebKit21MemoryPressureMonitor5startEvENK3$_7clEv () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#5  0x00007fa8cd40bc76 in  () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#6  0x00007fa8caca9570 in _ZN3WTF6Thread10entryPointEPNS0_16NewThreadContextE () at /app/webkit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#7  0x00007fa8cad04026 in _ZN3WTFL19wtfThreadEntryPointEPv () at /app/webkit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#8  0x00007fa8c783f4d2 in start_thread (arg=<optimized out>) at pthread_create.c:477
#9  0x00007fa8c53a34d3 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

It's asserting the isMain() check in NetworkProcessProxy.cpp networkProcessesSet(), added by r267763 (line 89 in https://trac.webkit.org/changeset/267763/webkit#file47).
Comment 1 Carlos Alberto Lopez Perez 2020-10-22 20:38:18 PDT
This test is also crashing in more bots than in gtk-release-wayland bot.

And it also times out sometimes.

This is a summary of what happened on the bots recently:

gtk-release: crashed 171 times on the last 4000 runs
gtk-debug: crashed 1 times and timed out 296 times on the last 4000 runs
wpe-release crashed 97 times and timed out 11 times on the last 4000 runs
wpe-debug: timed out 330 times on the last 4000 runs
Comment 2 Carlos Alberto Lopez Perez 2020-10-22 20:50:01 PDT
(In reply to Carlos Alberto Lopez Perez from comment #1)
> This test is also crashing in more bots than in gtk-release-wayland bot.
> 
> And it also times out sometimes.
> 
> This is a summary of what happened on the bots recently:
> 
> gtk-release: crashed 171 times on the last 4000 runs
> gtk-debug: crashed 1 times and timed out 296 times on the last 4000 runs
> wpe-release crashed 97 times and timed out 11 times on the last 4000 runs
> wpe-debug: timed out 330 times on the last 4000 runs

With "this test" I wanted to mean here the test js/throw-large-string-oom.html 

For the other 2 I no longer see them crashing. 

Updated expectations on r268903
Comment 3 Alicia Boya García 2020-12-10 08:14:53 PST
I have reproduced this in the minibrowser as well. I was attaching gdb to the WebProcess in a computer with very limited RAM. This caused enough RAM to be used to trigger the memory pressure handler in the MiniBrowser, and it crashed there.

Here is the backtrace, same as reported before, but resolving the pointers:

ASSERTION FAILED: RunLoop::isMain()
#0  WTFCrash() () at ../../Source/WTF/wtf/Assertions.cpp:295
#1  0x00007f6bdd2309b7 in CRASH_WITH_INFO(...) () at DerivedSources/ForwardingHeaders/wtf/Assertions.h:713
#2  0x00007f6bde1df52c in WebKit::networkProcessesSet() () at ../../Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp:90
#3  0x00007f6bde1df5ad in WebKit::NetworkProcessProxy::allNetworkProcesses() () at ../../Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp:98
#4  0x00007f6bdde8063a in WebKit::WebProcessPool::sendMemoryPressureEvent(bool) (this=0x7f6bc44d6000, isCritical=false) at ../../Source/WebKit/UIProcess/WebProcessPool.cpp:443
#5  0x00007f6bde2d4e53 in operator()() const (__closure=0x7f6bc44fa6a8) at ../../Source/WebKit/UIProcess/linux/MemoryPressureMonitor.cpp:383
#6  0x00007f6bde2d60a6 in WTF::Detail::CallableWrapper<WebKit::MemoryPressureMonitor::start()::<lambda()>, void>::call(void) (this=0x7f6bc44fa6a0) at DerivedSources/ForwardingHeaders/wtf/Function.h:52
#7  0x00007f6bdd233fd3 in WTF::Function<void ()>::operator()() const (this=0x7f6b7dbfac30) at DerivedSources/ForwardingHeaders/wtf/Function.h:83
#8  0x00007f6bce715839 in WTF::Thread::entryPoint(WTF::Thread::NewThreadContext*) (newThreadContext=0x7f6bc44eb410) at ../../Source/WTF/wtf/Threading.cpp:179
#9  0x00007f6bce7a6191 in WTF::wtfThreadEntryPoint(void*) (context=0x7f6bc44eb410) at ../../Source/WTF/wtf/posix/ThreadingPOSIX.cpp:213
#10 0x00007f6bc87674d2 in start_thread (arg=<optimized out>) at pthread_create.c:477
#11 0x00007f6bc64852a3 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

Note this is the code that starts and runs the MemoryPressureMonitor, very clearly on its own thread.

void MemoryPressureMonitor::start()
{
    if (m_started)
        return;

    m_started = true;

    Thread::create("MemoryPressureMonitor", [] {
        [...]
        while (true) {
            [...]
            if (usedPercentage >= s_memoryPresurePercentageThreshold) {
                [...]
                for (auto* processPool : WebProcessPool::allProcessPools())
                    processPool->sendMemoryPressureEvent([...]);

Then WebProcessPool::sendMemoryPressureEvent() needs to traverse allNetworkProcesses()

void WebProcessPool::sendMemoryPressureEvent([...])
{
    [...]
    for (auto networkProcess : NetworkProcessProxy::allNetworkProcesses())

But networkProcessesSet() assumes it will only ever be used in the main thread, which is not the case here.

static HashSet<NetworkProcessProxy*>& networkProcessesSet()
{
    ASSERT(RunLoop::isMain());
    static NeverDestroyed<HashSet<NetworkProcessProxy*>> set;
    return set;
}
Comment 4 Lauro Moura 2021-02-03 19:42:15 PST
Created attachment 419219 [details]
Patch
Comment 5 EWS 2021-02-04 00:47:39 PST
Committed r272362: <https://trac.webkit.org/changeset/272362>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 419219 [details].
Comment 6 Radar WebKit Bug Importer 2021-02-04 00:48:14 PST
<rdar://problem/73970229>