Bug 314187 – [Leak] Service worker fetch() retains ResourceResponses in the remote worker DocumentLoader

• Component: WebCore Misc.
• Status: RESOLVED FIXED

Created attachment 479496 [details]
screenshot of the memory been used and never cleaned after the

# Short version

A Service Worker fetch leak makes WebContent memory grow and stay alive after the page becomes idle.

The repro needs the Service Worker to intercept requests like this:

    event.respondWith(fetch(event.request));

Refreshing the page does not release the retained memory.

# Reproducing

## Setup

1. Use the files from this gist:
   https://gist.github.com/RatoX/60d8b7d6c68a8fc4d1c220fd4a5c61bb

2. Run the server:

       python3 server.py --port 8787

## Steps

1. Open this URL:

       http://127.0.0.1:8787

2. Click Start.
3. Wait until the hardcoded 20,000 requests finish.
4. Check the WebContent memory profile.

## Expected result

Memory can grow while the requests are running, but after the page becomes idle, the retained memory should be released.

## Actual result

The memory is not released after the workload finishes and the page becomes idle. See the attached screenshot.

In one local unpatched run:

```
After service worker install, before starting requests: 14.8M physical footprint
After 20,000 requests and idle settle:              77.6M physical footprint
Peak during the run:                              113.1M physical footprint
```

## Where I can reproduce it

Reproduced in Safari 16, current Safari production, Safari Technology Preview, and a local WebKit build from safari-7624.1.16.13-branch. And also in native mac and iOS apps using WKWebView.


--------


# Investigation and debbuging

## Context

I work for Canva.com and we found multiple crash coming from iOS due to low memory. We decided to investigate the problem and try to unblock the rollout of our feature on iOS devices.

## What I found 

I noticed the response data `ResourceResponse` from the Service Worker fetch was still alive after the page became idle. After tracing the retained objects, the owner looked like the service worker remote worker `DocumentLoader`, through `DocumentLoader::m_responses`.

That loader was already complete, but `m_stopRecordingResponses` was still false. Because of that, every worker side fetch response kept being appended to `m_responses`.

## What I found in the WebKit code

The backing frame is created through `Page::setupForRemoteWorker()`, which calls `FrameLoader::initForSynthesizedDocument()`. That function directly sets:

```
m_state = FrameState::Complete;
```

but it bypasses `FrameLoader::setState(FrameState::Complete)`, which is the normal path that calls:

```
documentLoader->stopRecordingResponses();
```

As result, the remote worker `DocumentLoader` can be complete while `m_stopRecordingResponses` is still false. Every worker side fetch response is then appended to `DocumentLoader::m_responses` and stays retained.

## What I did 

I was able to validate my hypothesis by applying this simple patch below into the code at branch: safari-7624.1.16.13-branch

```
diff --git a/Source/WebCore/loader/FrameLoader.cpp b/Source/WebCore/loader/FrameLoader.cpp
index b4f0ee8faf..e92238d5d9 100644
--- a/Source/WebCore/loader/FrameLoader.cpp
+++ b/Source/WebCore/loader/FrameLoader.cpp
@@ -458,6 +458,10 @@ void FrameLoader::initForSynthesizedDocument(const URL&)
     m_didCallImplicitClose = true;
     m_isComplete = true;
     m_state = FrameState::Complete;
+    // Synthesized documents bypass setState(FrameState::Complete), which normally
+    // stops recording responses on the DocumentLoader.
+    if (RefPtr documentLoader = m_documentLoader)
+        documentLoader->stopRecordingResponses();
     m_needsClear = true;

     m_networkingContext = m_client->createNetworkingContext();
```

## Results

```
Before the patch, settled run:
  StringImpl      18.3M
  VectorBuffer     8.2M
  system-malloc    4.1M
  total           ~30.6M

After the patch, settled run:
  StringImpl       9.6M
  VectorBuffer     2.1M
  system-malloc    2.2M
  total           ~13.9M
```

Pull request: https://github.com/WebKit/WebKit/pull/64384

[Radar WebKit Bug Importer]

<rdar://problem/176389228>

[EWS]

Committed 312779@main (c7cf80229641): <https://commits.webkit.org/312779@main>

Reviewed commits have been landed. Closing PR #64384 and removing active labels.

[EWS]

Committed 305413.860@safari-7624-branch (b8924fcb50ba): <https://commits.webkit.org/305413.860@safari-7624-branch>

Reviewed commits have been landed. Closing PR #5217 and removing active labels.