Bug 256262

Summary:	HTMLCanvasElement is orphaned causing a HTMLDocument leak on YouTube video pages
Product:	WebKit	Reporter:	Ryan Reno <rreno>
Component:	Canvas	Assignee:	Ryan Reno <rreno>
Status:	RESOLVED FIXED
Severity:	Normal	CC:	dino, 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=251835

Description Ryan Reno 2023-05-03 09:56:50 PDT

See also https://webkit.org/b/251835

Quoting Simon from that bug:
>Launch a recent Safari or MiniBrowser build from terminal (so you can see the output), and visit a YouTube.com video page (or a page with YouTube.com in a subframe). In that tab, navigate to about:blank, then trigger the low memory >handler (to clear caches):
>> notifyutil -p "org.WebKit.lowMemory"
>
>then dump the list of live documents:
>> notifyutil -p  "com.apple.WebKit.showAllDocuments"

As of 263633@main you will likely see the YouTube document with ref count > 10 and referencingNodeCount anywhere from about 7500 - 15000 depending on when you navigated away from the page. If you apply the patch in the PR in 251835 then you will see the HTMLDocument has refCount 0 and referencingNodeCount 1. That 1 node is an HTMLCanvasElement.

From preliminary testing (adding Ref tracking to HTMLCanvasElement and to the Node ctor and dtors) it appears the pointer to the element may have been leaked from the smart pointer and never derefed.

Comment 1 Radar WebKit Bug Importer 2023-05-03 09:57:05 PDT

<rdar://problem/108845985>

Comment 2 Ryan Reno 2023-05-03 10:05:03 PDT

This should probably be in DOM not Canvas.

Comment 3 Ryan Reno 2023-05-03 16:52:14 PDT

YouTube appears to be drawing gradients in a canvas for use as background style information. It turns out that a CanvasRenderingContext2D will ref the HTMLCanvasElement when it is reffed. It also turns out that a CanvasGradient holds a Ref<> to the context.

CanvasStyle holds a Ref to a CanvasGradient. CanvasGradient holds a ref to CanvasRenderingContext. CanvasRenderingContext has a state stack which holds a Ref to a CanvasStyle.

In other words, a CanvasRenderingContext2D has a state stack which contains a circular reference to itself. This State object is never removed from the stack. I verified that the CanvasStyle is assigned to the State::fillStyle field but that the stack is never modified again from lldb.

Comment 4 Ryan Reno 2023-05-03 16:57:40 PDT

Here's a stack trace of the underef-ed CanvasGradient. Frame 20 is the point where the circular reference is created:

(I removed the frames with the hideous std::variant type information)

