diff --git a/Capnp.Net.Runtime.Tests/Dtbdct.cs b/Capnp.Net.Runtime.Tests/Dtbdct.cs index e5c8653..eedecb3 100644 --- a/Capnp.Net.Runtime.Tests/Dtbdct.cs +++ b/Capnp.Net.Runtime.Tests/Dtbdct.cs @@ -98,5 +98,23 @@ namespace Capnp.Net.Runtime.Tests { NewDtbdctTestbed().RunTest(Testsuite.BootstrapReuse); } + + [TestMethod] + public void Ownership1() + { + NewDtbdctTestbed().RunTest(Testsuite.Ownership1); + } + + [TestMethod] + public void Ownership2() + { + NewDtbdctTestbed().RunTest(Testsuite.Ownership2); + } + + [TestMethod] + public void Ownership3() + { + NewDtbdctTestbed().RunTest(Testsuite.Ownership3); + } } } diff --git a/Capnp.Net.Runtime.Tests/LocalRpc.cs b/Capnp.Net.Runtime.Tests/LocalRpc.cs index 095e0d7..2100293 100644 --- a/Capnp.Net.Runtime.Tests/LocalRpc.cs +++ b/Capnp.Net.Runtime.Tests/LocalRpc.cs @@ -12,6 +12,7 @@ using System.Threading.Tasks; namespace Capnp.Net.Runtime.Tests { [TestClass] + [TestCategory("Coverage")] public class LocalRpc: TestBase { [TestMethod] @@ -113,5 +114,23 @@ namespace Capnp.Net.Runtime.Tests { NewLocalTestbed().RunTest(Testsuite.Basic); } + + [TestMethod] + public void Ownership1() + { + NewLocalTestbed().RunTest(Testsuite.Ownership1); + } + + [TestMethod] + public void Ownership2() + { + NewLocalTestbed().RunTest(Testsuite.Ownership2); + } + + [TestMethod] + public void Ownership3() + { + NewLocalTestbed().RunTest(Testsuite.Ownership3); + } } } diff --git a/Capnp.Net.Runtime.Tests/TcpRpcErrorHandling.cs b/Capnp.Net.Runtime.Tests/TcpRpcErrorHandling.cs index be58c28..8bd35b2 100644 --- a/Capnp.Net.Runtime.Tests/TcpRpcErrorHandling.cs +++ b/Capnp.Net.Runtime.Tests/TcpRpcErrorHandling.cs @@ -15,6 +15,7 @@ using System.Threading.Tasks; namespace Capnp.Net.Runtime.Tests { [TestClass] + [TestCategory("Coverage")] public class TcpRpcErrorHandling: TestBase { class MemStreamEndpoint : IEndpoint diff --git a/Capnp.Net.Runtime.Tests/TcpRpcPorted.cs b/Capnp.Net.Runtime.Tests/TcpRpcPorted.cs index 0066c40..e878920 100644 --- a/Capnp.Net.Runtime.Tests/TcpRpcPorted.cs +++ b/Capnp.Net.Runtime.Tests/TcpRpcPorted.cs @@ -166,5 +166,23 @@ namespace Capnp.Net.Runtime.Tests Assert.IsTrue(impl.IsDisposed); } + + [TestMethod] + public void Ownership1() + { + NewLocalhostTcpTestbed().RunTest(Testsuite.Ownership1); + } + + [TestMethod] + public void Ownership2() + { + NewLocalhostTcpTestbed().RunTest(Testsuite.Ownership2); + } + + [TestMethod] + public void Ownership3() + { + NewLocalhostTcpTestbed().RunTest(Testsuite.Ownership3); + } } } diff --git a/Capnp.Net.Runtime.Tests/Testsuite.cs b/Capnp.Net.Runtime.Tests/Testsuite.cs index dc48702..1b3486d 100644 --- a/Capnp.Net.Runtime.Tests/Testsuite.cs +++ b/Capnp.Net.Runtime.Tests/Testsuite.cs @@ -532,5 +532,43 @@ namespace Capnp.Net.Runtime.Tests Assert.IsFalse(impl.IsDisposed); } } + + public static void Ownership1(ITestbed testbed) + { + var impl = new TestMoreStuffImpl(new Counters()); + using (var main = testbed.ConnectMain(impl)) + { + var tcs = new TaskCompletionSource(); + var ti = new TestInterfaceImpl(new Counters(), tcs); + testbed.MustComplete(main.CallFoo(ti, default)); + testbed.MustComplete(tcs.Task); + } + } + + public static void Ownership2(ITestbed testbed) + { + var impl = new TestMoreStuffImpl(new Counters()); + using (var main = testbed.ConnectMain(impl)) + using (var nullProxy = main.GetNull().Eager(true)) + { + var tcs = new TaskCompletionSource(); + var ti = new TestInterfaceImpl(new Counters(), tcs); + testbed.MustComplete(nullProxy.CallFoo(ti, default)); + testbed.MustComplete(tcs.Task); + } + } + + public static void Ownership3(ITestbed testbed) + { + var impl = new TestMoreStuffImpl(new Counters()); + using (var main = testbed.ConnectMain(impl)) + using (var nullProxy = main.GetNull(new CancellationToken(true)).Eager(true)) + { + var tcs = new TaskCompletionSource(); + var ti = new TestInterfaceImpl(new Counters(), tcs); + testbed.MustComplete(nullProxy.CallFoo(ti, default)); + testbed.MustComplete(tcs.Task); + } + } } } diff --git a/Capnp.Net.Runtime.Tests/Util/TestBase.cs b/Capnp.Net.Runtime.Tests/Util/TestBase.cs index 422e7b3..2edffbc 100644 --- a/Capnp.Net.Runtime.Tests/Util/TestBase.cs +++ b/Capnp.Net.Runtime.Tests/Util/TestBase.cs @@ -167,12 +167,12 @@ namespace Capnp.Net.Runtime.Tests void ITestbed.MustComplete(params Task[] tasks) { - Assert.IsTrue(Task.WhenAll(tasks).IsCompleted); + Assert.IsTrue(tasks.All(t => t.IsCompleted)); } void ITestbed.MustNotComplete(params Task[] tasks) { - Assert.IsFalse(Task.WhenAny(tasks).IsCompleted); + Assert.IsFalse(tasks.Any(t => t.IsCompleted)); } } @@ -248,14 +248,30 @@ namespace Capnp.Net.Runtime.Tests return _client.GetMain(); } + static Task[] GulpExceptions(Task[] tasks) + { + async Task Gulp(Task t) + { + try + { + await t; + } + catch + { + } + } + + return tasks.Select(Gulp).ToArray(); + } + void ITestbed.MustComplete(params Task[] tasks) { - Assert.IsTrue(Task.WaitAll(tasks, MediumNonDbgTimeout)); + Assert.IsTrue(Task.WaitAll(GulpExceptions(tasks), MediumNonDbgTimeout)); } void ITestbed.MustNotComplete(params Task[] tasks) { - Assert.AreEqual(-1, Task.WaitAny(tasks, ShortTimeout)); + Assert.AreEqual(-1, Task.WaitAny(GulpExceptions(tasks), ShortTimeout)); } void ITestbed.FlushCommunication() diff --git a/Capnp.Net.Runtime/DeserializerState.cs b/Capnp.Net.Runtime/DeserializerState.cs index ea02416..702176c 100644 --- a/Capnp.Net.Runtime/DeserializerState.cs +++ b/Capnp.Net.Runtime/DeserializerState.cs @@ -45,6 +45,7 @@ namespace Capnp /// The kind of object this state currently represents. /// public ObjectKind Kind { get; set; } + bool _disposed; /// /// The capabilities imported from the capability table. Only valid in RPC context. /// @@ -65,6 +66,7 @@ namespace Capnp StructPtrCount = 1; Kind = ObjectKind.Struct; Caps = null; + _disposed = false; } /// @@ -685,12 +687,14 @@ namespace Capnp public void Dispose() { - if (Caps != null) + if (Caps != null && !_disposed) { foreach (var cap in Caps) { cap?.Release(false); } + + _disposed = true; } } } diff --git a/Capnp.Net.Runtime/ObjectKind.cs b/Capnp.Net.Runtime/ObjectKind.cs index bdabf6f..80909eb 100644 --- a/Capnp.Net.Runtime/ObjectKind.cs +++ b/Capnp.Net.Runtime/ObjectKind.cs @@ -4,9 +4,7 @@ namespace Capnp { /// /// The different kinds of Cap'n Proto objects. - /// Despite this is a [Flags] enum, it does not make sense to mutually combine literals. /// - [Flags] public enum ObjectKind: byte { /// diff --git a/Capnp.Net.Runtime/Rpc/Impatient.cs b/Capnp.Net.Runtime/Rpc/Impatient.cs index e0ede4e..1e621b5 100644 --- a/Capnp.Net.Runtime/Rpc/Impatient.cs +++ b/Capnp.Net.Runtime/Rpc/Impatient.cs @@ -76,6 +76,7 @@ namespace Capnp.Rpc /// The underlying promise /// is null. /// The task was not registered using MakePipelineAware. + [Obsolete("Please re-generate capnp code-behind. GetAnswer(task).Access(...) was replaced by Access(task, ...)")] public static IPromisedAnswer GetAnswer(Task task) { if (!_taskTable.TryGetValue(task, out var answer)) diff --git a/Capnp.Net.Runtime/Rpc/LazyCapability.cs b/Capnp.Net.Runtime/Rpc/LazyCapability.cs index 2c06584..ae1eef4 100644 --- a/Capnp.Net.Runtime/Rpc/LazyCapability.cs +++ b/Capnp.Net.Runtime/Rpc/LazyCapability.cs @@ -91,12 +91,28 @@ namespace Capnp.Rpc async Task CallImpl(ulong interfaceId, ushort methodId, DynamicSerializerState args, CancellationToken cancellationToken) { - var cap = await WhenResolved; + ConsumedCapability? cap; + try + { + cap = await WhenResolved; + } + catch + { + args.Dispose(); + throw; + } - cancellationToken.ThrowIfCancellationRequested(); + if (cancellationToken.IsCancellationRequested) + { + args.Dispose(); + cancellationToken.ThrowIfCancellationRequested(); + } if (cap == null) + { + args.Dispose(); throw new RpcException("Broken capability"); + } using var proxy = new Proxy(cap); var call = proxy.Call(interfaceId, methodId, args, default); diff --git a/Capnp.Net.Runtime/Rpc/LocalAnswerCapability.cs b/Capnp.Net.Runtime/Rpc/LocalAnswerCapability.cs index ef5cc29..696dbbb 100644 --- a/Capnp.Net.Runtime/Rpc/LocalAnswerCapability.cs +++ b/Capnp.Net.Runtime/Rpc/LocalAnswerCapability.cs @@ -72,7 +72,10 @@ namespace Capnp.Rpc cancellationToken.ThrowIfCancellationRequested(); if (proxy.IsNull) + { + args.Dispose(); throw new RpcException("Broken capability"); + } var call = proxy.Call(interfaceId, methodId, args, default); var whenReturned = call.WhenReturned; diff --git a/Capnp.Net.Runtime/Rpc/Proxy.cs b/Capnp.Net.Runtime/Rpc/Proxy.cs index c4d0eed..b6dd739 100644 --- a/Capnp.Net.Runtime/Rpc/Proxy.cs +++ b/Capnp.Net.Runtime/Rpc/Proxy.cs @@ -50,17 +50,9 @@ namespace Capnp.Rpc static async void DisposeCtrWhenReturned(CancellationTokenRegistration ctr, IPromisedAnswer answer) { - try - { - await answer.WhenReturned; - } - catch - { - } - finally - { - ctr.Dispose(); - } + try { await answer.WhenReturned; } + catch { } + finally { ctr.Dispose(); } } /// @@ -80,10 +72,16 @@ namespace Capnp.Rpc bool obsoleteAndIgnored, CancellationToken cancellationToken = default) { if (_disposedValue) + { + args.Dispose(); throw new ObjectDisposedException(nameof(Proxy)); + } if (ConsumedCap == null) + { + args.Dispose(); throw new InvalidOperationException("Cannot call null capability"); + } var answer = ConsumedCap.DoCall(interfaceId, methodId, args); @@ -172,7 +170,7 @@ namespace Capnp.Rpc ~Proxy() { #if DebugFinalizers - Logger.LogWarning($"Caught orphaned Proxy, created from here: {CreatorStackTrace}."); + Logger?.LogWarning($"Caught orphaned Proxy, created from here: {CreatorStackTrace}."); #endif Dispose(false); diff --git a/Capnp.Net.Runtime/Rpc/RefCountingCapability.cs b/Capnp.Net.Runtime/Rpc/RefCountingCapability.cs index b6beae8..d47a46f 100644 --- a/Capnp.Net.Runtime/Rpc/RefCountingCapability.cs +++ b/Capnp.Net.Runtime/Rpc/RefCountingCapability.cs @@ -50,7 +50,7 @@ namespace Capnp.Rpc ~RefCountingCapability() { #if DebugFinalizers - Logger.LogWarning($"Caught orphaned capability, created from here: {CreatorStackTrace}."); + Logger?.LogWarning($"Caught orphaned capability, created from here: {CreatorStackTrace}."); #endif Dispose(false); diff --git a/Capnp.Net.Runtime/Rpc/RemoteAnswerCapability.cs b/Capnp.Net.Runtime/Rpc/RemoteAnswerCapability.cs index 8423581..4631f8b 100644 --- a/Capnp.Net.Runtime/Rpc/RemoteAnswerCapability.cs +++ b/Capnp.Net.Runtime/Rpc/RemoteAnswerCapability.cs @@ -112,9 +112,17 @@ namespace Capnp.Rpc if (!_question.StateFlags.HasFlag(PendingQuestion.State.TailCall) && _question.StateFlags.HasFlag(PendingQuestion.State.Returned)) { - if (ResolvedCap == null) + try { - throw new RpcException("Answer did not resolve to expected capability"); + if (ResolvedCap == null) + { + throw new RpcException("Answer did not resolve to expected capability"); + } + } + catch + { + args.Dispose(); + throw; } return CallOnResolution(interfaceId, methodId, args); @@ -126,11 +134,13 @@ namespace Capnp.Rpc #endif if (_question.StateFlags.HasFlag(PendingQuestion.State.Disposed)) { + args.Dispose(); throw new ObjectDisposedException(nameof(PendingQuestion)); } if (_question.StateFlags.HasFlag(PendingQuestion.State.FinishRequested)) { + args.Dispose(); throw new InvalidOperationException("Finish request was already sent"); } diff --git a/Capnp.Net.Runtime/Rpc/RemoteResolvingCapability.cs b/Capnp.Net.Runtime/Rpc/RemoteResolvingCapability.cs index 868e775..b6a9cf8 100644 --- a/Capnp.Net.Runtime/Rpc/RemoteResolvingCapability.cs +++ b/Capnp.Net.Runtime/Rpc/RemoteResolvingCapability.cs @@ -90,9 +90,13 @@ namespace Capnp.Rpc { // Two reasons for ignoring exceptions on the previous task (i.e. not _.Wait()ing): // 1. A faulting predecessor, especially due to cancellation, must not have any impact on this one. - // 2. A faulting disembargo request would be a fatal protocol error, resulting in Abort() - we're dead anyway. + // 2. A faulting disembargo request would imply that the other side cannot send pending requests anyway. - cancellationTokenSource.Token.ThrowIfCancellationRequested(); + if (cancellationTokenSource.Token.IsCancellationRequested) + { + args.Dispose(); + cancellationTokenSource.Token.ThrowIfCancellationRequested(); + } using var proxy = new Proxy(ResolvedCap); return proxy.Call(interfaceId, methodId, args, default); diff --git a/Capnp.Net.Runtime/SerializerState.cs b/Capnp.Net.Runtime/SerializerState.cs index 7bdb9e2..26549f7 100644 --- a/Capnp.Net.Runtime/SerializerState.cs +++ b/Capnp.Net.Runtime/SerializerState.cs @@ -11,7 +11,7 @@ namespace Capnp /// by the code generator. Particularly, those writer classes are actually specializations of SerializerState, adding convenience methods /// for accessing the struct's fields. /// - public class SerializerState : IStructSerializer + public class SerializerState : IStructSerializer, IDisposable { /// /// Constructs a SerializerState instance for use in RPC context. @@ -42,6 +42,7 @@ namespace Capnp internal uint CapabilityIndex { get; set; } SerializerState[]? _linkedStates; + bool _disposed; /// /// Constructs an unbound serializer state. @@ -1380,5 +1381,18 @@ namespace Capnp var cap = StructReadRawCap(slot); return new Rpc.BareProxy(cap); } + + public void Dispose() + { + if (Caps != null && !_disposed) + { + foreach (var cap in Caps) + { + cap?.Release(false); + } + + _disposed = true; + } + } } } \ No newline at end of file