From 113d7815c2133dcea906ceab59d74dd50a6da369 Mon Sep 17 00:00:00 2001 From: jasperd Date: Tue, 28 Jan 2020 21:39:00 +0100 Subject: [PATCH 01/12] Make internals visible to UnitTests --- src/NATS.Client/Properties/AssemblyInfo.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/NATS.Client/Properties/AssemblyInfo.cs b/src/NATS.Client/Properties/AssemblyInfo.cs index 346e5cb05..7226e4387 100644 --- a/src/NATS.Client/Properties/AssemblyInfo.cs +++ b/src/NATS.Client/Properties/AssemblyInfo.cs @@ -6,7 +6,6 @@ #if DEBUG [assembly: InternalsVisibleTo("UnitTests")] [assembly: InternalsVisibleTo("MicroBenchmarks")] - #else [assembly: InternalsVisibleTo("UnitTests, PublicKey=0024000004800000940000000602000000240000525341310004000001000100db7da1f2f89089327b47d26d69666fad20861f24e9acdb13965fb6c64dfee8da589b495df37a75e934ddbacb0752a42c40f3dbc79614eec9bb2a0b6741f9e2ad2876f95e74d54c23eef0063eb4efb1e7d824ee8a695b647c113c92834f04a3a83fb60f435814ddf5c4e5f66a168139c4c1b1a50a3e60c164d180e265b1f000cd")] [assembly: InternalsVisibleTo("MicroBenchmarks, PublicKey=0024000004800000940000000602000000240000525341310004000001000100db7da1f2f89089327b47d26d69666fad20861f24e9acdb13965fb6c64dfee8da589b495df37a75e934ddbacb0752a42c40f3dbc79614eec9bb2a0b6741f9e2ad2876f95e74d54c23eef0063eb4efb1e7d824ee8a695b647c113c92834f04a3a83fb60f435814ddf5c4e5f66a168139c4c1b1a50a3e60c164d180e265b1f000cd")] From c02975a7fe7086a15ef276e245622a648d0c6db6 Mon Sep 17 00:00:00 2001 From: jasperd Date: Tue, 28 Jan 2020 21:40:04 +0100 Subject: [PATCH 02/12] Add unit tests for InFlightRequest - Move InFlightRequest to its own file - Make InFlightRequest internal --- src/NATS.Client/Conn.cs | 40 --------- src/NATS.Client/Internals/InFlightRequest.cs | 72 +++++++++++++++ .../Internals/InFlightRequestTests.cs | 88 +++++++++++++++++++ 3 files changed, 160 insertions(+), 40 deletions(-) create mode 100644 src/NATS.Client/Internals/InFlightRequest.cs create mode 100644 src/Tests/UnitTests/Internals/InFlightRequestTests.cs diff --git a/src/NATS.Client/Conn.cs b/src/NATS.Client/Conn.cs index 207169dd0..7dad3ac8f 100644 --- a/src/NATS.Client/Conn.cs +++ b/src/NATS.Client/Conn.cs @@ -158,46 +158,6 @@ public Options Opts private readonly ConcurrentDictionary waitingRequests = new ConcurrentDictionary(StringComparer.OrdinalIgnoreCase); - // Handles in-flight requests when using the new-style request/reply behavior - private sealed class InFlightRequest : IDisposable - { - internal InFlightRequest(string id, CancellationToken token, int timeout, Action onCompleted) - { - this.Id = id; - this.Waiter = new TaskCompletionSource(); - this.onCompleted = onCompleted; - this.tokenSource = token == CancellationToken.None - ? new CancellationTokenSource() - : CancellationTokenSource.CreateLinkedTokenSource(token); - - this.tokenRegistration = this.tokenSource.Token.Register(() => - { - if (timeout > 0) - this.Waiter.TrySetException(new NATSTimeoutException()); - - this.Waiter.TrySetCanceled(); - }); - - if(timeout > 0) - this.tokenSource.CancelAfter(timeout); - } - - public string Id { get; } - public TaskCompletionSource Waiter { get; } - public CancellationToken Token => tokenSource.Token; - - private readonly Action onCompleted; - private readonly CancellationTokenSource tokenSource; - private CancellationTokenRegistration tokenRegistration; - - public void Dispose() - { - this.tokenRegistration.Dispose(); - this.tokenSource?.Dispose(); - this.onCompleted?.Invoke(this.Id); - } - } - // Prepare protocol messages for efficiency private byte[] PING_P_BYTES = null; private int PING_P_BYTES_LEN; diff --git a/src/NATS.Client/Internals/InFlightRequest.cs b/src/NATS.Client/Internals/InFlightRequest.cs new file mode 100644 index 000000000..f1d42cc0d --- /dev/null +++ b/src/NATS.Client/Internals/InFlightRequest.cs @@ -0,0 +1,72 @@ +// Copyright 2015-2018 The NATS Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +using System; +using System.Threading; +using System.Threading.Tasks; + +namespace NATS.Client.Internals +{ + /// + /// Handles in-flight requests when using the default (i.e. not old) request/reply behavior + /// + internal sealed class InFlightRequest : IDisposable + { + private readonly Action _onCompleted; + private readonly CancellationTokenSource _tokenSource; + private readonly CancellationTokenRegistration _tokenRegistration; + + public string Id { get; } + public TaskCompletionSource Waiter { get; } + public CancellationToken Token => _tokenSource.Token; + + /// + /// Initializes a new instance of + /// + /// + /// + /// + /// + internal InFlightRequest(string id, CancellationToken token, int timeout, Action onCompleted) + { + Id = id; + Waiter = new TaskCompletionSource(); + _onCompleted = onCompleted; + _tokenSource = token == CancellationToken.None + ? new CancellationTokenSource() + : CancellationTokenSource.CreateLinkedTokenSource(token); + + _tokenRegistration = _tokenSource.Token.Register(() => + { + if (timeout > 0) + Waiter.TrySetException(new NATSTimeoutException()); + + Waiter.TrySetCanceled(); + }); + + if(timeout > 0) + _tokenSource.CancelAfter(timeout); + } + + /// + /// Releases all resources used by the object + /// and invokes the onCompleted delegate. + /// + public void Dispose() + { + _tokenRegistration.Dispose(); + _tokenSource?.Dispose(); + _onCompleted?.Invoke(Id); + } + } +} diff --git a/src/Tests/UnitTests/Internals/InFlightRequestTests.cs b/src/Tests/UnitTests/Internals/InFlightRequestTests.cs new file mode 100644 index 000000000..0b074dbc9 --- /dev/null +++ b/src/Tests/UnitTests/Internals/InFlightRequestTests.cs @@ -0,0 +1,88 @@ +using System.Threading; +using System.Threading.Tasks; +using NATS.Client; +using NATS.Client.Internals; +using Xunit; + +namespace UnitTests.Internals +{ + public class InFlightRequestTests + { + [Fact] + public async Task Timeout_ThrowsNatsTimeoutException() + { + // Arrange + var sut = new InFlightRequest("Foo", default, 1, _ => { }); + + // Assert + await Assert.ThrowsAsync(() => sut.Waiter.Task); + } + + [Fact] + public async Task TimeoutWithToken_ThrowsTaskCanceledExcpetion() + { + // Arrange + var cts = new CancellationTokenSource(); + var sut = new InFlightRequest("Foo", cts.Token, 1, _ => { }); + + // Assert + await Assert.ThrowsAsync(() => sut.Waiter.Task); + } + + [Fact] + public async Task Canceled_ThrowsTaskCanceledExcpetion() + { + // Arrange + var cts = new CancellationTokenSource(); + var sut = new InFlightRequest("Foo", cts.Token, 0, _ => { }); + + // Act + cts.Cancel(); + + // Assert + await Assert.ThrowsAsync(() => sut.Waiter.Task); + } + + [Fact] + public async Task CanceledWithTimeout_ThrowsNatsTimeoutException() + { + // Arrange + var cts = new CancellationTokenSource(); + var sut = new InFlightRequest("Foo", cts.Token, int.MaxValue, _ => { }); + + // Act + cts.Cancel(); + + // Assert + // NATSTimeoutException is somewhat unexpected here + await Assert.ThrowsAsync(() => sut.Waiter.Task); + } + + [Fact] + public void Dispose_InvokesOnCompletedDelegate() + { + // Arrange + var onCompletedArg = ""; + var sut = new InFlightRequest("Foo", default, 0, id => { onCompletedArg = id; }); + + // Act + sut.Dispose(); + + // Assert + Assert.Equal("Foo", onCompletedArg); + } + + [Fact] + public void Dispose_DoesNotThrowForNullDelegate() + { + // Arrange + var sut = new InFlightRequest("Foo", default, 0, null); + + // Act + var ex = Record.Exception(() => sut.Dispose()); + + // Assert + Assert.Null(ex); + } + } +} From 3db697955b96f5a57eed48a9bc2e105d485cf5a6 Mon Sep 17 00:00:00 2001 From: jasperd Date: Tue, 28 Jan 2020 21:43:49 +0100 Subject: [PATCH 03/12] Check for null in constructor of InFlightRequest - Remove unnecessary null checks in Dispose --- src/NATS.Client/Internals/InFlightRequest.cs | 7 ++++--- .../UnitTests/Internals/InFlightRequestTests.cs | 14 ++++---------- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/src/NATS.Client/Internals/InFlightRequest.cs b/src/NATS.Client/Internals/InFlightRequest.cs index f1d42cc0d..a06ac32c1 100644 --- a/src/NATS.Client/Internals/InFlightRequest.cs +++ b/src/NATS.Client/Internals/InFlightRequest.cs @@ -39,9 +39,10 @@ internal sealed class InFlightRequest : IDisposable /// internal InFlightRequest(string id, CancellationToken token, int timeout, Action onCompleted) { + _onCompleted = onCompleted ?? throw new ArgumentNullException(nameof(onCompleted)); Id = id; Waiter = new TaskCompletionSource(); - _onCompleted = onCompleted; + _tokenSource = token == CancellationToken.None ? new CancellationTokenSource() : CancellationTokenSource.CreateLinkedTokenSource(token); @@ -65,8 +66,8 @@ internal InFlightRequest(string id, CancellationToken token, int timeout, Action public void Dispose() { _tokenRegistration.Dispose(); - _tokenSource?.Dispose(); - _onCompleted?.Invoke(Id); + _tokenSource.Dispose(); + _onCompleted.Invoke(Id); } } } diff --git a/src/Tests/UnitTests/Internals/InFlightRequestTests.cs b/src/Tests/UnitTests/Internals/InFlightRequestTests.cs index 0b074dbc9..ebcde1bfb 100644 --- a/src/Tests/UnitTests/Internals/InFlightRequestTests.cs +++ b/src/Tests/UnitTests/Internals/InFlightRequestTests.cs @@ -1,4 +1,5 @@ -using System.Threading; +using System; +using System.Threading; using System.Threading.Tasks; using NATS.Client; using NATS.Client.Internals; @@ -73,16 +74,9 @@ public void Dispose_InvokesOnCompletedDelegate() } [Fact] - public void Dispose_DoesNotThrowForNullDelegate() + public void Ctor_ThrowsForNullArg() { - // Arrange - var sut = new InFlightRequest("Foo", default, 0, null); - - // Act - var ex = Record.Exception(() => sut.Dispose()); - - // Assert - Assert.Null(ex); + Assert.Throws("onCompleted", () => new InFlightRequest("Foo", default, 0, null)); } } } From 9bdbcaa54658af951df32e0984c1cd6a14e63c7a Mon Sep 17 00:00:00 2001 From: jasperd Date: Tue, 28 Jan 2020 21:46:13 +0100 Subject: [PATCH 04/12] Move TCS initialization out of ctor --- src/NATS.Client/Internals/InFlightRequest.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/NATS.Client/Internals/InFlightRequest.cs b/src/NATS.Client/Internals/InFlightRequest.cs index a06ac32c1..57bab4079 100644 --- a/src/NATS.Client/Internals/InFlightRequest.cs +++ b/src/NATS.Client/Internals/InFlightRequest.cs @@ -27,8 +27,8 @@ internal sealed class InFlightRequest : IDisposable private readonly CancellationTokenRegistration _tokenRegistration; public string Id { get; } - public TaskCompletionSource Waiter { get; } public CancellationToken Token => _tokenSource.Token; + public TaskCompletionSource Waiter { get; } = new TaskCompletionSource(); /// /// Initializes a new instance of @@ -41,8 +41,7 @@ internal InFlightRequest(string id, CancellationToken token, int timeout, Action { _onCompleted = onCompleted ?? throw new ArgumentNullException(nameof(onCompleted)); Id = id; - Waiter = new TaskCompletionSource(); - + _tokenSource = token == CancellationToken.None ? new CancellationTokenSource() : CancellationTokenSource.CreateLinkedTokenSource(token); From 9be02f0ae4405fe9f8c2647f9d9e219c44fcf90c Mon Sep 17 00:00:00 2001 From: jasperd Date: Tue, 28 Jan 2020 22:07:16 +0100 Subject: [PATCH 05/12] Create CTS only if necessary --- src/NATS.Client/Internals/InFlightRequest.cs | 27 ++++++++++++++------ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/src/NATS.Client/Internals/InFlightRequest.cs b/src/NATS.Client/Internals/InFlightRequest.cs index 57bab4079..21f6c5910 100644 --- a/src/NATS.Client/Internals/InFlightRequest.cs +++ b/src/NATS.Client/Internals/InFlightRequest.cs @@ -27,7 +27,7 @@ internal sealed class InFlightRequest : IDisposable private readonly CancellationTokenRegistration _tokenRegistration; public string Id { get; } - public CancellationToken Token => _tokenSource.Token; + public CancellationToken Token { get; } public TaskCompletionSource Waiter { get; } = new TaskCompletionSource(); /// @@ -42,11 +42,22 @@ internal InFlightRequest(string id, CancellationToken token, int timeout, Action _onCompleted = onCompleted ?? throw new ArgumentNullException(nameof(onCompleted)); Id = id; - _tokenSource = token == CancellationToken.None - ? new CancellationTokenSource() - : CancellationTokenSource.CreateLinkedTokenSource(token); + if (timeout > 0 && token == default) + { + _tokenSource = new CancellationTokenSource(); + Token = _tokenSource.Token; + } + else if (timeout > 0) + { + _tokenSource = CancellationTokenSource.CreateLinkedTokenSource(token); + Token = _tokenSource.Token; + } + else + { + Token = token; + } - _tokenRegistration = _tokenSource.Token.Register(() => + _tokenRegistration = Token.Register(() => { if (timeout > 0) Waiter.TrySetException(new NATSTimeoutException()); @@ -59,13 +70,13 @@ internal InFlightRequest(string id, CancellationToken token, int timeout, Action } /// - /// Releases all resources used by the object - /// and invokes the onCompleted delegate. + /// Releases all resources used by the current instance of the + /// class and invokes the onCompleted delegate. /// public void Dispose() { _tokenRegistration.Dispose(); - _tokenSource.Dispose(); + _tokenSource?.Dispose(); _onCompleted.Invoke(Id); } } From 2a3b06d44ef45ece5ba8cb106f507ac1d8d7cb0f Mon Sep 17 00:00:00 2001 From: jasperd Date: Wed, 29 Jan 2020 02:13:46 +0100 Subject: [PATCH 06/12] Avoid closing over TCS --- src/NATS.Client/Internals/InFlightRequest.cs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/NATS.Client/Internals/InFlightRequest.cs b/src/NATS.Client/Internals/InFlightRequest.cs index 21f6c5910..1f2233ab5 100644 --- a/src/NATS.Client/Internals/InFlightRequest.cs +++ b/src/NATS.Client/Internals/InFlightRequest.cs @@ -57,13 +57,15 @@ internal InFlightRequest(string id, CancellationToken token, int timeout, Action Token = token; } - _tokenRegistration = Token.Register(() => + _tokenRegistration = Token.Register((req) => { + var request = req as InFlightRequest; + if (timeout > 0) - Waiter.TrySetException(new NATSTimeoutException()); + request.Waiter.TrySetException(new NATSTimeoutException()); - Waiter.TrySetCanceled(); - }); + request.Waiter.TrySetCanceled(); + }, this); if(timeout > 0) _tokenSource.CancelAfter(timeout); From a02ee1e24e16a913ad54a596d6ec487f42f39ef7 Mon Sep 17 00:00:00 2001 From: jasperd Date: Wed, 29 Jan 2020 02:23:09 +0100 Subject: [PATCH 07/12] Throw TCE instead of NATSTimeoutException --- src/NATS.Client/Internals/InFlightRequest.cs | 10 ++++++---- src/Tests/UnitTests/Internals/InFlightRequestTests.cs | 3 +-- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/NATS.Client/Internals/InFlightRequest.cs b/src/NATS.Client/Internals/InFlightRequest.cs index 1f2233ab5..46326d42e 100644 --- a/src/NATS.Client/Internals/InFlightRequest.cs +++ b/src/NATS.Client/Internals/InFlightRequest.cs @@ -29,6 +29,7 @@ internal sealed class InFlightRequest : IDisposable public string Id { get; } public CancellationToken Token { get; } public TaskCompletionSource Waiter { get; } = new TaskCompletionSource(); + private readonly CancellationToken _clientProvidedToken; /// /// Initializes a new instance of @@ -40,6 +41,7 @@ internal sealed class InFlightRequest : IDisposable internal InFlightRequest(string id, CancellationToken token, int timeout, Action onCompleted) { _onCompleted = onCompleted ?? throw new ArgumentNullException(nameof(onCompleted)); + _clientProvidedToken = token; Id = id; if (timeout > 0 && token == default) @@ -61,13 +63,13 @@ internal InFlightRequest(string id, CancellationToken token, int timeout, Action { var request = req as InFlightRequest; - if (timeout > 0) - request.Waiter.TrySetException(new NATSTimeoutException()); + if (request._clientProvidedToken.IsCancellationRequested || timeout < 1) + request.Waiter.TrySetCanceled(); - request.Waiter.TrySetCanceled(); + request.Waiter.TrySetException(new NATSTimeoutException()); }, this); - if(timeout > 0) + if (timeout > 0) _tokenSource.CancelAfter(timeout); } diff --git a/src/Tests/UnitTests/Internals/InFlightRequestTests.cs b/src/Tests/UnitTests/Internals/InFlightRequestTests.cs index ebcde1bfb..a1a330905 100644 --- a/src/Tests/UnitTests/Internals/InFlightRequestTests.cs +++ b/src/Tests/UnitTests/Internals/InFlightRequestTests.cs @@ -55,8 +55,7 @@ public async Task CanceledWithTimeout_ThrowsNatsTimeoutException() cts.Cancel(); // Assert - // NATSTimeoutException is somewhat unexpected here - await Assert.ThrowsAsync(() => sut.Waiter.Task); + await Assert.ThrowsAsync(() => sut.Waiter.Task); } [Fact] From ea2d2c4103943d3499c49eafd4c1d48a52405ad3 Mon Sep 17 00:00:00 2001 From: jasperd Date: Wed, 29 Jan 2020 02:32:43 +0100 Subject: [PATCH 08/12] Add license headers --- src/NATS.Client/Internals/InFlightRequest.cs | 2 +- .../UnitTests/Internals/InFlightRequestTests.cs | 15 ++++++++++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/NATS.Client/Internals/InFlightRequest.cs b/src/NATS.Client/Internals/InFlightRequest.cs index 46326d42e..ccfba0192 100644 --- a/src/NATS.Client/Internals/InFlightRequest.cs +++ b/src/NATS.Client/Internals/InFlightRequest.cs @@ -1,4 +1,4 @@ -// Copyright 2015-2018 The NATS Authors +// Copyright 2017-2020 The NATS Authors // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. // You may obtain a copy of the License at diff --git a/src/Tests/UnitTests/Internals/InFlightRequestTests.cs b/src/Tests/UnitTests/Internals/InFlightRequestTests.cs index a1a330905..bcef99bbd 100644 --- a/src/Tests/UnitTests/Internals/InFlightRequestTests.cs +++ b/src/Tests/UnitTests/Internals/InFlightRequestTests.cs @@ -1,4 +1,17 @@ -using System; +// Copyright 2020 The NATS Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +using System; using System.Threading; using System.Threading.Tasks; using NATS.Client; From 086f1035c13c1fea32b7d22d14391d997c39ff8c Mon Sep 17 00:00:00 2001 From: jasperd Date: Wed, 29 Jan 2020 03:03:51 +0100 Subject: [PATCH 09/12] Add InFlightRequest benchmarks --- .../InFlightRequestBenchmark.cs | 56 +++++++++++++++++++ src/Benchmarks/MicroBenchmarks/Program.cs | 2 +- 2 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 src/Benchmarks/MicroBenchmarks/InFlightRequestBenchmark.cs diff --git a/src/Benchmarks/MicroBenchmarks/InFlightRequestBenchmark.cs b/src/Benchmarks/MicroBenchmarks/InFlightRequestBenchmark.cs new file mode 100644 index 000000000..282d4dd13 --- /dev/null +++ b/src/Benchmarks/MicroBenchmarks/InFlightRequestBenchmark.cs @@ -0,0 +1,56 @@ +// Copyright 2020 The NATS Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +using System.Threading; +using BenchmarkDotNet.Attributes; +using BenchmarkDotNet.Configs; +using BenchmarkDotNet.Jobs; +using NATS.Client.Internals; + +namespace MicroBenchmarks +{ + [MemoryDiagnoser] + [MarkdownExporterAttribute.GitHub] + [SimpleJob(RuntimeMoniker.Net462)] + [SimpleJob(RuntimeMoniker.NetCoreApp31)] + [GroupBenchmarksBy(BenchmarkLogicalGroupRule.ByCategory)] + [CategoriesColumn] + public class InFlightRequestBenchmark + { + private static void OnCompleted(string _) { } + + private static CancellationToken _ct = new CancellationTokenSource().Token; + + [Params(0, 999_999)] + public int Timeout { get; set; } + + [BenchmarkCategory("DefaultToken")] + [Benchmark] + public string InFlightRequest() + { + var request = new InFlightRequest("a", default, Timeout, OnCompleted); + var id = request.Id; + request.Dispose(); + return id; + } + [BenchmarkCategory("ClientToken")] + [Benchmark] + public string InFlightRequestClientToken() + { + var request = new InFlightRequest("a", _ct, Timeout, OnCompleted); + var id = request.Id; + request.Dispose(); + return id; + } + } +} diff --git a/src/Benchmarks/MicroBenchmarks/Program.cs b/src/Benchmarks/MicroBenchmarks/Program.cs index 7ff6d2d6c..3eb52cd85 100644 --- a/src/Benchmarks/MicroBenchmarks/Program.cs +++ b/src/Benchmarks/MicroBenchmarks/Program.cs @@ -19,7 +19,7 @@ public class Program { public static void Main(string[] args) { - var summary = BenchmarkRunner.Run(); + BenchmarkSwitcher.FromAssembly(typeof(Program).Assembly).Run(args); } } } From 5e35dafae8c4bfc46a51e28cd6e8a7167df7f5ab Mon Sep 17 00:00:00 2001 From: jasperd Date: Wed, 29 Jan 2020 18:30:05 +0100 Subject: [PATCH 10/12] Adjust test name to reflect changed behaviour --- src/Tests/UnitTests/Internals/InFlightRequestTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tests/UnitTests/Internals/InFlightRequestTests.cs b/src/Tests/UnitTests/Internals/InFlightRequestTests.cs index bcef99bbd..7a9fd98f5 100644 --- a/src/Tests/UnitTests/Internals/InFlightRequestTests.cs +++ b/src/Tests/UnitTests/Internals/InFlightRequestTests.cs @@ -58,7 +58,7 @@ public async Task Canceled_ThrowsTaskCanceledExcpetion() } [Fact] - public async Task CanceledWithTimeout_ThrowsNatsTimeoutException() + public async Task CanceledWithTimeout_ThrowsTaskCanceledException() { // Arrange var cts = new CancellationTokenSource(); From 3c7517e295ba3fdbca0e99998a74fe4d584384c7 Mon Sep 17 00:00:00 2001 From: jasperd Date: Wed, 29 Jan 2020 20:15:26 +0100 Subject: [PATCH 11/12] Don't close over timeout either & add docs - Enforce avoidance of closure by using a static method --- src/NATS.Client/Internals/InFlightRequest.cs | 48 ++++++++++++-------- 1 file changed, 28 insertions(+), 20 deletions(-) diff --git a/src/NATS.Client/Internals/InFlightRequest.cs b/src/NATS.Client/Internals/InFlightRequest.cs index ccfba0192..fe791dd60 100644 --- a/src/NATS.Client/Internals/InFlightRequest.cs +++ b/src/NATS.Client/Internals/InFlightRequest.cs @@ -18,26 +18,32 @@ namespace NATS.Client.Internals { /// - /// Handles in-flight requests when using the default (i.e. not old) request/reply behavior + /// Represents an in-flight request/reply operation. /// + /// + /// This class is not used when using the legacy request/reply + /// pattern (see ). + /// internal sealed class InFlightRequest : IDisposable { private readonly Action _onCompleted; private readonly CancellationTokenSource _tokenSource; private readonly CancellationTokenRegistration _tokenRegistration; - - public string Id { get; } - public CancellationToken Token { get; } - public TaskCompletionSource Waiter { get; } = new TaskCompletionSource(); private readonly CancellationToken _clientProvidedToken; + public readonly string Id; + public readonly CancellationToken Token; + public readonly TaskCompletionSource Waiter = new TaskCompletionSource(); + /// - /// Initializes a new instance of + /// Initializes a new instance of class. /// - /// - /// - /// - /// + /// The id associated with the request. + /// The cancellation token used to cancel the request. + /// A timeout (ms) after which the request is canceled. + /// The delegate that will be executed after the request ended. + /// Thrown if the request is cancelled by before receiving a response. + /// Thrown if the request is cancelled because period has elapsed before receiving a response. internal InFlightRequest(string id, CancellationToken token, int timeout, Action onCompleted) { _onCompleted = onCompleted ?? throw new ArgumentNullException(nameof(onCompleted)); @@ -53,26 +59,28 @@ internal InFlightRequest(string id, CancellationToken token, int timeout, Action { _tokenSource = CancellationTokenSource.CreateLinkedTokenSource(token); Token = _tokenSource.Token; - } + } else { Token = token; } - _tokenRegistration = Token.Register((req) => - { - var request = req as InFlightRequest; - - if (request._clientProvidedToken.IsCancellationRequested || timeout < 1) - request.Waiter.TrySetCanceled(); - - request.Waiter.TrySetException(new NATSTimeoutException()); - }, this); + _tokenRegistration = Token.Register(CancellationCallback, this); if (timeout > 0) _tokenSource.CancelAfter(timeout); } + private static void CancellationCallback(object req) + { + var request = req as InFlightRequest; + + if (request._clientProvidedToken.IsCancellationRequested) + request.Waiter.TrySetCanceled(); + + request.Waiter.TrySetException(new NATSTimeoutException()); + } + /// /// Releases all resources used by the current instance of the /// class and invokes the onCompleted delegate. From e225b977549cc2b9f1128f30c846aa812ee315b9 Mon Sep 17 00:00:00 2001 From: jasperd Date: Wed, 29 Jan 2020 22:24:30 +0100 Subject: [PATCH 12/12] Don't pack MicroBenchmarks --- src/Benchmarks/MicroBenchmarks/MicroBenchmarks.csproj | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Benchmarks/MicroBenchmarks/MicroBenchmarks.csproj b/src/Benchmarks/MicroBenchmarks/MicroBenchmarks.csproj index 7fe690cf8..4276fc4a8 100644 --- a/src/Benchmarks/MicroBenchmarks/MicroBenchmarks.csproj +++ b/src/Benchmarks/MicroBenchmarks/MicroBenchmarks.csproj @@ -7,6 +7,7 @@ AnyCPU pdbonly true + false