RefTracker: Backtrace for token 10727 (CanvasGradient)
1   0x135e581ac WTF::RefTracker::trackRef(WTF::String const&)
2   0x283fa7478 WebCore::CanvasGradient::trackRef() const
3   0x280956728 void WTF::RefTrackingTraits::refIfNotNull<WebCore::CanvasGradient>(WebCore::CanvasGradient*)
4   0x283fcd248 WTF::RefPtr<WebCore::CanvasGradient, WTF::RawPtrTraits<WebCore::CanvasGradient>, WTF::RefDerefTraits>::RefPtr(WTF::RefPtr<WebCore::CanvasGradient, WTF::RawPtrTraits<WebCore::CanvasGradient>, WTF::RefDerefTraits> const&)
5   0x283fcd1f0 WTF::RefPtr<WebCore::CanvasGradient, WTF::RawPtrTraits<WebCore::CanvasGradient>, WTF::RefDerefTraits>::RefPtr(WTF::RefPtr<WebCore::CanvasGradient, WTF::RawPtrTraits<WebCore::CanvasGradient>, WTF::RefDerefTraits> const&)
[snip - frames containing std::variant]
20  0x283fbb77c WebCore::CanvasStyle::operator=(WebCore::CanvasStyle const&)
21  0x283fbb92c WebCore::CanvasRenderingContext2DBase::setFillStyle(WebCore::CanvasStyle)
[snip - frames containing std::variant]
32  0x283fc4b80 WebCore::CanvasRenderingContext2DBase::setFillStyle(std::__1::variant<WTF::String, WTF::RefPtr<WebCore::CanvasGradient, WTF::RawPtrTraits<WebCore::CanvasGradient>, WTF::RefDerefTraits>, WTF::RefPtr<WebCore::CanvasPattern, WTF::RawPtrTraits<WebCore::CanvasPattern>, WTF::RefDerefTraits>>&&)
33  0x280959424 WebCore::setJSCanvasRenderingContext2D_fillStyleSetter(JSC::JSGlobalObject&, WebCore::JSCanvasRenderingContext2D&, JSC::JSValue)::'lambda'()::operator()() const
34  0x2809593f4 void WebCore::invokeFunctorPropagatingExceptionIfNecessary<WebCore::setJSCanvasRenderingContext2D_fillStyleSetter(JSC::JSGlobalObject&, WebCore::JSCanvasRenderingContext2D&, JSC::JSValue)::'lambda'()>(JSC::JSGlobalObject&, JSC::ThrowScope&, WebCore::setJSCanvasRenderingContext2D_fillStyleSetter(JSC::JSGlobalObject&, WebCore::JSCanvasRenderingContext2D&, JSC::JSValue)::'lambda'()&&)
35  0x280959360 WebCore::setJSCanvasRenderingContext2D_fillStyleSetter(JSC::JSGlobalObject&, WebCore::JSCanvasRenderingContext2D&, JSC::JSValue)
36  0x2808a4220 bool WebCore::IDLAttribute<WebCore::JSCanvasRenderingContext2D>::set<&WebCore::setJSCanvasRenderingContext2D_fillStyleSetter(JSC::JSGlobalObject&, WebCore::JSCanvasRenderingContext2D&, JSC::JSValue), (WebCore::CastedThisErrorBehavior)0>(JSC::JSGlobalObject&, long long, long long, JSC::PropertyName)
37  0x2808a40ec WebCore::setJSCanvasRenderingContext2D_fillStyle(JSC::JSGlobalObject*, long long, long long, JSC::PropertyName)
38  0x137c153e4 WTF::FunctionPtr<(WTF::PtrTag)28258, bool (JSC::JSGlobalObject*, long long, long long, JSC::PropertyName), (WTF::FunctionAttributes)1>::operator()(JSC::JSGlobalObject*, long long, long long, JSC::PropertyName) const
39  0x137c14b68 JSC::JSObject::putInlineSlow(JSC::JSGlobalObject*, JSC::PropertyName, JSC::JSValue, JSC::PutPropertySlot&)
40  0x13785dc04 JSC::JSObject::putInlineForJSObject(JSC::JSCell*, JSC::JSGlobalObject*, JSC::PropertyName, JSC::JSValue, JSC::PutPropertySlot&)
41  0x1372290d8 JSC::JSCell::putInline(JSC::JSGlobalObject*, JSC::PropertyName, JSC::JSValue, JSC::PutPropertySlot&)
42  0x13783a7dc JSC::JSValue::putInline(JSC::JSGlobalObject*, JSC::PropertyName, JSC::JSValue, JSC::PutPropertySlot&)
43  0x137839ba4 llint_slow_path_put_by_id
44  0x1364b8c98 llint_function_for_construct_arity_checkTagGateAfter
[snip]
55  0x137658314 JSC::Interpreter::executeProgram(JSC::SourceCode const&, JSC::JSGlobalObject*, JSC::JSObject*)
56  0x137996604 JSC::evaluate(JSC::JSGlobalObject*, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&)
57  0x137996754 JSC::profiledEvaluate(JSC::JSGlobalObject*, JSC::ProfilingReason, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&)
58  0x2830fd7b8 WebCore::JSExecState::profiledEvaluate(JSC::JSGlobalObject*, JSC::ProfilingReason, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&)
59  0x2830fd240 WebCore::ScriptController::evaluateInWorld(WebCore::ScriptSourceCode const&, WebCore::DOMWrapperWorld&)
60  0x2830fd070 WebCore::ScriptController::evaluateInWorldIgnoringException(WebCore::ScriptSourceCode const&, WebCore::DOMWrapperWorld&)
61  0x2830fdac4 WebCore::ScriptController::evaluateIgnoringException(WebCore::ScriptSourceCode const&)
62  0x283ae0174 WebCore::ScriptElement::executeClassicScript(WebCore::ScriptSourceCode const&)
63  0x283addd90 WebCore::ScriptElement::prepareScript(WTF::TextPosition const&, WebCore::ScriptElement::LegacyTypeSupport)
64  0x2840d580c WebCore::HTMLScriptRunner::runScript(WebCore::ScriptElement&, WTF::TextPosition const&)
65  0x2840d55e0 WebCore::HTMLScriptRunner::execute(WTF::Ref<WebCore::ScriptElement, WTF::RawPtrTraits<WebCore::ScriptElement>, WTF::RefDerefTraits>&&, WTF::TextPosition const&)
66  0x2840977b0 WebCore::HTMLDocumentParser::runScriptsForPausedTreeBuilder()
67  0x284097cbc WebCore::HTMLDocumentParser::pumpTokenizerLoop(WebCore::HTMLDocumentParser::SynchronousMode, bool, WebCore::PumpSession&)
68  0x284097048 WebCore::HTMLDocumentParser::pumpTokenizer(WebCore::HTMLDocumentParser::SynchronousMode)
69  0x2840973c0 WebCore::HTMLDocumentParser::resumeParsingAfterYield()
70  0x2840ca324 WebCore::HTMLParserScheduler::continueNextChunkTimerFired()
71  0x2840d31a4 decltype(*std::declval<WebCore::HTMLParserScheduler*&>().*std::declval<void (WebCore::HTMLParserScheduler::*&)()>()()) std::__1::__invoke[abi:v160000]<void (WebCore::HTMLParserScheduler::*&)(), WebCore::HTMLParserScheduler*&, void>(void (WebCore::HTMLParserScheduler::*&)(), WebCore::HTMLParserScheduler*&)
72  0x2840d310c std::__1::__bind_return<void (WebCore::HTMLParserScheduler::*)(), std::__1::tuple<WebCore::HTMLParserScheduler*>, std::__1::tuple<>, __is_valid_bind_return<void (WebCore::HTMLParserScheduler::*)(), std::__1::tuple<WebCore::HTMLParserScheduler*>, std::__1::tuple<>>::value>::type std::__1::__apply_functor[abi:v160000]<void (WebCore::HTMLParserScheduler::*)(), std::__1::tuple<WebCore::HTMLParserScheduler*>, 0ul, std::__1::tuple<>>(void (WebCore::HTMLParserScheduler::*&)(), std::__1::tuple<WebCore::HTMLParserScheduler*>&, std::__1::__tuple_indices<0ul>, std::__1::tuple<>&&)
73  0x2840d30c0 std::__1::__bind_return<void (WebCore::HTMLParserScheduler::*)(), std::__1::tuple<WebCore::HTMLParserScheduler*>, std::__1::tuple<>, __is_valid_bind_return<void (WebCore::HTMLParserScheduler::*)(), std::__1::tuple<WebCore::HTMLParserScheduler*>, std::__1::tuple<>>::value>::type std::__1::__bind<void (WebCore::HTMLParserScheduler::*&)(), WebCore::HTMLParserScheduler*>::operator()[abi:v160000]<>()
74  0x2840d305c WTF::Detail::CallableWrapper<std::__1::__bind<void (WebCore::HTMLParserScheduler::*&)(), WebCore::HTMLParserScheduler*>, void>::call()
75  0x2823c5ccc WTF::Function<void ()>::operator()() const
76  0x28304a0e0 WebCore::Timer::fired()
77  0x284b15c54 WebCore::ThreadTimers::sharedTimerFiredInternal()
78  0x284b1e9b0 WebCore::ThreadTimers::setSharedTimer(WebCore::SharedTimer*)::$_0::operator()() const
79  0x284b1e954 WTF::Detail::CallableWrapper<WebCore::ThreadTimers::setSharedTimer(WebCore::SharedTimer*)::$_0, void>::call()
80  0x2823c5ccc WTF::Function<void ()>::operator()() const
81  0x284ab6c68 WebCore::MainThreadSharedTimer::fired()
82  0x284bb3614 WebCore::timerFired(__CFRunLoopTimer*, void*)
[snip]

Comment 5 Ryan Reno 2023-05-03 21:28:53 PDT

I can't make the CanvasGradient hold a weak reference to a context - that doesn't make sense because the spec doesn't give me the option of throwing an exception if there is no associated context.

I can probably make the CanvasStyle hold a WeakPtr to the CanvasGradient though. That way the Stack object that holds the CanvasStyle won't create a circular reference via CanvasGradient in this case.

Comment 6 Ryan Reno 2023-05-04 14:56:08 PDT

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

Comment 7 EWS 2023-05-07 05:26:54 PDT

Committed 263774@main (ade36a336115): <https://commits.webkit.org/263774@main>

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