efl-mono: Fix value forwarding in promises/async

Summary:
Values returned from C# Then callbacks must release ownership of the
underlying native value, so Eina code can clean it up nicely and avoid
the Wrapper flushing it early.

The same issue applied to the Async wrappers. In this case the value
passed as the Task parameter could be released by an `using` block
awaiting the value.

Also Future creation was then-ing the wrong handle.

Also add better exception messages.

Reviewers: vitor.sousa, felipealmeida

Reviewed By: vitor.sousa

Subscribers: cedric, #reviewers, #committers

Tags: #efl

Differential Revision: https://phab.enlightenment.org/D9197

diff --git a/src/bindings/mono/eina_mono/eina_promises.cs b/src/bindings/mono/eina_mono/eina_promises.cs
index 25077a3..34561a5 100644 (file)
--- a/src/bindings/mono/eina_mono/eina_promises.cs
+++ b/src/bindings/mono/eina_mono/eina_promises.cs
@@ -180,6 +180,8 @@ public class Promise : IDisposable
     {
         SanityChecks();
         eina_promise_resolve(this.Handle, value);
+        // Promise will take care of releasing this value correctly.
+        value.ReleaseOwnership();
         this.Handle = IntPtr.Zero;
         // Resolving a cb does *not* call its cancellation callback, so we have to release the
         // lambda created in the constructor for cleanup.
@@ -224,11 +226,12 @@ public class Future
     /// </summary>
     public Future(IntPtr handle)
     {
-        Handle = ThenRaw(handle, (Eina.Value value) =>
+        handle = ThenRaw(handle, (Eina.Value value) =>
         {
             Handle = IntPtr.Zero;
             return value;
         });
+        Handle = handle;
     }
 
     /// <summary>
@@ -304,7 +307,12 @@ public class Future
         ResolvedCb cb = handle.Target as ResolvedCb;
         if (cb != null)
         {
-            value = cb(value);
+            Eina.Value managedValue = cb(value);
+            // Both `value` and `managedValue` will point to the same internal value data.
+            // Avoid C# wrapper invalidating the underlying C Eina_Value as the eina_future.c
+            // code will release it.
+            value = managedValue.GetNative();
+            managedValue.ReleaseOwnership();
         }
         else
         {

diff --git a/src/bindings/mono/eina_mono/eina_value.cs b/src/bindings/mono/eina_mono/eina_value.cs
index 99cf097..a19d3d4 100644 (file)
--- a/src/bindings/mono/eina_mono/eina_value.cs
+++ b/src/bindings/mono/eina_mono/eina_value.cs
@@ -47,6 +47,9 @@ static internal class UnsafeNativeMethods
     internal static extern void eina_value_free(IntPtr type);
 
     [DllImport(efl.Libs.Eina)]
+    internal static extern IntPtr eina_value_type_name_get(IntPtr type);
+
+    [DllImport(efl.Libs.Eina)]
     [return: MarshalAsAttribute(UnmanagedType.U1)]
     internal static extern bool eina_value_convert(IntPtr handle, IntPtr convert);
 
@@ -725,7 +728,16 @@ static class ValueTypeBridge
             LoadTypes();
         }
 
-        return NativeToManaged[native];
+        try
+        {
+            return NativeToManaged[native];
+        }
+        catch (KeyNotFoundException ex)
+        {
+            var name_ptr = eina_value_type_name_get(native);
+            var name = Marshal.PtrToStringAnsi(name_ptr);
+            throw new Efl.EflException($"Unknown native eina value Type: 0x{native.ToInt64():x} with name {name}");
+        }
     }
 
     public static IntPtr GetNative(ValueType valueType)
@@ -978,8 +990,7 @@ public class Value : IDisposable, IComparable<Value>, IEquatable<Value>
                 // free(), avoiding a call to eina_value_flush that would wipe the underlying value contents
                 // for pointer types like string.
                 tmp = MemoryNative.Alloc(Marshal.SizeOf(typeof(ValueNative)));
-                Marshal.StructureToPtr(value, tmp, false); // Can't get the address of a struct directly.
-                this.Handle = Alloc();
+                Marshal.StructureToPtr<ValueNative>(value, tmp, false); // Can't get the address of a struct directly.
 
                 // Copy is used to deep copy the pointed payload (e.g. strings) inside this struct, so we can own this value.
                 if (!eina_value_copy(tmp, this.Handle))
@@ -1482,7 +1493,7 @@ public class Value : IDisposable, IComparable<Value>, IEquatable<Value>
         {
             if (Disposed)
             {
-                throw new ObjectDisposedException(base.GetType().Name);
+                throw new ObjectDisposedException($"Value at 0x{this.Handle.ToInt64():x}");
             }
 
             return this.Handle;
@@ -1494,7 +1505,7 @@ public class Value : IDisposable, IComparable<Value>, IEquatable<Value>
     {
         if (Disposed)
         {
-            throw new ObjectDisposedException(base.GetType().Name);
+            throw new ObjectDisposedException($"Value at 0x{this.Handle.ToInt64():x}");
         }
 
         // Can't call setup with Empty value type (would give an eina error)
@@ -1540,7 +1551,7 @@ public class Value : IDisposable, IComparable<Value>, IEquatable<Value>
     {
         if (Disposed)
         {
-            throw new ObjectDisposedException(GetType().Name);
+            throw new ObjectDisposedException($"Value at 0x{this.Handle.ToInt64():x}");
         }
     }
 
@@ -1591,6 +1602,7 @@ public class Value : IDisposable, IComparable<Value>, IEquatable<Value>
     /// <summary>Get a ValueNative struct with the *value* pointed by this Eina.Value.</summary>
     public ValueNative GetNative()
     {
+        SanityChecks();
         ValueNative value = (ValueNative)Marshal.PtrToStructure(this.Handle, typeof(ValueNative));
         return value;
     }

diff --git a/src/bindings/mono/eo_mono/iwrapper.cs b/src/bindings/mono/eo_mono/iwrapper.cs
index 5fe1daa..82c08cc 100644 (file)
--- a/src/bindings/mono/eo_mono/iwrapper.cs
+++ b/src/bindings/mono/eo_mono/iwrapper.cs
@@ -530,8 +530,10 @@ public class Globals
                 }
                 else
                 {
-                    // Will mark the returned task below as completed.
-                    tcs.SetResult(received);
+                    // Async receiver methods may consume the value C# wrapper, like when awaiting in the start of an
+                    // using block. In order to continue to forward the value correctly to the next futures
+                    // in the chain, we give the Async wrapper a copy of the received wrapper.
+                    tcs.SetResult(new Eina.Value(received));
                 }
 
                 fulfilled = true;

diff --git a/src/tests/efl_mono/Promises.cs b/src/tests/efl_mono/Promises.cs
index b355a0e..fc0c089 100644 (file)
--- a/src/tests/efl_mono/Promises.cs
+++ b/src/tests/efl_mono/Promises.cs
@@ -42,6 +42,31 @@ class TestPromises
         Test.AssertEquals(received_value, reference_value);
     }
 
+    public static void test_simple_with_object()
+    {
+        bool callbackCalled = false;
+        Eina.Value receivedValue = null;
+
+        Efl.Loop loop = Efl.App.AppMain;
+        Eina.Promise promise = new Eina.Promise();
+        Eina.Future future = new Eina.Future(promise);
+
+        future = future.Then((Eina.Value value) => {
+            callbackCalled = true;
+            receivedValue = new Eina.Value(value);
+            return value;
+        });
+
+        Eina.Value referenceValue = new Eina.Value(Eina.ValueType.Array, Eina.ValueType.Int32);
+        referenceValue.Append(32);
+        promise.Resolve(new Eina.Value(referenceValue));
+
+        loop.Iterate();
+
+        Test.Assert(callbackCalled, "Future callback should have been called.");
+        Test.AssertEquals(receivedValue, referenceValue);
+    }
+
     public static void test_simple_reject()
     {
         bool callbackCalled = false;