From dbc9d8d0e7a98b48c010b571ce07fb6b7b7fbe06 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 3 Dec 2024 15:03:01 +0900 Subject: [PATCH 1/9] Switch to using named pipes for IPC --- osu.Framework/Platform/DesktopGameHost.cs | 4 +- .../Platform/NamedPipeIpcProvider.cs | 213 ++++++++++++++++++ 2 files changed, 215 insertions(+), 2 deletions(-) create mode 100644 osu.Framework/Platform/NamedPipeIpcProvider.cs diff --git a/osu.Framework/Platform/DesktopGameHost.cs b/osu.Framework/Platform/DesktopGameHost.cs index 40346e88e0..828a8ee96e 100644 --- a/osu.Framework/Platform/DesktopGameHost.cs +++ b/osu.Framework/Platform/DesktopGameHost.cs @@ -15,7 +15,7 @@ namespace osu.Framework.Platform { public abstract class DesktopGameHost : SDLGameHost { - private TcpIpcProvider ipcProvider; + private NamedPipeIpcProvider ipcProvider; private readonly int? ipcPort; protected DesktopGameHost(string gameName, HostOptions options = null) @@ -61,7 +61,7 @@ private void ensureIPCReady() if (ipcProvider != null) return; - ipcProvider = new TcpIpcProvider(ipcPort.Value); + ipcProvider = new NamedPipeIpcProvider(ipcPort.Value); ipcProvider.MessageReceived += OnMessageReceived; IsPrimaryInstance = ipcProvider.Bind(); diff --git a/osu.Framework/Platform/NamedPipeIpcProvider.cs b/osu.Framework/Platform/NamedPipeIpcProvider.cs new file mode 100644 index 0000000000..d0631ab8d6 --- /dev/null +++ b/osu.Framework/Platform/NamedPipeIpcProvider.cs @@ -0,0 +1,213 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System; +using System.IO; +using System.IO.Pipes; +using System.Text; +using System.Threading; +using System.Threading.Tasks; +using Newtonsoft.Json; +using Newtonsoft.Json.Linq; +using osu.Framework.Extensions; +using osu.Framework.Logging; + +namespace osu.Framework.Platform +{ + /// + /// An inter-process communication provider that runs over a specified named pipe. + /// This single class handles both binding as a server, or messaging another bound instance that is acting as a server. + /// + public class NamedPipeIpcProvider : IDisposable + { + /// + /// Invoked when a message is received when running as a server. + /// Returns either a response in the form of an , or null for no response. + /// + public event Func? MessageReceived; + + private readonly CancellationTokenSource cancellationSource = new CancellationTokenSource(); + + private readonly int port; + + private Task? listenTask; + + private NamedPipeServerStream? pipe; + + /// + /// Create a new provider. + /// + /// The port to operate on. + public NamedPipeIpcProvider(int port) + { + this.port = port; + } + + /// + /// Attempt to bind to the named pipe as a server, and start listening for incoming connections if successful. + /// + /// + /// Whether the bind was successful. + /// If false, another instance is likely already running (and can be messaged using or ). + /// + public bool Bind() + { + if (pipe != null) + throw new InvalidOperationException($"Can't {nameof(Bind)} more than once."); + + try + { + pipe = new NamedPipeServerStream($"osu-framework-{port}", PipeDirection.InOut); + + listenTask = listen(pipe); + + return true; + } + catch (IOException ex) + { + Logger.Error(ex, "Unable to bind IPC server"); + return false; + } + } + + private async Task listen(NamedPipeServerStream pipe) + { + var token = cancellationSource.Token; + + try + { + while (!token.IsCancellationRequested) + { + await pipe.WaitForConnectionAsync(token); + + try + { + var message = await receive(pipe, token); + + if (message == null) + continue; + + var response = MessageReceived?.Invoke(message); + + if (response != null) + await send(pipe, response).WaitAsync(token); + } + catch (Exception e) + { + Logger.Error(e, "Error handling incoming IPC request."); + } + } + } + catch (TaskCanceledException) + { + } + finally + { + try + { + pipe.Disconnect(); + } + catch + { + } + } + } + + /// + /// Send a message to the IPC server. + /// + /// The message to send. + public async Task SendMessageAsync(IpcMessage message) + { + using (var client = new NamedPipeClientStream($"osu-framework-{port}")) + { + await client.ConnectAsync().ConfigureAwait(false); + await send(client, message).ConfigureAwait(false); + } + } + + /// + /// Send a message to the IPC server. + /// + /// The message to send. + /// The response from the server. + public async Task SendMessageWithResponseAsync(IpcMessage message) + { + using (var client = new NamedPipeClientStream($"osu-framework-{port}")) + { + await client.ConnectAsync().ConfigureAwait(false); + await send(client, message).ConfigureAwait(false); + return await receive(client).ConfigureAwait(false); + } + } + + private static async Task send(Stream stream, IpcMessage message) + { + string str = JsonConvert.SerializeObject(message, Formatting.None); + byte[] data = Encoding.UTF8.GetBytes(str); + byte[] header = BitConverter.GetBytes(data.Length); + + await stream.WriteAsync(header.AsMemory()).ConfigureAwait(false); + await stream.WriteAsync(data.AsMemory()).ConfigureAwait(false); + await stream.FlushAsync().ConfigureAwait(false); + } + + private static async Task receive(Stream stream, CancellationToken cancellationToken = default) + { + const int header_length = sizeof(int); + + byte[] header = new byte[header_length]; + + int read = await stream.ReadAsync(header.AsMemory(), cancellationToken).ConfigureAwait(false); + + if (read < header_length) + return null; + + int contentLength = BitConverter.ToInt32(header, 0); + + if (contentLength == 0) + return null; + + byte[] data = await stream.ReadBytesToArrayAsync(contentLength, cancellationToken).ConfigureAwait(false); + + string str = Encoding.UTF8.GetString(data); + + var json = JToken.Parse(str); + + string? typeName = json["Type"]?.Value(); + + if (typeName == null) throw new InvalidOperationException("Response JSON has missing Type field."); + + var type = Type.GetType(typeName); + var value = json["Value"]; + + if (type == null) throw new InvalidOperationException($"Response type could not be mapped ({typeName})."); + if (value == null) throw new InvalidOperationException("Response JSON has missing Value field."); + + return new IpcMessage + { + Type = type.AssemblyQualifiedName, + Value = JsonConvert.DeserializeObject(value.ToString(), type), + }; + } + + public void Dispose() + { + const int thread_join_timeout = 2000; + + cancellationSource.Cancel(); + + if (listenTask != null) + { + try + { + listenTask.Wait(thread_join_timeout); + } + catch + { + Logger.Log($"IPC thread failed to exit in allocated time ({thread_join_timeout}ms).", LoggingTarget.Runtime, LogLevel.Important); + } + } + } + } +} From 4f28716a7c84ec90aa5614627aa7a51ffed832af Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 3 Dec 2024 15:11:52 +0900 Subject: [PATCH 2/9] Implement and expose the rest of `IIpcHost` flow --- osu.Framework/Platform/DesktopGameHost.cs | 9 ++++++++- osu.Framework/Platform/GameHost.cs | 3 +++ osu.Framework/Platform/IIpcHost.cs | 9 ++++++++- osu.Framework/Platform/IpcChannel.cs | 8 ++++++++ 4 files changed, 27 insertions(+), 2 deletions(-) diff --git a/osu.Framework/Platform/DesktopGameHost.cs b/osu.Framework/Platform/DesktopGameHost.cs index 828a8ee96e..0bde7dee11 100644 --- a/osu.Framework/Platform/DesktopGameHost.cs +++ b/osu.Framework/Platform/DesktopGameHost.cs @@ -97,7 +97,7 @@ public override bool PresentFileExternally(string filename) return true; } - private void openUsingShellExecute(string path) => Process.Start(new ProcessStartInfo + private static void openUsingShellExecute(string path) => Process.Start(new ProcessStartInfo { FileName = path, UseShellExecute = true //see https://github.com/dotnet/corefx/issues/10361 @@ -110,6 +110,13 @@ public override Task SendMessageAsync(IpcMessage message) return ipcProvider.SendMessageAsync(message); } + public override Task SendMessageWithResponseAsync(IpcMessage message) + { + ensureIPCReady(); + + return ipcProvider.SendMessageWithResponseAsync(message); + } + protected override void Dispose(bool isDisposing) { ipcProvider?.Dispose(); diff --git a/osu.Framework/Platform/GameHost.cs b/osu.Framework/Platform/GameHost.cs index 4eab59a416..f91363596d 100644 --- a/osu.Framework/Platform/GameHost.cs +++ b/osu.Framework/Platform/GameHost.cs @@ -144,6 +144,9 @@ public abstract class GameHost : IIpcHost, IDisposable protected IpcMessage OnMessageReceived(IpcMessage message) => MessageReceived?.Invoke(message); public virtual Task SendMessageAsync(IpcMessage message) => throw new NotSupportedException("This platform does not implement IPC."); + public virtual Task SendMessageWithResponseAsync(IpcMessage message) => throw new NotSupportedException("This platform does not implement IPC."); + + public virtual Task SendM(IpcMessage message) => throw new NotSupportedException("This platform does not implement IPC."); /// /// Requests that a file or folder be opened externally with an associated application, if available. diff --git a/osu.Framework/Platform/IIpcHost.cs b/osu.Framework/Platform/IIpcHost.cs index dc27d8e4bc..be2774c8a9 100644 --- a/osu.Framework/Platform/IIpcHost.cs +++ b/osu.Framework/Platform/IIpcHost.cs @@ -15,9 +15,16 @@ public interface IIpcHost event Func MessageReceived; /// - /// Send a message to the IPC server. + /// Asynchronously send a message to the IPC server. /// /// The message to send. Task SendMessageAsync(IpcMessage ipcMessage); + + /// + /// Asynchronously send a message to the IPC server and receive a response. + /// + /// The message to send. + /// The response from the server. + Task SendMessageWithResponseAsync(IpcMessage message); } } diff --git a/osu.Framework/Platform/IpcChannel.cs b/osu.Framework/Platform/IpcChannel.cs index ec28ba1eef..a079c9c9ea 100644 --- a/osu.Framework/Platform/IpcChannel.cs +++ b/osu.Framework/Platform/IpcChannel.cs @@ -5,6 +5,7 @@ using System; using System.Threading.Tasks; +using JetBrains.Annotations; namespace osu.Framework.Platform { @@ -25,6 +26,13 @@ public Task SendMessageAsync(T message) => host.SendMessageAsync(new IpcMessage Value = message, }); + [ItemCanBeNull] + public Task SendMessageWithResponseAsync(T message) => host.SendMessageWithResponseAsync(new IpcMessage + { + Type = typeof(T).AssemblyQualifiedName, + Value = message, + }); + private IpcMessage handleMessage(IpcMessage message) { if (message.Type != typeof(T).AssemblyQualifiedName) From f0e2b92d51bd730bbe1c7bfa7f117587bc26fd73 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 3 Dec 2024 15:17:41 +0900 Subject: [PATCH 3/9] Switch to using pipe name instead of port number --- osu.Framework/HostOptions.cs | 18 ++++++++++++++---- osu.Framework/Platform/DesktopGameHost.cs | 8 ++++---- osu.Framework/Platform/NamedPipeIpcProvider.cs | 14 +++++++------- 3 files changed, 25 insertions(+), 15 deletions(-) diff --git a/osu.Framework/HostOptions.cs b/osu.Framework/HostOptions.cs index c6e7d4dc2f..43042479fc 100644 --- a/osu.Framework/HostOptions.cs +++ b/osu.Framework/HostOptions.cs @@ -1,6 +1,8 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using System; +using System.Globalization; using osu.Framework.Platform; namespace osu.Framework @@ -11,12 +13,20 @@ namespace osu.Framework public class HostOptions { /// - /// The IPC port to bind. This port should be between 1024 and 49151, - /// should be shared by all instances of a given osu!framework app, - /// but be distinct from IPC ports specified by other osu!framework apps. + /// Use instead. + /// + [Obsolete("Use IPCPipeName instead.")] // can be removed 20250603. + public int? IPCPort + { + set => IPCPipeName = value?.ToString(CultureInfo.InvariantCulture); + } + + /// + /// The IPC pipe name to bind. This should be shared by all instances of + /// an osu!framework app that want to perform inter-process communications. /// See for more details on usage. /// - public int? IPCPort { get; set; } + public string? IPCPipeName { get; set; } /// /// Whether this is a portable installation. Will cause all game files to be placed alongside the executable, rather than in the standard data directory. diff --git a/osu.Framework/Platform/DesktopGameHost.cs b/osu.Framework/Platform/DesktopGameHost.cs index 0bde7dee11..b816040e13 100644 --- a/osu.Framework/Platform/DesktopGameHost.cs +++ b/osu.Framework/Platform/DesktopGameHost.cs @@ -16,12 +16,12 @@ namespace osu.Framework.Platform public abstract class DesktopGameHost : SDLGameHost { private NamedPipeIpcProvider ipcProvider; - private readonly int? ipcPort; + private readonly string ipcPipeName; protected DesktopGameHost(string gameName, HostOptions options = null) : base(gameName, options) { - ipcPort = Options.IPCPort; + ipcPipeName = Options.IPCPipeName; IsPortableInstallation = Options.PortableInstallation; } @@ -55,13 +55,13 @@ protected override void SetupForRun() private void ensureIPCReady() { - if (ipcPort == null) + if (ipcPipeName == null) return; if (ipcProvider != null) return; - ipcProvider = new NamedPipeIpcProvider(ipcPort.Value); + ipcProvider = new NamedPipeIpcProvider(ipcPipeName); ipcProvider.MessageReceived += OnMessageReceived; IsPrimaryInstance = ipcProvider.Bind(); diff --git a/osu.Framework/Platform/NamedPipeIpcProvider.cs b/osu.Framework/Platform/NamedPipeIpcProvider.cs index d0631ab8d6..6d270b126a 100644 --- a/osu.Framework/Platform/NamedPipeIpcProvider.cs +++ b/osu.Framework/Platform/NamedPipeIpcProvider.cs @@ -28,7 +28,7 @@ public class NamedPipeIpcProvider : IDisposable private readonly CancellationTokenSource cancellationSource = new CancellationTokenSource(); - private readonly int port; + private readonly string pipeName; private Task? listenTask; @@ -37,10 +37,10 @@ public class NamedPipeIpcProvider : IDisposable /// /// Create a new provider. /// - /// The port to operate on. - public NamedPipeIpcProvider(int port) + /// The port to operate on. + public NamedPipeIpcProvider(string pipeName) { - this.port = port; + this.pipeName = pipeName; } /// @@ -57,7 +57,7 @@ public bool Bind() try { - pipe = new NamedPipeServerStream($"osu-framework-{port}", PipeDirection.InOut); + pipe = new NamedPipeServerStream($"osu-framework-{pipeName}", PipeDirection.InOut); listenTask = listen(pipe); @@ -119,7 +119,7 @@ private async Task listen(NamedPipeServerStream pipe) /// The message to send. public async Task SendMessageAsync(IpcMessage message) { - using (var client = new NamedPipeClientStream($"osu-framework-{port}")) + using (var client = new NamedPipeClientStream($"osu-framework-{pipeName}")) { await client.ConnectAsync().ConfigureAwait(false); await send(client, message).ConfigureAwait(false); @@ -133,7 +133,7 @@ public async Task SendMessageAsync(IpcMessage message) /// The response from the server. public async Task SendMessageWithResponseAsync(IpcMessage message) { - using (var client = new NamedPipeClientStream($"osu-framework-{port}")) + using (var client = new NamedPipeClientStream($"osu-framework-{pipeName}")) { await client.ConnectAsync().ConfigureAwait(false); await send(client, message).ConfigureAwait(false); From 0bafbe7ee0c9262b2f2f5384a2ed31e0f0aef963 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 3 Dec 2024 15:52:36 +0900 Subject: [PATCH 4/9] Fix `IpcMessage` being exposed to the end user when sending a response --- osu.Framework/Platform/IIpcHost.cs | 2 +- osu.Framework/Platform/IpcChannel.cs | 48 +++++++++++++++++++++------- 2 files changed, 37 insertions(+), 13 deletions(-) diff --git a/osu.Framework/Platform/IIpcHost.cs b/osu.Framework/Platform/IIpcHost.cs index be2774c8a9..e50e688d66 100644 --- a/osu.Framework/Platform/IIpcHost.cs +++ b/osu.Framework/Platform/IIpcHost.cs @@ -12,7 +12,7 @@ public interface IIpcHost /// Invoked when a message is received by this IPC server. /// Returns either a response in the form of an , or null for no response. /// - event Func MessageReceived; + event Func? MessageReceived; /// /// Asynchronously send a message to the IPC server. diff --git a/osu.Framework/Platform/IpcChannel.cs b/osu.Framework/Platform/IpcChannel.cs index a079c9c9ea..220e99c5df 100644 --- a/osu.Framework/Platform/IpcChannel.cs +++ b/osu.Framework/Platform/IpcChannel.cs @@ -1,18 +1,23 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - using System; using System.Threading.Tasks; -using JetBrains.Annotations; namespace osu.Framework.Platform { - public class IpcChannel : IDisposable + /// + /// Setup an IPC channel which supports sending a specific pair of well-defined types. + /// + /// + /// + public class IpcChannel : IDisposable + where T : class + where TResponse : class { private readonly IIpcHost host; - public event Func MessageReceived; + + public event Func? MessageReceived; public IpcChannel(IIpcHost host) { @@ -26,19 +31,38 @@ public Task SendMessageAsync(T message) => host.SendMessageAsync(new IpcMessage Value = message, }); - [ItemCanBeNull] - public Task SendMessageWithResponseAsync(T message) => host.SendMessageWithResponseAsync(new IpcMessage + public async Task SendMessageWithResponseAsync(T message) { - Type = typeof(T).AssemblyQualifiedName, - Value = message, - }); + var response = await host.SendMessageWithResponseAsync(new IpcMessage + { + Type = typeof(T).AssemblyQualifiedName, + Value = message, + }); + + if (response == null) + return null; - private IpcMessage handleMessage(IpcMessage message) + if (response.Type != typeof(TResponse).AssemblyQualifiedName) + return null; + + return (TResponse)response.Value; + } + + private IpcMessage? handleMessage(IpcMessage message) { if (message.Type != typeof(T).AssemblyQualifiedName) return null; - return MessageReceived?.Invoke((T)message.Value); + var response = MessageReceived?.Invoke((T)message.Value); + + if (response == null) + return null; + + return new IpcMessage + { + Type = typeof(TResponse).AssemblyQualifiedName, + Value = response + }; } public void Dispose() From b3d873a608511839766d2d48676f40effb9fdeec Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 3 Dec 2024 15:52:49 +0900 Subject: [PATCH 5/9] Add comprehensive test coverage of all IPC usages --- .../Platform/HeadlessGameHostTest.cs | 42 ----- osu.Framework.Tests/Platform/IPCTest.cs | 155 ++++++++++++++++++ 2 files changed, 155 insertions(+), 42 deletions(-) create mode 100644 osu.Framework.Tests/Platform/IPCTest.cs diff --git a/osu.Framework.Tests/Platform/HeadlessGameHostTest.cs b/osu.Framework.Tests/Platform/HeadlessGameHostTest.cs index 6c02209e61..61f95372b4 100644 --- a/osu.Framework.Tests/Platform/HeadlessGameHostTest.cs +++ b/osu.Framework.Tests/Platform/HeadlessGameHostTest.cs @@ -5,7 +5,6 @@ using System; using System.Diagnostics.CodeAnalysis; -using System.Threading; using System.Threading.Tasks; using NUnit.Framework; using osu.Framework.Allocation; @@ -13,7 +12,6 @@ using osu.Framework.Extensions; using osu.Framework.Platform; using osu.Framework.Testing; -using osu.Framework.Tests.IO; namespace osu.Framework.Tests.Platform { @@ -86,46 +84,6 @@ public void TestThreadSafetyResetOnEnteringThread() } } - [Test] - public void TestIpc() - { - using (var server = new BackgroundGameHeadlessGameHost(@"server", new HostOptions { IPCPort = 45356 })) - using (var client = new HeadlessGameHost(@"client", new HostOptions { IPCPort = 45356 })) - { - Assert.IsTrue(server.IsPrimaryInstance, @"Server wasn't able to bind"); - Assert.IsFalse(client.IsPrimaryInstance, @"Client was able to bind when it shouldn't have been able to"); - - var serverChannel = new IpcChannel(server); - var clientChannel = new IpcChannel(client); - - async Task waitAction() - { - using (var received = new SemaphoreSlim(0)) - { - serverChannel.MessageReceived += message => - { - Assert.AreEqual("example", message.Bar); - // ReSharper disable once AccessToDisposedClosure - received.Release(); - return null; - }; - - await clientChannel.SendMessageAsync(new Foobar { Bar = "example" }).ConfigureAwait(false); - - if (!await received.WaitAsync(10000).ConfigureAwait(false)) - throw new TimeoutException("Message was not received in a timely fashion"); - } - } - - Assert.IsTrue(Task.Run(waitAction).Wait(10000), @"Message was not received in a timely fashion"); - } - } - - private class Foobar - { - public string Bar; - } - public class ExceptionDuringSetupGameHost : TestRunHeadlessGameHost { public ExceptionDuringSetupGameHost(string gameName) diff --git a/osu.Framework.Tests/Platform/IPCTest.cs b/osu.Framework.Tests/Platform/IPCTest.cs new file mode 100644 index 0000000000..e122d72968 --- /dev/null +++ b/osu.Framework.Tests/Platform/IPCTest.cs @@ -0,0 +1,155 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +#nullable disable + +using System; +using System.Threading; +using System.Threading.Tasks; +using NUnit.Framework; +using osu.Framework.Platform; +using osu.Framework.Tests.IO; + +namespace osu.Framework.Tests.Platform +{ + [TestFixture] + public partial class IPCTest + { + [Test] + public void TestNoPipeNameSpecifiedCoexist() + { + using (var host1 = new BackgroundGameHeadlessGameHost(@"server", new HostOptions { IPCPipeName = null })) + using (var host2 = new HeadlessGameHost(@"client", new HostOptions { IPCPipeName = null })) + { + Assert.IsTrue(host1.IsPrimaryInstance, @"Host 1 wasn't able to bind"); + Assert.IsTrue(host2.IsPrimaryInstance, @"Host 2 wasn't able to bind"); + } + } + + [Test] + public void TestDifferentPipeNamesCoexist() + { + using (var host1 = new BackgroundGameHeadlessGameHost(@"server", new HostOptions { IPCPipeName = "test-app" })) + using (var host2 = new HeadlessGameHost(@"client", new HostOptions { IPCPipeName = "test-app-2" })) + { + Assert.IsTrue(host1.IsPrimaryInstance, @"Host 1 wasn't able to bind"); + Assert.IsTrue(host2.IsPrimaryInstance, @"Host 2 wasn't able to bind"); + } + } + + [Test] + public void TestOneWay() + { + using (var server = new BackgroundGameHeadlessGameHost(@"server", new HostOptions { IPCPipeName = "test-app" })) + using (var client = new HeadlessGameHost(@"client", new HostOptions { IPCPipeName = "test-app" })) + { + Assert.IsTrue(server.IsPrimaryInstance, @"Server wasn't able to bind"); + Assert.IsFalse(client.IsPrimaryInstance, @"Client was able to bind when it shouldn't have been able to"); + + var serverChannel = new IpcChannel(server); + var clientChannel = new IpcChannel(client); + + async Task waitAction() + { + using (var received = new SemaphoreSlim(0)) + { + serverChannel.MessageReceived += message => + { + Assert.AreEqual("example", message.Bar); + // ReSharper disable once AccessToDisposedClosure + received.Release(); + return null; + }; + + await clientChannel.SendMessageAsync(new Foobar { Bar = "example" }).ConfigureAwait(false); + + if (!await received.WaitAsync(10000).ConfigureAwait(false)) + throw new TimeoutException("Message was not received in a timely fashion"); + } + } + + Assert.IsTrue(Task.Run(waitAction).Wait(10000), @"Message was not received in a timely fashion"); + } + } + + [Test] + public void TestTwoWay() + { + using (var server = new BackgroundGameHeadlessGameHost(@"server", new HostOptions { IPCPipeName = "test-app" })) + using (var client = new HeadlessGameHost(@"client", new HostOptions { IPCPipeName = "test-app" })) + { + Assert.IsTrue(server.IsPrimaryInstance, @"Server wasn't able to bind"); + Assert.IsFalse(client.IsPrimaryInstance, @"Client was able to bind when it shouldn't have been able to"); + + var serverChannel = new IpcChannel(server); + var clientChannel = new IpcChannel(client); + + async Task waitAction() + { + using (var received = new SemaphoreSlim(0)) + { + serverChannel.MessageReceived += message => + { + Assert.AreEqual("example", message.Bar); + // ReSharper disable once AccessToDisposedClosure + received.Release(); + + return new Foobar { Bar = "test response" }; + }; + + var response = await clientChannel.SendMessageWithResponseAsync(new Foobar { Bar = "example" }).ConfigureAwait(false); + + if (!await received.WaitAsync(10000).ConfigureAwait(false)) + throw new TimeoutException("Message was not received in a timely fashion"); + + Assert.That(response?.Bar, Is.EqualTo("test response")); + } + } + + Assert.IsTrue(Task.Run(waitAction).Wait(10000), @"Message was not received in a timely fashion"); + } + } + + [Test] + public void TestIpcLegacyPortSupport() + { +#pragma warning disable CS0612 // Type or member is obsolete + using (var server = new BackgroundGameHeadlessGameHost(@"server", new HostOptions { IPCPort = 45356 })) + using (var client = new HeadlessGameHost(@"client", new HostOptions { IPCPort = 45356 })) +#pragma warning restore CS0612 // Type or member is obsolete + { + Assert.IsTrue(server.IsPrimaryInstance, @"Server wasn't able to bind"); + Assert.IsFalse(client.IsPrimaryInstance, @"Client was able to bind when it shouldn't have been able to"); + + var serverChannel = new IpcChannel(server); + var clientChannel = new IpcChannel(client); + + async Task waitAction() + { + using (var received = new SemaphoreSlim(0)) + { + serverChannel.MessageReceived += message => + { + Assert.AreEqual("example", message.Bar); + // ReSharper disable once AccessToDisposedClosure + received.Release(); + return null; + }; + + await clientChannel.SendMessageAsync(new Foobar { Bar = "example" }).ConfigureAwait(false); + + if (!await received.WaitAsync(10000).ConfigureAwait(false)) + throw new TimeoutException("Message was not received in a timely fashion"); + } + } + + Assert.IsTrue(Task.Run(waitAction).Wait(10000), @"Message was not received in a timely fashion"); + } + } + + private class Foobar + { + public string Bar; + } + } +} From 8db5cd30dc9e821ce309e3ca83ed27782db905c0 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 3 Dec 2024 16:09:07 +0900 Subject: [PATCH 6/9] Fix issues with listening for too long in multiple places --- osu.Framework/Platform/NamedPipeIpcProvider.cs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/osu.Framework/Platform/NamedPipeIpcProvider.cs b/osu.Framework/Platform/NamedPipeIpcProvider.cs index 6d270b126a..9d747647f6 100644 --- a/osu.Framework/Platform/NamedPipeIpcProvider.cs +++ b/osu.Framework/Platform/NamedPipeIpcProvider.cs @@ -78,11 +78,11 @@ private async Task listen(NamedPipeServerStream pipe) { while (!token.IsCancellationRequested) { - await pipe.WaitForConnectionAsync(token); - try { - var message = await receive(pipe, token); + await pipe.WaitForConnectionAsync(token).ConfigureAwait(false); + + var message = await receive(pipe, token).ConfigureAwait(false); if (message == null) continue; @@ -90,7 +90,9 @@ private async Task listen(NamedPipeServerStream pipe) var response = MessageReceived?.Invoke(message); if (response != null) - await send(pipe, response).WaitAsync(token); + await send(pipe, response).ConfigureAwait(false); + + pipe.Disconnect(); } catch (Exception e) { @@ -202,6 +204,7 @@ public void Dispose() try { listenTask.Wait(thread_join_timeout); + pipe?.Dispose(); } catch { From fc4a637cd1879312697a468827997326677ef768 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 3 Dec 2024 16:25:24 +0900 Subject: [PATCH 7/9] Add back helper class for single directional messaging --- osu.Framework.Tests/Platform/IPCTest.cs | 4 ++-- osu.Framework/Platform/IpcChannel.cs | 22 +++++++++++++++++----- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/osu.Framework.Tests/Platform/IPCTest.cs b/osu.Framework.Tests/Platform/IPCTest.cs index e122d72968..23b8da91ff 100644 --- a/osu.Framework.Tests/Platform/IPCTest.cs +++ b/osu.Framework.Tests/Platform/IPCTest.cs @@ -46,8 +46,8 @@ public void TestOneWay() Assert.IsTrue(server.IsPrimaryInstance, @"Server wasn't able to bind"); Assert.IsFalse(client.IsPrimaryInstance, @"Client was able to bind when it shouldn't have been able to"); - var serverChannel = new IpcChannel(server); - var clientChannel = new IpcChannel(client); + var serverChannel = new IpcChannel(server); + var clientChannel = new IpcChannel(client); async Task waitAction() { diff --git a/osu.Framework/Platform/IpcChannel.cs b/osu.Framework/Platform/IpcChannel.cs index 220e99c5df..eff1a32c03 100644 --- a/osu.Framework/Platform/IpcChannel.cs +++ b/osu.Framework/Platform/IpcChannel.cs @@ -7,17 +7,29 @@ namespace osu.Framework.Platform { /// - /// Setup an IPC channel which supports sending a specific pair of well-defined types. + /// Define an IPC channel which supports sending a specific well-defined type. /// - /// - /// + /// The type to send. + public class IpcChannel : IpcChannel where T : class + { + public IpcChannel(IIpcHost host) + : base(host) + { + } + } + + /// + /// Define an IPC channel which supports sending and receiving a specific well-defined type. + /// + /// The type to send. + /// The type to receive. public class IpcChannel : IDisposable where T : class where TResponse : class { private readonly IIpcHost host; - public event Func? MessageReceived; + public event Func? MessageReceived; public IpcChannel(IIpcHost host) { @@ -37,7 +49,7 @@ public Task SendMessageAsync(T message) => host.SendMessageAsync(new IpcMessage { Type = typeof(T).AssemblyQualifiedName, Value = message, - }); + }).ConfigureAwait(false); if (response == null) return null; From caac5aee7c6589781123a90277da955a370bf4e4 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 4 Dec 2024 19:35:43 +0900 Subject: [PATCH 8/9] Fix some inspections --- osu.Framework.Tests/Platform/IPCTest.cs | 4 ++-- osu.Framework.Tests/Platform/UserInputManagerTest.cs | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/osu.Framework.Tests/Platform/IPCTest.cs b/osu.Framework.Tests/Platform/IPCTest.cs index 23b8da91ff..b138765ca9 100644 --- a/osu.Framework.Tests/Platform/IPCTest.cs +++ b/osu.Framework.Tests/Platform/IPCTest.cs @@ -113,10 +113,10 @@ async Task waitAction() [Test] public void TestIpcLegacyPortSupport() { -#pragma warning disable CS0612 // Type or member is obsolete +#pragma warning disable CS0618 // Type or member is obsolete using (var server = new BackgroundGameHeadlessGameHost(@"server", new HostOptions { IPCPort = 45356 })) using (var client = new HeadlessGameHost(@"client", new HostOptions { IPCPort = 45356 })) -#pragma warning restore CS0612 // Type or member is obsolete +#pragma warning restore CS0618 // Type or member is obsolete { Assert.IsTrue(server.IsPrimaryInstance, @"Server wasn't able to bind"); Assert.IsFalse(client.IsPrimaryInstance, @"Client was able to bind when it shouldn't have been able to"); diff --git a/osu.Framework.Tests/Platform/UserInputManagerTest.cs b/osu.Framework.Tests/Platform/UserInputManagerTest.cs index 14931218bc..45f3e2c671 100644 --- a/osu.Framework.Tests/Platform/UserInputManagerTest.cs +++ b/osu.Framework.Tests/Platform/UserInputManagerTest.cs @@ -13,7 +13,7 @@ public partial class UserInputManagerTest [Test] public void IsAliveTest() { - using (var client = new TestHeadlessGameHost(@"client", 45356)) + using (var client = new TestHeadlessGameHost(@"client")) { var testGame = new TestTestGame(); client.Run(testGame); @@ -25,8 +25,8 @@ private class TestHeadlessGameHost : TestRunHeadlessGameHost { public Drawable CurrentRoot => Root; - public TestHeadlessGameHost(string gameName, int? ipcPort) - : base(gameName, new HostOptions { IPCPort = ipcPort }) + public TestHeadlessGameHost(string gameName) + : base(gameName, new HostOptions { IPCPipeName = gameName }) { } } From bbec913c0a2463e8d436ba9521ed5d4f682812a0 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Fri, 6 Dec 2024 18:35:02 +0900 Subject: [PATCH 9/9] Remove unused method --- osu.Framework/Platform/GameHost.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/osu.Framework/Platform/GameHost.cs b/osu.Framework/Platform/GameHost.cs index f91363596d..400283a5ed 100644 --- a/osu.Framework/Platform/GameHost.cs +++ b/osu.Framework/Platform/GameHost.cs @@ -146,8 +146,6 @@ public abstract class GameHost : IIpcHost, IDisposable public virtual Task SendMessageAsync(IpcMessage message) => throw new NotSupportedException("This platform does not implement IPC."); public virtual Task SendMessageWithResponseAsync(IpcMessage message) => throw new NotSupportedException("This platform does not implement IPC."); - public virtual Task SendM(IpcMessage message) => throw new NotSupportedException("This platform does not implement IPC."); - /// /// Requests that a file or folder be opened externally with an associated application, if available. ///