🐞 [Bug] NSAttributedString.DocumentType.html main thread crash #767
Conversation
justinswart
force-pushed
the
html-main-thread-crash-fix
branch
from
430143d
to
080be7a
Compare
| if Thread.isMainThread { | ||
| return parsedHtml() | ||
| } else { | ||
| return DispatchQueue.main.sync { |
There was a problem hiding this comment.
Shouldn't this be async? 🤔
There was a problem hiding this comment.
We actually do want this to run synchronously on the main thread. The difference between async and sync here is essentially that the code after the closure won't be executed until the closure returns, this is also why we're able to just do return return DispatchQueue.main.sync { ... } as it's synchronously returning a value, async would asynchronously execute the closure and return early.
There was a problem hiding this comment.
I think a better way to say it is that it blocks the current thread, we also first check that whether we're on the main thread because it will crash if we try to dispatch it in this way if we're already on that thread.
| if Thread.isMainThread { | ||
| return parsedHtml() | ||
| } else { | ||
| return DispatchQueue.main.sync { |
There was a problem hiding this comment.
Same here, should this be async?
There was a problem hiding this comment.
No, sync is correct as per other answer.
| if Thread.isMainThread { | ||
| return parsedHtml() | ||
| } else { | ||
| return DispatchQueue.main.sync { |
📲 What
Attempts to fix an escalating crash that we've been seeing in our crash reports which appear to be linked to parsing HTML strings using
NSAttributedString.DocumentType.htmloutside of the main thread. Although I have not been able to reproduce this crash myself, this is my suggested fix which I'd like to ship with the next release to see if it brings the reported crashes down.🤔 Why
This crash has been rising in our crash reports (link below).
🛠 How
Ensure that functions in
String+SimpleHTML.swiftalways execute on the main thread.👀 See
https://fabric.io/kickstarter2/ios/apps/com.kickstarter.kickstarter/issues/5c19e0ddf8b88c2963607faa?time=last-seven-days
https://stackoverflow.com/questions/28915954/nsattributedstring-initwithdata-and-nshtmltextdocumenttype-crash-if-not-on-main
✅ Acceptance criteria