Merge remote-tracking branch 'upstream/v2.0' into fixEventHandler

azure-pipelines/build.yml

@@ -1,10 +1,32 @@
 steps:
-- task: DotNetCoreInstaller@0
+- task: UseDotNet@2
+  displayName: Install .NET Core 1.0.x
+  inputs:
+    packageType: runtime
+    version: 1.0.x
+
+- task: UseDotNet@2
+  displayName: Install .NET Core 2.0.x
+  inputs:
+    packageType: runtime
+    version: 2.0.x
+
+- task: UseDotNet@2
+  displayName: Install .NET Core 2.1.x
+  inputs:
+    packageType: runtime
+    version: 2.1.x
+
+- task: UseDotNet@2
   displayName: Install .NET Core SDK 2.2.104
   inputs:
     packageType: sdk
     version: 2.2.104
 
+- script: dotnet --info
+  workingDirectory: src
+  displayName: Show dotnet SDK info
+
 - script: |
     dotnet tool install --tool-path .. nbgv
     ..\nbgv cloud
@@ -21,9 +43,6 @@ steps:
 - ${{ if eq(variables['system.collectionId'], '011b8bdf-6d56-4f87-be0d-0092136884d9') }}:
   - template: azure-pipeline.microbuild.before.yml
 
-- script: dotnet --info
-  displayName: Show dotnet SDK info
-
   # We need nuget.exe on the PATH for our VSInsertion.ps1 script.
 - task: NuGetToolInstaller@0
   displayName: Install/Pin nuget.exe version

src/Benchmarks/.gitignore

@@ -0,0 +1 @@
+BenchmarkDotNet.Artifacts/

src/Benchmarks/Benchmarks.csproj

@@ -0,0 +1,19 @@
+<Project Sdk="Microsoft.NET.Sdk">
+
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <TargetFrameworks>netcoreapp2.2;net472</TargetFrameworks>
+    <CodeAnalysisRuleSet>..\StreamJsonRpc.Tests\StreamJsonRpc.Tests.ruleset</CodeAnalysisRuleSet>
+    <NoWarn>$(NoWarn);CS1591</NoWarn>
+    <IsPackable>false</IsPackable>
+  </PropertyGroup>
+
+  <ItemGroup>
+    <PackageReference Include="BenchmarkDotNet" version="0.11.4" />
+    <PackageReference Include="BenchmarkDotNet.Diagnostics.Windows" Version="0.11.4" />
+  </ItemGroup>
+
+  <ItemGroup>
+    <ProjectReference Include="..\StreamJsonRpc\StreamJsonRpc.csproj" />
+  </ItemGroup>
+</Project>

src/Benchmarks/InvokeBenchmarks.cs

@@ -0,0 +1,45 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+namespace Benchmarks
+{
+    using System;
+    using System.Threading.Tasks;
+    using BenchmarkDotNet.Attributes;
+    using Nerdbank.Streams;
+    using StreamJsonRpc;
+
+    [MemoryDiagnoser]
+    public class InvokeBenchmarks
+    {
+        private readonly JsonRpc clientRpc;
+        private readonly JsonRpc serverRpc;
+
+        public InvokeBenchmarks()
+        {
+            var duplex = FullDuplexStream.CreatePipePair();
+            this.clientRpc = new JsonRpc(new HeaderDelimitedMessageHandler(duplex.Item1, new JsonMessageFormatter()));
+            this.clientRpc.StartListening();
+
+            this.serverRpc = new JsonRpc(new HeaderDelimitedMessageHandler(duplex.Item2, new JsonMessageFormatter()));
+            this.serverRpc.AddLocalRpcTarget(new Server());
+            this.serverRpc.StartListening();
+        }
+
+        /// <summary>
+        /// Workaround https://github.com/dotnet/BenchmarkDotNet/issues/837.
+        /// </summary>
+        [GlobalSetup]
+        public void Workaround() => this.InvokeAsync_NoArgs();
+
+        [Benchmark]
+        public Task InvokeAsync_NoArgs() => this.clientRpc.InvokeAsync(nameof(Server.NoOp), Array.Empty<object>());
+
+        private class Server
+        {
+            public void NoOp()
+            {
+            }
+        }
+    }
+}

