fix(macos): Prevent NSExceptions when dropping a webview with registered protocols

[FabianLars]

fixes tauri-apps/tauri#7898

This one was quiet hard to fix, i really tried to not rely on a global variable but always ended up with either nsexceptions or stack traces for invalid memory access. The problem is that when you're dropping the webview, like tauri does on window close, is that protocol handlers may still be running. In Tauri this is even more of an issue because it uses a protocol for the whole IPC implementation so even the close-requested event will cause problems here.

If the webview is dropped before the handler finished, either _webview or task (or both) will be undefined, but in a way that objc's is_null still returns false, but if you check its class it will be returned as nil. Relying on that unfortunetely doesn't work because it requires 2 calls to the object / class (first to get the class, second to get the class name) and the second one really often causes memory issues. Also the timing is unpredictable, sometimes it fails on the didReceive call, sometimes on the didFinish call, rarely on the didReceiveResponse call.

Since wry consumers can't know when the handler was finished (especially the responder part inside wry) i believe it would be unreasonable to expect users to make sure handlers are done before dropping the webview and instead it's something we should make sure.

---

P.S. I kinda hope someone comes up with a better solution because i'm not the biggest fan of the global webview vec myself, it's just the only thing i found that works reliably.

P.P.S. Also semi related to #1214 which is why i'm opening this pr right now and not waiting for tomorrow morning 🙃

---

Edit: Another thing that just came to mind, which i didn't investigate is whether returning early from the function will leak memory? For example the NSData instance when we don't end up calling didReceive. Even if this leaks memory, i'm not sure how to handle that in objc so some input on that topic would be appreciated :)

[pewsheen]

Is #1214 handling the same issue? I would prefer the fix here and just check if the webview_id is still on the running list.

[FabianLars]

Is #1214 handling the same issue? I would prefer the fix here and just check if the webview_id is still on the running list.

No. My PR fixes a panic/mem issue when the InnerWebview struct is dropped while protocols are registered.

1214 fixes an issue when the dev aborts a request to a protocol via AbortHandler for example (a page reload may causes this too). This will make wkwebview send the stopTask event which is not triggered for the issue i'm trying to fix here.
For 1214 i do still think we should rustify it a bit more and use Lazy (like i said in the comment over there), if we end up using the PR.

[FabianLars]

don't mind the last commits, i'm so done for today 😂

[amrbashir]

I think the reason is is_null still returns false is that because it is not set to std::ptr::null, I have seen a similar issue in #1206

I think we should fix it like this:

1. pass the webview pointer to the responder closure
2. when dropping the webview, we explicitly set the value at the webview ptr address to a null ptr or just a unit (`()``
3. check inside the responder if the value pointed by the webview ptr is null

[FabianLars]

Thank you, i'll see if i can make that work.

[FabianLars]

Hmm, i may be doing something wrong, but i can't make it work like this. It also looks like the webview in the start_task and the one in drop point to different addresses.

[amrbashir]

cc @wusyong

[wusyong]

@lucasfernog Are you sure it's working? I believe @FabianLars mentioned it didn't work as intended.

[FabianLars]

The PR's current implementation works for me.
I just couldn't make Amr's suggestion work (didn't push anything to the branch to not break the PR).

[wusyong]

Ah sorry for the misunderstanding.