Reduce the scope of unsafe and ways to cause UB

[sftse]

This reduces the number of public API functions that can have bad interactions and cause UB.

This reduces it to only fn inner() and fn unmanage() while previously any combination of methods on StateManager were potentially perilous.

[github-actions[bot]]

Package Changes Through a472d51f683eea040fe68aadc92210f470bdaa6f

There are 8 changes which include tauri-bundler with minor, tauri with minor, tauri-cli with minor, tauri-codegen with minor, tauri-utils with minor, @tauri-apps/api with minor, @tauri-apps/cli with minor, tauri-runtime-wry with patch

Planned Package Versions

The following package releases are the planned based on the context of changes in this pull request.

package	current	next
@tauri-apps/api	2.5.0	2.6.0
tauri-utils	2.4.0	2.5.0
tauri-bundler	2.4.0	2.5.0
tauri-runtime	2.6.0	2.6.1
tauri-runtime-wry	2.6.0	2.6.1
tauri-codegen	2.2.0	2.3.0
tauri-macros	2.2.0	2.2.1
tauri-plugin	2.2.0	2.2.1
tauri-build	2.2.0	2.2.1
tauri	2.5.1	2.6.0
@tauri-apps/cli	2.5.0	2.6.0
tauri-cli	2.5.0	2.6.0

Add another change file through the GitHub UI by following this link.

---

Read about change files or the docs at github.com/jbolda/covector

[WSH032]

I noticed that many pieces of code/plugins frequently call manager::state, so personally, I care more about the performance aspect. (At least, I think Pin provides the same safety semantics as Arc 🤓)

[sftse]

@@ -154,10 +144,11 @@ impl StateManager {
     let map = self.map.lock().unwrap();
     let type_id = TypeId::of::<T>();
     let state = map.get(&type_id)?;
-    Some(State {
-      _life: PhantomData,
-      t: state.clone(),
-    })
+    Some(State(
+      Arc::downcast::<&T>(state.clone())
+        // SAFETY: the type of the key is the same as the type of the value
+        .expect("the type of the key should be same as the type of the value"),
+    ))
   }
 }

This downcast will fail, the type when it was inserted is T but this attempts to downcast to &T.

let mut map: HashMap<TypeId, Arc<dyn Any + Send + Sync>> = HashMap::new();
map.insert(TypeId::of::<u8>(), Arc::new(0) as Arc<_>);
let arc = map.get(&TypeId::of::<u8>()).unwrap();
assert!(Arc::downcast::<&u8>(arc.clone()).is_err());

There is a way to get this to work with these types, but it defeats the purpose of this PR. Creating the &'r T in fn try_get() means any combination of try_get+unmanage or get+unmanage (and by extension inner+unmanage because inner is a method on the return type of try_get or get) is potentially unsound. The Arc delays creating the reference until the call to inner, meaning if either inner or unmanage are never called, there's no unsoundness.

However, it is a bit contorted, if only to reduce the scope of UB without changing the public API. I don't think the Arc<&'r T> has any benefits over the previous code simply containing &'r T.

I noticed that many pieces of code/plugins frequently call manager::state, so personally, I care more about the performance aspect. (At least, I think Pin provides the same safety semantics as Arc 🤓)

I removed the Pin and the unsafe block because an unsafe block means the safety contract is upheld, but fn unmanage does move the value behind the Pin, violating the contract. This reads as if fn unmanage is unconditionally unsound.

[Legend-Master]

You're right, ignore my suggestions then, I thought the downcast would work but apparently no