Bug 220637 - REGRESSION (r271472): [ Mac WK2 ] intersection-observer/target-deleted.html is timing out
Summary: REGRESSION (r271472): [ Mac WK2 ] intersection-observer/target-deleted.html i...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tim Horton
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-01-14 15:32 PST by Truitt Savell
Modified: 2021-01-21 22:08 PST (History)
8 users (show)

See Also:


Attachments
Patch (2.14 KB, patch)
2021-01-21 21:21 PST, Tim Horton
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Truitt Savell 2021-01-14 15:32:34 PST
intersection-observer/target-deleted.html

This test is timing out constantly sense around r271473

History:
https://results.webkit.org/?suite=layout-tests&test=intersection-observer%2Ftarget-deleted.html


I can reproduce this with command:
run-webkit-tests --iterations 2000 --exit-after-n-failures 1 --exit-after-n-crashes-or-timeouts 1 --debug-rwt-logging --no-retry --force --no-build -f intersection-observer/target-deleted.html
Comment 1 Radar WebKit Bug Importer 2021-01-14 15:32:49 PST
<rdar://problem/73220571>
Comment 2 Truitt Savell 2021-01-14 16:00:30 PST
I was able to bisect this to r271471
Comment 3 Truitt Savell 2021-01-14 16:01:05 PST
(In reply to Truitt Savell from comment #2)
> I was able to bisect this to r271471

r2714712 fails while r271471 passes actually. misstyped
Comment 4 Simon Fraser (smfr) 2021-01-15 10:12:23 PST
You mean https://trac.webkit.org/changeset/271472/webkit.

Seems pretty unlikely that's the cause.
Comment 5 cathiechen 2021-01-18 04:24:04 PST
Hmm, I can't reproduce it on Catalina debug wk2.
It seems specific to the release versions.
I'll try to build release locally.
Comment 6 cathiechen 2021-01-19 23:45:54 PST
(In reply to cathiechen from comment #5)
> Hmm, I can't reproduce it on Catalina debug wk2.
> It seems specific to the release versions.
> I'll try to build release locally.

Tried the release version on macOS Catalina and Big Sur, but can't reproduce it.
Comment 7 Tim Horton 2021-01-21 13:05:00 PST
I can reproduce with Truitt's command.
Comment 8 Tim Horton 2021-01-21 13:07:56 PST
Even a single iteration is sufficient (which is consistent with the bots, and will make debugging easier).
Comment 9 Tim Horton 2021-01-21 20:18:02 PST
The observerShouldBeRemoved interval is firing forever, with 1 outstanding intersection observer live (i.e., the test is catching exactly the failure it was intended to fix...)
Comment 10 Tim Horton 2021-01-21 20:21:29 PST
Nothing even remotely near the regression point makes any sense, so I'm just going to see if I can debug.
Comment 11 Tim Horton 2021-01-21 20:23:36 PST
Test was introduced in https://trac.webkit.org/changeset/237880/webkit
Comment 12 Tim Horton 2021-01-21 21:00:01 PST
I think this is just a test depending on GC and the GC not happening (despite the attempt to force it). If I run the test with --no-timeout, it spins forever, UNTIL the instant I try to remote-web-inspect it, then suddenly the element is collected, the IntersectionObserver goes away, and the test completes...
Comment 13 Ryosuke Niwa 2021-01-21 21:00:45 PST
Oh, try increasing the number of observers and check if any observer is collected? I suspect the test is buggy. Since we have a conservative GC, we can't guarantee all objects will be collected.
Comment 14 Simon Fraser (smfr) 2021-01-21 21:09:26 PST
Maybe the test needs to do something non-trivial in the loop to make gc() do something
Comment 15 Tim Horton 2021-01-21 21:09:33 PST
Wait, also, m_intersectionObservers is a Vector of WeakPtrs, and we're just getting its length... which won't decrease if one of the WeakPtrs gets zeroed, right? This is all a bit questionable.
Comment 16 Ryosuke Niwa 2021-01-21 21:19:29 PST
(In reply to Simon Fraser (smfr) from comment #14)
> Maybe the test needs to do something non-trivial in the loop to make gc() do
> something

It's never guaranteed that gc() will collect all garbage because we have a conservative GC. Some objects can be kept alive even if they're logically dead.
Comment 17 Tim Horton 2021-01-21 21:21:48 PST
Created attachment 418108 [details]
Patch
Comment 18 EWS 2021-01-21 22:08:25 PST
Committed r271736: <https://trac.webkit.org/changeset/271736>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 418108 [details].