Fix short lived callback lifetimes (#90)

Signed-off-by: Kristupas Antanavičius <[email protected]>

Author: arg0d

CHANGELOG.md

@@ -1,3 +1,7 @@
+### UNRELEASED
+
+- **IMPORTANT**: Fix short-lived callback lifetimes (#79)
+
 ### v0.8.2+v0.25.0
 
 - Update C# callback syntax to work on iOS (#84)

bindgen/templates/CallbackInterfaceRuntime.cs

@@ -9,30 +9,25 @@ static class UniffiCallbackResponseCode {
 }
 
 class ConcurrentHandleMap<T> where T: notnull {
-    Dictionary<ulong, T> leftMap = new Dictionary<ulong, T>();
-    Dictionary<T, ulong> rightMap = new Dictionary<T, ulong>();
+    Dictionary<ulong, T> map = new Dictionary<ulong, T>();
 
     Object lock_ = new Object();
     ulong currentHandle = 0;
 
     public ulong Insert(T obj) {
         lock (lock_) {
-            ulong existingHandle = 0;
-            if (rightMap.TryGetValue(obj, out existingHandle)) {
-                return existingHandle;
-            }
             currentHandle += 1;
-            leftMap[currentHandle] = obj;
-            rightMap[obj] = currentHandle;
+            map[currentHandle] = obj;
             return currentHandle;
         }
     }
 
     public bool TryGet(ulong handle, out T result) {
-        // Possible null reference assignment
-        #pragma warning disable 8601
-        return leftMap.TryGetValue(handle, out result);
-        #pragma warning restore 8601
+        lock (lock_) {
+            #pragma warning disable 8601 // Possible null reference assignment
+            return map.TryGetValue(handle, out result);
+            #pragma warning restore 8601
+        }
     }
 
     public bool Remove(ulong handle) {
@@ -43,10 +38,9 @@ public bool Remove(ulong handle, out T result) {
         lock (lock_) {
             // Possible null reference assignment
             #pragma warning disable 8601
-            if (leftMap.TryGetValue(handle, out result)) {
+            if (map.TryGetValue(handle, out result)) {
             #pragma warning restore 8601
-                leftMap.Remove(handle);
-                rightMap.Remove(result);
+                map.Remove(handle);
                 return true;
             } else {
                 return false;

dotnet-tests/UniffiCS.BindingTests/TestCallbacksFixture.cs

@@ -189,4 +189,20 @@ public void VoidCallbackExceptions()
             );
         }
     }
+
+    [Fact]
+    public void ShortLivedCallbackDoesNotInvalidateLongerLivedCallback()
+    {
+        var stringifier = new CsharpStringifier();
+        using (var rustStringifier1 = new RustStringifier(stringifier))
+        {
+            using (var rustStringifier2 = new RustStringifier(stringifier))
+            {
+                Assert.Equal("C#: 123", rustStringifier2.FromSimpleType(123));
+            }
+            // `stringifier` must remain valid after `rustStringifier2` drops the reference
+
+            Assert.Equal("C#: 123", rustStringifier1.FromSimpleType(123));
+        }
+    }
 }

dotnet-tests/UniffiCS.BindingTests/TestNullToEmptyString.cs

@@ -12,6 +12,8 @@ public class TestNullToEmptyString
     public void NullToEmptyStringWorks()
     {
         Assert.Equal("hello", LibGreeter.HelloWorld("hello"));
+        #pragma warning disable 8625 // Cannot convert null literal to non-nullable reference type
         Assert.Equal("", LibGreeter.HelloWorld(null));
+        #pragma warning restore 8625
     }
 }

generate_bindings.sh

@@ -6,4 +6,4 @@ GEN_DIR="dotnet-tests/UniffiCS/gen"
 rm -rf "$GEN_DIR"
 mkdir -p "$GEN_DIR"
 
-target/debug/uniffi-bindgen-cs target/debug/libuniffi_fixtures.so --library --out-dir="$GEN_DIR"
+target/debug/uniffi-bindgen-cs target/debug/libuniffi_fixtures.so --library --out-dir="$GEN_DIR" --no-format