Skip to content
This repository was archived by the owner on Dec 18, 2018. It is now read-only.

Commit 6d7ddc4

Browse files
author
Cesar Blum Silveira
committed
Fix deadlock in SocketOutput (#1278).
1 parent 2351c1b commit 6d7ddc4

File tree

4 files changed

+119
-3
lines changed

4 files changed

+119
-3
lines changed

src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/SocketOutput.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ public Task WriteAsync(
169169
_tasksPending.Enqueue(new WaitingTask()
170170
{
171171
CancellationToken = cancellationToken,
172-
CancellationRegistration = cancellationToken.Register(_connectionCancellation, this),
172+
CancellationRegistration = cancellationToken.SafeRegister(_connectionCancellation, this),
173173
BytesToWrite = buffer.Count,
174174
CompletionSource = tcs
175175
});
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
using System;
5+
using System.Threading;
6+
7+
namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure
8+
{
9+
internal static class CancellationTokenExtensions
10+
{
11+
public static IDisposable SafeRegister(this CancellationToken cancellationToken, Action<object> callback, object state)
12+
{
13+
var callbackWrapper = new CancellationCallbackWrapper(callback, state);
14+
var registration = cancellationToken.Register(s => InvokeCallback(s), callbackWrapper);
15+
var disposeCancellationState = new DisposeCancellationState(callbackWrapper, registration);
16+
17+
return new DisposableAction(s => Dispose(s), disposeCancellationState);
18+
}
19+
20+
private static void InvokeCallback(object state)
21+
{
22+
((CancellationCallbackWrapper)state).TryInvoke();
23+
}
24+
25+
private static void Dispose(object state)
26+
{
27+
((DisposeCancellationState)state).TryDispose();
28+
}
29+
30+
private class DisposeCancellationState
31+
{
32+
private readonly CancellationCallbackWrapper _callbackWrapper;
33+
private readonly CancellationTokenRegistration _registration;
34+
35+
public DisposeCancellationState(CancellationCallbackWrapper callbackWrapper, CancellationTokenRegistration registration)
36+
{
37+
_callbackWrapper = callbackWrapper;
38+
_registration = registration;
39+
}
40+
41+
public void TryDispose()
42+
{
43+
if (_callbackWrapper.TrySetInvoked())
44+
{
45+
_registration.Dispose();
46+
}
47+
}
48+
}
49+
50+
private class CancellationCallbackWrapper
51+
{
52+
private readonly Action<object> _callback;
53+
private readonly object _state;
54+
private int _callbackInvoked;
55+
56+
public CancellationCallbackWrapper(Action<object> callback, object state)
57+
{
58+
_callback = callback;
59+
_state = state;
60+
}
61+
62+
public bool TrySetInvoked()
63+
{
64+
return Interlocked.Exchange(ref _callbackInvoked, 1) == 0;
65+
}
66+
67+
public void TryInvoke()
68+
{
69+
if (TrySetInvoked())
70+
{
71+
_callback(_state);
72+
}
73+
}
74+
}
75+
}
76+
}
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
using System;
5+
using System.Threading;
6+
7+
namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure
8+
{
9+
internal class DisposableAction : IDisposable
10+
{
11+
public static readonly DisposableAction Empty = new DisposableAction(() => { });
12+
13+
private Action<object> _action;
14+
private readonly object _state;
15+
16+
public DisposableAction(Action action)
17+
: this(state => ((Action)state).Invoke(), state: action)
18+
{
19+
}
20+
21+
public DisposableAction(Action<object> action, object state)
22+
{
23+
_action = action;
24+
_state = state;
25+
}
26+
27+
protected virtual void Dispose(bool disposing)
28+
{
29+
if (disposing)
30+
{
31+
Interlocked.Exchange(ref _action, (state) => { }).Invoke(_state);
32+
}
33+
}
34+
35+
public void Dispose()
36+
{
37+
Dispose(true);
38+
}
39+
}
40+
}

test/Microsoft.AspNetCore.Server.KestrelTests/SocketOutputTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -753,7 +753,7 @@ public async Task OnlyAllowsUpToThreeConcurrentWrites(KestrelServerOptions optio
753753
Action<int> triggerNextCompleted;
754754
Assert.True(completeQueue.TryDequeue(out triggerNextCompleted));
755755
triggerNextCompleted(0);
756-
await mockLibuv.OnPostTask;
756+
await mockLibuv.OnPostTask;
757757
Assert.True(writeCalled);
758758

759759
// Cleanup
@@ -862,4 +862,4 @@ public async Task ProducingStartAndProducingCompleteCanBeCalledAfterConnectionCl
862862
}
863863
}
864864
}
865-
}
865+
}

0 commit comments

Comments
 (0)