Bug 252059

Summary: [run-webkit-tests] found several memory leak while running with --world-leaks
Product: WebKit Reporter: cathiechen <cathiechen>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: NEW ---    
Severity: Normal CC: ap, cathiechen, koivisto, mmaxfield, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=251834

Description cathiechen 2023-02-10 09:11:27 PST
While run-webkit-tests with --world-leaks on Mac WK2, it reports some memory leak.

For instance, run it for imported/w3c/web-platform-tests/css/css-sizing/

14:46:42.469 25391 Testing completed, Exit status: 9
=> Results: 366/389 tests passed (94.1%)

=> Tests to be fixed (24):
     13 image-only failures      (54.2%)
      8 leaks                    (33.3%)

=> Tests that will only be fixed if they crash (WONTFIX) (0):


Unexpected flakiness: leaks (8)
  imported/w3c/web-platform-tests/css/css-sizing/animation/aspect-ratio-interpolation.html [ Pass Leak ]
  imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio-affects-container-width-when-height-changes.html [ Pass Leak ]
  imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/abspos-001.html [ Pass Leak ]
  imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/parsing/aspect-ratio-computed.html [ Pass Leak ]
  imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/zero-or-infinity-006.html [ Pass Leak ]
  imported/w3c/web-platform-tests/css/css-sizing/contain-intrinsic-size/animation/contain-intrinsic-size-interpolation.html [ Pass Leak ]
  imported/w3c/web-platform-tests/css/css-sizing/contain-intrinsic-size/auto-001.html [ Pass Leak ]
  imported/w3c/web-platform-tests/css/css-sizing/contain-intrinsic-size/parsing/contain-intrinsic-size-computed.html [ Pass Leak ]

Tried code on Jan 31, b4048150686363c, there is no complaint.
Comment 1 Radar WebKit Bug Importer 2023-02-17 09:12:19 PST
<rdar://problem/105599530>
Comment 2 Simon Fraser (smfr) 2023-02-17 10:18:16 PST
This suggests that something is triggering a retain cycle between the Document and other objects
Comment 3 cathiechen 2023-02-20 10:47:38 PST
[1] https://bugs.webkit.org/show_bug.cgi?id=251834

Looks like [1] introduced this.
Yes, before this change, run-webkit-tests with --world-leaks crashes.
I used below to work around it temporarily.

diff --git a/Tools/WebKitTestRunner/TestController.cpp b/Tools/WebKitTestRunner/TestController.cpp
index 9f9bf7fa7e3e..b808251b7b56 100644
--- a/Tools/WebKitTestRunner/TestController.cpp
+++ b/Tools/WebKitTestRunner/TestController.cpp
@@ -1784,8 +1784,10 @@ void TestController::didReceiveLiveDocumentsList(WKArrayRef liveDocumentList)
 
     HashMap<uint64_t, String> documentInfo;
     for (size_t i = 0; i < numDocuments; ++i) {
-        if (auto dictionary = dictionaryValue(WKArrayGetItemAtIndex(liveDocumentList, i)))
-            documentInfo.add(uint64Value(dictionary, "id"), toWTFString(stringValue(dictionary, "url")));
+        if (auto dictionary = dictionaryValue(WKArrayGetItemAtIndex(liveDocumentList, i))) {
+            if (auto id = uint64Value(dictionary, "id"))
+                documentInfo.add(id, toWTFString(stringValue(dictionary, "url")));
+        }
     }
 
     if (!documentInfo.size()) {
Comment 4 Simon Fraser (smfr) 2023-02-20 11:05:21 PST
I fixed that recently on trunk.
Comment 5 cathiechen 2023-02-20 11:40:38 PST
So the document leak is true?
We didn't find out because run-webkit-tests with --world-leaks crashes.
Comment 6 Simon Fraser (smfr) 2023-02-20 11:45:11 PST
Oh, you're saying that it might not be a regression, simply because the detection didn't work before? That might be true. You'd have to apply the WebKitTestRunner fix to older builds to test.
Comment 7 cathiechen 2023-02-20 12:26:03 PST
OK, applied the WebKitTestRunner fix to the code on Jan 31, b4048150686363c, there is memory leak too.
So it is not a regression from that commit...