Repository: stellar/rs-soroban-env
Pull request #1182 (2023-11-08 01:42:38)

Add fn to find out if Host can be finished
### What
Add fn to find out if Host can be finished.

### Why
So that the SDK can ask the Host if it has no more references left and it is able to be finished without calling try_finish which is destructive.

I plan to use this in:
- https://github.com/stellar/rs-soroban-sdk/pull/1137

dmkozh: I'm not necessarily opposed to this change, but I wonder if it's needed for your purpose. You can get read-only access to storage and events at any point - there is really no need to destruct host to do that (and IIUC the linked PR doesn't actually call `try_finish`). `try_finish` is there mostly as a minor optimization for e2e flows and it really just ensures that there is no bug that causes something to hold a reference.
leighmcculloch: > I'm not necessarily opposed to this change, but I wonder if it's needed for your purpose.

The SDK needs to know if the `Host` has a strong ref count greater than `1` because it is the only way the SDK can tell if the Env being dropped is the last holder of the Host. Env's get dropped all the time, but they all hold a shared ref to the Host via the internal Rc

I could add a `ref_count` fn that simply returns that. But given the Host has a concept of being "finishable" that is directly tied to whether the host is a unique ref or not (a ref count of 1), it feels like it works to add that function based on that terminology.
dmkozh: I see, thanks for the explanation.

Patch:
diff --git a/soroban-env-host/src/host.rs b/soroban-env-host/src/host.rs
index 52fe94aa..84bafc1a 100644
--- a/soroban-env-host/src/host.rs
+++ b/soroban-env-host/src/host.rs
@@ -486,9 +486,20 @@ impl Host {
         self.as_budget().with_shadow_mode(f, e)
     }
 
+    /// Returns whether the Host can be finished by calling
+    /// [`Host::try_finish`].
+    ///
+    /// Returns true if the host reference is unique, refcount = 1.
+    pub fn can_finish(&self) -> bool {
+        Rc::strong_count(&self.0) == 1
+    }
+
     /// Accept a _unique_ (refcount = 1) host reference and destroy the
     /// underlying [`HostImpl`], returning its finalized components containing
     /// processing side effects  to the caller as a tuple wrapped in `Ok(...)`.
+    ///
+    /// Use [`Host::can_finish`] to determine before calling the function if it
+    /// will succeed.
     pub fn try_finish(self) -> Result<(Storage, Events), HostError> {
         let events = self.try_borrow_events()?.externalize(&self)?;
         Rc::try_unwrap(self.0)


Test patch:
diff --git a/soroban-env-host/src/test.rs b/soroban-env-host/src/test.rs
index 5bc8bf86..6b93b6a3 100644
--- a/soroban-env-host/src/test.rs
+++ b/soroban-env-host/src/test.rs
@@ -24,4 +24,5 @@ mod symbol;
 mod tuple;
 mod vec;
 
+mod finish;
 mod metering_benchmark;
diff --git a/soroban-env-host/src/test/finish.rs b/soroban-env-host/src/test/finish.rs
new file mode 100644
index 00000000..96dd9954
--- /dev/null
+++ b/soroban-env-host/src/test/finish.rs
@@ -0,0 +1,31 @@
+use crate::Host;
+
+#[test]
+fn can_finish_on_one_reference() {
+    let host1 = Host::test_host_with_recording_footprint();
+    assert!(host1.can_finish());
+    assert!(host1.try_finish().is_ok());
+}
+
+#[test]
+fn cant_finish_on_multiple_reference() {
+    let host1 = Host::test_host_with_recording_footprint();
+    let host2 = host1.clone();
+    assert!(!host1.can_finish());
+    assert!(!host2.can_finish());
+    assert!(host1.try_finish().is_err());
+    _ = host2;
+}
+
+#[test]
+fn can_finish_on_reduced_to_one_reference_count() {
+    let host1 = Host::test_host_with_recording_footprint();
+    {
+        let host2 = host1.clone();
+        assert!(!host1.can_finish());
+        assert!(!host2.can_finish());
+        _ = host1;
+    }
+    assert!(host1.can_finish());
+    assert!(host1.try_finish().is_ok());
+}