Bug 213363 - Fix for crash in AXIsolatedObject::relativeFrame.
Summary: Fix for crash in AXIsolatedObject::relativeFrame.
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: Andres Gonzalez
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-06-18 19:17 PDT by Andres Gonzalez
Modified: 2020-06-26 12:27 PDT (History)
9 users (show)

See Also:


Attachments
Patch (4.70 KB, patch)
2020-06-18 19:30 PDT, Andres Gonzalez
no flags Details | Formatted Diff | Diff
Patch (1.65 KB, patch)
2020-06-26 11:54 PDT, Andres Gonzalez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andres Gonzalez 2020-06-18 19:17:04 PDT
Fix for crash in AXIsolatedObject::relativeFrame.
Comment 1 Andres Gonzalez 2020-06-18 19:30:24 PDT
Created attachment 402263 [details]
Patch
Comment 2 chris fleizach 2020-06-18 22:21:54 PDT
Comment on attachment 402263 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=402263&action=review

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:1976
> +        return Accessibility::retrieveAutoreleasedValueFromMainThread<NSValue *>([protectedSelf = retainPtr(self)] () -> RetainPtr<NSValue> {

why can't we access relativeFrame off the main thread? that's a big win

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:2314
> +    CFRetain((__bridge CFTypeRef)self);

feels like we should just be able to do

RetainPtr<WebAccessibilityObjectWrapper> protectedWrapper(self);

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:3076
> +        return Accessibility::retrieveAutoreleasedValueFromMainThread<NSValue *>([protectedSelf = retainPtr(self)] () -> RetainPtr<NSValue> {

how come this has to be access on main thread?
Comment 3 chris fleizach 2020-06-18 22:22:14 PDT
can you include the crash trace as a comment here
Comment 4 Andres Gonzalez 2020-06-26 11:39:55 PDT
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0xbbadbeef)
    frame #0: 0x000000064939115e JavaScriptCore`::WTFCrash() at Assertions.cpp:295:35
    frame #1: 0x000000062d7fa26b WebCore`WTFCrashWithInfo((null)=647, (null)="/Users/ag/s/web/OpenSource/WebKitBuild/Debug/usr/local/include/wtf/HashTable.h", (null)="void WTF::HashTable<unsigned long, WTF::KeyValuePair<unsigned long, WTF::RefPtr<WebCore::AccessibilityObject, WTF::DumbPtrTraits<WebCore::AccessibilityObject> > >, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<unsigned long, WTF::RefPtr<WebCore::AccessibilityObject, WTF::DumbPtrTraits<WebCore::AccessibilityObject> > > >, WTF::IntHash<unsigned long>, WTF::HashMap<unsigned long, WTF::RefPtr<WebCore::AccessibilityObject, WTF::DumbPtrTraits<WebCore::AccessibilityObject> >, WTF::IntHash<unsigned long>, WTF::HashTraits<unsigned long>, WTF::HashTraits<WTF::RefPtr<WebCore::AccessibilityObject, WTF::DumbPtrTraits<WebCore::AccessibilityObject> > > >::KeyValuePairTraits, WTF::HashTraits<unsigned long> >::checkKey(const T &) [Key = unsigned long, Value = WTF::KeyValuePair<unsigned long, WTF::RefPtr<WebCore::AccessibilityObject, WTF::DumbPtrTraits<WebCore::AccessibilityObject> > >, Extractor = WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<unsigned long, WTF::RefPtr<WebCore::AccessibilityObject, WTF::DumbPtrTraits<WebC"..., (null)=61) at Assertions.h:671:5
    frame #2: 0x000000062fa6d1fb WebCore`void WTF::HashTable<unsigned long, WTF::KeyValuePair<unsigned long, WTF::RefPtr<WebCore::AccessibilityObject, WTF::DumbPtrTraits<WebCore::AccessibilityObject> > >, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<unsigned long, WTF::RefPtr<WebCore::AccessibilityObject, WTF::DumbPtrTraits<WebCore::AccessibilityObject> > > >, WTF::IntHash<unsigned long>, WTF::HashMap<unsigned long, WTF::RefPtr<WebCore::AccessibilityObject, WTF::DumbPtrTraits<WebCore::AccessibilityObject> >, WTF::IntHash<unsigned long>, WTF::HashTraits<unsigned long>, WTF::HashTraits<WTF::RefPtr<WebCore::AccessibilityObject, WTF::DumbPtrTraits<WebCore::AccessibilityObject> > > >::KeyValuePairTraits, WTF::HashTraits<unsigned long> >::checkKey<WTF::HashMapTranslatorAdapter<WTF::HashMap<unsigned long, WTF::RefPtr<WebCore::AccessibilityObject, WTF::DumbPtrTraits<WebCore::AccessibilityObject> >, WTF::IntHash<unsigned long>, WTF::HashTraits<unsigned long>, WTF::HashTraits<WTF::RefPtr<WebCore::AccessibilityObject, WTF::DumbPtrTraits<WebCore::AccessibilityObject> > > >::KeyValuePairTraits, WTF::IdentityHashTranslator<WTF::HashMap<unsigned long, WTF::RefPtr<WebCore::AccessibilityObject, WTF::DumbPtrTraits<WebCore::AccessibilityObject> >, WTF::IntHash<unsigned long>, WTF::HashTraits<unsigned long>, WTF::HashTraits<WTF::RefPtr<WebCore::AccessibilityObject, WTF::DumbPtrTraits<WebCore::AccessibilityObject> > > >::KeyValuePairTraits, WTF::IntHash<unsigned long> > >, unsigned long>(this=0x000000065d9b7798, key=0x00007ffee3eb3540) at HashTable.h:647:9
    frame #3: 0x000000062fa6d084 WebCore`WTF::KeyValuePair<unsigned long, WTF::RefPtr<WebCore::AccessibilityObject, WTF::DumbPtrTraits<WebCore::AccessibilityObject> > >* WTF::HashTable<unsigned long, WTF::KeyValuePair<unsigned long, WTF::RefPtr<WebCore::AccessibilityObject, WTF::DumbPtrTraits<WebCore::AccessibilityObject> > >, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<unsigned long, WTF::RefPtr<WebCore::AccessibilityObject, WTF::DumbPtrTraits<WebCore::AccessibilityObject> > > >, WTF::IntHash<unsigned long>, WTF::HashMap<unsigned long, WTF::RefPtr<WebCore::AccessibilityObject, WTF::DumbPtrTraits<WebCore::AccessibilityObject> >, WTF::IntHash<unsigned long>, WTF::HashTraits<unsigned long>, WTF::HashTraits<WTF::RefPtr<WebCore::AccessibilityObject, WTF::DumbPtrTraits<WebCore::AccessibilityObject> > > >::KeyValuePairTraits, WTF::HashTraits<unsigned long> >::inlineLookup<WTF::HashMapTranslatorAdapter<WTF::HashMap<unsigned long, WTF::RefPtr<WebCore::AccessibilityObject, WTF::DumbPtrTraits<WebCore::AccessibilityObject> >, WTF::IntHash<unsigned long>, WTF::HashTraits<unsigned long>, WTF::HashTraits<WTF::RefPtr<WebCore::AccessibilityObject, WTF::DumbPtrTraits<WebCore::AccessibilityObject> > > >::KeyValuePairTraits, WTF::IdentityHashTranslator<WTF::HashMap<unsigned long, WTF::RefPtr<WebCore::AccessibilityObject, WTF::DumbPtrTraits<WebCore::AccessibilityObject> >, WTF::IntHash<unsigned long>, WTF::HashTraits<unsigned long>, WTF::HashTraits<WTF::RefPtr<WebCore::AccessibilityObject, WTF::DumbPtrTraits<WebCore::AccessibilityObject> > > >::KeyValuePairTraits, WTF::IntHash<unsigned long> > >, unsigned long>(this=0x000000065d9b7798, key=0x00007ffee3eb3540) at HashTable.h:668:9
    frame #4: 0x000000062fa6d00d WebCore`WTF::KeyValuePair<unsigned long, WTF::RefPtr<WebCore::AccessibilityObject, WTF::DumbPtrTraits<WebCore::AccessibilityObject> > >* WTF::HashTable<unsigned long, WTF::KeyValuePair<unsigned long, WTF::RefPtr<WebCore::AccessibilityObject, WTF::DumbPtrTraits<WebCore::AccessibilityObject> > >, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<unsigned long, WTF::RefPtr<WebCore::AccessibilityObject, WTF::DumbPtrTraits<WebCore::AccessibilityObject> > > >, WTF::IntHash<unsigned long>, WTF::HashMap<unsigned long, WTF::RefPtr<WebCore::AccessibilityObject, WTF::DumbPtrTraits<WebCore::AccessibilityObject> >, WTF::IntHash<unsigned long>, WTF::HashTraits<unsigned long>, WTF::HashTraits<WTF::RefPtr<WebCore::AccessibilityObject, WTF::DumbPtrTraits<WebCore::AccessibilityObject> > > >::KeyValuePairTraits, WTF::HashTraits<unsigned long> >::lookup<WTF::HashMapTranslatorAdapter<WTF::HashMap<unsigned long, WTF::RefPtr<WebCore::AccessibilityObject, WTF::DumbPtrTraits<WebCore::AccessibilityObject> >, WTF::IntHash<unsigned long>, WTF::HashTraits<unsigned long>, WTF::HashTraits<WTF::RefPtr<WebCore::AccessibilityObject, WTF::DumbPtrTraits<WebCore::AccessibilityObject> > > >::KeyValuePairTraits, WTF::IdentityHashTranslator<WTF::HashMap<unsigned long, WTF::RefPtr<WebCore::AccessibilityObject, WTF::DumbPtrTraits<WebCore::AccessibilityObject> >, WTF::IntHash<unsigned long>, WTF::HashTraits<unsigned long>, WTF::HashTraits<WTF::RefPtr<WebCore::AccessibilityObject, WTF::DumbPtrTraits<WebCore::AccessibilityObject> > > >::KeyValuePairTraits, WTF::IntHash<unsigned long> > >, unsigned long>(this=0x000000065d9b7798, key=0x00007ffee3eb3540) at HashTable.h:661:16
    frame #5: 0x000000062fa6cf9d WebCore`WebCore::AccessibilityObject* WTF::HashMap<unsigned long, WTF::RefPtr<WebCore::AccessibilityObject, WTF::DumbPtrTraits<WebCore::AccessibilityObject> >, WTF::IntHash<unsigned long>, WTF::HashTraits<unsigned long>, WTF::HashTraits<WTF::RefPtr<WebCore::AccessibilityObject, WTF::DumbPtrTraits<WebCore::AccessibilityObject> > > >::get<WTF::IdentityHashTranslator<WTF::HashMap<unsigned long, WTF::RefPtr<WebCore::AccessibilityObject, WTF::DumbPtrTraits<WebCore::AccessibilityObject> >, WTF::IntHash<unsigned long>, WTF::HashTraits<unsigned long>, WTF::HashTraits<WTF::RefPtr<WebCore::AccessibilityObject, WTF::DumbPtrTraits<WebCore::AccessibilityObject> > > >::KeyValuePairTraits, WTF::IntHash<unsigned long> >, unsigned long>(this=0x000000065d9b7798, value=0x00007ffee3eb3540) const at HashMap.h:321:63
    frame #6: 0x000000062fa4986d WebCore`WTF::HashMap<unsigned long, WTF::RefPtr<WebCore::AccessibilityObject, WTF::DumbPtrTraits<WebCore::AccessibilityObject> >, WTF::IntHash<unsigned long>, WTF::HashTraits<unsigned long>, WTF::HashTraits<WTF::RefPtr<WebCore::AccessibilityObject, WTF::DumbPtrTraits<WebCore::AccessibilityObject> > > >::get(this=0x000000065d9b7798, key=0x00007ffee3eb3540) const at HashMap.h:436:12
    frame #7: 0x000000062fb1b6f6 WebCore`WebCore::AXObjectCache::objectFromAXID(this=0x000000065d9b7780, id=0) const at AXObjectCache.h:225:75
  * frame #8: 0x000000062fb04e45 WebCore`WebCore::AXIsolatedObject::associatedAXObject(this=0x000000065d4a8680) const at AXIsolatedObject.h:87:59
    frame #9: 0x000000062fb4c2c8 WebCore`WebCore::AXIsolatedObject::relativeFrame(this=0x0000700009040fd0) const::$_40::operator()() const at AXIsolatedObject.cpp:928:30
    frame #10: 0x000000062fb4c64d WebCore`WebCore::FloatRect WebCore::Accessibility::retrieveValueFromMainThread<WebCore::FloatRect, WebCore::AXIsolatedObject::relativeFrame() const::$_40>(this=0x000000065b3bc830) const::$_40&&)::'lambda'()::operator()() const at AccessibilityObjectInterface.h:1267:17
    frame #11: 0x000000062fb4c5fe WebCore`WTF::Detail::CallableWrapper<WebCore::FloatRect WebCore::Accessibility::retrieveValueFromMainThread<WebCore::FloatRect, WebCore::AXIsolatedObject::relativeFrame() const::$_40>(WebCore::AXIsolatedObject::relativeFrame() const::$_40&&)::'lambda'(), void>::call(this=0x000000065b3bc828) at Function.h:52:39
    frame #12: 0x00000006493b9102 JavaScriptCore`WTF::Function<void ()>::operator(this=0x000000065d48b468)() const at Function.h:84:35
    frame #13: 0x00000006493f649c JavaScriptCore`WTF::callOnMainAndWait(this=0x000000065d48b468)>&&, WTF::MainStyle)::$_0::operator()() const at MainThread.cpp:144:9
    frame #14: 0x00000006493f63ce JavaScriptCore`WTF::Detail::CallableWrapper<WTF::callOnMainAndWait(WTF::Function<void ()>&&, WTF::MainStyle)::$_0, void>::call(this=0x000000065d48b460) at Function.h:52:39
    frame #15: 0x00000006493b9102 JavaScriptCore`WTF::Function<void ()>::operator(this=0x00007ffee3eb36b8)() const at Function.h:84:35
    frame #16: 0x00000006493f566d JavaScriptCore`WTF::dispatchFunctionsFromMainThread() at MainThread.cpp:84:9
    frame #17: 0x00000006493f80c3 JavaScriptCore`WTF::Detail::CallableWrapper<void (*)(), void>::call(this=0x000000065e17ff00) at Function.h:52:39
    frame #18: 0x00000006493b9102 JavaScriptCore`WTF::Function<void ()>::operator(this=0x00007ffee3eb3738)() const at Function.h:84:35
    frame #19: 0x0000000649429b08 JavaScriptCore`WTF::RunLoop::performWork(this=0x00000006501f5000) at RunLoop.cpp:146:9
    frame #20: 0x000000064942a4e1 JavaScriptCore`WTF::RunLoop::performWork(context=0x00000006501f5000) at RunLoopCF.cpp:38:37
    frame #21: 0x00007fff3032aa71 CoreFoundation`__CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17
    frame #22: 0x00007fff3032aa20 CoreFoundation`__CFRunLoopDoSource0 + 157
    frame #23: 0x00007fff3032a7fd CoreFoundation`__CFRunLoopDoSources0 + 222
    frame #24: 0x00007fff3032950a CoreFoundation`__CFRunLoopRun + 890
    frame #25: 0x00007fff30328b1a CoreFoundation`CFRunLoopRunSpecific + 534
    frame #26: 0x00007fff32ce7521 Foundation`-[NSRunLoop(NSRunLoop) runMode:beforeDate:] + 212
    frame #27: 0x00007fff32d75634 Foundation`-[NSRunLoop(NSRunLoop) run] + 76
    frame #28: 0x00007fff70aeab7f libxpc.dylib`_xpc_objc_main + 760
    frame #29: 0x00007fff70aea648 libxpc.dylib`xpc_main + 437
    frame #30: 0x0000000620879ee1 WebKit`WebKit::XPCServiceMain(argc=1, argv=0x00007ffee3eb49b8) at XPCServiceMain.mm:180:5
    frame #31: 0x0000000621cc71eb WebKit`WKXPCServiceMain(argc=1, argv=0x00007ffee3eb49b8) at WKMain.mm:33:12
    frame #32: 0x000000010bd4ee92 com.apple.WebKit.WebContent.Development`main(argc=1, argv=0x00007ffee3eb49b8) at AuxiliaryProcessMain.cpp:30:12
    frame #33: 0x00007fff7086ec71 libdyld.dylib`start + 1
    frame #34: 0x00007fff7086ec71 libdyld.dylib`start + 1
(lldb)
Comment 5 Andres Gonzalez 2020-06-26 11:41:46 PDT
After fixing other timing crashes, now this one is more reproducible and the cause is clear. Uploading a new patch.
Comment 6 Andres Gonzalez 2020-06-26 11:45:23 PDT
The relative frame is already being retrieved from the main thread:
    // Retrieve this on the main thread because we require the scroll ancestor to convert to the right scroll offset.
Comment 7 Andres Gonzalez 2020-06-26 11:48:00 PDT
The cause is that between the time the isolated object dispatches the method to the main thread, and the lambda is executed, the isolated object is detached, and hence its object ID is invalid.
Comment 8 Andres Gonzalez 2020-06-26 11:54:41 PDT
Created attachment 402883 [details]
Patch
Comment 9 EWS 2020-06-26 12:26:27 PDT
Committed r263573: <https://trac.webkit.org/changeset/263573>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 402883 [details].
Comment 10 Radar WebKit Bug Importer 2020-06-26 12:27:15 PDT
<rdar://problem/64817348>