src/Benchmarks/NotifyBenchmarks.cs

@@ -0,0 +1,31 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+namespace Benchmarks
+{
+    using System;
+    using System.IO;
+    using System.Threading.Tasks;
+    using BenchmarkDotNet.Attributes;
+    using StreamJsonRpc;
+
+    [MemoryDiagnoser]
+    public class NotifyBenchmarks
+    {
+        private readonly JsonRpc clientRpc;
+
+        public NotifyBenchmarks()
+        {
+            this.clientRpc = new JsonRpc(Stream.Null);
+        }
+
+        /// <summary>
+        /// Workaround https://github.com/dotnet/BenchmarkDotNet/issues/837.
+        /// </summary>
+        [GlobalSetup]
+        public void Workaround() => this.NotifyAsync_NoArgs();
+
+        [Benchmark]
+        public Task NotifyAsync_NoArgs() => this.clientRpc.NotifyAsync("NoOp", Array.Empty<object>());
+    }
+}

src/Benchmarks/Program.cs

@@ -0,0 +1,33 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+namespace Benchmarks
+{
+    using System;
+    using System.Threading.Tasks;
+    using BenchmarkDotNet.Running;
+
+    internal static class Program
+    {
+        private static async Task Main(string[] args)
+        {
+            // Allow a special "manual" argument for convenient perfview.exe-monitored runs for GC pressure analysis.
+            if (args.Length == 1 && args[0] == "manual")
+            {
+                var b = new InvokeBenchmarks();
+                await b.InvokeAsync_NoArgs();
+
+                await Task.Delay(2000);
+
+                for (int i = 0; i < 1000; i++)
+                {
+                    await b.InvokeAsync_NoArgs();
+                }
+            }
+            else
+            {
+                var summaries = BenchmarkSwitcher.FromAssembly(typeof(Program).Assembly).Run(args);
+            }
+        }
+    }
+}

src/Benchmarks/run.cmd

@@ -0,0 +1,3 @@
+@pushd "%~dp0\"
+dotnet run -f netcoreapp2.2 -c release -- --runtimes net472 netcoreapp2.2 %*
+@popd

src/Directory.Build.props

@@ -11,7 +11,6 @@
 
     <LangVersion>7.3</LangVersion>
     <GenerateDocumentationFile>true</GenerateDocumentationFile>
-    <LangVersion>7.3</LangVersion>
 
     <PackageRequireLicenseAcceptance>false</PackageRequireLicenseAcceptance>
     <Authors>Microsoft</Authors>

src/StreamJsonRpc.Tests/JsonMessageFormatterTests.cs

@@ -59,24 +59,33 @@ public void ProtocolVersion_RejectsOtherVersions()
     [Fact]
     public void EncodingProperty_UsedToFormat()
     {
-        JsonRpcRequest msg = new JsonRpcRequest { Method = "a" };
-        var builder = new Sequence<byte>();
-        var formatter = new JsonMessageFormatter();
+        var msg = new JsonRpcRequest { Method = "a" };
 
-        formatter.Encoding = Encoding.ASCII;
-        formatter.Serialize(builder, msg);
-        long asciiLength = builder.AsReadOnlySequence.Length;
-        var readMsg = (JsonRpcRequest)formatter.Deserialize(builder.AsReadOnlySequence);
-        Assert.Equal(msg.Method, readMsg.Method);
+        var formatter = new JsonMessageFormatter(Encoding.ASCII);
+        long asciiLength = MeasureLength(msg, formatter);
 
-        builder.Reset();
         formatter.Encoding = Encoding.UTF32;
-        formatter.Serialize(builder, msg);
-        long utf32Length = builder.AsReadOnlySequence.Length;
-        readMsg = (JsonRpcRequest)formatter.Deserialize(builder.AsReadOnlySequence);
-        Assert.Equal(msg.Method, readMsg.Method);
+        var utf32Length = MeasureLength(msg, formatter);
+        Assert.Equal(asciiLength * 4, utf32Length - Encoding.UTF32.GetPreamble().Length);
+    }
+
+    [Fact]
+    public void EncodingPreambleWrittenOnlyOncePerMessage()
+    {
+        // Contrive a very long message, designed to exceed any buffer that would be used internally by the formatter.
+        // The goal here is to result in multiple write operations in order to coerce a second preamble to be written if there were a bug.
+        var msg = new JsonRpcRequest { Method = new string('a', 16 * 1024) };
 
-        Assert.Equal(utf32Length - Encoding.UTF32.GetPreamble().Length, asciiLength * 4);
+        var formatter = new JsonMessageFormatter(Encoding.ASCII);
+        long asciiLength = MeasureLength(msg, formatter);
+
+        formatter.Encoding = Encoding.UTF32;
+        var utf32Length = MeasureLength(msg, formatter);
+        Assert.Equal(asciiLength * 4, utf32Length - Encoding.UTF32.GetPreamble().Length);
+
+        // Measure UTF32 again to verify the length doesn't change (and the preamble is thus applied to each message).
+        var utf32Length2 = MeasureLength(msg, formatter);
+        Assert.Equal(utf32Length, utf32Length2);
     }
 
     [Fact]
