Bug 256262

Summary: HTMLCanvasElement is orphaned causing a HTMLDocument leak on YouTube video pages
Product: WebKit Reporter: Ryan Reno <rreno>
Component: CanvasAssignee: 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
https://bugs.webkit.org/show_bug.cgi?id=277234

Ryan Reno
Reported 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.
Attachments
Radar WebKit Bug Importer
Comment 1 2023-05-03 09:57:05 PDT
Ryan Reno
Comment 2 2023-05-03 10:05:03 PDT
This should probably be in DOM not Canvas.
Ryan Reno
Comment 3 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.
Ryan Reno
Comment 4 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]
Ryan Reno
Comment 5 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.
Ryan Reno
Comment 6 2023-05-04 14:56:08 PDT
EWS
Comment 7 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.
Note You need to log in before you can comment on or make changes to this bug.