fix(macOS): race conditions around WKURLSchemeTask

Fixes two issues which could cause race conditions when navigating between pages while custom protocol requests are in progress on macOS and iOS:
- The `obc2/catch-all` feature turns all Objective-C exceptions into panics, preventing `obc2::exception::catch(...)` from working.
- The `webview` and `task` objects passed to `start_task` were not retained, leading to use-after-free issues in the `responder`.
- `WryWebView::remove_custom_task_key` is called from both `stop_task` and `response` which may be in different threads, so `custom_protocol_task_ids` needs to be protected by a `Mutex`.

Closes #1189

.changes/objc-exceptions.md

@@ -0,0 +1,5 @@
+---
+wry: minor
+---
+
+Removed `obj-exception` feature.

Cargo.toml

@@ -26,9 +26,8 @@ rustc-args = ["--cfg", "docsrs"]
 rustdoc-args = ["--cfg", "docsrs"]
 
 [features]
-default = ["drag-drop", "objc-exception", "protocol", "os-webview"]
+default = ["drag-drop", "protocol", "os-webview"]
 serde = ["dpi/serde"]
-objc-exception = ["objc2/catch-all"]
 drag-drop = []
 protocol = []
 devtools = []

src/wkwebview/class/url_scheme_handler.rs

@@ -16,7 +16,7 @@ use http::{
 use objc2::{
   rc::Retained,
   runtime::{AnyClass, AnyObject, ClassBuilder, ProtocolObject},
-  AllocAnyThread, ClassType,
+  AllocAnyThread, ClassType, Message,
 };
 use objc2_foundation::{
   NSData, NSHTTPURLResponse, NSMutableDictionary, NSObject, NSObjectProtocol, NSString, NSURL,
@@ -54,8 +54,8 @@ pub fn create(name: &str) -> &AnyClass {
 extern "C" fn start_task(
   this: &AnyObject,
   _sel: objc2::runtime::Sel,
-  webview: &'static WryWebView,
-  task: &'static ProtocolObject<dyn WKURLSchemeTask>,
+  webview: &WryWebView,
+  task: &ProtocolObject<dyn WKURLSchemeTask>,
 ) {
   unsafe {
     #[cfg(feature = "tracing")]
@@ -180,12 +180,14 @@ extern "C" fn start_task(
       // send response
       match http_request.body(sent_form_body) {
         Ok(final_request) => {
+          let webview = webview.retain();
+          let task = task.retain();
           let responder: Box<dyn FnOnce(HttpResponse<Cow<'static, [u8]>>)> =
             Box::new(move |sent_response| {
               // Consolidate checks before calling into `did*` methods.
               let validate = || -> crate::Result<()> {
                 check_webview_id_valid(webview_id)?;
-                check_task_is_valid(webview, task_key, task_uuid.clone())?;
+                check_task_is_valid(&webview, task_key, task_uuid.clone())?;
                 Ok(())
               };
 
@@ -198,9 +200,9 @@ extern "C" fn start_task(
 
               unsafe fn response(
                 // FIXME: though we give it a static lifetime, it's not guaranteed to be valid.
-                task: &'static ProtocolObject<dyn WKURLSchemeTask>,
+                task: Retained<ProtocolObject<dyn WKURLSchemeTask>>,
                 // FIXME: though we give it a static lifetime, it's not guaranteed to be valid.
-                webview: &'static WryWebView,
+                webview: Retained<WryWebView>,
                 task_key: usize,
                 task_uuid: Retained<NSUUID>,
                 webview_id: &str,
@@ -209,7 +211,7 @@ extern "C" fn start_task(
               ) -> crate::Result<()> {
                 // Validate
                 check_webview_id_valid(webview_id)?;
-                check_task_is_valid(webview, task_key, task_uuid.clone())?;
+                check_task_is_valid(&webview, task_key, task_uuid.clone())?;
 
                 let content = sent_response.body();
                 // default: application/octet-stream, but should be provided by the client
@@ -253,7 +255,7 @@ extern "C" fn start_task(
 
                 // Re-validate before calling didReceiveResponse
                 check_webview_id_valid(webview_id)?;
-                check_task_is_valid(webview, task_key, task_uuid.clone())?;
+                check_task_is_valid(&webview, task_key, task_uuid.clone())?;
 
                 // Use map_err to convert Option<Retained<Exception>> to crate::Error
                 objc2::exception::catch(AssertUnwindSafe(|| {
@@ -273,15 +275,15 @@ extern "C" fn start_task(
 
                 // Check validity again
                 check_webview_id_valid(webview_id)?;
-                check_task_is_valid(webview, task_key, task_uuid.clone())?;
+                check_task_is_valid(&webview, task_key, task_uuid.clone())?;
 
                 objc2::exception::catch(AssertUnwindSafe(|| {
                   task.didReceiveData(&data);
                 }))
                 .map_err(|_e| crate::Error::CustomProtocolTaskInvalid)?;
 
                 check_webview_id_valid(webview_id)?;
-                check_task_is_valid(webview, task_key, task_uuid.clone())?;
+                check_task_is_valid(&webview, task_key, task_uuid.clone())?;
 
                 objc2::exception::catch(AssertUnwindSafe(|| {
                   task.didFinish();

src/wkwebview/class/wry_web_view.rs

@@ -2,7 +2,7 @@
 // SPDX-License-Identifier: Apache-2.0
 // SPDX-License-Identifier: MIT
 
-use std::{cell::RefCell, collections::HashMap};
+use std::{cell::RefCell, collections::HashMap, sync::Mutex};
 
 #[cfg(target_os = "macos")]
 use objc2::runtime::ProtocolObject;
@@ -29,7 +29,7 @@ pub struct WryWebViewIvars {
   pub(crate) drag_drop_handler: Box<dyn Fn(DragDropEvent) -> bool>,
   #[cfg(target_os = "macos")]
   pub(crate) accept_first_mouse: objc2::runtime::Bool,
-  pub(crate) custom_protocol_task_ids: RefCell<HashMap<usize, Retained<NSUUID>>>,
+  pub(crate) custom_protocol_task_ids: Mutex<HashMap<usize, Retained<NSUUID>>>,
 }
 
 define_class!(
@@ -117,22 +117,25 @@ impl WryWebView {
     self
       .ivars()
       .custom_protocol_task_ids
-      .borrow_mut()
+      .lock()
+      .unwrap()
       .insert(task_id, task_uuid.clone());
     task_uuid
   }
   pub(crate) fn remove_custom_task_key(&self, task_id: usize) {
     self
       .ivars()
       .custom_protocol_task_ids
-      .borrow_mut()
+      .lock()
+      .unwrap()
       .remove(&task_id);
   }
   pub(crate) fn get_custom_task_uuid(&self, task_id: usize) -> Option<Retained<NSUUID>> {
     self
       .ivars()
       .custom_protocol_task_ids
-      .borrow()
+      .lock()
+      .unwrap()
       .get(&task_id)
       .cloned()
   }