@@ -102,4 +111,15 @@ public void JTokenParserHonorsSettingsOnSerializer()
         Assert.IsType<string>(value);
         Assert.Equal("2019-01-29T03:37:28.4433841Z", value);
     }
+
+    private static long MeasureLength(JsonRpcRequest msg, JsonMessageFormatter formatter)
+    {
+        var builder = new Sequence<byte>();
+        formatter.Serialize(builder, msg);
+        var length = builder.AsReadOnlySequence.Length;
+        var readMsg = (JsonRpcRequest)formatter.Deserialize(builder.AsReadOnlySequence);
+        Assert.Equal(msg.Method, readMsg.Method);
+
+        return length;
+    }
 }

src/StreamJsonRpc.Tests/JsonRpcProxyGenerationTests.cs

@@ -39,6 +39,10 @@ public interface IServer
 
         event EventHandler<CustomEventArgs> TreeGrown;
 
+        event EventHandler<CustomNonDerivingEventArgs> AppleGrown;
+
+        event EventHandler<bool> BoolEvent;
+
         Task<string> SayHiAsync();
 
         Task<string> SayHiAsync(string name);
@@ -370,6 +374,44 @@ public async Task GenericEventRaisedOnClient()
         Assert.False(tcs.Task.IsCompleted);
     }
 
+    [Fact]
+    public async Task GenericEventWithoutEventArgsBaseTypeRaisedOnClient()
+    {
+        var tcs = new TaskCompletionSource<CustomNonDerivingEventArgs>();
+        EventHandler<CustomNonDerivingEventArgs> handler = (sender, args) => tcs.SetResult(args);
+        this.clientRpc.AppleGrown += handler;
+        var expectedArgs = new CustomNonDerivingEventArgs { Color = "Red" };
+        this.server.OnAppleGrown(expectedArgs);
+        var actualArgs = await tcs.Task.WithCancellation(this.TimeoutToken);
+        Assert.Equal(expectedArgs.Color, actualArgs.Color);
+
+        // Now unregister and confirm we don't get notified.
+        this.clientRpc.AppleGrown -= handler;
+        tcs = new TaskCompletionSource<CustomNonDerivingEventArgs>();
+        this.server.OnAppleGrown(expectedArgs);
+        await Assert.ThrowsAsync<TimeoutException>(() => tcs.Task.WithTimeout(ExpectedTimeout));
+        Assert.False(tcs.Task.IsCompleted);
+    }
+
+    [Fact]
+    public async Task GenericEventWithBoolArgRaisedOnClient()
+    {
+        var tcs = new TaskCompletionSource<bool>();
+        EventHandler<bool> handler = (sender, args) => tcs.SetResult(args);
+        this.clientRpc.BoolEvent += handler;
+        var expectedArgs = true;
+        this.server.OnBoolEvent(expectedArgs);
+        var actualArgs = await tcs.Task.WithCancellation(this.TimeoutToken);
+        Assert.Equal(expectedArgs, actualArgs);
+
+        // Now unregister and confirm we don't get notified.
+        this.clientRpc.BoolEvent -= handler;
+        tcs = new TaskCompletionSource<bool>();
+        this.server.OnBoolEvent(expectedArgs);
+        await Assert.ThrowsAsync<TimeoutException>(() => tcs.Task.WithTimeout(ExpectedTimeout));
+        Assert.False(tcs.Task.IsCompleted);
+    }
+
     [Fact]
     public async Task NonGenericEventRaisedOnClient()
     {
@@ -504,12 +546,26 @@ public class CustomEventArgs : EventArgs
         public int Seeds { get; set; }
     }
 
