Bug 243343 – REGRESSION (iOS 16 Beta): Crash adding / removing ScriptMessageHandlers to WKUserContentController

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit API

[Sean Reinhardt]

Created attachment 461296 [details]
Crash log - addScriptMessageHandler:name

Observed frequent EXC_BAD_ACCESS crashes (~ 1 out of 4 attempts) when adding `[WKUserContentController addScriptMessageHandler: name]` or removing `[WKUserContentController removeAllScriptMessageHandlers]` script message handlers to a WKWebView. 

Observed on iOS 16 betas 1-4, stable on all other OS's.

Crash statement (full log attached):
```
Thread 0 name:   Dispatch queue: com.apple.main-thread
Thread 0 Crashed:
0   JavaScriptCore                	       0x19e76ba54 WTF::equal(WTF::StringImpl const*, unsigned char const*, unsigned int) + 200
1   JavaScriptCore                	       0x19e717c70 WTF::HashTableAddResult<WTF::HashTableIterator<WTF::HashTable<WTF::Packed<WTF::StringImpl*>, WTF::Packed<WTF::StringImpl*>, WTF::IdentityExtractor, WTF::DefaultHash<WTF::Packed<WTF::StringImpl*> >, WTF::HashTraits<WTF::Packed<WTF::StringImpl*> >, WTF::HashTraits<WTF::Packed<WTF::StringImpl*> > >, WTF::Packed<WTF::StringImpl*>, WTF::Packed<WTF::StringImpl*>, WTF::IdentityExtractor, WTF::DefaultHash<WTF::Packed<WTF::StringImpl*> >, WTF::HashTraits<WTF::Packed<WTF::StringImpl*> >, WTF::HashTraits<WTF::Packed<WTF::StringImpl*> > > > WTF::HashTable<WTF::Packed<WTF::StringImpl*>, WTF::Packed<WTF::StringImpl*>, WTF::IdentityExtractor, WTF::DefaultHash<WTF::Packed<WTF::StringImpl*> >, WTF::HashTraits<WTF::Packed<WTF::StringImpl*> >, WTF::HashTraits<WTF::Packed<WTF::StringImpl*> > >::addPassingHashCode<WTF::HashSetTranslatorAdapter<WTF::LCharBufferTranslator>, WTF::HashTranslatorCharBuffer<unsigned char> const&, WTF::HashTranslatorCharBuffer<unsigned char> const&>(WTF::HashTranslatorCharBuffer<unsigned char> const&, WTF::HashTranslatorCharBuffer<unsigned char> const&) + 184
2   JavaScriptCore                	       0x19e714f20 WTF::AtomStringImpl::add(unsigned char const*, unsigned int) + 244
3   WebKit                        	       0x1a25fbc90 -[WKUserContentController addScriptMessageHandler:name:] + 80
4   HyprMX                        	       0x104ebe7b8 +[HYPRWebView addScriptsToWebView:withMessageHandler:] + 592

```

[Radar WebKit Bug Importer]

<rdar://problem/97789620>

[Alex Christensen]

None of the code in that exact stack has changed since iOS 15 and I'm unable to reproduce the crash.  I definitely believe that something has broken, though.  Would you be willing to upload or send me a project that reproduces the issue, even if sometimes?

[Alex Christensen]

It looks to me like you're passing a nil name to addScriptMessageHandler:name: but I could be wrong.

[Sean Reinhardt]

Created attachment 461345 [details]
Xcode Project that reproduces the crash

Test on iOS 16 bets - Press button on the screen until it crashes.  Will take a few attempts.
The call in and back from a JSContext in the app appears to be required for the crash to occur, and a similar thing occurs in the original crash case.

[Sean Reinhardt]

Created attachment 461346 [details]
Another Xcode project repro case

A second way we were able to cause the crash to happen on iOS 16 without JSContext involved, but required using a background thread (despite the observed warning).  Our original crash did not occur on a background thread, but attached in case its helpful.

[Jeremy Ellison]

This still appears to be an issue in the latest iOS beta (beta 7). Has there been any attempt to mitigate this crash? We're able to reproduce it easily and repeatibly.

[Tim Horton]

The test case (WKWebViewBridgeCrashBackgroundThread) that is calling addScriptMessageHandler on a background queue is invalid; this is absolutely unsafe and can cause bugs exactly like the one you're experiencing. I think we should ignore that test case.

The original test case (WKWebViewBridgeCrashJSContext) does eventually reproduce, though, and doesn't have the same kind of obvious cause...

[Alex Christensen]

I think this is related to making WebScriptMessageHandler.m_name an AtomString instead of a String in https://bugs.webkit.org/show_bug.cgi?id=239950
The solution will likely involve reverting that part of that change, but may also need something else.  I'm looking to verify that and understand a little more of what is going on in this case.

[Alex Christensen]

Pull request: https://github.com/WebKit/WebKit/pull/4449

[EWS]

Committed 254599@main (e7898844fe5a): <https://commits.webkit.org/254599@main>

Reviewed commits have been landed. Closing PR #4449 and removing active labels.

[Alex Christensen]

Thanks for the report and the project!