Potential race condition when client releases capability, but the same capability is re-imported from server

This commit is contained in:
Christian Köllner 2019-08-25 22:06:29 +02:00
parent d090697fa4
commit 77467e606a
2 changed files with 29 additions and 30 deletions

View File

@ -42,19 +42,18 @@ namespace Capnp.Rpc
} }
catch (ArgumentException) catch (ArgumentException)
{ {
// Force .NET to create a new Task instance if (rtask.IsCompleted)
var stask = rtask;
async Task<T> AwaitAgain()
{ {
return await stask; // Force .NET to create a new Task instance
} var stask = rtask;
rtask = new Task<T>(() => stask.Result);
rtask = AwaitAgain();
_taskTable.Add(rtask, promise); _taskTable.Add(rtask, promise);
} }
else
{
throw new InvalidOperationException("What the heck is wrong with Task?");
}
}
return rtask; return rtask;
} }

View File

@ -6,6 +6,8 @@ namespace Capnp.Rpc
{ {
abstract class RefCountingCapability: ConsumedCapability abstract class RefCountingCapability: ConsumedCapability
{ {
readonly object _reentrancyBlocker = new object();
// Note on reference counting: Works in analogy to COM. AddRef() adds a reference, // Note on reference counting: Works in analogy to COM. AddRef() adds a reference,
// Release() removes it. When the reference count reaches zero, the capability must be // Release() removes it. When the reference count reaches zero, the capability must be
// released remotely, i.e. we need to tell the remote peer that it can remove this // released remotely, i.e. we need to tell the remote peer that it can remove this
@ -64,35 +66,33 @@ namespace Capnp.Rpc
internal sealed override void AddRef() internal sealed override void AddRef()
{ {
if (Interlocked.Increment(ref _refCount) <= 1) lock (_reentrancyBlocker)
{
if (++_refCount <= 1)
{ {
throw new ObjectDisposedException(nameof(ConsumedCapability)); throw new ObjectDisposedException(nameof(ConsumedCapability));
} }
} }
}
internal sealed override void Release() internal sealed override void Release()
{ {
while (true) lock (_reentrancyBlocker)
{ {
if ((Interlocked.CompareExchange(ref _refCount, 0, 2) == 2) || switch (_refCount)
(Interlocked.CompareExchange(ref _refCount, 0, 1) == 1))
{ {
case 1: // initial state, actually ref. count 0
case 2: // actually ref. count 1
_refCount = 0;
Dispose(true); Dispose(true);
GC.SuppressFinalize(this); GC.SuppressFinalize(this);
return; break;
}
long oldCount = Interlocked.Read(ref _refCount); case var _ when _refCount > 2:
--_refCount;
break;
if (oldCount > 2) default:
{
if (Interlocked.CompareExchange(ref _refCount, oldCount - 1, oldCount) == oldCount)
{
return;
}
}
else if (oldCount <= 0)
{
throw new InvalidOperationException("Capability is already disposed"); throw new InvalidOperationException("Capability is already disposed");
} }
} }