+    /// <summary>
+    /// This class serves as a type argument to <see cref="EventHandler{TEventArgs}"/>
+    /// but intentionally does *not* derive from <see cref="EventArgs"/>
+    /// since that is no longer a requirement as of .NET 4.5.
+    /// </summary>
+    public class CustomNonDerivingEventArgs
+    {
+        public string Color { get; set; }
+    }
+
     internal class Server : IServerDerived, IServer2, IServer3
     {
         public event EventHandler ItHappened;
 
         public event EventHandler<CustomEventArgs> TreeGrown;
 
+        public event EventHandler<CustomNonDerivingEventArgs> AppleGrown;
+
+        public event EventHandler<bool> BoolEvent;
+
         public AsyncManualResetEvent MethodEntered { get; } = new AsyncManualResetEvent();
 
         public AsyncManualResetEvent ResumeMethod { get; } = new AsyncManualResetEvent(initialState: true);
@@ -569,5 +625,9 @@ public async Task<int> SumOfParameterObject(Newtonsoft.Json.Linq.JToken paramObj
         internal void OnItHappened(EventArgs args) => this.ItHappened?.Invoke(this, args);
 
         internal void OnTreeGrown(CustomEventArgs args) => this.TreeGrown?.Invoke(this, args);
+
+        internal void OnAppleGrown(CustomNonDerivingEventArgs args) => this.AppleGrown?.Invoke(this, args);
+
+        internal void OnBoolEvent(bool args) => this.BoolEvent?.Invoke(this, args);
     }
 }

src/StreamJsonRpc.sln

@@ -1,7 +1,7 @@
 
 Microsoft Visual Studio Solution File, Format Version 12.00
-# Visual Studio 15
-VisualStudioVersion = 15.0.26620.0
+# Visual Studio Version 16
+VisualStudioVersion = 16.0.28916.44
 MinimumVisualStudioVersion = 10.0.40219.1
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "StreamJsonRpc", "StreamJsonRpc\StreamJsonRpc.csproj", "{DFBD1BCA-EAE0-4454-9E97-FA9BD9A0F03A}"
 EndProject
@@ -19,6 +19,8 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution
 		version.json = version.json
 	EndProjectSection
 EndProject
+Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Benchmarks", "Benchmarks\Benchmarks.csproj", "{CEF0F77F-19EB-4C76-A050-854984BB0364}"
+EndProject
 Global
 	GlobalSection(SolutionConfigurationPlatforms) = preSolution
 		Debug|Any CPU = Debug|Any CPU
@@ -33,6 +35,10 @@ Global
 		{8BF355B2-E3B0-4615-BFC1-7563EADC4F8B}.Debug|Any CPU.Build.0 = Debug|Any CPU
 		{8BF355B2-E3B0-4615-BFC1-7563EADC4F8B}.Release|Any CPU.ActiveCfg = Release|Any CPU
 		{8BF355B2-E3B0-4615-BFC1-7563EADC4F8B}.Release|Any CPU.Build.0 = Release|Any CPU
+		{CEF0F77F-19EB-4C76-A050-854984BB0364}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{CEF0F77F-19EB-4C76-A050-854984BB0364}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{CEF0F77F-19EB-4C76-A050-854984BB0364}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{CEF0F77F-19EB-4C76-A050-854984BB0364}.Release|Any CPU.Build.0 = Release|Any CPU
 	EndGlobalSection
 	GlobalSection(SolutionProperties) = preSolution
 		HideSolutionNode = FALSE