Bug 117842 – WebProcess hangs loading eff.org (Waiting forever on a sync XHR, NetworkProcess unable to service it)

• Component: WebKit2
• Status: RESOLVED FIXED

[Brady Eidson]

WebProcess hangs loading eff.org (Waiting forever on a sync XHR, NetworkProcess unable to service it)

This is the known theoretical "6 + 1 problem" where 6 asynchronous requests to a host are already outstanding while a 7th sync XHR is made.

In single WebProcess mode and WebKit 1, the sync XHR will time out and the WebProcess will recover.

With the NetworkProcess the WebProcess waits forever on the NetworkProcess, and the NetworkProcess doesn't even try to serve the 7th request.

A lot more could be said about this.  But the TLDR; is that we need to bump the platform connection-per-host limit to 7, and only use that 7th slot for a sync XHR if 6 loads are already in progress.

In radar as <rdar://problem/14112329>

[Brady Eidson]

Created attachment 205108 [details]
Patch v1

[Alexey Proskuryakov]

Comment on attachment 205108 [details]
Patch v1 

View in context: https://bugs.webkit.org/attachment.cgi?id=205108&action=review

> Source/WebKit2/NetworkProcess/HostRecord.cpp:198
> +    ASSERT(loader && loader->connectionToWebProcess());

Two lines.

> Source/WebKit2/NetworkProcess/HostRecord.cpp:204
> +    if (loader->connectionToWebProcess()->isSerialLoadingEnabled() && m_loadersInProgress.size() >= 1)
> +        return true;

What is serial loading? Sounds like we have the exactly the same issue with it if there is one outstanding request, and WebProcess starts a sync one.

> Source/WebKit2/NetworkProcess/HostRecord.cpp:213
> +    // If we're already past the limit of the number of loaders in flight, we won't even serve new synchronous requests right now.
> +    if (m_loadersInProgress.size() > m_maxRequestsInFlight)
> +        return true;

Perhaps assert that at least one active loader is synchronous?

> Source/WebKit2/NetworkProcess/NetworkResourceLoadScheduler.h:29
> +#include "HostRecord.h"

Why?

[Brady Eidson]

(In reply to comment #2)
> (From update of attachment 205108 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=205108&action=review
> 
> > Source/WebKit2/NetworkProcess/HostRecord.cpp:198
> > +    ASSERT(loader && loader->connectionToWebProcess());
> 
> Two lines.

Yup!

> > Source/WebKit2/NetworkProcess/HostRecord.cpp:204
> > +    if (loader->connectionToWebProcess()->isSerialLoadingEnabled() && m_loadersInProgress.size() >= 1)
> > +        return true;
> 
> What is serial loading? Sounds like we have the exactly the same issue with it if there is one outstanding request, and WebProcess starts a sync one.

It's SPI for DRT/WKTR for a tiny subset of tests.  None with sync XHR.  Shouldn't be a factor in real browsing.

> > Source/WebKit2/NetworkProcess/HostRecord.cpp:213
> > +    // If we're already past the limit of the number of loaders in flight, we won't even serve new synchronous requests right now.
> > +    if (m_loadersInProgress.size() > m_maxRequestsInFlight)
> > +        return true;
> 
> Perhaps assert that at least one active loader is synchronous?

It'll be a lot of debug-only code, but I think it's worthwhile.

> > Source/WebKit2/NetworkProcess/NetworkResourceLoadScheduler.h:29
> > +#include "HostRecord.h"
> 
> Why?

Leftover from earlier patch, and I'd fixed locally but not committed - Thanks for spotting it.

Thanks!

[Brady Eidson]

http://trac.webkit.org/changeset/151795