Some WebKit clients crash in FrameLoader::stopAllLoaders via [WebView dealloc] inside ~ObjCEventListener while deleting event targets. <rdar://problem/40888424>
Created attachment 367769 [details] Fixes the bug
Created attachment 367770 [details] Fixes the bug
Comment on attachment 367770 [details] Fixes the bug View in context: https://bugs.webkit.org/attachment.cgi?id=367770&action=review > Source/WebKitLegacy/mac/DOM/ObjCEventListener.mm:75 > + // Avoid executing arbitrary code during GC; e.g. inside Node::~Node. > + [m_listener.get() retain]; > + [m_listener.get() autorelease]; Some day we will move to ARC. To be prepared for when we do, it will be much more future-proof to use CFRetain and CFAutorelease, which can be used for this purpose even under ARC. I suggest doing that. CFRetain((__bridge CFTypeRef)m_listener.get()); CFAutorelease((__bridge CFTypeRef)m_listener.get()); To make it cleaner, if this idiom comes up more often, we could add a RetainPtr member function designed for this purpose. Unfortunately, the current RetainPtr::autorelease is not what we want because it compiles to a bridging release under ARC, not an autorelease.
(In reply to Darin Adler from comment #3) > Comment on attachment 367770 [details] > Fixes the bug > > View in context: > https://bugs.webkit.org/attachment.cgi?id=367770&action=review > > > Source/WebKitLegacy/mac/DOM/ObjCEventListener.mm:75 > > + // Avoid executing arbitrary code during GC; e.g. inside Node::~Node. > > + [m_listener.get() retain]; > > + [m_listener.get() autorelease]; > > Some day we will move to ARC. To be prepared for when we do, it will be much > more future-proof to use CFRetain and CFAutorelease, which can be used for > this purpose even under ARC. I suggest doing that. > > CFRetain((__bridge CFTypeRef)m_listener.get()); > CFAutorelease((__bridge CFTypeRef)m_listener.get()); > > To make it cleaner, if this idiom comes up more often, we could add a > RetainPtr member function designed for this purpose. Unfortunately, the > current RetainPtr::autorelease is not what we want because it compiles to a > bridging release under ARC, not an autorelease. Okay, sure, I'll use CF*.
Committed r244459: <https://trac.webkit.org/changeset/244459>