Branch: refs/heads/main
Home: https://github.com/WebKit/WebKit
Commit: dbe37108181c9061fdc12b5c61ab669c48f1419b
https://github.com/WebKit/WebKit/commit/dbe37108181c9061fdc12b5c61ab669c48f1419b
Author: Ryan Reno <[email protected]>
Date: 2024-07-15 (Mon, 15 Jul 2024)
Changed paths:
A
LayoutTests/requestidlecallback/requestidlecallback-does-not-leak-document-expected.txt
A
LayoutTests/requestidlecallback/requestidlecallback-does-not-leak-document.html
A LayoutTests/requestidlecallback/resources/requestidlecallback-frame.html
M Source/WebCore/Modules/WebGPU/GPUDevice.cpp
M Source/WebCore/Modules/entriesapi/ErrorCallback.h
M Source/WebCore/Modules/entriesapi/ErrorCallback.idl
M Source/WebCore/Modules/entriesapi/FileCallback.h
M Source/WebCore/Modules/entriesapi/FileCallback.idl
M Source/WebCore/Modules/entriesapi/FileSystemEntriesCallback.h
M Source/WebCore/Modules/entriesapi/FileSystemEntriesCallback.idl
M Source/WebCore/Modules/entriesapi/FileSystemEntryCallback.h
M Source/WebCore/Modules/entriesapi/FileSystemEntryCallback.idl
M Source/WebCore/Modules/geolocation/PositionCallback.h
M Source/WebCore/Modules/geolocation/PositionCallback.idl
M Source/WebCore/Modules/geolocation/PositionErrorCallback.h
M Source/WebCore/Modules/geolocation/PositionErrorCallback.idl
M Source/WebCore/Modules/notifications/NotificationPermissionCallback.h
M Source/WebCore/Modules/notifications/NotificationPermissionCallback.idl
M Source/WebCore/Modules/remoteplayback/RemotePlaybackAvailabilityCallback.h
M
Source/WebCore/Modules/remoteplayback/RemotePlaybackAvailabilityCallback.idl
M Source/WebCore/Modules/web-locks/WebLockGrantedCallback.h
M Source/WebCore/Modules/web-locks/WebLockGrantedCallback.idl
M Source/WebCore/Modules/webaudio/AudioBufferCallback.h
M Source/WebCore/Modules/webaudio/AudioBufferCallback.idl
M Source/WebCore/Modules/webaudio/AudioWorkletProcessorConstructor.h
M Source/WebCore/Modules/webaudio/AudioWorkletProcessorConstructor.idl
M Source/WebCore/Modules/webdatabase/DatabaseCallback.h
M Source/WebCore/Modules/webdatabase/DatabaseCallback.idl
M Source/WebCore/Modules/webdatabase/SQLStatementCallback.h
M Source/WebCore/Modules/webdatabase/SQLStatementCallback.idl
M Source/WebCore/Modules/webdatabase/SQLStatementErrorCallback.h
M Source/WebCore/Modules/webdatabase/SQLStatementErrorCallback.idl
M Source/WebCore/Modules/webdatabase/SQLTransactionCallback.h
M Source/WebCore/Modules/webdatabase/SQLTransactionCallback.idl
M Source/WebCore/Modules/webdatabase/SQLTransactionErrorCallback.h
M Source/WebCore/Modules/webdatabase/SQLTransactionErrorCallback.idl
M Source/WebCore/Modules/webxr/WebXRSystem.cpp
M Source/WebCore/Modules/webxr/XRFrameRequestCallback.h
M Source/WebCore/Modules/webxr/XRFrameRequestCallback.idl
M Source/WebCore/animation/CustomEffectCallback.h
M Source/WebCore/animation/CustomEffectCallback.idl
M Source/WebCore/bindings/js/JSCallbackData.cpp
M Source/WebCore/bindings/js/JSCallbackData.h
M Source/WebCore/bindings/scripts/CodeGeneratorJS.pm
M Source/WebCore/bindings/scripts/IDLAttributes.json
M Source/WebCore/bindings/scripts/test/JS/JSTestCallbackFunction.cpp
M Source/WebCore/bindings/scripts/test/JS/JSTestCallbackFunction.h
A
Source/WebCore/bindings/scripts/test/JS/JSTestCallbackFunctionGenerateIsReachable.cpp
A
Source/WebCore/bindings/scripts/test/JS/JSTestCallbackFunctionGenerateIsReachable.h
M Source/WebCore/bindings/scripts/test/JS/JSTestCallbackFunctionRethrow.cpp
M Source/WebCore/bindings/scripts/test/JS/JSTestCallbackFunctionRethrow.h
R Source/WebCore/bindings/scripts/test/JS/JSTestCallbackFunctionStrong.cpp
R Source/WebCore/bindings/scripts/test/JS/JSTestCallbackFunctionStrong.h
M
Source/WebCore/bindings/scripts/test/JS/JSTestCallbackFunctionWithThisObject.cpp
M
Source/WebCore/bindings/scripts/test/JS/JSTestCallbackFunctionWithThisObject.h
M
Source/WebCore/bindings/scripts/test/JS/JSTestCallbackFunctionWithTypedefs.cpp
M
Source/WebCore/bindings/scripts/test/JS/JSTestCallbackFunctionWithTypedefs.h
M
Source/WebCore/bindings/scripts/test/JS/JSTestCallbackFunctionWithVariadic.cpp
M
Source/WebCore/bindings/scripts/test/JS/JSTestCallbackFunctionWithVariadic.h
M Source/WebCore/bindings/scripts/test/JS/JSTestCallbackInterface.cpp
M Source/WebCore/bindings/scripts/test/JS/JSTestCallbackInterface.h
M Source/WebCore/bindings/scripts/test/JS/JSTestVoidCallbackFunction.cpp
M Source/WebCore/bindings/scripts/test/JS/JSTestVoidCallbackFunction.h
M Source/WebCore/bindings/scripts/test/SupplementalDependencies.dep
A
Source/WebCore/bindings/scripts/test/TestCallbackFunctionGenerateIsReachable.idl
R Source/WebCore/bindings/scripts/test/TestCallbackFunctionStrong.idl
M Source/WebCore/css/CSSPaintCallback.h
M Source/WebCore/css/CSSPaintCallback.idl
M Source/WebCore/dom/AbortAlgorithm.h
M Source/WebCore/dom/AbortAlgorithm.idl
M Source/WebCore/dom/IdleRequestCallback.h
M Source/WebCore/dom/IdleRequestCallback.idl
M Source/WebCore/dom/RequestAnimationFrameCallback.h
M Source/WebCore/dom/RequestAnimationFrameCallback.idl
M Source/WebCore/dom/StringCallback.h
M Source/WebCore/dom/StringCallback.idl
M Source/WebCore/dom/ViewTransitionUpdateCallback.h
M Source/WebCore/dom/ViewTransitionUpdateCallback.idl
M Source/WebCore/fileapi/BlobCallback.h
M Source/WebCore/fileapi/BlobCallback.idl
M Source/WebCore/html/VideoFrameRequestCallback.h
M Source/WebCore/html/VideoFrameRequestCallback.idl
M Source/WebCore/html/VoidCallback.h
M Source/WebCore/html/VoidCallback.idl
M Source/WebCore/inspector/RTCLogsCallback.h
M Source/WebCore/inspector/RTCLogsCallback.idl
M Source/WebCore/inspector/agents/InspectorDatabaseAgent.cpp
M Source/WebCore/page/NavigationInterceptHandler.h
M Source/WebCore/page/NavigationInterceptHandler.idl
M Source/WebCore/testing/XRSimulateUserActivationFunction.h
M Source/WebCore/testing/XRSimulateUserActivationFunction.idl
M Source/WebCore/xml/CustomXPathNSResolver.h
M Source/WebCore/xml/CustomXPathNSResolver.idl
Log Message:
-----------
Make all callbacks Weak handles
https://bugs.webkit.org/show_bug.cgi?id=276563
rdar://131646743
Reviewed by Ryosuke Niwa.
Callback functions in Web APIs have been a frequent source of memory
leaks on real websites. This is due to the fact that the underlying
JavaScript function object was created as a JSC::Strong handle which
made it a root in the GC heap. This makes it simple for memory
mangagement for an API with a callback as the function object becomes
ineligible for garbage collection unless all references from C++ to
the wrapper are released. However, this makes it incredibly easy to create
subtle memory retention bugs due to circular references if the JS wrapper
lifetime is not managed appropriately on the C++ side (which can be
quite nontrivial). Usually the correct thing to do would be to make
callbacks Weak handles by using an extended IDL attribute and using
JS wrapper lifecycle management facilities like opaque roots to keep
the callbacks alive as long as needed by the owning object.
In 280611@main I replaced the `IsWeakCallback` attribute with
`IsStrongCallback` which would mean a developer would need to go out of
their way to make the callback handles Strong. This mitigation has
proven to be useful in testing on real sites. However, the design is
fragile and leaves us open to reintroducing leaks in the future. This patch
takes it one step further by making it impossible to make Strong handles for
callbacks.
We reuse the `GenerateIsReachable=ImplScriptExecutionContext` extended
IDL attribute to tell the code generator that we want to tie the
lifetime of the callback object to that of the script context (usually a
Document or Worker). This gives us most of the benefits of Strong
without allowing for circular references and greatly reducing leaks.
Note that it's not necessarily correct to tie the lifetime of the
callback to the ScriptExecutionContext but it makes it likely impossible
to cause Document leaks with a callback with this implementation.
By default, then, an unannotated callback in IDL will be a Weak handle
which will require some other object to keep it alive which is unchanged
behavior.
* LayoutTests/requestidlecallback/:
Added a test to prove that a callback with
`GenerateIsReachable=ImplScriptExecutionContext`
will not leak the document.
* Source/WebCore:
Every callback IDL that contained the attribute `IsStrongCallback`
was replaced with `GenerateIsReachable=ImplScriptExecutionContext`.
Additionally, every callback base class now needs to declare a
pure virtual `hasCallback` member function for the generated wrapper
code to override so those changes were also made.
* Source/WebCore/bindings/js/JSCallbackData.cpp:
* Source/WebCore/bindings/js/JSCallbackData.h:
Fold JSCallbackDataWeak into JSCallbackData and delete
JSCallbackDataStrong. The code generator is responsible for passing
the correct owner pointer into the JSCallbackData constructor so
that isReachableFromOpaqueRoots will query the visitor for the
correct opaque root.
* Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:
(GenerateCallbackHeaderContent):
(GenerateCallbackImplementationContent):
* Source/WebCore/bindings/scripts/IDLAttributes.json:
* Source/WebCore/bindings/scripts/test/JS/:
bindings test rebaselines to account for the new codegen behavior.
Canonical link: https://commits.webkit.org/280975@main
To unsubscribe from these emails, change your notification settings at
https://github.com/WebKit/WebKit/settings/notifications
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
