CommonMarkAttributedString Crash: Simultaneous accesses to 0x… but modification requires exclusive access.

Crash: Simultaneous accesses to 0x… but modification requires exclusive access.

Open regexident opened this issue 4 years ago • 5 comments

I'm getting a crash of …

Simultaneous accesses to 0x… but modification requires exclusive access.

… at this line …

let mutableAttributedString = try NSMutableAttributedString(
    data: data,
    options: options,
    documentAttributes: &documentAttributes
)

… in a SwiftUI app when trying to convert the following markdown string …

"E = mc<sup>2</sup>"

… to an NSAttributedString (while doing the same with "E = mc^2" works just fine).

Userland stack-trace:

#0	0x00007fff2c80f85a in ViewRendererHost.render(interval:updateDisplayList:) ()
#1	0x00007fff2c452e2e in closure #1 in _UIHostingView.requestImmediateUpdate() ()
#2	0x00007fff2c44d959 in thunk for @escaping @callee_guaranteed () -> () ()
#3	0x0000000106895f11 in _dispatch_call_block_and_release ()
#4	0x0000000106896e8e in _dispatch_client_callout ()
#5	0x00000001068a4d97 in _dispatch_main_queue_callback_4CF ()
#6	0x00007fff23da0909 in __CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ ()
#7	0x00007fff23d9b459 in __CFRunLoopRun ()
#8	0x00007fff23d9a944 in CFRunLoopRunSpecific ()
#9	0x00007fff481778be in -[NSHTMLReader _loadUsingWebKit] ()
#10	0x00007fff4817882b in -[NSHTMLReader attributedString] ()
#11	0x00007fff48101ef0 in _NSReadAttributedStringFromURLOrData ()
#12	0x00007fff480ff915 in -[NSAttributedString(NSAttributedStringUIFoundationAdditions) initWithData:options:documentAttributes:error:] ()
#13	0x000000010655812e in @nonobjc NSMutableAttributedString.init(data:options:documentAttributes:) ()
#14	0x0000000106557f5a in NSMutableAttributedString.__allocating_init(data:options:documentAttributes:) ()
#15	0x00000001065578e1 in NSAttributedString.init(html:attributes:) at ~/Library/Developer/Xcode/DerivedData/Crasher-alelyvwrvwarxvekibncuyjaqpcs/SourcePackages/checkouts/CommonMarkAttributedString/Sources/CommonMarkAttributedString/NSAttributedString+Extensions.swift:35
#16	0x000000010654fe0a in Node.attributedString(attributes:attachments:) at ~/Library/Developer/Xcode/DerivedData/Crasher-alelyvwrvwarxvekibncuyjaqpcs/SourcePackages/checkouts/CommonMarkAttributedString/Sources/CommonMarkAttributedString/CommonMark+Extensions.swift:42
#17	0x000000010655206a in @objc Node.attributedString(attributes:attachments:) ()
#18	0x0000000106550b86 in closure #2 in Node.attributedString(attributes:attachments:) at ~/Library/Developer/Xcode/DerivedData/Crasher-alelyvwrvwarxvekibncuyjaqpcs/SourcePackages/checkouts/CommonMarkAttributedString/Sources/CommonMarkAttributedString/CommonMark+Extensions.swift:38
#19	0x0000000106551ed4 in partial apply for closure #2 in Node.attributedString(attributes:attachments:) ()
#20	0x0000000106550cc0 in thunk for @callee_guaranteed (@guaranteed Node & Block) -> (@owned NSAttributedString, @error @owned Error) ()
#21	0x0000000106551ef4 in partial apply for thunk for @callee_guaranteed (@guaranteed Node & Block) -> (@owned NSAttributedString, @error @owned Error) ()
#22	0x00007fff510663c5 in Collection.map<A>(_:) ()
#23	0x000000010654f4d8 in Node.attributedString(attributes:attachments:) at ~/Library/Developer/Xcode/DerivedData/Crasher-alelyvwrvwarxvekibncuyjaqpcs/SourcePackages/checkouts/CommonMarkAttributedString/Sources/CommonMarkAttributedString/CommonMark+Extensions.swift:38
#24	0x000000010655206a in @objc Node.attributedString(attributes:attachments:) ()
#25	0x000000010655713b in NSAttributedString.init(commonmark:attributes:attachments:) at ~/Library/Developer/Xcode/DerivedData/Crasher-alelyvwrvwarxvekibncuyjaqpcs/SourcePackages/checkouts/CommonMarkAttributedString/Sources/CommonMarkAttributedString/NSAttributedString+Extensions.swift:22
#26	0x000000010654d941 in ContentView.body.getter at ~/Downloads/Crasher/Crasher/ContentView.swift:15
#27	0x000000010654dbf9 in protocol witness for View.body.getter in conformance ContentView ()
#28	0x00007fff2c403bc2 in ViewBody.apply(_:) ()
#29	0x00007fff2c406520 in protocol witness for static UntypedAttribute._update(_:graph:attribute:) in conformance ViewBody<A> ()
#30	0x00007fff2fc78309 in partial apply ()
#31	0x00007fff2fc60d3d in AG::Graph::UpdateStack::update() ()
#32	0x00007fff2fc6124b in AG::Graph::update_attribute(unsigned int, bool) ()
#33	0x00007fff2fc65d53 in AG::Subgraph::update(unsigned int) ()
#34	0x00007fff2c5359e0 in ViewGraph.runTransaction(in:) ()
#35	0x00007fff2c535db7 in closure #1 in ViewGraph.updateOutputs(at:) ()
#36	0x00007fff2c535a9d in ViewGraph.updateOutputs(at:) ()
#37	0x00007fff2c81a7db in closure #1 in closure #1 in ViewRendererHost.render(interval:updateDisplayList:) ()
#38	0x00007fff2c819c33 in closure #1 in ViewRendererHost.render(interval:updateDisplayList:) ()
#39	0x00007fff2c80f785 in ViewRendererHost.render(interval:updateDisplayList:) ()
#40	0x00007fff2c96f9e2 in _UIHostingView.layoutSubviews() ()
#41	0x00007fff2c96fa05 in @objc _UIHostingView.layoutSubviews() ()
#42	0x00007fff49193678 in -[UIView(CALayerDelegate) layoutSublayersOfLayer:] ()
#43	0x00007fff2b4c6398 in -[CALayer layoutSublayers] ()
#44	0x00007fff2b4cc523 in CA::Layer::layout_if_needed(CA::Transaction*) ()
#45	0x00007fff2b4d7bba in CA::Layer::layout_and_display_if_needed(CA::Transaction*) ()
#46	0x00007fff2b420c04 in CA::Context::commit_transaction(CA::Transaction*, double) ()
#47	0x00007fff2b4545ef in CA::Transaction::commit() ()
#48	0x00007fff48ca3747 in __34-[UIApplication _firstCommitBlock]_block_invoke_2 ()
#49	0x00007fff23da0b5c in __CFRUNLOOP_IS_CALLING_OUT_TO_A_BLOCK__ ()
#50	0x00007fff23da0253 in __CFRunLoopDoBlocks ()
#51	0x00007fff23d9b043 in __CFRunLoopRun ()
#52	0x00007fff23d9a944 in CFRunLoopRunSpecific ()
#53	0x00007fff38ba6c1a in GSEventRunModal ()
#54	0x00007fff48c8b9ec in UIApplicationMain ()
#55	0x000000010654c5bb in main at ~/Downloads/Crasher/Crasher/AppDelegate.swift:12
#56	0x00007fff51a231fd in start ()
#57	0x00007fff51a231fd in start ()
Enqueued from com.apple.main-thread (Thread 1) Queue : com.apple.main-thread (serial)
#0	0x000000010689ad46 in dispatch_async ()
#1	0x00007fff514a12e0 in OS_dispatch_queue.async(group:qos:flags:execute:) ()
#2	0x00007fff2c452d23 in _UIHostingView.requestImmediateUpdate() ()
#3	0x00007fff2c52fe97 in ViewGraph.transform.setter ()
#4	0x00007fff2c451eac in _UIHostingView.updateTransform() ()
#5	0x00007fff2c96f8d3 in @objc _UIHostingView._geometryChanges(_:forAncestor:) ()
#6	0x00007fff49170089 in -[UIView _notifyGeometryObserversWithChangeInfo:] ()
#7	0x00007fff4918cc50 in -[UIView(Internal) _addSubview:positioned:relativeTo:] ()
#8	0x00007fff49138cab in -[UIDropShadowView setContentView:] ()
#9	0x00007fff4850223d in -[_UISheetPresentationController presentationTransitionWillBegin] ()
#10	0x00007fff484f3b5c in __71-[UIPresentationController _initViewHierarchyForPresentationSuperview:]_block_invoke ()
#11	0x00007fff484f13b3 in __56-[UIPresentationController runTransitionForCurrentState]_block_invoke.465 ()
#12	0x00007fff484f7569 in +[UIPresentationController _scheduleTransition:] ()
#13	0x00007fff484f0f26 in -[UIPresentationController runTransitionForCurrentState] ()
#14	0x00007fff484ee058 in -[UIPresentationController _presentWithAnimationController:interactionController:target:didEndSelector:] ()
#15	0x00007fff48cc4452 in -[UIWindow addRootViewControllerViewIfPossible] ()
#16	0x00007fff48cc3986 in -[UIWindow _updateLayerOrderingAndSetLayerHidden:actionBlock:] ()
#17	0x00007fff48cc4a11 in -[UIWindow _setHidden:forced:] ()
#18	0x00007fff48cd7e4d in -[UIWindow _mainQueue_makeKeyAndVisible] ()
#19	0x000000010654d12b in SceneDelegate.scene(_:willConnectTo:options:) at ~/Downloads/Crasher/Crasher/SceneDelegate.swift:30
#20	0x000000010654d306 in @objc SceneDelegate.scene(_:willConnectTo:options:) ()
#21	0x00007fff481e7cfb in +[UIScene _sceneForFBSScene:create:withSession:connectionOptions:] ()
#22	0x00007fff48c87d2d in -[UIApplication _connectUISceneFromFBSScene:transitionContext:] ()
#23	0x00007fff48c88064 in -[UIApplication workspace:didCreateScene:withTransitionContext:completion:] ()
#24	0x00007fff487da8dc in -[UIApplicationSceneClientAgent scene:didInitializeWithEvent:completion:] ()
#25	0x00007fff36cacd2e in -[FBSSceneImpl _callOutQueue_agent_didCreateWithTransitionContext:completion:] ()
#26	0x00007fff36cd2dc1 in __86-[FBSWorkspaceScenesClient sceneID:createWithParameters:transitionContext:completion:]_block_invoke.154 ()
#27	0x00007fff36cb7757 in -[FBSWorkspace _calloutQueue_executeCalloutFromSource:withBlock:] ()
#28	0x00007fff36cd2a52 in __86-[FBSWorkspaceScenesClient sceneID:createWithParameters:transitionContext:completion:]_block_invoke ()
#29	0x0000000106896e8e in _dispatch_client_callout ()
#30	0x0000000106899da2 in _dispatch_block_invoke_direct ()
#31	0x00007fff36cf86e9 in __FBSSERIALQUEUE_IS_CALLING_OUT_TO_A_BLOCK__ ()
#32	0x00007fff36cf83d7 in -[FBSSerialQueue _queue_performNextIfPossible] ()
#33	0x00007fff36cf88e6 in -[FBSSerialQueue _performNextFromRunLoopSource] ()
#34	0x00007fff23da0d31 in __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ ()
#35	0x00007fff23da0c5c in __CFRunLoopDoSource0 ()
#36	0x00007fff23da048c in __CFRunLoopDoSources0 ()
#37	0x00007fff23d9b02e in __CFRunLoopRun ()
#38	0x00007fff23d9a944 in CFRunLoopRunSpecific ()
#39	0x00007fff38ba6c1a in GSEventRunModal ()
#40	0x00007fff48c8b9ec in UIApplicationMain ()
#41	0x000000010654c5bb in main at …/AppDelegate.swift:12
#42	0x00007fff51a231fd in start ()

What's apparently not causing the issue

I tried isolating/reproducing the issue in a unit test in a fork of CommonMarkAttributedString:

func testCrash() throws {
    let commonmark = #"""
    E = mc<sup>2</sup>
    """#

    #if canImport(UIKit)
    let _ = try NSAttributedString(commonmark: commonmark)
    #elseif canImport(AppKit)
    let _ = try NSAttributedString(commonmark: commonmark)
    #endif
}

func testCrash2() throws {
    let html = #"""
    <p>E = mc<!-- raw HTML omitted -->2<!-- raw HTML omitted --></p>\n
    """#

    #if canImport(UIKit)
    let _ = try NSAttributedString(html: html, attributes: [:])
    #elseif canImport(AppKit)
    let _ = try NSAttributedString(html: html, attributes: [:])
    #endif
}

But there things worked just fine. No crash. 😕

So next I tried replacing documentAttributes: &documentAttributes with documentAttributes: nil, since it is actually not used afterwards, and changed Nio's dependency to that fork, hoping it might solve the crasher. It didn't.

How to reproduce

Create new single-view SwiftUI iOS app.
Add https://github.com/mattt/CommonMarkAttributedString.git SPM package.

Change the contents of ContentView.body to …

import SwiftUI

import CommonMarkAttributedString

struct ContentView: View {
    var body: some View {
        let string = try! NSAttributedString(
            commonmark: "E = mc<sup>2</sup>"
        )

        return Text("Hello, World!")
    }
}

Start the app in iOS simulator and watch it crash.

May 23 '20 11:05 regexident

Great writeup as always, @regexident! I have half a mind to start an Etsy shop that sells framed prints of your bug reports 😍

The good news is that I'm pretty sure what the underlying cause is. This old-school documentation from Apple has a good writeup:

Multicore considerations: Since OS X v10.4, NSAttributedString has used WebKit for all import (but not for export) of HTML documents. Because WebKit document loading is not thread safe, this has not been safe to use on background threads. For applications linked on OS X v10.5 and later, if NSAttributedString imports HTML documents on any but the main thread, the use of WebKit is transferred to the main thread via performSelectorOnMainThread:withObject:waitUntilDone:. This makes the operation thread safe, but it requires that the main thread be executing the run loop in one of the common modes. This behavior can be overridden by setting the value of the standard user default NSRunWebKitOnAppKitThread to either YES (to obtain the new behavior regardless of linkage) or NO (to obtain the old behavior regardless of linkage).

Although this was last updated in 2014, I believe the diagnosis is still correct. As for the prescribed treatment... it may work, but it's far from optimal.

I must admit that when I was first writing this library, I had a vague memory of HTML → NSAttributedString having thread safety issues. But in an effort to get something out, I punted on that. That's my bad for not taking the time to look into this more — at least enough to document this limitation in the README or open an issue. So I apologize for that.

Where do we go from here?

Like I said, I'm not a fan of the workaround prescribed in Apple's documentation. I think the best long-term solution will involve finding or creating a new library (preferably, using Markup) for doing the HTML conversion.

May 23 '20 15:05 mattt

Ugh, I didn't mean to close this. 🙄

Great writeup as always, @regexident! I have half a mind to start an Etsy shop that sells framed prints of your bug reports 😍

😆 Having been on the receiving end of quite a few horribly impudent and lazy issues myself I just try my best to be a good open source citizen. 😅

Where do we go from here?

Like I said, I'm not a fan of the workaround prescribed in Apple's documentation. I think the best long-term solution will involve finding or creating a new library (preferably, using Markup) for doing the HTML conversion.

Probably the best solution, yeah. Or maybe just skip HTML entirely and go straight from Markdown to NSAttributedString?

Either way, lemme know if I can help you here in any way.

Doing things ourselves would also give us more control over how the output is formatted. The current formatting doesn't quite match the output produced by most common Markdown editors/viewers and the paragraph margins are a bit off, I think.

(also cough https://github.com/mattt/CommonMarkAttributedString/issues/6 cough 😏)

May 23 '20 18:05 regexident

Probably the best solution, yeah. Or maybe just skip HTML entirely and go straight from Markdown to NSAttributedString?

To clarify, I'm proposing that, for each HTMLBlock and InlineHTML CommonMark node, to use this proposed library to parse and convert the raw HTML to an NSAttributedString. We probably don't want to convert CommonMark to HTML and then to NSAttributedString if we can avoid it.

Either way, lemme know if I can help you here in any way.

Will do! This seems "fun" enough that I may well have a sketch of this within the next couple days.

May 23 '20 18:05 mattt

I have the same problem. It is possible that the main thread is causing this problem This is my code. It will cause a crash. ❌ Thread 1: Simultaneous accesses to 0x7f882bc1ec40, but modification requires exclusive access

    func updateUIView(_ label: UILabel, context: Context) {
        let parser = MarkdownParser()
        let html = parser.html(from: string)
        if let attributedString = try? NSAttributedString(
            data: Data(html.utf8),
            options: [.documentType: NSAttributedString.DocumentType.html],
            documentAttributes: nil
        ) {
            label.attributedText = attributedString
        }
    }

It works if I put them into the global queue. ✅

    func updateUIView(_ label: UILabel, context: Context) {
        let parser = MarkdownParser()
        let html = parser.html(from: string)
        DispatchQueue.global().async {
            if let attributedString = try? NSAttributedString(
                data: Data(html.utf8),
                options: [.documentType: NSAttributedString.DocumentType.html],
                documentAttributes: nil
            ) {
                DispatchQueue.main.async {
                    label.attributedText = attributedString
                }
            }
        }
    }

Jan 15 '21 09:01 Mas0nSun

A nice, I also debugged this as part of Nio. It doesn't seem to be a threading issue (both accesses are in one stack), but really a concurrent Swift access thing due to some weird WebKit/SwiftUI nesting. (I think it may happen because the thing is nested within a Swift constructor, but not quite sure)

I think NSXMLParser is based on libxml2 and supports HTML (or is that just NSXMLDocument?). If so, one could probably use that to parse some basic HTML instead.

Oh, and I forgot: This isn't actually a bug in CommonMarkAttributedString, but in NSAttributedString(data:options:documentAttributes:), just in case someone bothers and wants to file a Radar.

Mar 09 '21 16:03 helje5

CommonMarkAttributedString CommonMarkAttributedString copied to clipboard

Crash: Simultaneous accesses to 0x… but modification requires exclusive access.

What's apparently not causing the issue

How to reproduce

CommonMarkAttributedString
CommonMarkAttributedString copied to clipboard