From b478c96debb780fe49658af8236afb0e3bf0a67c Mon Sep 17 00:00:00 2001 From: DJGosnell Date: Wed, 13 Sep 2023 18:23:45 -0400 Subject: [PATCH 01/15] Added simple benchmark. Started work on duplex pipe concurrent usage tests. Removed DoNotPassFlushCancellationToken configuration for alternate cancellation method. --- src/NexNet.IntegrationTests/BaseTests.cs | 1 - src/NexNet.IntegrationTests/ConsoleLogger.cs | 9 ++- .../Pipes/NexusClientTests_NexusDuplexPipe.cs | 7 +- src/NexNet.Quic/QuicTransport.cs | 5 +- src/NexNet.sln | 6 ++ src/NexNet/Cache/CachedDuplexPipe.cs | 6 +- src/NexNet/Cache/CachedRentedDuplexPipe.cs | 6 +- src/NexNet/Internals/NexusSession.Sending.cs | 71 ++++++++++++---- src/NexNet/Internals/NexusSession.cs | 11 +-- src/NexNet/Pipes/NexusDuplexPipe.cs | 71 +++++++++++----- src/NexNet/Pipes/NexusPipeManager.cs | 10 +-- src/NexNet/Pipes/RentedNexusDuplexPipe.cs | 9 ++- src/NexNet/Transports/TcpTlsTransport.cs | 6 +- .../Transports/TransportConfiguration.cs | 10 +-- src/NexNetBenchmarks/BenchmarkConfig.cs | 56 +++++++++++++ src/NexNetBenchmarks/InvocationBenchmarks.cs | 81 +++++++++++++++++++ src/NexNetBenchmarks/NexNetBenchmarks.csproj | 25 ++++++ src/NexNetBenchmarks/Nexuses.cs | 72 +++++++++++++++++ src/NexNetBenchmarks/Program.cs | 20 +++++ 19 files changed, 399 insertions(+), 83 deletions(-) create mode 100644 src/NexNetBenchmarks/BenchmarkConfig.cs create mode 100644 src/NexNetBenchmarks/InvocationBenchmarks.cs create mode 100644 src/NexNetBenchmarks/NexNetBenchmarks.csproj create mode 100644 src/NexNetBenchmarks/Nexuses.cs create mode 100644 src/NexNetBenchmarks/Program.cs diff --git a/src/NexNet.IntegrationTests/BaseTests.cs b/src/NexNet.IntegrationTests/BaseTests.cs index 9c22af65..a9b866f1 100644 --- a/src/NexNet.IntegrationTests/BaseTests.cs +++ b/src/NexNet.IntegrationTests/BaseTests.cs @@ -38,7 +38,6 @@ public enum Type [OneTimeSetUp] public void OneTimeSetUp() { - //_logger = new ConsoleLogger(); Trace.Listeners.Add(new ConsoleTraceListener()); _socketDirectory = Directory.CreateTempSubdirectory("socketTests"); diff --git a/src/NexNet.IntegrationTests/ConsoleLogger.cs b/src/NexNet.IntegrationTests/ConsoleLogger.cs index 389df6b3..82e1b14a 100644 --- a/src/NexNet.IntegrationTests/ConsoleLogger.cs +++ b/src/NexNet.IntegrationTests/ConsoleLogger.cs @@ -15,6 +15,8 @@ public class ConsoleLogger : INexusLogger private readonly ConsoleLogger _baseLogger; + public INexusLogger.LogLevel MinLogLevel { get; set; } = INexusLogger.LogLevel.Trace; + public ConsoleLogger() { _baseLogger = this; @@ -37,16 +39,19 @@ public void Log(INexusLogger.LogLevel logLevel, string? category, Exception? exc if (!_baseLogger.LogEnabled) return; + if (logLevel < MinLogLevel) + return; + _outWriter.WriteLine($"[{_sw.ElapsedTicks/(double)Stopwatch.Frequency:0.000000}]{_prefix} [{category}]: {message} {exception}"); } public INexusLogger CreateLogger(string? category) { - return new ConsoleLogger(_baseLogger, category, _prefix); + return new ConsoleLogger(_baseLogger, category, _prefix) { MinLogLevel = MinLogLevel }; } public INexusLogger CreateLogger(string? category, string prefix) { - return new ConsoleLogger(_baseLogger, category, prefix); + return new ConsoleLogger(_baseLogger, category, prefix) { MinLogLevel = MinLogLevel }; } } diff --git a/src/NexNet.IntegrationTests/Pipes/NexusClientTests_NexusDuplexPipe.cs b/src/NexNet.IntegrationTests/Pipes/NexusClientTests_NexusDuplexPipe.cs index feda095c..42dfa007 100644 --- a/src/NexNet.IntegrationTests/Pipes/NexusClientTests_NexusDuplexPipe.cs +++ b/src/NexNet.IntegrationTests/Pipes/NexusClientTests_NexusDuplexPipe.cs @@ -1,6 +1,7 @@ using System.Buffers; using System.IO.Pipelines; using NUnit.Framework; +using NUnit.Framework.Internal; namespace NexNet.IntegrationTests.Pipes; @@ -13,11 +14,11 @@ internal class NexusClientTests_NexusDuplexPipe : BasePipeTests public async Task Client_PipeReaderReceivesDataMultipleTimes(Type type) { //BlockForClose = true; - var (_, sNexus, _, cNexus, tcs) = await Setup(type); + var (_, sNexus, _, cNexus, tcs) = await Setup(type, true); int count = 0; // TODO: Review adding a test for increased iterations as this has been found to sometimes fail on CI. - const int iterations = 10; + const int iterations = 1000; cNexus.ClientTaskValueWithDuplexPipeEvent = async (nexus, pipe) => { var result = await pipe.Input.ReadAsync().Timeout(1); @@ -40,7 +41,7 @@ public async Task Client_PipeReaderReceivesDataMultipleTimes(Type type) await pipe.Output.WriteAsync(Data).Timeout(1); } - await tcs.Task.Timeout(1); + await tcs.Task.Timeout(100); } [TestCase(Type.Uds)] diff --git a/src/NexNet.Quic/QuicTransport.cs b/src/NexNet.Quic/QuicTransport.cs index b45946bf..de409e6a 100644 --- a/src/NexNet.Quic/QuicTransport.cs +++ b/src/NexNet.Quic/QuicTransport.cs @@ -29,10 +29,7 @@ private QuicTransport(QuicConnection quicConnection, QuicStream stream) Output = PipeWriter.Create(stream); } - public TransportConfiguration Configurations => new TransportConfiguration() - { - DoNotPassFlushCancellationToken = true - }; + public TransportConfiguration Configurations => new TransportConfiguration(); public ValueTask CloseAsync(bool linger) { diff --git a/src/NexNet.sln b/src/NexNet.sln index 19529815..dfe0d575 100644 --- a/src/NexNet.sln +++ b/src/NexNet.sln @@ -25,6 +25,8 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution NexNet.props = NexNet.props EndProjectSection EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "NexNetBenchmarks", "NexNetBenchmarks\NexNetBenchmarks.csproj", "{601A50B4-E03D-423F-A1FF-99805719C409}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -55,6 +57,10 @@ Global {AB75B6D0-0DC0-4E3A-A3A7-320DFB6B09C5}.Debug|Any CPU.Build.0 = Debug|Any CPU {AB75B6D0-0DC0-4E3A-A3A7-320DFB6B09C5}.Release|Any CPU.ActiveCfg = Release|Any CPU {AB75B6D0-0DC0-4E3A-A3A7-320DFB6B09C5}.Release|Any CPU.Build.0 = Release|Any CPU + {601A50B4-E03D-423F-A1FF-99805719C409}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {601A50B4-E03D-423F-A1FF-99805719C409}.Debug|Any CPU.Build.0 = Debug|Any CPU + {601A50B4-E03D-423F-A1FF-99805719C409}.Release|Any CPU.ActiveCfg = Release|Any CPU + {601A50B4-E03D-423F-A1FF-99805719C409}.Release|Any CPU.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE diff --git a/src/NexNet/Cache/CachedDuplexPipe.cs b/src/NexNet/Cache/CachedDuplexPipe.cs index a76ab38d..3e2fa056 100644 --- a/src/NexNet/Cache/CachedDuplexPipe.cs +++ b/src/NexNet/Cache/CachedDuplexPipe.cs @@ -13,7 +13,7 @@ public NexusDuplexPipe Rent(INexusSession session, byte initialId) if (!_cache.TryTake(out var cachedPipe)) cachedPipe = new NexusDuplexPipe(); - cachedPipe.IsInCached = false; + cachedPipe.IsInCache = false; cachedPipe.Setup(initialId, session); return cachedPipe; @@ -21,10 +21,10 @@ public NexusDuplexPipe Rent(INexusSession session, byte initialId) public void Return(NexusDuplexPipe pipe) { - if(pipe.IsInCached) + if(pipe.IsInCache) return; - pipe.IsInCached = true; + pipe.IsInCache = true; pipe.Reset(); _cache.Add(pipe); } diff --git a/src/NexNet/Cache/CachedRentedDuplexPipe.cs b/src/NexNet/Cache/CachedRentedDuplexPipe.cs index a4e266b4..0ab0082b 100644 --- a/src/NexNet/Cache/CachedRentedDuplexPipe.cs +++ b/src/NexNet/Cache/CachedRentedDuplexPipe.cs @@ -13,7 +13,7 @@ public RentedNexusDuplexPipe Rent(INexusSession session, byte initialId) if (!_cache.TryTake(out var cachedPipe)) cachedPipe = new RentedNexusDuplexPipe(); - cachedPipe.IsInCached = false; + cachedPipe.IsInCache = false; cachedPipe.Setup(initialId, session); return cachedPipe; @@ -21,10 +21,10 @@ public RentedNexusDuplexPipe Rent(INexusSession session, byte initialId) public void Return(RentedNexusDuplexPipe pipe) { - if(pipe.IsInCached) + if(pipe.IsInCache) return; - pipe.IsInCached = true; + pipe.IsInCache = true; pipe.Reset(); _cache.Add(pipe); } diff --git a/src/NexNet/Internals/NexusSession.Sending.cs b/src/NexNet/Internals/NexusSession.Sending.cs index 73894eba..6ce7f956 100644 --- a/src/NexNet/Internals/NexusSession.Sending.cs +++ b/src/NexNet/Internals/NexusSession.Sending.cs @@ -4,6 +4,7 @@ using System.Threading.Tasks; using System.Threading; using System; +using System.Runtime.CompilerServices; using MemoryPack; using Pipelines.Sockets.Unofficial.Arenas; @@ -40,6 +41,19 @@ public async ValueTask SendMessage(TMessage body, CancellationToken ca using var mutexResult = await _writeMutex.TryWaitAsync(cancellationToken).ConfigureAwait(false); + if (mutexResult.Success != true) + throw new InvalidOperationException("Could not acquire write lock"); + + // Register a cancellation token to cancel the flush operation. + CancellationTokenRegistration? ctRegistration = null; + if (cancellationToken.CanBeCanceled) + { + ctRegistration = cancellationToken.Register(static obj => + { + Unsafe.As(obj)?.CancelPendingFlush(); + }, _pipeOutput); + } + var header = _bufferWriter.GetMemory(3); _bufferWriter.Advance(3); MemoryPackSerializer.Serialize(_bufferWriter, body); @@ -66,13 +80,16 @@ public async ValueTask SendMessage(TMessage body, CancellationToken ca try { // ReSharper disable once MethodSupportsCancellation - result = _configDoNotPassFlushCancellationToken - ? await _pipeOutput.FlushAsync().ConfigureAwait(false) - : await _pipeOutput.FlushAsync(cancellationToken).ConfigureAwait(false); + result = await _pipeOutput.FlushAsync().ConfigureAwait(false); } catch (ObjectDisposedException) { - + // noop + } + finally + { + if (ctRegistration != null) + await ctRegistration.Value.DisposeAsync().ConfigureAwait(false); } OnSent?.Invoke(); @@ -116,6 +133,17 @@ public async ValueTask SendHeaderWithBody(MessageType type, ReadOnlyMemory if (mutexResult.Success != true) throw new InvalidOperationException("Could not acquire write lock"); + + // Register a cancellation token to cancel the flush operation. + CancellationTokenRegistration? ctRegistration = null; + if (cancellationToken.CanBeCanceled) + { + ctRegistration = cancellationToken.Register(static obj => + { + Unsafe.As(obj)?.CancelPendingFlush(); + }, _pipeOutput); + } + var length = (int)body.Length; var contentLength = checked((ushort)(length)); @@ -146,13 +174,16 @@ public async ValueTask SendHeaderWithBody(MessageType type, ReadOnlyMemory try { // ReSharper disable once MethodSupportsCancellation - result = _configDoNotPassFlushCancellationToken - ? await _pipeOutput.FlushAsync().ConfigureAwait(false) - : await _pipeOutput.FlushAsync(cancellationToken).ConfigureAwait(false); + result = await _pipeOutput.FlushAsync().ConfigureAwait(false); } catch (ObjectDisposedException) { - + // noop + } + finally + { + if (ctRegistration != null) + await ctRegistration.Value.DisposeAsync().ConfigureAwait(false); } OnSent?.Invoke(); @@ -196,19 +227,28 @@ public ValueTask SendHeader(MessageType type, CancellationToken cancellationToke /// Thrown when the write lock cannot be acquired. private async ValueTask SendHeaderCore(MessageType type, bool force, CancellationToken cancellationToken = default) { - if (_pipeOutput == null || cancellationToken.IsCancellationRequested) return; //if (State != ConnectionState.Connected && State != ConnectionState.Disconnecting) if (State != ConnectionState.Connected && !force) return; - + using var mutexResult = await _writeMutex.TryWaitAsync(cancellationToken).ConfigureAwait(false); if (mutexResult.Success != true) throw new InvalidOperationException("Could not acquire write lock"); + // Register a cancellation token to cancel the flush operation. + CancellationTokenRegistration? ctRegistration = null; + if (cancellationToken.CanBeCanceled) + { + ctRegistration = cancellationToken.Register(static obj => + { + Unsafe.As(obj)?.CancelPendingFlush(); + }, _pipeOutput); + } + _config.InternalOnSend?.Invoke(this, new[] { (byte)type }); _pipeOutput.GetSpan(1)[0] = (byte)type; @@ -220,13 +260,16 @@ private async ValueTask SendHeaderCore(MessageType type, bool force, Cancellatio try { // ReSharper disable once MethodSupportsCancellation - result = _configDoNotPassFlushCancellationToken - ? await _pipeOutput.FlushAsync().ConfigureAwait(false) - : await _pipeOutput.FlushAsync(cancellationToken).ConfigureAwait(false); + result = await _pipeOutput.FlushAsync().ConfigureAwait(false); } catch (ObjectDisposedException) { - + // noop + } + finally + { + if (ctRegistration != null) + await ctRegistration.Value.DisposeAsync().ConfigureAwait(false); } OnSent?.Invoke(); diff --git a/src/NexNet/Internals/NexusSession.cs b/src/NexNet/Internals/NexusSession.cs index 5a3e2276..ee734ebf 100644 --- a/src/NexNet/Internals/NexusSession.cs +++ b/src/NexNet/Internals/NexusSession.cs @@ -50,14 +50,6 @@ private record struct ProcessResult( private readonly TaskCompletionSource? _disconnectedTaskCompletionSource; private volatile int _state; - /// - /// If true, the transport will not pass the flush cancellation token to the underlying transport. - /// Currently exists due to an issue in the QUIC implementation. Should be removed once the issue is fixed. - /// https://github.com/dotnet/runtime/issues/82704 - /// https://github.com/dotnet/runtime/pull/90253 - /// - private readonly bool _configDoNotPassFlushCancellationToken; - public NexusPipeManager PipeManager { get; } public long Id { get; } @@ -100,7 +92,6 @@ public NexusSession(in NexusSessionConfigurations configurations _pipeInput = configurations.Transport.Input; _pipeOutput = configurations.Transport.Output; _transportConnection = configurations.Transport; - _configDoNotPassFlushCancellationToken = _transportConnection.Configurations.DoNotPassFlushCancellationToken; Config = _config = configurations.Configs; _readyTaskCompletionSource = configurations.ReadyTaskCompletionSource; _disconnectedTaskCompletionSource = configurations.DisconnectedTaskCompletionSource; @@ -249,7 +240,7 @@ private async ValueTask DisconnectCore(DisconnectReason reason, bool sendDisconn _registeredDisconnectReason = reason; - Logger?.LogInfo($"Session disconnected with reason: {reason}"); + Logger?.LogError($"Session disconnected with reason: {reason}"); if (sendDisconnect && !_config.InternalForceDisableSendingDisconnectSignal) { diff --git a/src/NexNet/Pipes/NexusDuplexPipe.cs b/src/NexNet/Pipes/NexusDuplexPipe.cs index c1d181d6..5c88e512 100644 --- a/src/NexNet/Pipes/NexusDuplexPipe.cs +++ b/src/NexNet/Pipes/NexusDuplexPipe.cs @@ -1,11 +1,13 @@ using System; using System.Buffers; +using System.Diagnostics; using System.IO.Pipelines; using System.Runtime.CompilerServices; using System.Threading; using System.Threading.Tasks; using NexNet.Internals; using NexNet.Messages; +using static NexNet.Pipes.NexusDuplexPipe; namespace NexNet.Pipes; @@ -21,7 +23,7 @@ internal class NexusDuplexPipe : INexusDuplexPipe, IPipeStateManager internal enum State : byte { /// - /// Unset state. + /// Unset state. This is the default state and indicates that the pipe is ready for setup /// /// Unset = 0, @@ -65,7 +67,16 @@ internal enum State : byte private TaskCompletionSource? _readyTcs; - private State _currentState = State.Unset; + private volatile State _currentState = State.Unset; + /*private State _currentState + { + get => __currentState; + set + { + Logger?.LogError($"{Id} Set new State from {__currentState} to {value}; From {new StackTrace().GetFrame(1).GetMethod().Name}"); + __currentState = value; + } + }*/ private INexusSession? _session; @@ -113,12 +124,12 @@ internal enum State : byte /// /// Logger. /// - private INexusLogger? _logger; + protected INexusLogger? Logger; /// /// True if the pipe is in the cache. /// - internal bool IsInCached = false; + internal bool IsInCache = false; internal NexusDuplexPipe() { @@ -128,7 +139,9 @@ internal NexusDuplexPipe() public ValueTask CompleteAsync() { - if (_currentState == State.Complete) + if (_currentState == State.Complete + || _currentState == State.Unset + || IsInCache) return default; var stateChanged = UpdateState(State.Complete); @@ -149,20 +162,20 @@ public ValueTask CompleteAsync() public void Setup(byte initialId, INexusSession session) { if (_currentState != State.Unset) - throw new InvalidOperationException("Can't setup a pipe that is already in use."); + throw new InvalidOperationException($"Can't setup a pipe that is already in use. State:{Id} Current State {_currentState}"); _readyTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); _session = session; - _logger = session.Logger?.CreateLogger(); + Logger = session.Logger?.CreateLogger(); InitialId = initialId; _outputPipeWriter.Setup( - _logger, + Logger, _session, _session.IsServer, _session.Config.NexusPipeFlushChunkSize); _inputPipeReader.Setup( - _logger, + Logger, _session.IsServer, _session.Config.NexusPipeHighWaterMark, _session.Config.NexusPipeHighWaterCutoff, @@ -192,10 +205,19 @@ public ValueTask PipeReady(ushort id) /// public void Reset() { + if(_currentState == State.Unset) + return; + // Close everything. - if(_currentState != State.Complete) + if (_currentState != State.Complete) + { + Logger?.LogError($"Resetting and Sending pipe: {Id} {_currentState}"); UpdateState(State.Complete); + } + StateId++; + _session = null; + Id = 0; InitialId = 0; _currentState = State.Unset; InitiatingPipe = false; @@ -205,8 +227,8 @@ public void Reset() _inputPipeReader.Reset(); _outputPipeWriter.Reset(); + - Interlocked.Increment(ref StateId); } /// @@ -250,7 +272,7 @@ public async ValueTask NotifyState() catch (Exception e) { // Possible that the session was closed before we could send the message. - _logger?.LogInfo(e, $"Exception occurred while updating state to: {currentState}. Closing pipe."); + Logger?.LogInfo(e, $"Exception occurred while updating state to: {currentState}. Closing pipe."); // Close the pipe. _currentState = State.Complete; @@ -267,20 +289,24 @@ public bool UpdateState(State updatedState, bool remove = false) { // Get a copy of the current state. var currentState = _currentState; + var stateId = StateId; if (_session == null || (!remove && currentState.HasFlag(updatedState)) || (remove && !currentState.HasFlag(updatedState))) { - _logger?.LogTrace($"Current State: {currentState}; Canceled state update of: {updatedState} because it is already set."); + Logger?.LogTrace($"Current State: {currentState}; Canceled state update of: {updatedState} because it is already set."); return false; } - _logger?.LogTrace($"Current State: {currentState}; Update State: {updatedState}"); + var isServer = _session.IsServer; + + Logger?.LogError($"{Id}: Current State: {currentState}; Update State: {updatedState}; From {new StackTrace().GetFrame(1).GetMethod().Name}"); if (currentState == State.Unset) { if (updatedState == State.Ready) { + Logger?.LogError($"--Updated {Id} state to ready from Unset to: {updatedState}"); _currentState = State.Ready; // Set the ready task. @@ -293,7 +319,7 @@ public bool UpdateState(State updatedState, bool remove = false) // If we are not ready, then we can't update the state. // This normally happens when the pipe is reset before it is ready or after it has been reset. // Honestly, we shouldn't reach here. - _logger?.LogTrace($"Ignored update state of : {updatedState} because the pipe was never readied."); + Logger?.LogTrace($"Ignored update state of : {updatedState} because the pipe was never readied."); _readyTcs?.TrySetCanceled(); return false; } @@ -301,7 +327,7 @@ public bool UpdateState(State updatedState, bool remove = false) if (HasState(updatedState, currentState, State.ClientReaderServerWriterComplete)) { - if (_session.IsServer) + if (isServer) { // Close output pipe. _outputPipeWriter.SetComplete(); @@ -316,7 +342,7 @@ public bool UpdateState(State updatedState, bool remove = false) if (HasState(updatedState, currentState, State.ClientWriterServerReaderComplete)) { - if (_session.IsServer) + if (isServer) { // Close input pipe. _inputPipeReader.CompleteNoNotify(); @@ -330,32 +356,35 @@ public bool UpdateState(State updatedState, bool remove = false) } // Back pressure - if (HasState(updatedState, currentState, State.ClientWriterPause) && _session.IsServer) + if (HasState(updatedState, currentState, State.ClientWriterPause) && isServer) { // Back pressure was added _outputPipeWriter.PauseWriting = true; } else if (remove && currentState.HasFlag(State.ClientWriterPause) && updatedState.HasFlag(State.ClientWriterPause) - && _session.IsServer) + && isServer) { // Back pressure was removed. _outputPipeWriter.PauseWriting = false; } - if (HasState(updatedState, currentState, State.ServerWriterPause) && !_session.IsServer) + if (HasState(updatedState, currentState, State.ServerWriterPause) && !isServer) { // Back pressure was added _outputPipeWriter.PauseWriting = true; } else if (remove && currentState.HasFlag(State.ServerWriterPause) && updatedState.HasFlag(State.ServerWriterPause) - && !_session.IsServer) + && !isServer) { // Back pressure was removed. _outputPipeWriter.PauseWriting = false; } + if (updatedState == State.Complete && stateId != StateId || IsInCache) + return false; + if (remove) { // Remove the state from the current state. diff --git a/src/NexNet/Pipes/NexusPipeManager.cs b/src/NexNet/Pipes/NexusPipeManager.cs index f218496b..4e3f23b3 100644 --- a/src/NexNet/Pipes/NexusPipeManager.cs +++ b/src/NexNet/Pipes/NexusPipeManager.cs @@ -81,8 +81,6 @@ public async ValueTask ReturnPipe(IRentedNexusDuplexPipe pipe) if(nexusPipe.CurrentState != State.Complete) { await pipe.CompleteAsync().ConfigureAwait(false); - // Return the pipe to the cache. - nexusPipe.Reset(); } _session.CacheManager.NexusRentedDuplexPipeCache.Return(nexusPipe); @@ -119,18 +117,18 @@ public async ValueTask RegisterPipe(byte otherId) public async ValueTask DeregisterPipe(INexusDuplexPipe pipe) { - _logger?.LogTrace($"DeregisterPipe({pipe.Id});"); + _logger?.LogError($"DeregisterPipe({pipe.Id});"); if (!_activePipes.TryRemove(pipe.Id, out var nexusPipe)) + { + _logger?.LogError($"Cant Remove ({pipe.Id});"); return; + } var (clientId, serverId) = GetClientAndServerId(pipe.Id); await nexusPipe.Pipe.CompleteAsync().ConfigureAwait(false); - // Return the pipe to the cache. - nexusPipe.Pipe.Reset(); - _session.CacheManager.NexusDuplexPipeCache.Return(nexusPipe.Pipe); lock (_usedIds) diff --git a/src/NexNet/Pipes/RentedNexusDuplexPipe.cs b/src/NexNet/Pipes/RentedNexusDuplexPipe.cs index d1da7072..fe699256 100644 --- a/src/NexNet/Pipes/RentedNexusDuplexPipe.cs +++ b/src/NexNet/Pipes/RentedNexusDuplexPipe.cs @@ -17,9 +17,14 @@ public RentedNexusDuplexPipe() public ValueTask DisposeAsync() { var manager = Interlocked.Exchange(ref Manager, null); - - if(manager == null) + + if (manager == null) + { + //Logger?.LogError($"Failed to dispose rented pipe {_rentedStateId} {StateId}"); return default; + } + + //Logger?.LogError($"Returning rented pipe {_rentedStateId} {StateId}"); return manager!.ReturnPipe(this); } diff --git a/src/NexNet/Transports/TcpTlsTransport.cs b/src/NexNet/Transports/TcpTlsTransport.cs index 7d6a35ca..392eecbc 100644 --- a/src/NexNet/Transports/TcpTlsTransport.cs +++ b/src/NexNet/Transports/TcpTlsTransport.cs @@ -26,11 +26,7 @@ private TcpTlsTransport(Socket socket, NetworkStream networkStream, SslStream ss Output = PipeWriter.Create(sslStream); } - public TransportConfiguration Configurations => new TransportConfiguration() - { - DoNotPassFlushCancellationToken = true - }; - + public TransportConfiguration Configurations => new TransportConfiguration(); public ValueTask CloseAsync(bool linger) { if (!linger) diff --git a/src/NexNet/Transports/TransportConfiguration.cs b/src/NexNet/Transports/TransportConfiguration.cs index c4c1854f..94c98ce7 100644 --- a/src/NexNet/Transports/TransportConfiguration.cs +++ b/src/NexNet/Transports/TransportConfiguration.cs @@ -5,13 +5,5 @@ /// public class TransportConfiguration { - /// - /// If true, the transport will not pass the flush cancellation token to the underlying transport. - /// Currently exists due to an issue in the QUIC implementation. Should be removed once the issue is fixed. - /// - /// - /// https://github.com/dotnet/runtime/issues/82704 - /// https://github.com/dotnet/runtime/pull/90253 - /// - public bool DoNotPassFlushCancellationToken { get; init; } = false; + // Configurations for the transport. } diff --git a/src/NexNetBenchmarks/BenchmarkConfig.cs b/src/NexNetBenchmarks/BenchmarkConfig.cs new file mode 100644 index 00000000..e318e964 --- /dev/null +++ b/src/NexNetBenchmarks/BenchmarkConfig.cs @@ -0,0 +1,56 @@ +using System.Collections.Generic; +using System.Linq; +using BenchmarkDotNet.Analysers; +using BenchmarkDotNet.Columns; +using BenchmarkDotNet.Configs; +using BenchmarkDotNet.Diagnosers; +using BenchmarkDotNet.Environments; +using BenchmarkDotNet.Exporters; +using BenchmarkDotNet.Exporters.Csv; +using BenchmarkDotNet.Jobs; +using BenchmarkDotNet.Loggers; + +namespace NexNetBenchmarks +{ + public class BenchmarkConfig + { + /// + /// Get a custom configuration + /// + /// + public static IConfig Get() + { + return ManualConfig.CreateEmpty() + // Jobs + .AddJob(Job.Default + .WithRuntime(CoreRuntime.Core70) + .WithPlatform(Platform.X64) + .WithMinWarmupCount(1) + .WithMaxWarmupCount(3) + .WithMinIterationCount(1) + .WithMaxIterationCount(7)) + .AddDiagnoser(MemoryDiagnoser.Default) + .AddColumnProvider(DefaultColumnProviders.Instance) + .AddColumn(StatisticColumn.OperationsPerSecond) + .AddLogger(ConsoleLogger.Default) + //.AddExporter(CsvExporter.Default) + //.AddExporter(HtmlExporter.Default) + .AddAnalyser(GetAnalysers().ToArray()); + } + + /// + /// Get analyser for the cutom configuration + /// + /// + private static IEnumerable GetAnalysers() + { + yield return EnvironmentAnalyser.Default; + yield return OutliersAnalyser.Default; + yield return MinIterationTimeAnalyser.Default; + //yield return MultimodalDistributionAnalyzer.Default; + yield return RuntimeErrorAnalyser.Default; + yield return ZeroMeasurementAnalyser.Default; + //yield return BaselineCustomAnalyzer.Default; + } + } +} diff --git a/src/NexNetBenchmarks/InvocationBenchmarks.cs b/src/NexNetBenchmarks/InvocationBenchmarks.cs new file mode 100644 index 00000000..d3389585 --- /dev/null +++ b/src/NexNetBenchmarks/InvocationBenchmarks.cs @@ -0,0 +1,81 @@ +using System; +using System.IO; +using System.Net.Sockets; +using System.Threading.Tasks; +using BenchmarkDotNet; +using BenchmarkDotNet.Attributes; +using NexNet; +using NexNet.Transports; + +namespace NexNetBenchmarks +{ + public class InvocationBenchmarks + { + private NexusClient _client; + private NexusServer _server; + private ReadOnlyMemory _uploadBuffer; + + [GlobalSetup] + public async Task GlobalSetup() + { + _uploadBuffer = new byte[1024 * 16]; + var path = "test.sock"; + if (File.Exists(path)) + File.Delete(path); + + var serverConfig = new UdsServerConfig() + { + EndPoint = new UnixDomainSocketEndPoint(path), + }; + var clientConfig = new UdsClientConfig() + { + EndPoint = new UnixDomainSocketEndPoint(path), + }; + + _client = ClientNexus.CreateClient(clientConfig, new ClientNexus()); + _server = ServerNexus.CreateServer(serverConfig, static () => new ServerNexus()); + await _server.StartAsync(); + await _client.ConnectAsync(); + } + + [GlobalCleanup] + public async Task GlobalCleanup() + { + await _client.DisconnectAsync(); + await _server.StopAsync(); + } + + //[Benchmark] + public async ValueTask InvocationNoArgument() + { + await _client.Proxy.InvocationNoArgument(); + } + + //[Benchmark] + public async ValueTask InvocationUnmanagedArgument() + { + await _client.Proxy.InvocationUnmanagedArgument(12345); + } + + //[Benchmark] + public async ValueTask InvocationUnmanagedMultipleArguments() + { + await _client.Proxy.InvocationUnmanagedMultipleArguments(12345, 128475129847, 24812, 298471920875185871, 19818479124.12871924821d); + } + + //[Benchmark] + public async ValueTask InvocationNoArgumentWithResult() + { + await _client.Proxy.InvocationNoArgumentWithResult(); + } + + [Benchmark] + public async ValueTask InvocationWithDuplexPipe_Upload() + { + await using var pipe = _client.CreatePipe(); + await _client.Proxy.InvocationWithDuplexPipe_Upload(pipe); + await pipe.ReadyTask; + await pipe.Output.WriteAsync(_uploadBuffer); + } + } +} diff --git a/src/NexNetBenchmarks/NexNetBenchmarks.csproj b/src/NexNetBenchmarks/NexNetBenchmarks.csproj new file mode 100644 index 00000000..e3ff6cdb --- /dev/null +++ b/src/NexNetBenchmarks/NexNetBenchmarks.csproj @@ -0,0 +1,25 @@ + + + net7.0 + Exe + + + AnyCPU + pdbonly + true + true + true + Release + false + + + + + + + + + + + + diff --git a/src/NexNetBenchmarks/Nexuses.cs b/src/NexNetBenchmarks/Nexuses.cs new file mode 100644 index 00000000..51e41b01 --- /dev/null +++ b/src/NexNetBenchmarks/Nexuses.cs @@ -0,0 +1,72 @@ +using System.IO; +using System.IO.Pipelines; +using System.Threading.Tasks; +using NexNet; +using NexNet.Pipes; + +namespace NexNetBenchmarks; + +interface IClientNexus +{ + +} + +interface IServerNexus +{ + ValueTask InvocationNoArgument(); + ValueTask InvocationUnmanagedArgument(int argument); + ValueTask InvocationUnmanagedMultipleArguments(int argument1, long argument2, ushort argument3, ulong argument4, double argument5); + ValueTask InvocationNoArgumentWithResult(); + + ValueTask InvocationWithDuplexPipe_Upload(INexusDuplexPipe duplexPipe); +} + +[Nexus(NexusType = NexusType.Client)] +partial class ClientNexus +{ + +} + +[Nexus(NexusType = NexusType.Server)] +partial class ServerNexus +{ + public ValueTask InvocationNoArgument() + { + // Do Work. + return ValueTask.CompletedTask; + } + + public ValueTask InvocationUnmanagedArgument(int argument) + { + return ValueTask.CompletedTask; + } + + public ValueTask InvocationUnmanagedMultipleArguments( + int argument1, + long argument2, + ushort argument3, + ulong argument4, + double argument5) + { + return ValueTask.CompletedTask; + } + + public ValueTask InvocationNoArgumentWithResult() + { + return new ValueTask(12345); + } + + public async ValueTask InvocationWithDuplexPipe_Upload(INexusDuplexPipe duplexPipe) + { + var reader = duplexPipe.Input; + while (true) + { + var result = await reader.ReadAsync(); + + if(result.IsCompleted) + break; + + reader.AdvanceTo(result.Buffer.End); + } + } +} diff --git a/src/NexNetBenchmarks/Program.cs b/src/NexNetBenchmarks/Program.cs new file mode 100644 index 00000000..673761b5 --- /dev/null +++ b/src/NexNetBenchmarks/Program.cs @@ -0,0 +1,20 @@ +using BenchmarkDotNet.Running; + +namespace NexNetBenchmarks +{ + public class Program + { + public static void Main(string[] args) + { + // If arguments are available use BenchmarkSwitcher to run benchmarks + if (args.Length > 0) + { + var summaries = BenchmarkSwitcher.FromAssembly(typeof(Program).Assembly) + .Run(args, BenchmarkConfig.Get()); + return; + } + // Else, use BenchmarkRunner + var summary = BenchmarkRunner.Run(BenchmarkConfig.Get()); + } + } +} From a8a0dcbdee678ac0483dd3b7161d81554675e6a0 Mon Sep 17 00:00:00 2001 From: DJGosnell Date: Thu, 14 Sep 2023 17:22:15 -0400 Subject: [PATCH 02/15] Major re-write of pipes. Now pipes no longer are cached. --- .../Pipes/NexusChannelReaderTests.cs | 32 +--- .../Pipes/NexusChannelReaderUnmanagedTests.cs | 16 +- .../Pipes/NexusChannelReaderWriterTestBase.cs | 91 +++++++++ .../Pipes/NexusChannelReaderWriterTests.cs | 73 +------- .../NexusChannelReaderWriterUnmanagedTests.cs | 69 +------ .../Pipes/NexusChannelTestBase.cs | 64 +++++++ .../Pipes/NexusChannelWriterTests.cs | 12 +- .../Pipes/NexusChannelWriterUnmanagedTests.cs | 67 +------ .../Pipes/NexusClientTests_NexusDuplexPipe.cs | 10 +- .../Pipes/NexusDuplexPipeReaderTests.cs | 16 +- .../Pipes/NexusDuplexPipeWriterTests.cs | 9 +- src/NexNet/Cache/CacheManager.cs | 6 +- src/NexNet/Cache/CachedDuplexPipe.cs | 36 ---- src/NexNet/Cache/CachedRentedDuplexPipe.cs | 36 ---- src/NexNet/Internals/NexusSession.Sending.cs | 46 ++++- src/NexNet/Invocation/ProxyInvocationBase.cs | 2 +- src/NexNet/Pipes/NexusDuplexPipe.cs | 143 ++++---------- src/NexNet/Pipes/NexusPipeManager.cs | 176 ++++++++---------- src/NexNet/Pipes/NexusPipeReader.cs | 54 +++--- src/NexNet/Pipes/NexusPipeWriter.cs | 92 +++++---- src/NexNet/Pipes/RentedNexusDuplexPipe.cs | 12 +- 21 files changed, 447 insertions(+), 615 deletions(-) create mode 100644 src/NexNet.IntegrationTests/Pipes/NexusChannelReaderWriterTestBase.cs create mode 100644 src/NexNet.IntegrationTests/Pipes/NexusChannelTestBase.cs delete mode 100644 src/NexNet/Cache/CachedDuplexPipe.cs delete mode 100644 src/NexNet/Cache/CachedRentedDuplexPipe.cs diff --git a/src/NexNet.IntegrationTests/Pipes/NexusChannelReaderTests.cs b/src/NexNet.IntegrationTests/Pipes/NexusChannelReaderTests.cs index 69db45db..6f444212 100644 --- a/src/NexNet.IntegrationTests/Pipes/NexusChannelReaderTests.cs +++ b/src/NexNet.IntegrationTests/Pipes/NexusChannelReaderTests.cs @@ -6,12 +6,12 @@ namespace NexNet.IntegrationTests.Pipes; -internal class NexusChannelReaderTests +internal class NexusChannelReaderTests : NexusChannelTestBase { [Test] public async Task ReadsData() { - var pipeReader = new NexusPipeReader(new DummyPipeStateManager()); + var pipeReader = new NexusPipeReader(new DummyPipeStateManager(), null, true, 0, 0, 0); var reader = new NexusChannelReader(pipeReader); var baseObject = ComplexMessage.Random(); @@ -48,7 +48,7 @@ public async Task ReadsData() [Test] public async Task ReadsPartialData() { - var pipeReader = new NexusPipeReader(new DummyPipeStateManager()); + var pipeReader = new NexusPipeReader(new DummyPipeStateManager(), null, true, 0, 0, 0); var reader = new NexusChannelReader(pipeReader); var baseObject = ComplexMessage.Random(); @@ -87,7 +87,7 @@ public async Task ReadsPartialData() [Test] public async Task CancelsReadDelayed() { - var pipeReader = new NexusPipeReader(new DummyPipeStateManager()); + var pipeReader = new NexusPipeReader(new DummyPipeStateManager(), null, true, 0, 0, 0); var reader = new NexusChannelReader(pipeReader); var cts = new CancellationTokenSource(100); var result = await reader.ReadAsync(cts.Token).Timeout(1); @@ -100,7 +100,7 @@ public async Task CancelsReadDelayed() [Test] public async Task CancelsReadImmediate() { - var pipeReader = new NexusPipeReader(new DummyPipeStateManager()); + var pipeReader = new NexusPipeReader(new DummyPipeStateManager(), null, true, 0, 0, 0); var reader = new NexusChannelReader(pipeReader); var cts = new CancellationTokenSource(100); cts.Cancel(); @@ -114,7 +114,7 @@ public async Task CancelsReadImmediate() [Test] public async Task Completes() { - var pipeReader = new NexusPipeReader(new DummyPipeStateManager()); + var pipeReader = new NexusPipeReader(new DummyPipeStateManager(), null, true, 0, 0, 0); var reader = new NexusChannelReader(pipeReader); // ReSharper disable once MethodHasAsyncOverload await pipeReader.CompleteAsync(); @@ -130,7 +130,7 @@ public async Task Completes() public async Task WaitsForFullData() { var tcs = new TaskCompletionSource(); - var pipeReader = new NexusPipeReader(new DummyPipeStateManager()); + var pipeReader = new NexusPipeReader(new DummyPipeStateManager(), null, true, 0, 0, 0); var reader = new NexusChannelReader(pipeReader); var baseObject = ComplexMessage.Random(); var bytes = new ReadOnlySequence(MemoryPackSerializer.Serialize(baseObject)); @@ -153,7 +153,7 @@ public async Task WaitsForFullData() public async Task ReadsMultiple() { const int iterations = 1000; - var pipeReader = new NexusPipeReader(new DummyPipeStateManager()); + var pipeReader = new NexusPipeReader(new DummyPipeStateManager(), null, true, 0, 0, 0); var reader = new NexusChannelReader(pipeReader); var baseObject = ComplexMessage.Random(); var bytes = new ReadOnlySequence(MemoryPackSerializer.Serialize(baseObject)); @@ -171,20 +171,4 @@ public async Task ReadsMultiple() Assert.AreEqual(iterations, result.Count()); } - private class DummyPipeStateManager : IPipeStateManager - { - public ushort Id { get; } = 0; - public ValueTask NotifyState() - { - return default; - } - - public bool UpdateState(NexusDuplexPipe.State updatedState, bool remove = false) - { - CurrentState |= updatedState; - return true; - } - - public NexusDuplexPipe.State CurrentState { get; private set; } = NexusDuplexPipe.State.Ready; - } } diff --git a/src/NexNet.IntegrationTests/Pipes/NexusChannelReaderUnmanagedTests.cs b/src/NexNet.IntegrationTests/Pipes/NexusChannelReaderUnmanagedTests.cs index ce7067b6..e8078277 100644 --- a/src/NexNet.IntegrationTests/Pipes/NexusChannelReaderUnmanagedTests.cs +++ b/src/NexNet.IntegrationTests/Pipes/NexusChannelReaderUnmanagedTests.cs @@ -19,7 +19,7 @@ internal class NexusChannelReaderUnmanagedTests public async Task ReadsData(T inputData) where T : unmanaged { - var pipeReader = new NexusPipeReader(new DummyPipeStateManager()); + var pipeReader = new NexusPipeReader(new DummyPipeStateManager(), new ConsoleLogger(), true, 0, 0, 0); var reader = new NexusChannelReaderUnmanaged(pipeReader); await pipeReader.BufferData(Utilities.GetBytes(inputData)); @@ -32,7 +32,7 @@ public async Task ReadsData(T inputData) public async Task CancelsReadDelayed() where T : unmanaged { - var pipeReader = new NexusPipeReader(new DummyPipeStateManager()); + var pipeReader = new NexusPipeReader(new DummyPipeStateManager(), new ConsoleLogger(), true, 0, 0, 0); var reader = new NexusChannelReaderUnmanaged(pipeReader); var cts = new CancellationTokenSource(100); var result = await reader.ReadAsync(cts.Token).Timeout(1); @@ -45,7 +45,7 @@ public async Task CancelsReadDelayed() [Test] public async Task CancelsReadImmediate() { - var pipeReader = new NexusPipeReader(new DummyPipeStateManager()); + var pipeReader = new NexusPipeReader(new DummyPipeStateManager(), new ConsoleLogger(), true, 0, 0, 0); var reader = new NexusChannelReaderUnmanaged(pipeReader); var cts = new CancellationTokenSource(100); cts.Cancel(); @@ -59,7 +59,7 @@ public async Task CancelsReadImmediate() [Test] public async Task Completes() { - var pipeReader = new NexusPipeReader(new DummyPipeStateManager()); + var pipeReader = new NexusPipeReader(new DummyPipeStateManager(), new ConsoleLogger(), true, 0, 0, 0); var reader = new NexusChannelReaderUnmanaged(pipeReader); // ReSharper disable once MethodHasAsyncOverload @@ -87,7 +87,7 @@ public async Task WaitsForFullData(T inputData) where T : unmanaged { var tcs = new TaskCompletionSource(); - var pipeReader = new NexusPipeReader(new DummyPipeStateManager()); + var pipeReader = new NexusPipeReader(new DummyPipeStateManager(), new ConsoleLogger(), true, 0, 0, 0); var reader = new NexusChannelReaderUnmanaged(pipeReader); _ = Task.Run(async () => @@ -121,7 +121,7 @@ public async Task ReadsMultiple(T inputData) where T : unmanaged { const int iterations = 1000; - var pipeReader = new NexusPipeReader(new DummyPipeStateManager()); + var pipeReader = new NexusPipeReader(new DummyPipeStateManager(), new ConsoleLogger(), true, 0, 0, 0); var reader = new NexusChannelReaderUnmanaged(pipeReader); var data = Utilities.GetBytes(inputData); var count = 0; @@ -155,7 +155,7 @@ public async Task ReadsMultipleParallel(T inputData) where T : unmanaged { const int iterations = 1000; - var pipeReader = new NexusPipeReader(new DummyPipeStateManager()); + var pipeReader = new NexusPipeReader(new DummyPipeStateManager(), new ConsoleLogger(), true, 0, 0, 0); var reader = new NexusChannelReaderUnmanaged(pipeReader); var data = Utilities.GetBytes(inputData); var count = 0; @@ -202,7 +202,7 @@ await Task.Run(async () => public async Task ReadsWithPartialWrites(T inputData) where T : unmanaged { - var pipeReader = new NexusPipeReader(new DummyPipeStateManager()); + var pipeReader = new NexusPipeReader(new DummyPipeStateManager(), new ConsoleLogger(), true, 0, 0, 0); var reader = new NexusChannelReaderUnmanaged(pipeReader); var data = Utilities.GetBytes(inputData); diff --git a/src/NexNet.IntegrationTests/Pipes/NexusChannelReaderWriterTestBase.cs b/src/NexNet.IntegrationTests/Pipes/NexusChannelReaderWriterTestBase.cs new file mode 100644 index 00000000..e76be849 --- /dev/null +++ b/src/NexNet.IntegrationTests/Pipes/NexusChannelReaderWriterTestBase.cs @@ -0,0 +1,91 @@ +using System.Buffers; +using NexNet.Internals; +using NexNet.Messages; +using NexNet.Pipes; + +namespace NexNet.IntegrationTests.Pipes; + +internal class NexusChannelReaderWriterTestBase +{ + + protected class DummyPipeStateManager : IPipeStateManager + { + public ushort Id { get; } = 0; + public ValueTask NotifyState() + { + return default; + } + + public bool UpdateState(NexusDuplexPipe.State updatedState, bool remove = false) + { + CurrentState |= updatedState; + return true; + } + + public NexusDuplexPipe.State CurrentState { get; private set; } = NexusDuplexPipe.State.Ready; + } + + protected class DummySessionMessenger : ISessionMessenger + { + public Func?, ReadOnlySequence, ValueTask> + OnMessageSent + { get; set; } = null!; + public ValueTask SendMessage(TMessage body, CancellationToken cancellationToken = default) where TMessage : IMessageBase + { + throw new NotImplementedException(); + } + + public ValueTask SendHeaderWithBody(MessageType type, ReadOnlySequence body, CancellationToken cancellationToken = default) + { + throw new NotImplementedException(); + } + + public ValueTask SendHeader(MessageType type, CancellationToken cancellationToken = default) + { + throw new NotImplementedException(); + } + + public ValueTask SendHeaderWithBody(MessageType type, ReadOnlyMemory? messageHeader, ReadOnlySequence body, + CancellationToken cancellationToken = default) + { + if (OnMessageSent == null) + throw new InvalidOperationException("No handler for OnMessageSent"); + + return OnMessageSent.Invoke(type, messageHeader, body); + } + + public Task DisconnectAsync(DisconnectReason reason) + { + throw new NotImplementedException(); + } + } + + protected (NexusPipeWriter, NexusPipeReader) GetConnectedPipeReaderWriter() + { + NexusPipeWriter nexusPipeWriter = null!; + NexusPipeReader nexusPipeReader = null!; + + var messenger = new DummySessionMessenger() + { + OnMessageSent = async (type, memory, arg3) => + { + await nexusPipeReader!.BufferData(arg3).Timeout(1); + } + }; + nexusPipeWriter = new NexusPipeWriter( + new DummyPipeStateManager(), + new ConsoleLogger(), + messenger, + true, + ushort.MaxValue); + nexusPipeReader = new NexusPipeReader( + new DummyPipeStateManager(), + new ConsoleLogger(), + true, + ushort.MaxValue, + 0, + 0); + + return (nexusPipeWriter, nexusPipeReader); + } +} diff --git a/src/NexNet.IntegrationTests/Pipes/NexusChannelReaderWriterTests.cs b/src/NexNet.IntegrationTests/Pipes/NexusChannelReaderWriterTests.cs index 400cf551..e0084f2f 100644 --- a/src/NexNet.IntegrationTests/Pipes/NexusChannelReaderWriterTests.cs +++ b/src/NexNet.IntegrationTests/Pipes/NexusChannelReaderWriterTests.cs @@ -1,12 +1,9 @@ -using System.Buffers; -using NexNet.Internals; -using NexNet.Messages; -using NexNet.Pipes; +using NexNet.Pipes; using NUnit.Framework; namespace NexNet.IntegrationTests.Pipes; -internal class NexusChannelReaderWriterTests +internal class NexusChannelReaderWriterTests : NexusChannelReaderWriterTestBase { [Test] public async Task WritesAndReadsData() @@ -92,72 +89,12 @@ public async Task ReaderCompletesOnPartialRead() } private (NexusChannelWriter, NexusChannelReader) GetReaderWriter() { - var nexusPipeWriter = new NexusPipeWriter(new DummyPipeStateManager()); - var nexusPipeReader = new NexusPipeReader(new DummyPipeStateManager()); - var messenger = new DummySessionMessenger() - { - OnMessageSent = async (type, memory, arg3) => - { - await nexusPipeReader.BufferData(arg3).Timeout(1); - } - }; - nexusPipeWriter.Setup(new ConsoleLogger(), messenger, true, ushort.MaxValue); + var (pipeWriter, pipeReader) = GetConnectedPipeReaderWriter(); - var writer = new NexusChannelWriter(nexusPipeWriter); - var reader = new NexusChannelReader(nexusPipeReader); + var writer = new NexusChannelWriter(pipeWriter); + var reader = new NexusChannelReader(pipeReader); return (writer, reader); } - private class DummyPipeStateManager : IPipeStateManager - { - public ushort Id { get; } = 0; - public ValueTask NotifyState() - { - return default; - } - - public bool UpdateState(NexusDuplexPipe.State updatedState, bool remove = false) - { - CurrentState |= updatedState; - return true; - } - - public NexusDuplexPipe.State CurrentState { get; private set; } = NexusDuplexPipe.State.Ready; - } - - private class DummySessionMessenger : ISessionMessenger - { - public Func?, ReadOnlySequence, ValueTask> - OnMessageSent - { get; set; } = null!; - public ValueTask SendMessage(TMessage body, CancellationToken cancellationToken = default) where TMessage : IMessageBase - { - throw new NotImplementedException(); - } - - public ValueTask SendHeaderWithBody(MessageType type, ReadOnlySequence body, CancellationToken cancellationToken = default) - { - throw new NotImplementedException(); - } - - public ValueTask SendHeader(MessageType type, CancellationToken cancellationToken = default) - { - throw new NotImplementedException(); - } - - public ValueTask SendHeaderWithBody(MessageType type, ReadOnlyMemory? messageHeader, ReadOnlySequence body, - CancellationToken cancellationToken = default) - { - if (OnMessageSent == null) - throw new InvalidOperationException("No handler for OnMessageSent"); - - return OnMessageSent.Invoke(type, messageHeader, body); - } - - public Task DisconnectAsync(DisconnectReason reason) - { - throw new NotImplementedException(); - } - } } diff --git a/src/NexNet.IntegrationTests/Pipes/NexusChannelReaderWriterUnmanagedTests.cs b/src/NexNet.IntegrationTests/Pipes/NexusChannelReaderWriterUnmanagedTests.cs index ee75ebb1..5bc0ff80 100644 --- a/src/NexNet.IntegrationTests/Pipes/NexusChannelReaderWriterUnmanagedTests.cs +++ b/src/NexNet.IntegrationTests/Pipes/NexusChannelReaderWriterUnmanagedTests.cs @@ -6,7 +6,7 @@ namespace NexNet.IntegrationTests.Pipes; -internal class NexusChannelReaderWriterUnmanagedTests +internal class NexusChannelReaderWriterUnmanagedTests : NexusChannelReaderWriterTestBase { [Test] public async Task WritesAndReadsData() @@ -91,72 +91,11 @@ public async Task ReaderCompletesOnPartialRead() private (NexusChannelWriterUnmanaged, NexusChannelReaderUnmanaged) GetReaderWriter() where T : unmanaged { - var nexusPipeWriter = new NexusPipeWriter(new DummyPipeStateManager()); - var nexusPipeReader = new NexusPipeReader(new DummyPipeStateManager()); - var messenger = new DummySessionMessenger() - { - OnMessageSent = async (type, memory, arg3) => - { - await nexusPipeReader.BufferData(arg3).Timeout(1); - } - }; - nexusPipeWriter.Setup(new ConsoleLogger(), messenger, true, ushort.MaxValue); + var (pipeWriter, pipeReader) = GetConnectedPipeReaderWriter(); - var writer = new NexusChannelWriterUnmanaged(nexusPipeWriter); - var reader = new NexusChannelReaderUnmanaged(nexusPipeReader); + var writer = new NexusChannelWriterUnmanaged(pipeWriter); + var reader = new NexusChannelReaderUnmanaged(pipeReader); return (writer, reader); } - - private class DummyPipeStateManager : IPipeStateManager - { - public ushort Id { get; } = 0; - public ValueTask NotifyState() - { - return default; - } - - public bool UpdateState(NexusDuplexPipe.State updatedState, bool remove = false) - { - CurrentState |= updatedState; - return true; - } - - public NexusDuplexPipe.State CurrentState { get; private set; } = NexusDuplexPipe.State.Ready; - } - - private class DummySessionMessenger : ISessionMessenger - { - public Func?, ReadOnlySequence, ValueTask> - OnMessageSent - { get; set; } = null!; - public ValueTask SendMessage(TMessage body, CancellationToken cancellationToken = default) where TMessage : IMessageBase - { - throw new NotImplementedException(); - } - - public ValueTask SendHeaderWithBody(MessageType type, ReadOnlySequence body, CancellationToken cancellationToken = default) - { - throw new NotImplementedException(); - } - - public ValueTask SendHeader(MessageType type, CancellationToken cancellationToken = default) - { - throw new NotImplementedException(); - } - - public ValueTask SendHeaderWithBody(MessageType type, ReadOnlyMemory? messageHeader, ReadOnlySequence body, - CancellationToken cancellationToken = default) - { - if (OnMessageSent == null) - throw new InvalidOperationException("No handler for OnMessageSent"); - - return OnMessageSent.Invoke(type, messageHeader, body); - } - - public Task DisconnectAsync(DisconnectReason reason) - { - throw new NotImplementedException(); - } - } } diff --git a/src/NexNet.IntegrationTests/Pipes/NexusChannelTestBase.cs b/src/NexNet.IntegrationTests/Pipes/NexusChannelTestBase.cs new file mode 100644 index 00000000..2f500456 --- /dev/null +++ b/src/NexNet.IntegrationTests/Pipes/NexusChannelTestBase.cs @@ -0,0 +1,64 @@ +using System.Buffers; +using NexNet.Internals; +using NexNet.Messages; +using NexNet.Pipes; + +namespace NexNet.IntegrationTests.Pipes; + +internal class NexusChannelTestBase +{ + protected class DummySessionMessenger : ISessionMessenger + { + public Func?, ReadOnlySequence, ValueTask> OnMessageSent { get; set; } = + null!; + + public ValueTask SendMessage(TMessage body, CancellationToken cancellationToken = default) + where TMessage : IMessageBase + { + throw new NotImplementedException(); + } + + public ValueTask SendHeaderWithBody(MessageType type, ReadOnlySequence body, + CancellationToken cancellationToken = default) + { + throw new NotImplementedException(); + } + + public ValueTask SendHeader(MessageType type, CancellationToken cancellationToken = default) + { + throw new NotImplementedException(); + } + + public ValueTask SendHeaderWithBody(MessageType type, ReadOnlyMemory? messageHeader, + ReadOnlySequence body, + CancellationToken cancellationToken = default) + { + if (OnMessageSent == null) + throw new InvalidOperationException("No handler for OnMessageSent"); + + return OnMessageSent.Invoke(type, messageHeader, body); + } + + public Task DisconnectAsync(DisconnectReason reason) + { + throw new NotImplementedException(); + } + } + + protected class DummyPipeStateManager : IPipeStateManager + { + public ushort Id { get; } = 0; + public ValueTask NotifyState() + { + return default; + } + + public bool UpdateState(NexusDuplexPipe.State updatedState, bool remove = false) + { + CurrentState |= updatedState; + return true; + } + + public NexusDuplexPipe.State CurrentState { get; private set; } = NexusDuplexPipe.State.Ready; + } +} diff --git a/src/NexNet.IntegrationTests/Pipes/NexusChannelWriterTests.cs b/src/NexNet.IntegrationTests/Pipes/NexusChannelWriterTests.cs index ff69e19f..1f12f90c 100644 --- a/src/NexNet.IntegrationTests/Pipes/NexusChannelWriterTests.cs +++ b/src/NexNet.IntegrationTests/Pipes/NexusChannelWriterTests.cs @@ -13,9 +13,8 @@ internal class NexusChannelWriterTests [Test] public async Task WritesData() { - var nexusPipeWriter = new NexusPipeWriter(new DummyPipeStateManager()); var messenger = new DummySessionMessenger(); - nexusPipeWriter.Setup(new ConsoleLogger(), messenger, true, ushort.MaxValue); + var nexusPipeWriter = new NexusPipeWriter(new DummyPipeStateManager(), new ConsoleLogger(), messenger, true, ushort.MaxValue); var writer = new NexusChannelWriter(nexusPipeWriter); var bufferWriter = BufferWriter.Create(); var baseObject = ComplexMessage.Random(); @@ -35,9 +34,8 @@ public async Task WritesData() [Test] public async Task WritesDataWithPartialFlush() { - var nexusPipeWriter = new NexusPipeWriter(new DummyPipeStateManager()); var messenger = new DummySessionMessenger(); - nexusPipeWriter.Setup(new ConsoleLogger(), messenger, true, 1); + var nexusPipeWriter = new NexusPipeWriter(new DummyPipeStateManager(), new ConsoleLogger(), messenger, true, 1); var writer = new NexusChannelWriter(nexusPipeWriter); var bufferWriter = BufferWriter.Create(); var baseObject = ComplexMessage.Random(); @@ -57,13 +55,12 @@ public async Task WritesDataWithPartialFlush() [Test] public async Task WritesCancels() { - var nexusPipeWriter = new NexusPipeWriter(new DummyPipeStateManager()) + var nexusPipeWriter = new NexusPipeWriter(new DummyPipeStateManager(), new ConsoleLogger(), new DummySessionMessenger(), true, ushort.MaxValue) { PauseWriting = true }; var writer = new NexusChannelWriter(nexusPipeWriter); - nexusPipeWriter.Setup(new ConsoleLogger(), new DummySessionMessenger(), true, ushort.MaxValue); var cts = new CancellationTokenSource(100); var writeResult = await writer.WriteAsync(ComplexMessage.Random(), cts.Token).Timeout(1); @@ -75,13 +72,12 @@ public async Task WritesCancels() [Test] public async Task WritesCancelsImmediately() { - var nexusPipeWriter = new NexusPipeWriter(new DummyPipeStateManager()) + var nexusPipeWriter = new NexusPipeWriter(new DummyPipeStateManager(), new ConsoleLogger(), new DummySessionMessenger(), true, ushort.MaxValue) { PauseWriting = true }; var writer = new NexusChannelWriter(nexusPipeWriter); - nexusPipeWriter.Setup(new ConsoleLogger(), new DummySessionMessenger(), true, ushort.MaxValue); var cts = new CancellationTokenSource(); cts.Cancel(); diff --git a/src/NexNet.IntegrationTests/Pipes/NexusChannelWriterUnmanagedTests.cs b/src/NexNet.IntegrationTests/Pipes/NexusChannelWriterUnmanagedTests.cs index 33f68d0e..33d9f54e 100644 --- a/src/NexNet.IntegrationTests/Pipes/NexusChannelWriterUnmanagedTests.cs +++ b/src/NexNet.IntegrationTests/Pipes/NexusChannelWriterUnmanagedTests.cs @@ -1,6 +1,4 @@ using System.Buffers; -using NexNet.Internals; -using NexNet.Messages; using NexNet.Pipes; using NUnit.Framework; using Pipelines.Sockets.Unofficial.Arenas; @@ -8,7 +6,7 @@ namespace NexNet.IntegrationTests.Pipes; -internal class NexusChannelWriterUnmanagedTests +internal class NexusChannelWriterUnmanagedTests : NexusChannelTestBase { [TestCase((sbyte)-54)] [TestCase((byte)200)] @@ -24,9 +22,8 @@ internal class NexusChannelWriterUnmanagedTests public async Task WritesData(T inputData) where T : unmanaged { - var nexusPipeWriter = new NexusPipeWriter(new DummyPipeStateManager()); var messenger = new DummySessionMessenger(); - nexusPipeWriter.Setup(new ConsoleLogger(), messenger, true, ushort.MaxValue); + var nexusPipeWriter = new NexusPipeWriter(new DummyPipeStateManager(), null, messenger, true, ushort.MaxValue); var writer = new NexusChannelWriterUnmanaged(nexusPipeWriter); var bufferWriter = BufferWriter.Create(); messenger.OnMessageSent = (type, header, body) => @@ -54,13 +51,12 @@ public async Task WritesData(T inputData) public async Task WritesDataWithPartialFlush(T inputData) where T : unmanaged { - var nexusPipeWriter = new NexusPipeWriter(new DummyPipeStateManager()); var messenger = new DummySessionMessenger(); + var nexusPipeWriter = new NexusPipeWriter(new DummyPipeStateManager(), new ConsoleLogger(), messenger, true, 1); + var writer = new NexusChannelWriterUnmanaged(nexusPipeWriter); var bufferWriter = BufferWriter.Create(); - nexusPipeWriter.Setup(new ConsoleLogger(), messenger, true, 1); - messenger.OnMessageSent = (type, header, body) => { bufferWriter.Write(body.ToArray()); @@ -75,13 +71,12 @@ public async Task WritesDataWithPartialFlush(T inputData) [Test] public async Task WritesCancels() { - var nexusPipeWriter = new NexusPipeWriter(new DummyPipeStateManager()) + var nexusPipeWriter = new NexusPipeWriter(new DummyPipeStateManager(), new ConsoleLogger(), new DummySessionMessenger(), true, ushort.MaxValue) { PauseWriting = true }; var writer = new NexusChannelWriterUnmanaged(nexusPipeWriter); - nexusPipeWriter.Setup(new ConsoleLogger(), new DummySessionMessenger(), true, ushort.MaxValue); var cts = new CancellationTokenSource(100); var writeResult = await writer.WriteAsync(123456789L, cts.Token).Timeout(1); @@ -93,13 +88,12 @@ public async Task WritesCancels() [Test] public async Task WritesCancelsImmediately() { - var nexusPipeWriter = new NexusPipeWriter(new DummyPipeStateManager()) + var nexusPipeWriter = new NexusPipeWriter(new DummyPipeStateManager(), new ConsoleLogger(), new DummySessionMessenger(), true, ushort.MaxValue) { PauseWriting = true }; var writer = new NexusChannelWriterUnmanaged(nexusPipeWriter); - nexusPipeWriter.Setup(new ConsoleLogger(), new DummySessionMessenger(), true, ushort.MaxValue); var cts = new CancellationTokenSource(); cts.Cancel(); @@ -108,53 +102,4 @@ public async Task WritesCancelsImmediately() Assert.IsFalse(writeResult); Assert.IsFalse(writer.IsComplete); } - private class DummyPipeStateManager : IPipeStateManager - { - public ushort Id { get; } = 0; - public ValueTask NotifyState() - { - return default; - } - - public bool UpdateState(NexusDuplexPipe.State updatedState, bool remove = false) - { - CurrentState |= updatedState; - return true; - } - - public NexusDuplexPipe.State CurrentState { get; private set; } = NexusDuplexPipe.State.Ready; - } - - private class DummySessionMessenger : ISessionMessenger - { - public Func?, ReadOnlySequence, ValueTask> OnMessageSent { get; set; } = null!; - public ValueTask SendMessage(TMessage body, CancellationToken cancellationToken = default) where TMessage : IMessageBase - { - throw new NotImplementedException(); - } - - public ValueTask SendHeaderWithBody(MessageType type, ReadOnlySequence body, CancellationToken cancellationToken = default) - { - throw new NotImplementedException(); - } - - public ValueTask SendHeader(MessageType type, CancellationToken cancellationToken = default) - { - throw new NotImplementedException(); - } - - public ValueTask SendHeaderWithBody(MessageType type, ReadOnlyMemory? messageHeader, ReadOnlySequence body, - CancellationToken cancellationToken = default) - { - if (OnMessageSent == null) - throw new InvalidOperationException("No handler for OnMessageSent"); - - return OnMessageSent.Invoke(type, messageHeader, body); - } - - public Task DisconnectAsync(DisconnectReason reason) - { - throw new NotImplementedException(); - } - } } diff --git a/src/NexNet.IntegrationTests/Pipes/NexusClientTests_NexusDuplexPipe.cs b/src/NexNet.IntegrationTests/Pipes/NexusClientTests_NexusDuplexPipe.cs index 42dfa007..e4aa1497 100644 --- a/src/NexNet.IntegrationTests/Pipes/NexusClientTests_NexusDuplexPipe.cs +++ b/src/NexNet.IntegrationTests/Pipes/NexusClientTests_NexusDuplexPipe.cs @@ -11,14 +11,16 @@ internal class NexusClientTests_NexusDuplexPipe : BasePipeTests [TestCase(Type.Tcp)] [TestCase(Type.TcpTls)] [TestCase(Type.Quic)] + [Repeat(10)] public async Task Client_PipeReaderReceivesDataMultipleTimes(Type type) { + //this.Logger.MinLogLevel = INexusLogger.LogLevel.Critical; //BlockForClose = true; - var (_, sNexus, _, cNexus, tcs) = await Setup(type, true); + var (_, sNexus, _, cNexus, tcs) = await Setup(type); int count = 0; // TODO: Review adding a test for increased iterations as this has been found to sometimes fail on CI. - const int iterations = 1000; + const int iterations = 8000; cNexus.ClientTaskValueWithDuplexPipeEvent = async (nexus, pipe) => { var result = await pipe.Input.ReadAsync().Timeout(1); @@ -41,7 +43,7 @@ public async Task Client_PipeReaderReceivesDataMultipleTimes(Type type) await pipe.Output.WriteAsync(Data).Timeout(1); } - await tcs.Task.Timeout(100); + await tcs.Task.Timeout(10); } [TestCase(Type.Uds)] @@ -75,7 +77,7 @@ public async Task Client_PipeReaderReceivesData(Type type) [TestCase(Type.Quic)] public async Task Client_PipeWriterSendsData(Type type) { - var (_, sNexus, _, cNexus, _) = await Setup(type); + var (_, sNexus, _, cNexus, _) = await Setup(type, true); cNexus.ClientTaskValueWithDuplexPipeEvent = async (nexus, pipe) => { diff --git a/src/NexNet.IntegrationTests/Pipes/NexusDuplexPipeReaderTests.cs b/src/NexNet.IntegrationTests/Pipes/NexusDuplexPipeReaderTests.cs index dae75b30..ed5b94ad 100644 --- a/src/NexNet.IntegrationTests/Pipes/NexusDuplexPipeReaderTests.cs +++ b/src/NexNet.IntegrationTests/Pipes/NexusDuplexPipeReaderTests.cs @@ -10,8 +10,8 @@ internal class NexusDuplexPipeReaderTests private NexusPipeReader CreateReader(IPipeStateManager? stateManager = null) { stateManager ??= new PipeStateManagerStub(); - var reader = new NexusPipeReader(stateManager); - reader.Setup( + var reader = new NexusPipeReader( + stateManager, null,//new ConsoleLogger(""), true, 1024 * 128, @@ -373,8 +373,9 @@ public async Task ReaderNotifiesBackPressure_HighWaterCutoff() { var stateManager = new PipeStateManagerStub(NexusDuplexPipe.State.Ready); var data = new ReadOnlySequence(new byte[1024]); - var reader = CreateReader(stateManager); - reader.Setup( + stateManager ??= new PipeStateManagerStub(); + var reader = new NexusPipeReader( + stateManager, null,//new ConsoleLogger(""), true, 1024 * 128, @@ -393,14 +394,17 @@ public async Task ReadAsyncNotifiesBackPressure_ReachesLowWaterMark() { var stateManager = new PipeStateManagerStub(NexusDuplexPipe.State.Ready); var data = new ReadOnlySequence(new byte[1024]); - var reader = CreateReader(stateManager); - reader.Setup( + stateManager ??= new PipeStateManagerStub(); + var reader = new NexusPipeReader( + stateManager, null,//new ConsoleLogger(""), true, 1024 * 128, 1024 * 1024, 1024 * 32); + + for (var j = 0; j < 10; j++) { // Reach the high water mark. diff --git a/src/NexNet.IntegrationTests/Pipes/NexusDuplexPipeWriterTests.cs b/src/NexNet.IntegrationTests/Pipes/NexusDuplexPipeWriterTests.cs index 6869d1c3..214d3f62 100644 --- a/src/NexNet.IntegrationTests/Pipes/NexusDuplexPipeWriterTests.cs +++ b/src/NexNet.IntegrationTests/Pipes/NexusDuplexPipeWriterTests.cs @@ -50,8 +50,9 @@ private NexusPipeWriter CreateWriter(SessionMessengerStub? messenger = null, IPi { stateManager ??= new PipeStateManagerStub(); messenger ??= new SessionMessengerStub(); - var writer = new NexusPipeWriter(stateManager); - writer.Setup(null, + var writer = new NexusPipeWriter( + stateManager, + null, messenger, true, 1024); @@ -84,10 +85,6 @@ public async Task WriterChunks() var simpleData = new byte[512 * 3]; var messenger = new SessionMessengerStub(); var writer = CreateWriter(messenger); - writer.Setup(null, - messenger, - true, - 1024); var invocations = 0; messenger.OnSendCustomHeaderWithBody = (type, header, body, token) => diff --git a/src/NexNet/Cache/CacheManager.cs b/src/NexNet/Cache/CacheManager.cs index ce2ec1bf..3de51bc3 100644 --- a/src/NexNet/Cache/CacheManager.cs +++ b/src/NexNet/Cache/CacheManager.cs @@ -19,8 +19,8 @@ internal class CacheManager public readonly CachedResettableItem RegisteredInvocationStateCache = new(); public readonly CachedPipeManager PipeManagerCache = new(); public readonly CachedCts CancellationTokenSourceCache = new(); - public readonly CachedDuplexPipe NexusDuplexPipeCache = new(); - public readonly CachedRentedDuplexPipe NexusRentedDuplexPipeCache = new(); + //public readonly CachedDuplexPipe NexusDuplexPipeCache = new(); + //public readonly CachedRentedDuplexPipe NexusRentedDuplexPipeCache = new(); public readonly ConcurrentBag> BufferWriterCache = new(); private readonly ICachedMessage?[] _messageCaches = new ICachedMessage?[50]; @@ -79,7 +79,5 @@ public virtual void Clear() RegisteredInvocationStateCache.Clear(); CancellationTokenSourceCache.Clear(); PipeManagerCache.Clear(); - NexusDuplexPipeCache.Clear(); - NexusRentedDuplexPipeCache.Clear(); } } diff --git a/src/NexNet/Cache/CachedDuplexPipe.cs b/src/NexNet/Cache/CachedDuplexPipe.cs deleted file mode 100644 index 3e2fa056..00000000 --- a/src/NexNet/Cache/CachedDuplexPipe.cs +++ /dev/null @@ -1,36 +0,0 @@ -using System.Collections.Concurrent; -using NexNet.Internals; -using NexNet.Pipes; - -namespace NexNet.Cache; - -internal class CachedDuplexPipe -{ - private readonly ConcurrentBag _cache = new(); - - public NexusDuplexPipe Rent(INexusSession session, byte initialId) - { - if (!_cache.TryTake(out var cachedPipe)) - cachedPipe = new NexusDuplexPipe(); - - cachedPipe.IsInCache = false; - cachedPipe.Setup(initialId, session); - - return cachedPipe; - } - - public void Return(NexusDuplexPipe pipe) - { - if(pipe.IsInCache) - return; - - pipe.IsInCache = true; - pipe.Reset(); - _cache.Add(pipe); - } - - public void Clear() - { - _cache.Clear(); - } -} diff --git a/src/NexNet/Cache/CachedRentedDuplexPipe.cs b/src/NexNet/Cache/CachedRentedDuplexPipe.cs deleted file mode 100644 index 0ab0082b..00000000 --- a/src/NexNet/Cache/CachedRentedDuplexPipe.cs +++ /dev/null @@ -1,36 +0,0 @@ -using System.Collections.Concurrent; -using NexNet.Internals; -using NexNet.Pipes; - -namespace NexNet.Cache; - -internal class CachedRentedDuplexPipe -{ - private readonly ConcurrentBag _cache = new(); - - public RentedNexusDuplexPipe Rent(INexusSession session, byte initialId) - { - if (!_cache.TryTake(out var cachedPipe)) - cachedPipe = new RentedNexusDuplexPipe(); - - cachedPipe.IsInCache = false; - cachedPipe.Setup(initialId, session); - - return cachedPipe; - } - - public void Return(RentedNexusDuplexPipe pipe) - { - if(pipe.IsInCache) - return; - - pipe.IsInCache = true; - pipe.Reset(); - _cache.Add(pipe); - } - - public void Clear() - { - _cache.Clear(); - } -} diff --git a/src/NexNet/Internals/NexusSession.Sending.cs b/src/NexNet/Internals/NexusSession.Sending.cs index 6ce7f956..2d58fa75 100644 --- a/src/NexNet/Internals/NexusSession.Sending.cs +++ b/src/NexNet/Internals/NexusSession.Sending.cs @@ -50,8 +50,10 @@ public async ValueTask SendMessage(TMessage body, CancellationToken ca { ctRegistration = cancellationToken.Register(static obj => { - Unsafe.As(obj)?.CancelPendingFlush(); - }, _pipeOutput); + var (pipeOutput, logger) = ((PipeWriter?, INexusLogger?))obj!; + logger?.LogTrace("Cancelling pending flush1"); + pipeOutput?.CancelPendingFlush(); + }, (_pipeOutput, Logger), false); } var header = _bufferWriter.GetMemory(3); @@ -81,6 +83,10 @@ public async ValueTask SendMessage(TMessage body, CancellationToken ca { // ReSharper disable once MethodSupportsCancellation result = await _pipeOutput.FlushAsync().ConfigureAwait(false); + + // Return if the operation was canceled. + if (result.IsCanceled) + return; } catch (ObjectDisposedException) { @@ -94,7 +100,7 @@ public async ValueTask SendMessage(TMessage body, CancellationToken ca OnSent?.Invoke(); - if (result.IsCanceled || result.IsCompleted) + if (result.IsCompleted) await DisconnectCore(DisconnectReason.SocketClosedWhenWriting, false).ConfigureAwait(false); } @@ -121,7 +127,11 @@ public ValueTask SendHeaderWithBody(MessageType type, ReadOnlySequence bod /// | Body | Variable | The body of the message. Its length is specified by the 'Content Length'. /// /// A ValueTask representing the asynchronous operation. - public async ValueTask SendHeaderWithBody(MessageType type, ReadOnlyMemory? messageHeader, ReadOnlySequence body, CancellationToken cancellationToken = default) + public async ValueTask SendHeaderWithBody( + MessageType type, + ReadOnlyMemory? messageHeader, + ReadOnlySequence body, + CancellationToken cancellationToken = default) { if (_pipeOutput == null || cancellationToken.IsCancellationRequested) return; @@ -140,8 +150,10 @@ public async ValueTask SendHeaderWithBody(MessageType type, ReadOnlyMemory { ctRegistration = cancellationToken.Register(static obj => { - Unsafe.As(obj)?.CancelPendingFlush(); - }, _pipeOutput); + var (pipeOutput, logger) = ((PipeWriter?, INexusLogger?))obj!; + logger?.LogTrace("Cancelling pending flush2"); + pipeOutput?.CancelPendingFlush(); + }, (_pipeOutput, Logger), false); } var length = (int)body.Length; @@ -175,11 +187,19 @@ public async ValueTask SendHeaderWithBody(MessageType type, ReadOnlyMemory { // ReSharper disable once MethodSupportsCancellation result = await _pipeOutput.FlushAsync().ConfigureAwait(false); + + // Return if the operation was canceled. + if (result.IsCanceled) + return; } catch (ObjectDisposedException) { // noop } + catch (Exception e) + { + Logger?.LogCritical(e, "Error while flushing"); + } finally { if (ctRegistration != null) @@ -188,7 +208,7 @@ public async ValueTask SendHeaderWithBody(MessageType type, ReadOnlyMemory OnSent?.Invoke(); - if (result.IsCanceled || result.IsCompleted) + if (result.IsCompleted) await DisconnectCore(DisconnectReason.SocketClosedWhenWriting, false).ConfigureAwait(false); } @@ -245,8 +265,10 @@ private async ValueTask SendHeaderCore(MessageType type, bool force, Cancellatio { ctRegistration = cancellationToken.Register(static obj => { - Unsafe.As(obj)?.CancelPendingFlush(); - }, _pipeOutput); + var (pipeOutput, logger) = ((PipeWriter?, INexusLogger?))obj!; + logger?.LogTrace("Cancelling pending flush3"); + pipeOutput?.CancelPendingFlush(); + }, (_pipeOutput, Logger), false); } _config.InternalOnSend?.Invoke(this, new[] { (byte)type }); @@ -261,6 +283,10 @@ private async ValueTask SendHeaderCore(MessageType type, bool force, Cancellatio { // ReSharper disable once MethodSupportsCancellation result = await _pipeOutput.FlushAsync().ConfigureAwait(false); + + // Return if the operation was canceled. + if (result.IsCanceled) + return; } catch (ObjectDisposedException) { @@ -274,7 +300,7 @@ private async ValueTask SendHeaderCore(MessageType type, bool force, Cancellatio OnSent?.Invoke(); - if (result.IsCanceled || result.IsCompleted) + if (result.IsCompleted) await DisconnectCore(DisconnectReason.SocketClosedWhenWriting, false).ConfigureAwait(false); } } diff --git a/src/NexNet/Invocation/ProxyInvocationBase.cs b/src/NexNet/Invocation/ProxyInvocationBase.cs index 80cae7c1..4a942455 100644 --- a/src/NexNet/Invocation/ProxyInvocationBase.cs +++ b/src/NexNet/Invocation/ProxyInvocationBase.cs @@ -362,7 +362,7 @@ protected async ValueTask __ProxyInvokeAndWaitForResultCore(us protected static byte __ProxyGetDuplexPipeInitialId(INexusDuplexPipe? pipe) { ArgumentNullException.ThrowIfNull(pipe); - return Unsafe.As(pipe).InitialId; + return Unsafe.As(pipe).LocalId; } diff --git a/src/NexNet/Pipes/NexusDuplexPipe.cs b/src/NexNet/Pipes/NexusDuplexPipe.cs index 5c88e512..8b97d121 100644 --- a/src/NexNet/Pipes/NexusDuplexPipe.cs +++ b/src/NexNet/Pipes/NexusDuplexPipe.cs @@ -7,6 +7,7 @@ using System.Threading.Tasks; using NexNet.Internals; using NexNet.Messages; +using static System.Collections.Specialized.BitVector32; using static NexNet.Pipes.NexusDuplexPipe; namespace NexNet.Pipes; @@ -14,7 +15,7 @@ namespace NexNet.Pipes; /// /// Pipe used for transmission of binary data from a one nexus to another. /// -internal class NexusDuplexPipe : INexusDuplexPipe, IPipeStateManager +internal class NexusDuplexPipe : INexusDuplexPipe, IPipeStateManager, IDisposable { /// /// Represents the state of a duplex pipe. @@ -64,27 +65,15 @@ internal enum State : byte //private readonly Pipe _inputPipe = new Pipe(); private readonly NexusPipeReader _inputPipeReader; private readonly NexusPipeWriter _outputPipeWriter; - private TaskCompletionSource? _readyTcs; - - + private readonly TaskCompletionSource? _readyTcs; + private readonly INexusSession? _session; private volatile State _currentState = State.Unset; - /*private State _currentState - { - get => __currentState; - set - { - Logger?.LogError($"{Id} Set new State from {__currentState} to {value}; From {new StackTrace().GetFrame(1).GetMethod().Name}"); - __currentState = value; - } - }*/ - - private INexusSession? _session; // // Id which changes based upon completion of the pipe. Used to make sure the // Pipe is in the same state upon completion of writing/reading. // - internal int StateId; + //internal int StateId; /// /// True if this pipe is the pipe used for initiating the pipe connection. @@ -99,7 +88,7 @@ internal enum State : byte /// /// Initial ID of this side of the connection. /// - public byte InitialId { get; set; } + public byte LocalId { get; } /// /// Gets the pipe reader for this connection. @@ -126,110 +115,44 @@ internal enum State : byte /// protected INexusLogger? Logger; - /// - /// True if the pipe is in the cache. - /// - internal bool IsInCache = false; - - internal NexusDuplexPipe() - { - _inputPipeReader = new NexusPipeReader(this); - _outputPipeWriter = new NexusPipeWriter(this); - } - - public ValueTask CompleteAsync() - { - if (_currentState == State.Complete - || _currentState == State.Unset - || IsInCache) - return default; - - var stateChanged = UpdateState(State.Complete); - if (stateChanged) - return NotifyState(); - - return default; - } - NexusPipeWriter INexusDuplexPipe.WriterCore => _outputPipeWriter; NexusPipeReader INexusDuplexPipe.ReaderCore => _inputPipeReader; - /// - /// Sets up the duplex pipe with the given parameters. - /// - /// The partial initial identifier. - /// The Nexus session. - public void Setup(byte initialId, INexusSession session) + internal NexusDuplexPipe(byte localId, INexusSession session) { - if (_currentState != State.Unset) - throw new InvalidOperationException($"Can't setup a pipe that is already in use. State:{Id} Current State {_currentState}"); - _readyTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); _session = session; Logger = session.Logger?.CreateLogger(); - InitialId = initialId; - _outputPipeWriter.Setup( - Logger, - _session, - _session.IsServer, - _session.Config.NexusPipeFlushChunkSize); + LocalId = localId; - _inputPipeReader.Setup( + _inputPipeReader = new NexusPipeReader( + this, Logger, _session.IsServer, _session.Config.NexusPipeHighWaterMark, _session.Config.NexusPipeHighWaterCutoff, _session.Config.NexusPipeLowWaterMark); - } + _outputPipeWriter = new NexusPipeWriter( + this, + Logger, + _session, + _session.IsServer, + _session.Config.NexusPipeFlushChunkSize); + } - /// - /// Signals that the pipe is ready to receive and send messages. - /// - /// The ID of the pipe. - public ValueTask PipeReady(ushort id) + public ValueTask CompleteAsync() { - if (_session == null) + if (_currentState == State.Complete + || _currentState == State.Unset) return default; - Id = id; - - if (UpdateState(State.Ready)) + var stateChanged = UpdateState(State.Complete); + if (stateChanged) return NotifyState(); return default; } - - /// - /// Resets the pipe to its initial state while incrementing the StateId to indicate that the pipe has been reset. - /// - public void Reset() - { - if(_currentState == State.Unset) - return; - - // Close everything. - if (_currentState != State.Complete) - { - Logger?.LogError($"Resetting and Sending pipe: {Id} {_currentState}"); - UpdateState(State.Complete); - } - - StateId++; - _session = null; - Id = 0; - InitialId = 0; - _currentState = State.Unset; - InitiatingPipe = false; - - // Set the task to canceled in case the pipe was reset before it was ready. - _readyTcs?.TrySetCanceled(); - - _inputPipeReader.Reset(); - _outputPipeWriter.Reset(); - - - } /// /// Writes data from upstream reader into the input pipe @@ -289,7 +212,6 @@ public bool UpdateState(State updatedState, bool remove = false) { // Get a copy of the current state. var currentState = _currentState; - var stateId = StateId; if (_session == null || (!remove && currentState.HasFlag(updatedState)) || (remove && !currentState.HasFlag(updatedState))) @@ -382,9 +304,6 @@ public bool UpdateState(State updatedState, bool remove = false) _outputPipeWriter.PauseWriting = false; } - if (updatedState == State.Complete && stateId != StateId || IsInCache) - return false; - if (remove) { // Remove the state from the current state. @@ -404,4 +323,22 @@ private static bool HasState(State has, State notHave, State flag) { return has.HasFlag(flag) && !notHave.HasFlag(flag); } + + public void Dispose() + { + _inputPipeReader.Dispose(); + _outputPipeWriter.Dispose(); + + // Close everything. + if (_currentState != State.Complete) + { + Logger?.LogError($"Resetting and Sending pipe: {Id} {_currentState}"); + UpdateState(State.Complete); + } + + // Set the task to canceled in case the pipe was reset before it was ready. + _readyTcs?.TrySetCanceled(); + + return; + } } diff --git a/src/NexNet/Pipes/NexusPipeManager.cs b/src/NexNet/Pipes/NexusPipeManager.cs index 4e3f23b3..bbc78954 100644 --- a/src/NexNet/Pipes/NexusPipeManager.cs +++ b/src/NexNet/Pipes/NexusPipeManager.cs @@ -2,6 +2,7 @@ using System.Buffers; using System.Collections; using System.Collections.Concurrent; +using System.IO.Pipelines; using System.Runtime.CompilerServices; using System.Threading.Tasks; using NexNet.Internals; @@ -11,30 +12,17 @@ namespace NexNet.Pipes; internal class NexusPipeManager { - private class PipeAndState - { - public readonly NexusDuplexPipe Pipe; - private readonly int _state; - - public bool IsStateValid => _state == Pipe.StateId; - - public PipeAndState(NexusDuplexPipe pipe) - { - Pipe = pipe; - _state = pipe.StateId; - } - } private INexusLogger? _logger; private INexusSession _session = null!; - private readonly ConcurrentDictionary _activePipes = new(); - private readonly ConcurrentDictionary _initializingPipes = new(); + private readonly ConcurrentDictionary _activePipes = new(); + //private readonly ConcurrentDictionary _initializingPipes = new(); private readonly BitArray _usedIds = new(256, false); //private readonly Stack _availableIds = new Stack(); - private int _currentId; + private int _currentId = 1; private bool _isCanceled; @@ -46,16 +34,21 @@ public void Setup(INexusSession session) _logger = session.Logger?.CreateLogger(); } + public IRentedNexusDuplexPipe? RentPipe() { if (_isCanceled) return null; - var id = GetLocalId(); - var pipe = _session.CacheManager.NexusRentedDuplexPipeCache.Rent(_session, id); - pipe.InitiatingPipe = true; - pipe.Manager = this; - _initializingPipes.TryAdd(id, new PipeAndState(pipe)); + var localId = GetNewLocalId(); + var partialId = GetPartialIdFromLocalId(localId); + var pipe = new RentedNexusDuplexPipe(localId, _session) + { + InitiatingPipe = true, + Manager = this + }; + + _activePipes.TryAdd(partialId, pipe); return pipe; } @@ -68,31 +61,24 @@ public void Setup(INexusSession session) public async ValueTask ReturnPipe(IRentedNexusDuplexPipe pipe) { _logger?.LogTrace($"ReturnPipe({pipe.Id});"); - _activePipes.TryRemove(pipe.Id, out _); - - var (clientId, serverId) = GetClientAndServerId(pipe.Id); - var localId = _session.IsServer ? serverId : clientId; - - _activePipes.TryRemove(pipe.Id, out _); - _initializingPipes.TryRemove(localId, out _); - - var nexusPipe = (RentedNexusDuplexPipe)pipe; + if (!_activePipes.TryRemove(pipe.Id, out var nexusPipe)) + return; if(nexusPipe.CurrentState != State.Complete) { await pipe.CompleteAsync().ConfigureAwait(false); } - _session.CacheManager.NexusRentedDuplexPipeCache.Return(nexusPipe); + nexusPipe.Dispose(); lock (_usedIds) { // Return the local ID to the available IDs list. - _usedIds.Set(localId, false); + _usedIds.Set(nexusPipe.LocalId, false); } - } + public async ValueTask RegisterPipe(byte otherId) { if (_session == null) @@ -102,15 +88,20 @@ public async ValueTask RegisterPipe(byte otherId) if (_isCanceled) throw new InvalidOperationException("Can't register duplex pipe due to cancellation."); - var id = GetCompleteId(otherId, out var thisId); - - var pipe = _session.CacheManager.NexusDuplexPipeCache.Rent(_session, thisId); + var id = GetCompleteId(otherId, out var localId); + var pipe = new NexusDuplexPipe(localId, _session) + { + InitiatingPipe = false, + }; - if (!_activePipes.TryAdd(id, new PipeAndState(pipe))) + if (!_activePipes.TryAdd(id, pipe)) throw new Exception("Could not add NexusDuplexPipe to the list of current pipes."); - await pipe.PipeReady(id).ConfigureAwait(false); + // Signal that the pipe is ready to receive and send messages. + pipe.UpdateState(State.Ready); + pipe.Id = id; + await pipe.NotifyState().ConfigureAwait(false); return pipe; } @@ -125,16 +116,19 @@ public async ValueTask DeregisterPipe(INexusDuplexPipe pipe) return; } - var (clientId, serverId) = GetClientAndServerId(pipe.Id); + var localId = ExtractLocalId(pipe.Id, _session.IsServer); - await nexusPipe.Pipe.CompleteAsync().ConfigureAwait(false); + if (nexusPipe.CurrentState != State.Complete) + { + await pipe.CompleteAsync().ConfigureAwait(false); + } - _session.CacheManager.NexusDuplexPipeCache.Return(nexusPipe.Pipe); + //_session.CacheManager.NexusDuplexPipeCache.Return(nexusPipe.Pipe); lock (_usedIds) { // Return the local ID to the available IDs list. - _usedIds.Set(_session.IsServer ? serverId : clientId, false); + _usedIds.Set(localId, false); } } @@ -150,17 +144,11 @@ public ValueTask BufferIncomingData(ushort id, ReadOnlySe if (_isCanceled) return new ValueTask(NexusPipeBufferResult.DataIgnored); - if (_activePipes.TryGetValue(id, out var pipeWrapper)) + if (_activePipes.TryGetValue(id, out var pipe)) { - if (!pipeWrapper.IsStateValid) - { - _logger?.LogTrace($"Ignored data due to pipe changing state form last ."); - return new ValueTask(NexusPipeBufferResult.DataIgnored); - } - // Check to see if we have exceeded the high water cutoff for the pipe. // If we have, then disconnect the connection. - return pipeWrapper.Pipe.WriteFromUpstream(data); + return pipe.WriteFromUpstream(data); } _logger?.LogError($"Received data on NexusDuplexPipe id: {id} but no stream is open on this id."); @@ -173,30 +161,24 @@ public void UpdateState(ushort id, NexusDuplexPipe.State state) if (_isCanceled) return; - if (_activePipes.TryGetValue(id, out var pipeWrapper)) + if (_activePipes.TryGetValue(id, out var pipe)) { - if (!pipeWrapper.IsStateValid) - { - _logger?.LogTrace($"State update of {state} ignored due to state change."); - return; - } - - pipeWrapper.Pipe.UpdateState(state); + pipe.UpdateState(state); } else { - var (clientId, serverId) = GetClientAndServerId(id); - var thisId = _session.IsServer ? serverId : clientId; - if (!_initializingPipes.TryRemove(thisId, out var initialPipe)) + var localIdByte = ExtractLocalId(id, _session.IsServer); + var partialId = GetPartialIdFromLocalId(localIdByte); + if (!_activePipes.TryRemove(partialId, out pipe)) { - _logger?.LogTrace($"Could not find pipe with initial ID of {thisId}"); + _logger?.LogError($"Could not find pipe with initial ID of {localIdByte}"); return; } // Move the pipe to the main active pipes. - _activePipes.TryAdd(id, initialPipe); - initialPipe.Pipe.Id = id; - initialPipe.Pipe.UpdateState(state); + _activePipes.TryAdd(id, pipe); + pipe.Id = id; + pipe.UpdateState(state); } } @@ -205,32 +187,9 @@ public void CancelAll() _logger?.LogTrace($"CancelAll();"); _isCanceled = true; - // Update all the states of the pipes to complete. - foreach (var pipeWrapper in _initializingPipes) + foreach (var pipe in _activePipes) { - if (!pipeWrapper.Value.IsStateValid) - { - _logger?.LogTrace($"Did not cancel initializing pipe {pipeWrapper.Key} due to state change."); - continue; - } - - pipeWrapper.Value.Pipe.UpdateState(NexusDuplexPipe.State.Complete); - } - - _initializingPipes.Clear(); - - foreach (var pipeWrapper in _activePipes) - { - if (!pipeWrapper.Value.IsStateValid) - { - _logger?.LogTrace($"Did not cancel active pipe {pipeWrapper.Key} due to state change."); - continue; - } - - pipeWrapper.Value.Pipe.UpdateState(NexusDuplexPipe.State.Complete); - // Todo: Return the pipe to the cache. Can't in this flow here since it will reset the - // result to not completed while a reading process is still ongoing. - //_session.CacheManager.NexusDuplexPipeCache.Return(pipe.Value); + pipe.Value.UpdateState(NexusDuplexPipe.State.Complete); } _activePipes.Clear(); @@ -243,11 +202,11 @@ private ushort GetCompleteId(byte otherId, out byte thisId) if (_session.IsServer) { idBytes[0] = otherId; // Client - thisId = idBytes[1] = GetLocalId(); // Server + thisId = idBytes[1] = GetNewLocalId(); // Server } else { - thisId = idBytes[0] = GetLocalId(); // Client + thisId = idBytes[0] = GetNewLocalId(); // Client idBytes[1] = otherId; // Server } @@ -255,14 +214,41 @@ private ushort GetCompleteId(byte otherId, out byte thisId) } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static (byte ClientId, byte ServerId) GetClientAndServerId(ushort id) + private ushort GetPartialIdFromLocalId(byte localId) + { + Span idBytes = stackalloc byte[sizeof(ushort)]; + if (_session.IsServer) + { + idBytes[0] = 0; // Client + idBytes[1] = localId; // Server + } + else + { + idBytes[0] = localId; // Client + idBytes[1] = 0; // Server + } + + return BitConverter.ToUInt16(idBytes); + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static (byte ClientId, byte ServerId) ExtractClientAndServerId(ushort id) { Span bytes = stackalloc byte[sizeof(ushort)]; Unsafe.As(ref bytes[0]) = id; return (bytes[0], bytes[1]); } - private byte GetLocalId() + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static byte ExtractLocalId(ushort id, bool isServer) + { + Span bytes = stackalloc byte[sizeof(ushort)]; + Unsafe.As(ref bytes[0]) = id; + return isServer ? bytes[1] : bytes[0]; + } + + + private byte GetNewLocalId() { lock (_usedIds) { @@ -272,7 +258,7 @@ private byte GetLocalId() { // Loop around if we reach the end of the available IDs. if (++incrementedId == 256) - incrementedId = 0; + incrementedId = 1; // If the Id is not available, then try the next one. if (_usedIds.Get(incrementedId)) diff --git a/src/NexNet/Pipes/NexusPipeReader.cs b/src/NexNet/Pipes/NexusPipeReader.cs index ce3c5860..137cce83 100644 --- a/src/NexNet/Pipes/NexusPipeReader.cs +++ b/src/NexNet/Pipes/NexusPipeReader.cs @@ -10,7 +10,7 @@ namespace NexNet.Pipes; -internal class NexusPipeReader : PipeReader +internal class NexusPipeReader : PipeReader, IDisposable { //private readonly NexusDuplexPipe _nexusDuplexPipe; private readonly SemaphoreSlim _readSemaphore = new SemaphoreSlim(0, 1); @@ -24,13 +24,13 @@ private record CancellationRegistrationArgs(SemaphoreSlim Semaphore); private bool _isCompleted; private bool _isCanceled; - private int _highWaterMark; - private int _highWaterCutoff; - private int _lowWaterMark; + private readonly int _highWaterMark; + private readonly int _highWaterCutoff; + private readonly int _lowWaterMark; - private NexusDuplexPipe.State _backPressureFlag; - private NexusDuplexPipe.State _writingCompleteFlag; - private INexusLogger? _logger; + private readonly NexusDuplexPipe.State _backPressureFlag; + private readonly NexusDuplexPipe.State _writingCompleteFlag; + private readonly INexusLogger? _logger; private long _examinedPosition; private long _bufferTailPosition; @@ -51,20 +51,20 @@ public long BufferedLength /// public bool IsCompleted => _isCompleted; - public NexusPipeReader(IPipeStateManager stateManager) + public NexusPipeReader( + IPipeStateManager stateManager, + INexusLogger? logger, + bool isServer, + int highWaterMark, + int highWaterCutoff, + int lowWaterMark) { _stateManager = stateManager; _cancelReadingArgs = new CancellationRegistrationArgs(_readSemaphore); - } - - public void Setup(INexusLogger? logger, bool isServer, int highWaterMark, int highWaterCutoff, int lowWaterMark) - { _logger = logger; _highWaterMark = highWaterMark; //_session!.Config.NexusPipeHighWaterMark; _highWaterCutoff = highWaterCutoff; //_session!.Config.NexusPipeHighWaterCutoff; _lowWaterMark = lowWaterMark; //_session!.Config.NexusPipeLowWaterMark; - _examinedPosition = 0; - _bufferTailPosition = 0; _backPressureFlag = !isServer ? NexusDuplexPipe.State.ServerWriterPause : NexusDuplexPipe.State.ClientWriterPause; @@ -72,24 +72,7 @@ public void Setup(INexusLogger? logger, bool isServer, int highWaterMark, int hi _writingCompleteFlag = isServer ? NexusDuplexPipe.State.ClientWriterServerReaderComplete : NexusDuplexPipe.State.ClientReaderServerWriterComplete; - } - - /// - /// Resets the reader to it's initial state for reuse. - /// - public void Reset() - { - _isCompleted = false; - _isCanceled = false; - _logger = null; - - lock (_buffer) - { - _buffer.Reset(); - } - // Reset the semaphore to it's original state. - _readSemaphore.Wait(0); } /// @@ -423,4 +406,13 @@ public override void CancelPendingRead() _isCanceled = true; Utilities.TryReleaseSemaphore(_readSemaphore); } + + public void Dispose() + { + _readSemaphore.Dispose(); + lock (_buffer) + { + _buffer.Reset(); + } + } } diff --git a/src/NexNet/Pipes/NexusPipeWriter.cs b/src/NexNet/Pipes/NexusPipeWriter.cs index 4ac2f9cf..a83e0a7e 100644 --- a/src/NexNet/Pipes/NexusPipeWriter.cs +++ b/src/NexNet/Pipes/NexusPipeWriter.cs @@ -9,7 +9,7 @@ namespace NexNet.Pipes; -internal class NexusPipeWriter : PipeWriter +internal class NexusPipeWriter : PipeWriter, IDisposable { private readonly IPipeStateManager _stateManager; @@ -18,17 +18,13 @@ internal class NexusPipeWriter : PipeWriter private bool _isCompleted; private CancellationTokenSource? _flushCts; private readonly Memory _pipeId = new byte[sizeof(ushort)]; - private int _chunkSize; + private readonly int _chunkSize; private readonly SemaphoreSlim _pauseSemaphore = new SemaphoreSlim(0, 1); private bool _pauseWriting; - private INexusLogger? _logger; - private ISessionMessenger? _messenger; - private NexusDuplexPipe.State _completedFlag; - - public NexusPipeWriter(IPipeStateManager stateManager) - { - _stateManager = stateManager; - } + private readonly INexusLogger? _logger; + private readonly ISessionMessenger? _messenger; + private readonly NexusDuplexPipe.State _completedFlag; + private bool _hasPipeId; /// /// Set to true to pause writing to the pipe. @@ -46,11 +42,14 @@ public bool PauseWriting } } - /// - /// Sets up the pipe writer for use. - /// - public void Setup(INexusLogger? logger, ISessionMessenger sessionMessenger, bool isServer, int chunkSize) + public NexusPipeWriter( + IPipeStateManager stateManager, + INexusLogger? logger, + ISessionMessenger sessionMessenger, + bool isServer, int chunkSize) { + _stateManager = stateManager; + _messenger = sessionMessenger ?? throw new ArgumentNullException(nameof(sessionMessenger)); _logger = logger; _chunkSize = chunkSize; // _session.Config.NexusPipeFlushChunkSize; @@ -60,23 +59,6 @@ public void Setup(INexusLogger? logger, ISessionMessenger sessionMessenger, bool : NexusDuplexPipe.State.ClientWriterServerReaderComplete; } - /// - /// Resets the pipe writer for reuse. - /// - public void Reset() - { - _bufferWriter.Reset(); - _isCanceled = false; - _isCompleted = false; - _flushCts?.Dispose(); - _flushCts = null; - PauseWriting = false; - - // Reset the pause Semaphore back to 0. - _pauseSemaphore.Wait(0); - } - - [MethodImpl(MethodImplOptions.AggressiveInlining)] public override void Advance(int bytes) { @@ -126,17 +108,11 @@ public void SetComplete() _isCompleted = true; } - public override async ValueTask FlushAsync( - CancellationToken cancellationToken = new CancellationToken()) + public override async ValueTask FlushAsync(CancellationToken cancellationToken = default) { if (_isCompleted) return new FlushResult(_isCanceled, _isCompleted); - static void CancelCallback(object? ctsObject) - { - Unsafe.As(ctsObject)!.Cancel(); - } - if (_flushCts?.IsCancellationRequested == true) return new FlushResult(_isCanceled, _isCompleted); @@ -150,7 +126,19 @@ static void CancelCallback(object? ctsObject) // ReSharper disable once UseAwaitUsing // TODO: Review only calling when the token can be canceled. - using var reg = cancellationToken.Register(CancelCallback, _flushCts); + + CancellationTokenRegistration? cancellationTokenRegistration = null; + + // Only register the cancellation token if it can be canceled. + if (cancellationToken.CanBeCanceled) + { + cancellationTokenRegistration = cancellationToken.Register(static (object? ctsObject) => + { + var (cts, logger) = ((CancellationTokenSource, INexusLogger?))ctsObject!; + logger?.LogTrace("Canceled Nexus Pipe Writer"); + cts.Cancel(); + }, (_flushCts, _logger)); + } // If we are paused, wait for the semaphore to be released. if (PauseWriting) @@ -166,9 +154,18 @@ static void CancelCallback(object? ctsObject) _flushCts = null; return new FlushResult(true, _isCompleted); } + finally + { + if(cancellationTokenRegistration != null) + await cancellationTokenRegistration.Value.DisposeAsync(); + } } - BitConverter.TryWriteBytes(_pipeId.Span, _stateManager.Id); + if (!_hasPipeId) + { + BitConverter.TryWriteBytes(_pipeId.Span, _stateManager.Id); + _hasPipeId = true; + } var buffer = _bufferWriter.GetBuffer(); @@ -181,7 +178,12 @@ static void CancelCallback(object? ctsObject) var flushPosition = 0; if (_messenger == null) + { + if (cancellationTokenRegistration != null) + await cancellationTokenRegistration.Value.DisposeAsync(); + throw new InvalidOperationException("Session is null."); + } while (true) { @@ -234,6 +236,16 @@ await _messenger.SendHeaderWithBody( await _stateManager.NotifyState().ConfigureAwait(false); } + if(cancellationTokenRegistration != null) + await cancellationTokenRegistration.Value.DisposeAsync(); + return new FlushResult(_isCanceled, _isCompleted); } + + public void Dispose() + { + _bufferWriter.Dispose(); + _flushCts?.Dispose(); + _pauseSemaphore.Dispose(); + } } diff --git a/src/NexNet/Pipes/RentedNexusDuplexPipe.cs b/src/NexNet/Pipes/RentedNexusDuplexPipe.cs index fe699256..da929d3b 100644 --- a/src/NexNet/Pipes/RentedNexusDuplexPipe.cs +++ b/src/NexNet/Pipes/RentedNexusDuplexPipe.cs @@ -1,15 +1,14 @@ using System.Threading; using System.Threading.Tasks; +using NexNet.Internals; namespace NexNet.Pipes; internal class RentedNexusDuplexPipe : NexusDuplexPipe, IRentedNexusDuplexPipe { - private readonly int _rentedStateId; - - public RentedNexusDuplexPipe() + public RentedNexusDuplexPipe(byte localId, INexusSession session) + : base(localId, session) { - _rentedStateId = StateId; } internal NexusPipeManager? Manager; @@ -19,12 +18,7 @@ public ValueTask DisposeAsync() var manager = Interlocked.Exchange(ref Manager, null); if (manager == null) - { - //Logger?.LogError($"Failed to dispose rented pipe {_rentedStateId} {StateId}"); return default; - } - - //Logger?.LogError($"Returning rented pipe {_rentedStateId} {StateId}"); return manager!.ReturnPipe(this); } From 19f688db63820ae8067b907a20ade0b06095dfdf Mon Sep 17 00:00:00 2001 From: DJGosnell Date: Thu, 14 Sep 2023 18:13:05 -0400 Subject: [PATCH 03/15] cleanup --- .../Pipes/NexusClientTests_NexusDuplexPipe.cs | 1 - src/NexNet/Internals/NexusSession.Sending.cs | 18 ++++++------------ src/NexNet/Pipes/NexusPipeReader.cs | 1 - src/NexNet/Pipes/NexusPipeWriter.cs | 6 ++---- 4 files changed, 8 insertions(+), 18 deletions(-) diff --git a/src/NexNet.IntegrationTests/Pipes/NexusClientTests_NexusDuplexPipe.cs b/src/NexNet.IntegrationTests/Pipes/NexusClientTests_NexusDuplexPipe.cs index e4aa1497..d63333a6 100644 --- a/src/NexNet.IntegrationTests/Pipes/NexusClientTests_NexusDuplexPipe.cs +++ b/src/NexNet.IntegrationTests/Pipes/NexusClientTests_NexusDuplexPipe.cs @@ -11,7 +11,6 @@ internal class NexusClientTests_NexusDuplexPipe : BasePipeTests [TestCase(Type.Tcp)] [TestCase(Type.TcpTls)] [TestCase(Type.Quic)] - [Repeat(10)] public async Task Client_PipeReaderReceivesDataMultipleTimes(Type type) { //this.Logger.MinLogLevel = INexusLogger.LogLevel.Critical; diff --git a/src/NexNet/Internals/NexusSession.Sending.cs b/src/NexNet/Internals/NexusSession.Sending.cs index 2d58fa75..511467f5 100644 --- a/src/NexNet/Internals/NexusSession.Sending.cs +++ b/src/NexNet/Internals/NexusSession.Sending.cs @@ -50,10 +50,8 @@ public async ValueTask SendMessage(TMessage body, CancellationToken ca { ctRegistration = cancellationToken.Register(static obj => { - var (pipeOutput, logger) = ((PipeWriter?, INexusLogger?))obj!; - logger?.LogTrace("Cancelling pending flush1"); - pipeOutput?.CancelPendingFlush(); - }, (_pipeOutput, Logger), false); + Unsafe.As(obj)?.CancelPendingFlush(); + }, _pipeOutput, false); } var header = _bufferWriter.GetMemory(3); @@ -150,10 +148,8 @@ public async ValueTask SendHeaderWithBody( { ctRegistration = cancellationToken.Register(static obj => { - var (pipeOutput, logger) = ((PipeWriter?, INexusLogger?))obj!; - logger?.LogTrace("Cancelling pending flush2"); - pipeOutput?.CancelPendingFlush(); - }, (_pipeOutput, Logger), false); + Unsafe.As(obj)?.CancelPendingFlush(); + }, _pipeOutput, false); } var length = (int)body.Length; @@ -265,10 +261,8 @@ private async ValueTask SendHeaderCore(MessageType type, bool force, Cancellatio { ctRegistration = cancellationToken.Register(static obj => { - var (pipeOutput, logger) = ((PipeWriter?, INexusLogger?))obj!; - logger?.LogTrace("Cancelling pending flush3"); - pipeOutput?.CancelPendingFlush(); - }, (_pipeOutput, Logger), false); + Unsafe.As(obj)?.CancelPendingFlush(); + }, _pipeOutput, false); } _config.InternalOnSend?.Invoke(this, new[] { (byte)type }); diff --git a/src/NexNet/Pipes/NexusPipeReader.cs b/src/NexNet/Pipes/NexusPipeReader.cs index 137cce83..9daeb5e0 100644 --- a/src/NexNet/Pipes/NexusPipeReader.cs +++ b/src/NexNet/Pipes/NexusPipeReader.cs @@ -149,7 +149,6 @@ public async ValueTask BufferData(ReadOnlySequence _logger?.LogTrace($"Pipe {_stateManager.Id} has buffered {length} new bytes."); - // Copy the data to the buffer. data.CopyTo(_buffer.GetSpan(length)); _buffer.Advance(length); diff --git a/src/NexNet/Pipes/NexusPipeWriter.cs b/src/NexNet/Pipes/NexusPipeWriter.cs index a83e0a7e..ffd01d42 100644 --- a/src/NexNet/Pipes/NexusPipeWriter.cs +++ b/src/NexNet/Pipes/NexusPipeWriter.cs @@ -134,10 +134,8 @@ public override async ValueTask FlushAsync(CancellationToken cancel { cancellationTokenRegistration = cancellationToken.Register(static (object? ctsObject) => { - var (cts, logger) = ((CancellationTokenSource, INexusLogger?))ctsObject!; - logger?.LogTrace("Canceled Nexus Pipe Writer"); - cts.Cancel(); - }, (_flushCts, _logger)); + Unsafe.As(ctsObject)?.Cancel(); + }, _flushCts); } // If we are paused, wait for the semaphore to be released. From 0140c85bb7fde5887e6c8b15bc260bd3d7886d35 Mon Sep 17 00:00:00 2001 From: DJGosnell Date: Fri, 15 Sep 2023 11:11:18 -0400 Subject: [PATCH 04/15] Fixed issue with the pipe flush canceling the connection sometimes. --- .../Pipes/NexusChannelReaderUnmanagedTests.cs | 16 +++--- .../Pipes/NexusChannelReaderWriterTestBase.cs | 9 ++-- .../Pipes/NexusClientTests_NexusDuplexPipe.cs | 9 ++-- .../Pipes/NexusServerTests_NexusDuplexPipe.cs | 4 +- src/NexNet/Internals/ISessionMessenger.cs | 1 + .../Internals/NexusSession.Receiving.cs | 4 +- src/NexNet/Internals/NexusSession.Sending.cs | 4 +- src/NexNet/Internals/NexusSession.cs | 1 + .../SessionInvocationStateManager.cs | 1 - src/NexNet/Pipes/NexusDuplexPipe.cs | 17 +++---- src/NexNet/Pipes/NexusPipeManager.cs | 49 ++++++++++++++----- src/NexNet/Pipes/NexusPipeWriter.cs | 5 +- 12 files changed, 76 insertions(+), 44 deletions(-) diff --git a/src/NexNet.IntegrationTests/Pipes/NexusChannelReaderUnmanagedTests.cs b/src/NexNet.IntegrationTests/Pipes/NexusChannelReaderUnmanagedTests.cs index e8078277..4363636a 100644 --- a/src/NexNet.IntegrationTests/Pipes/NexusChannelReaderUnmanagedTests.cs +++ b/src/NexNet.IntegrationTests/Pipes/NexusChannelReaderUnmanagedTests.cs @@ -19,7 +19,7 @@ internal class NexusChannelReaderUnmanagedTests public async Task ReadsData(T inputData) where T : unmanaged { - var pipeReader = new NexusPipeReader(new DummyPipeStateManager(), new ConsoleLogger(), true, 0, 0, 0); + var pipeReader = new NexusPipeReader(new DummyPipeStateManager(), null, true, 0, 0, 0); var reader = new NexusChannelReaderUnmanaged(pipeReader); await pipeReader.BufferData(Utilities.GetBytes(inputData)); @@ -32,7 +32,7 @@ public async Task ReadsData(T inputData) public async Task CancelsReadDelayed() where T : unmanaged { - var pipeReader = new NexusPipeReader(new DummyPipeStateManager(), new ConsoleLogger(), true, 0, 0, 0); + var pipeReader = new NexusPipeReader(new DummyPipeStateManager(), null, true, 0, 0, 0); var reader = new NexusChannelReaderUnmanaged(pipeReader); var cts = new CancellationTokenSource(100); var result = await reader.ReadAsync(cts.Token).Timeout(1); @@ -45,7 +45,7 @@ public async Task CancelsReadDelayed() [Test] public async Task CancelsReadImmediate() { - var pipeReader = new NexusPipeReader(new DummyPipeStateManager(), new ConsoleLogger(), true, 0, 0, 0); + var pipeReader = new NexusPipeReader(new DummyPipeStateManager(), null, true, 0, 0, 0); var reader = new NexusChannelReaderUnmanaged(pipeReader); var cts = new CancellationTokenSource(100); cts.Cancel(); @@ -59,7 +59,7 @@ public async Task CancelsReadImmediate() [Test] public async Task Completes() { - var pipeReader = new NexusPipeReader(new DummyPipeStateManager(), new ConsoleLogger(), true, 0, 0, 0); + var pipeReader = new NexusPipeReader(new DummyPipeStateManager(), null, true, 0, 0, 0); var reader = new NexusChannelReaderUnmanaged(pipeReader); // ReSharper disable once MethodHasAsyncOverload @@ -87,7 +87,7 @@ public async Task WaitsForFullData(T inputData) where T : unmanaged { var tcs = new TaskCompletionSource(); - var pipeReader = new NexusPipeReader(new DummyPipeStateManager(), new ConsoleLogger(), true, 0, 0, 0); + var pipeReader = new NexusPipeReader(new DummyPipeStateManager(), null, true, 0, 0, 0); var reader = new NexusChannelReaderUnmanaged(pipeReader); _ = Task.Run(async () => @@ -121,7 +121,7 @@ public async Task ReadsMultiple(T inputData) where T : unmanaged { const int iterations = 1000; - var pipeReader = new NexusPipeReader(new DummyPipeStateManager(), new ConsoleLogger(), true, 0, 0, 0); + var pipeReader = new NexusPipeReader(new DummyPipeStateManager(), null, true, 0, 0, 0); var reader = new NexusChannelReaderUnmanaged(pipeReader); var data = Utilities.GetBytes(inputData); var count = 0; @@ -155,7 +155,7 @@ public async Task ReadsMultipleParallel(T inputData) where T : unmanaged { const int iterations = 1000; - var pipeReader = new NexusPipeReader(new DummyPipeStateManager(), new ConsoleLogger(), true, 0, 0, 0); + var pipeReader = new NexusPipeReader(new DummyPipeStateManager(), null, true, 0, 0, 0); var reader = new NexusChannelReaderUnmanaged(pipeReader); var data = Utilities.GetBytes(inputData); var count = 0; @@ -202,7 +202,7 @@ await Task.Run(async () => public async Task ReadsWithPartialWrites(T inputData) where T : unmanaged { - var pipeReader = new NexusPipeReader(new DummyPipeStateManager(), new ConsoleLogger(), true, 0, 0, 0); + var pipeReader = new NexusPipeReader(new DummyPipeStateManager(), null, true, 0, 0, 0); var reader = new NexusChannelReaderUnmanaged(pipeReader); var data = Utilities.GetBytes(inputData); diff --git a/src/NexNet.IntegrationTests/Pipes/NexusChannelReaderWriterTestBase.cs b/src/NexNet.IntegrationTests/Pipes/NexusChannelReaderWriterTestBase.cs index e76be849..ed9e42fe 100644 --- a/src/NexNet.IntegrationTests/Pipes/NexusChannelReaderWriterTestBase.cs +++ b/src/NexNet.IntegrationTests/Pipes/NexusChannelReaderWriterTestBase.cs @@ -2,6 +2,7 @@ using NexNet.Internals; using NexNet.Messages; using NexNet.Pipes; +using NUnit.Framework.Internal; namespace NexNet.IntegrationTests.Pipes; @@ -60,7 +61,7 @@ public Task DisconnectAsync(DisconnectReason reason) } } - protected (NexusPipeWriter, NexusPipeReader) GetConnectedPipeReaderWriter() + protected (NexusPipeWriter, NexusPipeReader) GetConnectedPipeReaderWriter(bool log = false) { NexusPipeWriter nexusPipeWriter = null!; NexusPipeReader nexusPipeReader = null!; @@ -73,14 +74,14 @@ public Task DisconnectAsync(DisconnectReason reason) } }; nexusPipeWriter = new NexusPipeWriter( - new DummyPipeStateManager(), - new ConsoleLogger(), + new DummyPipeStateManager(), + log ? new ConsoleLogger() : null, messenger, true, ushort.MaxValue); nexusPipeReader = new NexusPipeReader( new DummyPipeStateManager(), - new ConsoleLogger(), + log ? new ConsoleLogger() : null, true, ushort.MaxValue, 0, diff --git a/src/NexNet.IntegrationTests/Pipes/NexusClientTests_NexusDuplexPipe.cs b/src/NexNet.IntegrationTests/Pipes/NexusClientTests_NexusDuplexPipe.cs index d63333a6..366c8455 100644 --- a/src/NexNet.IntegrationTests/Pipes/NexusClientTests_NexusDuplexPipe.cs +++ b/src/NexNet.IntegrationTests/Pipes/NexusClientTests_NexusDuplexPipe.cs @@ -13,13 +13,14 @@ internal class NexusClientTests_NexusDuplexPipe : BasePipeTests [TestCase(Type.Quic)] public async Task Client_PipeReaderReceivesDataMultipleTimes(Type type) { + //this.Logger.MinLogLevel = INexusLogger.LogLevel.Critical; //BlockForClose = true; var (_, sNexus, _, cNexus, tcs) = await Setup(type); int count = 0; - + // TODO: Review adding a test for increased iterations as this has been found to sometimes fail on CI. - const int iterations = 8000; + const int iterations = 10000; cNexus.ClientTaskValueWithDuplexPipeEvent = async (nexus, pipe) => { var result = await pipe.Input.ReadAsync().Timeout(1); @@ -30,7 +31,7 @@ public async Task Client_PipeReaderReceivesDataMultipleTimes(Type type) Assert.AreEqual(Data, result.Buffer.ToArray()); } - if(++count == iterations) + if (++count == iterations) tcs.SetResult(); }; @@ -42,7 +43,7 @@ public async Task Client_PipeReaderReceivesDataMultipleTimes(Type type) await pipe.Output.WriteAsync(Data).Timeout(1); } - await tcs.Task.Timeout(10); + await tcs.Task.Timeout(1); } [TestCase(Type.Uds)] diff --git a/src/NexNet.IntegrationTests/Pipes/NexusServerTests_NexusDuplexPipe.cs b/src/NexNet.IntegrationTests/Pipes/NexusServerTests_NexusDuplexPipe.cs index 4db874ca..835584fa 100644 --- a/src/NexNet.IntegrationTests/Pipes/NexusServerTests_NexusDuplexPipe.cs +++ b/src/NexNet.IntegrationTests/Pipes/NexusServerTests_NexusDuplexPipe.cs @@ -16,7 +16,7 @@ public async Task Server_PipeReaderReceivesDataMultipleTimes(Type type) var count = 0; // TODO: Review adding a test for increased iterations as this has been found to sometimes fail on CI. - const int iterations = 10; + const int iterations = 10000; sNexus.ServerTaskValueWithDuplexPipeEvent = async (nexus, pipe) => { var result = await pipe.Input.ReadAsync().Timeout(1); @@ -118,7 +118,6 @@ public async Task Server_PipeReaderCompletesUponCompleteAsync(Type type) [TestCase(Type.Quic)] public async Task Server_PipeWriterCompletesUponCompleteAsync(Type type) { - //Console.WriteLine("Starting test"); var (_, sNexus, _, cNexus, tcs) = await Setup(type); var completedTcs = new TaskCompletionSource(); @@ -132,7 +131,6 @@ public async Task Server_PipeWriterCompletesUponCompleteAsync(Type type) { result = await pipe.Output.WriteAsync(Data).Timeout(1); - //Console.WriteLine($"Result Comp:{result.IsCompleted}, Can:{result.IsCanceled}"); if (result.IsCompleted) break; diff --git a/src/NexNet/Internals/ISessionMessenger.cs b/src/NexNet/Internals/ISessionMessenger.cs index b64d8874..530bc1b4 100644 --- a/src/NexNet/Internals/ISessionMessenger.cs +++ b/src/NexNet/Internals/ISessionMessenger.cs @@ -1,5 +1,6 @@ using System; using System.Buffers; +using System.Runtime.CompilerServices; using System.Threading; using System.Threading.Tasks; using NexNet.Messages; diff --git a/src/NexNet/Internals/NexusSession.Receiving.cs b/src/NexNet/Internals/NexusSession.Receiving.cs index 9b900e9d..32e1cd6c 100644 --- a/src/NexNet/Internals/NexusSession.Receiving.cs +++ b/src/NexNet/Internals/NexusSession.Receiving.cs @@ -437,7 +437,9 @@ static async void InvocationTask(object? argumentsObj) case MessageType.DuplexPipeUpdateState: { var updateStateMessage = message.As(); - PipeManager.UpdateState(updateStateMessage.PipeId, updateStateMessage.State); + var updateStateResult = PipeManager.UpdateState(updateStateMessage.PipeId, updateStateMessage.State); + if (updateStateResult != DisconnectReason.None) + return updateStateResult; break; } diff --git a/src/NexNet/Internals/NexusSession.Sending.cs b/src/NexNet/Internals/NexusSession.Sending.cs index 511467f5..fdea26c2 100644 --- a/src/NexNet/Internals/NexusSession.Sending.cs +++ b/src/NexNet/Internals/NexusSession.Sending.cs @@ -7,6 +7,7 @@ using System.Runtime.CompilerServices; using MemoryPack; using Pipelines.Sockets.Unofficial.Arenas; +using System.Diagnostics; namespace NexNet.Internals; @@ -74,13 +75,14 @@ public async ValueTask SendMessage(TMessage body, CancellationToken ca _bufferWriter.Reset(); _pipeOutput.Advance(length); - Logger?.LogTrace($"Sending {TMessage.Type} header and body with {length} total bytes."); + Logger?.LogTrace($"Sending {TMessage.Type} header and body with {length} total bytes."); FlushResult result = default; try { // ReSharper disable once MethodSupportsCancellation result = await _pipeOutput.FlushAsync().ConfigureAwait(false); + Logger?.LogTrace($"FlushResult: {result.IsCanceled}, {result.IsCompleted}"); // Return if the operation was canceled. if (result.IsCanceled) diff --git a/src/NexNet/Internals/NexusSession.cs b/src/NexNet/Internals/NexusSession.cs index ee734ebf..2db0615d 100644 --- a/src/NexNet/Internals/NexusSession.cs +++ b/src/NexNet/Internals/NexusSession.cs @@ -8,6 +8,7 @@ using System.Threading; using System; using System.Collections.Concurrent; +using System.Diagnostics; using Pipelines.Sockets.Unofficial.Threading; using Pipelines.Sockets.Unofficial.Buffers; using System.Runtime.CompilerServices; diff --git a/src/NexNet/Invocation/SessionInvocationStateManager.cs b/src/NexNet/Invocation/SessionInvocationStateManager.cs index ed3c9f6a..ea9b57ba 100644 --- a/src/NexNet/Invocation/SessionInvocationStateManager.cs +++ b/src/NexNet/Invocation/SessionInvocationStateManager.cs @@ -121,7 +121,6 @@ public void UpdateInvocationResult(InvocationResultMessage message) { static void Callback(object? arg) { - //Console.WriteLine("Canceled"); var state = (RegisteredInvocationState)arg!; state.TrySetCanceled(true); } diff --git a/src/NexNet/Pipes/NexusDuplexPipe.cs b/src/NexNet/Pipes/NexusDuplexPipe.cs index 8b97d121..210d60d3 100644 --- a/src/NexNet/Pipes/NexusDuplexPipe.cs +++ b/src/NexNet/Pipes/NexusDuplexPipe.cs @@ -65,7 +65,7 @@ internal enum State : byte //private readonly Pipe _inputPipe = new Pipe(); private readonly NexusPipeReader _inputPipeReader; private readonly NexusPipeWriter _outputPipeWriter; - private readonly TaskCompletionSource? _readyTcs; + private readonly TaskCompletionSource _readyTcs; private readonly INexusSession? _session; private volatile State _currentState = State.Unset; @@ -103,7 +103,7 @@ internal enum State : byte /// /// Task which completes when the pipe is ready for usage on the invoking side. /// - public Task ReadyTask => _readyTcs!.Task; + public Task ReadyTask => _readyTcs.Task; /// /// State of the pipe. @@ -222,17 +222,16 @@ public bool UpdateState(State updatedState, bool remove = false) var isServer = _session.IsServer; - Logger?.LogError($"{Id}: Current State: {currentState}; Update State: {updatedState}; From {new StackTrace().GetFrame(1).GetMethod().Name}"); + Logger?.LogTrace($"{Id}: Current State: {currentState}; Update State: {updatedState};"); if (currentState == State.Unset) { if (updatedState == State.Ready) { - Logger?.LogError($"--Updated {Id} state to ready from Unset to: {updatedState}"); _currentState = State.Ready; // Set the ready task. - _readyTcs?.TrySetResult(); + _readyTcs.TrySetResult(); return true; } @@ -242,7 +241,7 @@ public bool UpdateState(State updatedState, bool remove = false) // This normally happens when the pipe is reset before it is ready or after it has been reset. // Honestly, we shouldn't reach here. Logger?.LogTrace($"Ignored update state of : {updatedState} because the pipe was never readied."); - _readyTcs?.TrySetCanceled(); + _readyTcs.TrySetCanceled(); return false; } } @@ -326,6 +325,9 @@ private static bool HasState(State has, State notHave, State flag) public void Dispose() { + // Set the task to canceled in case the pipe was reset before it was ready. + _readyTcs.TrySetCanceled(); + _inputPipeReader.Dispose(); _outputPipeWriter.Dispose(); @@ -336,9 +338,6 @@ public void Dispose() UpdateState(State.Complete); } - // Set the task to canceled in case the pipe was reset before it was ready. - _readyTcs?.TrySetCanceled(); - return; } } diff --git a/src/NexNet/Pipes/NexusPipeManager.cs b/src/NexNet/Pipes/NexusPipeManager.cs index bbc78954..ba1e623a 100644 --- a/src/NexNet/Pipes/NexusPipeManager.cs +++ b/src/NexNet/Pipes/NexusPipeManager.cs @@ -6,6 +6,7 @@ using System.Runtime.CompilerServices; using System.Threading.Tasks; using NexNet.Internals; +using NexNet.Messages; using static NexNet.Pipes.NexusDuplexPipe; namespace NexNet.Pipes; @@ -22,7 +23,7 @@ internal class NexusPipeManager private readonly BitArray _usedIds = new(256, false); //private readonly Stack _availableIds = new Stack(); - private int _currentId = 1; + private int _currentId = 0; private bool _isCanceled; @@ -156,10 +157,10 @@ public ValueTask BufferIncomingData(ushort id, ReadOnlySe return new ValueTask(NexusPipeBufferResult.DataIgnored); } - public void UpdateState(ushort id, NexusDuplexPipe.State state) + public DisconnectReason UpdateState(ushort id, NexusDuplexPipe.State state) { if (_isCanceled) - return; + return DisconnectReason.None; if (_activePipes.TryGetValue(id, out var pipe)) { @@ -167,19 +168,32 @@ public void UpdateState(ushort id, NexusDuplexPipe.State state) } else { - var localIdByte = ExtractLocalId(id, _session.IsServer); - var partialId = GetPartialIdFromLocalId(localIdByte); - if (!_activePipes.TryRemove(partialId, out pipe)) + if (state == State.Ready) + { + var localIdByte = ExtractLocalId(id, _session.IsServer); + var partialId = GetPartialIdFromLocalId(localIdByte); + if (!_activePipes.TryRemove(partialId, out pipe)) + { + _logger?.LogTrace($"Could not find pipe with Full ID of {id} and initial ID of {localIdByte}"); + return DisconnectReason.ProtocolError; + } + + // Move the pipe to the main active pipes. + _activePipes.TryAdd(id, pipe); + pipe.Id = id; + pipe.UpdateState(state); + return DisconnectReason.None; + } + else if (state == State.Complete) { - _logger?.LogError($"Could not find pipe with initial ID of {localIdByte}"); - return; + _logger?.LogTrace($"Pipe is already complete and ignored state update of {state} with Full ID of {id}"); + return DisconnectReason.None; } - // Move the pipe to the main active pipes. - _activePipes.TryAdd(id, pipe); - pipe.Id = id; - pipe.UpdateState(state); + _logger?.LogTrace($"Ignored state update of {state} with Full ID of {id}"); } + + return DisconnectReason.None; } public void CancelAll() @@ -247,6 +261,17 @@ private static byte ExtractLocalId(ushort id, bool isServer) return isServer ? bytes[1] : bytes[0]; } + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static bool IsIdLocalIdOnly(ushort id, bool isServer, out byte localId) + { + Span bytes = stackalloc byte[sizeof(ushort)]; + Unsafe.As(ref bytes[0]) = id; + localId = isServer ? bytes[1] : bytes[0]; + + // If the other ID is 0, then this is a local ID only. + return (!isServer ? bytes[1] : bytes[0]) == 0; + } + private byte GetNewLocalId() { diff --git a/src/NexNet/Pipes/NexusPipeWriter.cs b/src/NexNet/Pipes/NexusPipeWriter.cs index ffd01d42..25aa9c84 100644 --- a/src/NexNet/Pipes/NexusPipeWriter.cs +++ b/src/NexNet/Pipes/NexusPipeWriter.cs @@ -190,11 +190,14 @@ public override async ValueTask FlushAsync(CancellationToken cancel try { + // We are passing the cancellation token from the method instead of the _flushCts due to + // the fact that the _flushCts can be canceled even after this method is completed due + // to some transport implementations such as QUIC. await _messenger.SendHeaderWithBody( MessageType.DuplexPipeWrite, _pipeId, sendingBuffer, - _flushCts.Token).ConfigureAwait(false); + cancellationToken).ConfigureAwait(false); } catch (InvalidOperationException) { From 937ebc9bb05e8805c7f3e779626fc3309633119d Mon Sep 17 00:00:00 2001 From: DJGosnell Date: Fri, 15 Sep 2023 14:50:28 -0400 Subject: [PATCH 05/15] Simplified cache returns in messages. Changed CachedMessage to use ThreadStatic instead of ConcurrentQueue for quicker retrieval and addition instead of reduced object instances. --- src/NexNet/Cache/CachedMessage.cs | 55 +++++++++++++++---- src/NexNet/Messages/ClientGreeting.cs | 6 +- .../Messages/DuplexPipeUpdateStateMessage.cs | 7 +-- src/NexNet/Messages/IMessageBase.cs | 4 +- .../Messages/InvocationCancellationMessage.cs | 9 +-- src/NexNet/Messages/InvocationMessage.cs | 2 +- src/NexNet/Messages/ServerGreetingMessage.cs | 7 +-- 7 files changed, 54 insertions(+), 36 deletions(-) diff --git a/src/NexNet/Cache/CachedMessage.cs b/src/NexNet/Cache/CachedMessage.cs index e8535938..1a4da9ab 100644 --- a/src/NexNet/Cache/CachedMessage.cs +++ b/src/NexNet/Cache/CachedMessage.cs @@ -1,5 +1,8 @@ -using System.Buffers; +using System; +using System.Buffers; +using System.Collections; using System.Collections.Concurrent; +using System.Collections.Generic; using System.Runtime.CompilerServices; using MemoryPack; using NexNet.Messages; @@ -9,12 +12,21 @@ namespace NexNet.Cache; internal class CachedCachedMessage : ICachedMessage where T : class, IMessageBase, new() { - private readonly ConcurrentBag _cache = new(); + /// + /// Thread-local cache of message items. + /// + [ThreadStatic] + private static Stack? _cache; + private static readonly ConcurrentBag> _caches = new(); + public T Rent() { - if (!_cache.TryTake(out var cachedItem)) - cachedItem = new T() { MessageCache = this }; + InitStack(); + if (!_cache!.TryPop(out var cachedItem)) + cachedItem = new T(); + + cachedItem.MessageCache = this; return cachedItem; } @@ -22,8 +34,11 @@ public T Rent() [MethodImpl(MethodImplOptions.AggressiveInlining)] public T? Deserialize(in ReadOnlySequence bodySequence) { - if (!_cache.TryTake(out var cachedItem)) - cachedItem = new T() { MessageCache = this }; + InitStack(); + if (!_cache!.TryPop(out var cachedItem)) + cachedItem = new T(); + + cachedItem.MessageCache = this; MemoryPackSerializer.Deserialize(bodySequence, ref cachedItem); @@ -33,8 +48,11 @@ public T Rent() [MethodImpl(MethodImplOptions.AggressiveInlining)] public IMessageBase DeserializeInterface(in ReadOnlySequence bodySequence) { - if (!_cache.TryTake(out var cachedItem)) - cachedItem = new T() { MessageCache = this }; + InitStack(); + if (!_cache!.TryPop(out var cachedItem)) + cachedItem = new T(); + + cachedItem.MessageCache = this; MemoryPackSerializer.Deserialize(bodySequence, ref cachedItem); @@ -44,12 +62,27 @@ public IMessageBase DeserializeInterface(in ReadOnlySequence bodySequence) [MethodImpl(MethodImplOptions.AggressiveInlining)] public void Return(IMessageBase item) { - item.MessageCache = null; - _cache.Add(Unsafe.As(item)); + InitStack(); + _cache!.Push(Unsafe.As(item)); } public void Clear() { - _cache.Clear(); + foreach (var cache in _caches) + { + cache.Clear(); + } + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private void InitStack() + { + if (_cache == null) + { + _cache = new Stack(); + + // Add the cache to the list of caches so that it can be cleared later. + _caches.Add(_cache); + } } } diff --git a/src/NexNet/Messages/ClientGreeting.cs b/src/NexNet/Messages/ClientGreeting.cs index a7b59d69..57854903 100644 --- a/src/NexNet/Messages/ClientGreeting.cs +++ b/src/NexNet/Messages/ClientGreeting.cs @@ -56,8 +56,6 @@ public void Dispose() if (cache == null) return; - cache.Return(this); - if (_isArgumentPoolArray) { @@ -67,8 +65,10 @@ public void Dispose() if (AuthenticationToken.IsEmpty) return; - IMessageBase.Return(AuthenticationToken); + IMessageBase.ReturnMemoryPackMemory(AuthenticationToken); AuthenticationToken = default; } + + cache.Return(this); } } diff --git a/src/NexNet/Messages/DuplexPipeUpdateStateMessage.cs b/src/NexNet/Messages/DuplexPipeUpdateStateMessage.cs index b9d862be..49cc2df7 100644 --- a/src/NexNet/Messages/DuplexPipeUpdateStateMessage.cs +++ b/src/NexNet/Messages/DuplexPipeUpdateStateMessage.cs @@ -26,11 +26,6 @@ public ICachedMessage? MessageCache public void Dispose() { - var cache = Interlocked.Exchange(ref _messageCache, null); - - if (cache == null) - return; - - cache.Return(this); + Interlocked.Exchange(ref _messageCache, null)?.Return(this); } } diff --git a/src/NexNet/Messages/IMessageBase.cs b/src/NexNet/Messages/IMessageBase.cs index 23aac1cc..73b72d33 100644 --- a/src/NexNet/Messages/IMessageBase.cs +++ b/src/NexNet/Messages/IMessageBase.cs @@ -22,8 +22,8 @@ internal interface IMessageBase : IDisposable ///// //public void Reset(); - protected static void Return(Memory memory) => Return((ReadOnlyMemory)memory); - protected static void Return(ReadOnlyMemory memory) + protected static void ReturnMemoryPackMemory(Memory memory) => ReturnMemoryPackMemory((ReadOnlyMemory)memory); + protected static void ReturnMemoryPackMemory(ReadOnlyMemory memory) { if (MemoryMarshal.TryGetArray(memory, out var segment) && segment.Array is { Length: > 0 }) { diff --git a/src/NexNet/Messages/InvocationCancellationMessage.cs b/src/NexNet/Messages/InvocationCancellationMessage.cs index e072d217..4218b26b 100644 --- a/src/NexNet/Messages/InvocationCancellationMessage.cs +++ b/src/NexNet/Messages/InvocationCancellationMessage.cs @@ -9,7 +9,7 @@ internal partial class InvocationCancellationMessage : IMessageBase { public static MessageType Type { get; } = MessageType.InvocationCancellation; - private ICachedMessage? _messageCache = null!; + internal ICachedMessage? _messageCache = null!; [MemoryPackIgnore] public ICachedMessage? MessageCache @@ -33,11 +33,6 @@ public InvocationCancellationMessage(int invocationId) public void Dispose() { - var cache = Interlocked.Exchange(ref _messageCache, null); - - if (cache == null) - return; - - cache.Return(this); + Interlocked.Exchange(ref _messageCache, null)?.Return(this); } } diff --git a/src/NexNet/Messages/InvocationMessage.cs b/src/NexNet/Messages/InvocationMessage.cs index 6958b124..91ed4a1c 100644 --- a/src/NexNet/Messages/InvocationMessage.cs +++ b/src/NexNet/Messages/InvocationMessage.cs @@ -91,7 +91,7 @@ public void Dispose() { // Reset the pool flag. _isArgumentPoolArray = false; - IMessageBase.Return(Arguments); + IMessageBase.ReturnMemoryPackMemory(Arguments); Arguments = default; } diff --git a/src/NexNet/Messages/ServerGreetingMessage.cs b/src/NexNet/Messages/ServerGreetingMessage.cs index bee847a7..85d99745 100644 --- a/src/NexNet/Messages/ServerGreetingMessage.cs +++ b/src/NexNet/Messages/ServerGreetingMessage.cs @@ -22,11 +22,6 @@ public ICachedMessage? MessageCache public void Dispose() { - var cache = Interlocked.Exchange(ref _messageCache, null); - - if (cache == null) - return; - - cache.Return(this); + Interlocked.Exchange(ref _messageCache, null)?.Return(this); } } From 909856865983e739484aa7a97982f97fea3c8634 Mon Sep 17 00:00:00 2001 From: DJGosnell Date: Fri, 15 Sep 2023 16:40:35 -0400 Subject: [PATCH 06/15] Added Server_PipeReaderReceivesDataMultipleTimesWithLargeData. Work on pipe timeouts. --- src/NexNet.IntegrationTests/BaseTests.cs | 6 + src/NexNet.IntegrationTests/ConsoleLogger.cs | 48 ++++++- .../Pipes/NexusChannelReaderWriterTestBase.cs | 1 + .../Pipes/NexusChannelWriterTests.cs | 8 +- .../Pipes/NexusChannelWriterUnmanagedTests.cs | 6 +- .../Pipes/NexusServerTests_NexusDuplexPipe.cs | 60 ++++++++- src/NexNet/Internals/NexusSession.Sending.cs | 1 - src/NexNet/Internals/NexusSession.cs | 2 +- src/NexNet/Pipes/NexusPipeManager.cs | 17 +-- src/NexNet/Pipes/NexusPipeWriter.cs | 9 +- src/NexNetBenchmarks/BenchmarkConfig.cs | 4 +- src/NexNetBenchmarks/InvocationBenchmarks.cs | 127 ++++++++++-------- src/NexNetBenchmarks/NexNetBenchmarks.csproj | 6 +- 13 files changed, 207 insertions(+), 88 deletions(-) diff --git a/src/NexNet.IntegrationTests/BaseTests.cs b/src/NexNet.IntegrationTests/BaseTests.cs index a9b866f1..b316f295 100644 --- a/src/NexNet.IntegrationTests/BaseTests.cs +++ b/src/NexNet.IntegrationTests/BaseTests.cs @@ -8,6 +8,7 @@ using NexNet.Quic; using NexNet.Transports; using NUnit.Framework; +using NUnit.Framework.Interfaces; using NUnit.Framework.Internal; namespace NexNet.IntegrationTests; @@ -61,6 +62,11 @@ public virtual void SetUp() [TearDown] public virtual void TearDown() { + if (TestContext.CurrentContext.Result.Outcome != ResultState.Success) + { + _logger.Flush(TestContext.Out); + } + CurrentPath = null; CurrentTcpPort = null; CurrentUdpPort = null; diff --git a/src/NexNet.IntegrationTests/ConsoleLogger.cs b/src/NexNet.IntegrationTests/ConsoleLogger.cs index 82e1b14a..c952b1c1 100644 --- a/src/NexNet.IntegrationTests/ConsoleLogger.cs +++ b/src/NexNet.IntegrationTests/ConsoleLogger.cs @@ -1,5 +1,6 @@ -using System.Diagnostics; -using NUnit.Framework; +using System; +using System.Diagnostics; +using System.IO; namespace NexNet.IntegrationTests; @@ -7,7 +8,9 @@ public class ConsoleLogger : INexusLogger { private readonly string _prefix = ""; private readonly Stopwatch _sw; - private readonly TextWriter _outWriter; + private readonly string[]? _lines; + private int _currentLineIndex = 0; + private int _totalLinesWritten = 0; public string? Category { get; } @@ -17,11 +20,16 @@ public class ConsoleLogger : INexusLogger public INexusLogger.LogLevel MinLogLevel { get; set; } = INexusLogger.LogLevel.Trace; - public ConsoleLogger() + public int TotalLinesWritten { + get => _totalLinesWritten; + } + + public ConsoleLogger(int maxLines = 500) + { + _lines = new string[maxLines]; _baseLogger = this; _sw = Stopwatch.StartNew(); - _outWriter = TestContext.Out; } private ConsoleLogger(ConsoleLogger baseLogger, string? category, string prefix = "") @@ -30,7 +38,6 @@ private ConsoleLogger(ConsoleLogger baseLogger, string? category, string prefix _prefix = prefix; Category = category; _sw = baseLogger._sw; - _outWriter = baseLogger._outWriter; } @@ -42,7 +49,13 @@ public void Log(INexusLogger.LogLevel logLevel, string? category, Exception? exc if (logLevel < MinLogLevel) return; - _outWriter.WriteLine($"[{_sw.ElapsedTicks/(double)Stopwatch.Frequency:0.000000}]{_prefix} [{category}]: {message} {exception}"); + lock (_baseLogger._lines!) + { + _baseLogger._lines[_baseLogger._currentLineIndex] = + $"[{_sw.ElapsedTicks / (double)Stopwatch.Frequency:0.000000}]{_prefix} [{category}]: {message} {exception}"; + _baseLogger._currentLineIndex = (_baseLogger._currentLineIndex + 1) % _baseLogger._lines.Length; + _baseLogger._totalLinesWritten++; + } } public INexusLogger CreateLogger(string? category) @@ -54,4 +67,25 @@ public INexusLogger CreateLogger(string? category, string prefix) { return new ConsoleLogger(_baseLogger, category, prefix) { MinLogLevel = MinLogLevel }; } + + public void Flush(TextWriter writer) + { + if (_baseLogger._totalLinesWritten == 0) + return; + + var startIndex = _baseLogger._currentLineIndex; + var maxLines = _baseLogger._lines!.Length; + + if (_baseLogger._totalLinesWritten > maxLines) + { + writer.WriteLine($"Truncating Log. Showing only last {maxLines} out of {_baseLogger._totalLinesWritten} total lines written."); + } + for (int i = 0; i < maxLines; i++) + { + writer.WriteLine(_baseLogger._lines![(startIndex + i) % maxLines]); + } + + _baseLogger._currentLineIndex = 0; + _baseLogger._totalLinesWritten = 0; + } } diff --git a/src/NexNet.IntegrationTests/Pipes/NexusChannelReaderWriterTestBase.cs b/src/NexNet.IntegrationTests/Pipes/NexusChannelReaderWriterTestBase.cs index ed9e42fe..f884ca92 100644 --- a/src/NexNet.IntegrationTests/Pipes/NexusChannelReaderWriterTestBase.cs +++ b/src/NexNet.IntegrationTests/Pipes/NexusChannelReaderWriterTestBase.cs @@ -2,6 +2,7 @@ using NexNet.Internals; using NexNet.Messages; using NexNet.Pipes; +using NUnit.Framework; using NUnit.Framework.Internal; namespace NexNet.IntegrationTests.Pipes; diff --git a/src/NexNet.IntegrationTests/Pipes/NexusChannelWriterTests.cs b/src/NexNet.IntegrationTests/Pipes/NexusChannelWriterTests.cs index 1f12f90c..5ca51562 100644 --- a/src/NexNet.IntegrationTests/Pipes/NexusChannelWriterTests.cs +++ b/src/NexNet.IntegrationTests/Pipes/NexusChannelWriterTests.cs @@ -14,7 +14,7 @@ internal class NexusChannelWriterTests public async Task WritesData() { var messenger = new DummySessionMessenger(); - var nexusPipeWriter = new NexusPipeWriter(new DummyPipeStateManager(), new ConsoleLogger(), messenger, true, ushort.MaxValue); + var nexusPipeWriter = new NexusPipeWriter(new DummyPipeStateManager(), null, messenger, true, ushort.MaxValue); var writer = new NexusChannelWriter(nexusPipeWriter); var bufferWriter = BufferWriter.Create(); var baseObject = ComplexMessage.Random(); @@ -35,7 +35,7 @@ public async Task WritesData() public async Task WritesDataWithPartialFlush() { var messenger = new DummySessionMessenger(); - var nexusPipeWriter = new NexusPipeWriter(new DummyPipeStateManager(), new ConsoleLogger(), messenger, true, 1); + var nexusPipeWriter = new NexusPipeWriter(new DummyPipeStateManager(), null, messenger, true, 1); var writer = new NexusChannelWriter(nexusPipeWriter); var bufferWriter = BufferWriter.Create(); var baseObject = ComplexMessage.Random(); @@ -55,7 +55,7 @@ public async Task WritesDataWithPartialFlush() [Test] public async Task WritesCancels() { - var nexusPipeWriter = new NexusPipeWriter(new DummyPipeStateManager(), new ConsoleLogger(), new DummySessionMessenger(), true, ushort.MaxValue) + var nexusPipeWriter = new NexusPipeWriter(new DummyPipeStateManager(), null, new DummySessionMessenger(), true, ushort.MaxValue) { PauseWriting = true }; @@ -72,7 +72,7 @@ public async Task WritesCancels() [Test] public async Task WritesCancelsImmediately() { - var nexusPipeWriter = new NexusPipeWriter(new DummyPipeStateManager(), new ConsoleLogger(), new DummySessionMessenger(), true, ushort.MaxValue) + var nexusPipeWriter = new NexusPipeWriter(new DummyPipeStateManager(), null, new DummySessionMessenger(), true, ushort.MaxValue) { PauseWriting = true }; diff --git a/src/NexNet.IntegrationTests/Pipes/NexusChannelWriterUnmanagedTests.cs b/src/NexNet.IntegrationTests/Pipes/NexusChannelWriterUnmanagedTests.cs index 33d9f54e..7a55e9c4 100644 --- a/src/NexNet.IntegrationTests/Pipes/NexusChannelWriterUnmanagedTests.cs +++ b/src/NexNet.IntegrationTests/Pipes/NexusChannelWriterUnmanagedTests.cs @@ -52,7 +52,7 @@ public async Task WritesDataWithPartialFlush(T inputData) where T : unmanaged { var messenger = new DummySessionMessenger(); - var nexusPipeWriter = new NexusPipeWriter(new DummyPipeStateManager(), new ConsoleLogger(), messenger, true, 1); + var nexusPipeWriter = new NexusPipeWriter(new DummyPipeStateManager(), null, messenger, true, 1); var writer = new NexusChannelWriterUnmanaged(nexusPipeWriter); var bufferWriter = BufferWriter.Create(); @@ -71,7 +71,7 @@ public async Task WritesDataWithPartialFlush(T inputData) [Test] public async Task WritesCancels() { - var nexusPipeWriter = new NexusPipeWriter(new DummyPipeStateManager(), new ConsoleLogger(), new DummySessionMessenger(), true, ushort.MaxValue) + var nexusPipeWriter = new NexusPipeWriter(new DummyPipeStateManager(), null, new DummySessionMessenger(), true, ushort.MaxValue) { PauseWriting = true }; @@ -88,7 +88,7 @@ public async Task WritesCancels() [Test] public async Task WritesCancelsImmediately() { - var nexusPipeWriter = new NexusPipeWriter(new DummyPipeStateManager(), new ConsoleLogger(), new DummySessionMessenger(), true, ushort.MaxValue) + var nexusPipeWriter = new NexusPipeWriter(new DummyPipeStateManager(), null, new DummySessionMessenger(), true, ushort.MaxValue) { PauseWriting = true }; diff --git a/src/NexNet.IntegrationTests/Pipes/NexusServerTests_NexusDuplexPipe.cs b/src/NexNet.IntegrationTests/Pipes/NexusServerTests_NexusDuplexPipe.cs index 835584fa..6699a2cf 100644 --- a/src/NexNet.IntegrationTests/Pipes/NexusServerTests_NexusDuplexPipe.cs +++ b/src/NexNet.IntegrationTests/Pipes/NexusServerTests_NexusDuplexPipe.cs @@ -12,7 +12,7 @@ internal class NexusServerTests_NexusDuplexPipe : BasePipeTests [TestCase(Type.Quic)] public async Task Server_PipeReaderReceivesDataMultipleTimes(Type type) { - var (_, sNexus, _, cNexus, tcs) = await Setup(type); + var (_, sNexus, _, cNexus, tcs) = await Setup(type, true); var count = 0; // TODO: Review adding a test for increased iterations as this has been found to sometimes fail on CI. @@ -42,6 +42,64 @@ public async Task Server_PipeReaderReceivesDataMultipleTimes(Type type) await tcs.Task.Timeout(1); } + + [TestCase(Type.Uds)] + [TestCase(Type.Tcp)] + [TestCase(Type.TcpTls)] + [TestCase(Type.Quic)] + public async Task Server_PipeReaderReceivesDataMultipleTimesWithLargeData(Type type) + { + var (_, sNexus, _, cNexus, tcs) = await Setup(type, true); + var count = 0; + var largeData = new byte[1024 * 32]; + // TODO: Review adding a test for increased iterations as this has been found to sometimes fail on CI. + const int iterations = 100000; + sNexus.ServerTaskValueWithDuplexPipeEvent = async (nexus, pipe) => + { + var result = await pipe.Input.ReadAsync().Timeout(1); + + if (++count == iterations) + tcs.SetResult(); + }; + + for (var i = 0; i < iterations; i++) + { + await using var pipe = cNexus.Context.CreatePipe(); + await cNexus.Context.Proxy.ServerTaskValueWithDuplexPipe(pipe).Timeout(1); + await pipe.ReadyTask.Timeout(1); + await pipe.Output.WriteAsync(largeData).Timeout(1); + } + + await tcs.Task.Timeout(1); + } + + [TestCase(Type.Uds)] + [TestCase(Type.Tcp)] + [TestCase(Type.TcpTls)] + [TestCase(Type.Quic)] + public async Task Server_PipeReaderCreatesAndDestroysPipeMultipleTimes(Type type) + { + var (_, sNexus, _, cNexus, tcs) = await Setup(type, true); + var count = 0; + + // TODO: Review adding a test for increased iterations as this has been found to sometimes fail on CI. + const int iterations = 10000; + sNexus.ServerTaskValueWithDuplexPipeEvent = async (nexus, pipe) => + { + if (++count == iterations) + tcs.SetResult(); + }; + + for (var i = 0; i < iterations; i++) + { + await using var pipe = cNexus.Context.CreatePipe(); + await cNexus.Context.Proxy.ServerTaskValueWithDuplexPipe(pipe).Timeout(1); + await pipe.ReadyTask.Timeout(1); + } + + await tcs.Task.Timeout(1); + } + [TestCase(Type.Uds)] [TestCase(Type.Tcp)] [TestCase(Type.TcpTls)] diff --git a/src/NexNet/Internals/NexusSession.Sending.cs b/src/NexNet/Internals/NexusSession.Sending.cs index fdea26c2..5c87683a 100644 --- a/src/NexNet/Internals/NexusSession.Sending.cs +++ b/src/NexNet/Internals/NexusSession.Sending.cs @@ -82,7 +82,6 @@ public async ValueTask SendMessage(TMessage body, CancellationToken ca { // ReSharper disable once MethodSupportsCancellation result = await _pipeOutput.FlushAsync().ConfigureAwait(false); - Logger?.LogTrace($"FlushResult: {result.IsCanceled}, {result.IsCompleted}"); // Return if the operation was canceled. if (result.IsCanceled) diff --git a/src/NexNet/Internals/NexusSession.cs b/src/NexNet/Internals/NexusSession.cs index 2db0615d..8c958868 100644 --- a/src/NexNet/Internals/NexusSession.cs +++ b/src/NexNet/Internals/NexusSession.cs @@ -241,7 +241,7 @@ private async ValueTask DisconnectCore(DisconnectReason reason, bool sendDisconn _registeredDisconnectReason = reason; - Logger?.LogError($"Session disconnected with reason: {reason}"); + Logger?.LogInfo($"Session disconnected with reason: {reason}"); if (sendDisconnect && !_config.InternalForceDisableSendingDisconnectSignal) { diff --git a/src/NexNet/Pipes/NexusPipeManager.cs b/src/NexNet/Pipes/NexusPipeManager.cs index ba1e623a..d9d2b9ce 100644 --- a/src/NexNet/Pipes/NexusPipeManager.cs +++ b/src/NexNet/Pipes/NexusPipeManager.cs @@ -13,29 +13,22 @@ namespace NexNet.Pipes; internal class NexusPipeManager { - private INexusLogger? _logger; private INexusSession _session = null!; private readonly ConcurrentDictionary _activePipes = new(); - //private readonly ConcurrentDictionary _initializingPipes = new(); - - private readonly BitArray _usedIds = new(256, false); - //private readonly Stack _availableIds = new Stack(); - private int _currentId = 0; - private bool _isCanceled; public void Setup(INexusSession session) { + _currentId = 0; _usedIds.SetAll(false); _isCanceled = false; _session = session; _logger = session.Logger?.CreateLogger(); } - public IRentedNexusDuplexPipe? RentPipe() { if (_isCanceled) @@ -94,6 +87,7 @@ public async ValueTask RegisterPipe(byte otherId) var pipe = new NexusDuplexPipe(localId, _session) { InitiatingPipe = false, + Id = id }; if (!_activePipes.TryAdd(id, pipe)) @@ -101,15 +95,14 @@ public async ValueTask RegisterPipe(byte otherId) // Signal that the pipe is ready to receive and send messages. pipe.UpdateState(State.Ready); - pipe.Id = id; await pipe.NotifyState().ConfigureAwait(false); - + _logger?.LogError($"Sending Ready Notification"); return pipe; } public async ValueTask DeregisterPipe(INexusDuplexPipe pipe) { - _logger?.LogError($"DeregisterPipe({pipe.Id});"); + _logger?.LogTrace($"DeregisterPipe({pipe.Id});"); if (!_activePipes.TryRemove(pipe.Id, out var nexusPipe)) { @@ -159,6 +152,7 @@ public ValueTask BufferIncomingData(ushort id, ReadOnlySe public DisconnectReason UpdateState(ushort id, NexusDuplexPipe.State state) { + _logger?.LogError($"Pipe {id} update state {state}"); if (_isCanceled) return DisconnectReason.None; @@ -182,6 +176,7 @@ public DisconnectReason UpdateState(ushort id, NexusDuplexPipe.State state) _activePipes.TryAdd(id, pipe); pipe.Id = id; pipe.UpdateState(state); + _logger?.LogError($"Readies pipe {id}"); return DisconnectReason.None; } else if (state == State.Complete) diff --git a/src/NexNet/Pipes/NexusPipeWriter.cs b/src/NexNet/Pipes/NexusPipeWriter.cs index 25aa9c84..9a40fbc2 100644 --- a/src/NexNet/Pipes/NexusPipeWriter.cs +++ b/src/NexNet/Pipes/NexusPipeWriter.cs @@ -81,7 +81,14 @@ public override Span GetSpan(int sizeHint = 0) public override void CancelPendingFlush() { _isCanceled = true; - _flushCts?.Cancel(); + try + { + _flushCts?.Cancel(); + } + catch (ObjectDisposedException) + { + // noop + } } public override ValueTask CompleteAsync(Exception? exception = null) diff --git a/src/NexNetBenchmarks/BenchmarkConfig.cs b/src/NexNetBenchmarks/BenchmarkConfig.cs index e318e964..3186a09d 100644 --- a/src/NexNetBenchmarks/BenchmarkConfig.cs +++ b/src/NexNetBenchmarks/BenchmarkConfig.cs @@ -27,8 +27,8 @@ public static IConfig Get() .WithPlatform(Platform.X64) .WithMinWarmupCount(1) .WithMaxWarmupCount(3) - .WithMinIterationCount(1) - .WithMaxIterationCount(7)) + .WithMinIterationCount(50) + .WithMaxIterationCount(51)) .AddDiagnoser(MemoryDiagnoser.Default) .AddColumnProvider(DefaultColumnProviders.Instance) .AddColumn(StatisticColumn.OperationsPerSecond) diff --git a/src/NexNetBenchmarks/InvocationBenchmarks.cs b/src/NexNetBenchmarks/InvocationBenchmarks.cs index d3389585..d1d71a03 100644 --- a/src/NexNetBenchmarks/InvocationBenchmarks.cs +++ b/src/NexNetBenchmarks/InvocationBenchmarks.cs @@ -1,81 +1,96 @@ using System; +using System.Collections.Generic; using System.IO; +using System.Linq; using System.Net.Sockets; using System.Threading.Tasks; using BenchmarkDotNet; using BenchmarkDotNet.Attributes; using NexNet; +using NexNet.IntegrationTests; using NexNet.Transports; -namespace NexNetBenchmarks +namespace NexNetBenchmarks; + +public class InvocationBenchmarks { - public class InvocationBenchmarks + private NexusClient _client; + private NexusServer _server; + private ReadOnlyMemory _uploadBuffer; + private ConsoleLogger _log; + [GlobalSetup] + public async Task GlobalSetup() { - private NexusClient _client; - private NexusServer _server; - private ReadOnlyMemory _uploadBuffer; + _uploadBuffer = new byte[(1024 * 16)]; + var path = $"test.sock"; + if (File.Exists(path)) + File.Delete(path); - [GlobalSetup] - public async Task GlobalSetup() + _log = new ConsoleLogger(500) { MinLogLevel = INexusLogger.LogLevel.Information }; + + var serverConfig = new UdsServerConfig() { - _uploadBuffer = new byte[1024 * 16]; - var path = "test.sock"; - if (File.Exists(path)) - File.Delete(path); + EndPoint = new UnixDomainSocketEndPoint(path), + //Logger = _log.CreateLogger("SV"), + }; + var clientConfig = new UdsClientConfig() + { + EndPoint = new UnixDomainSocketEndPoint(path), + //Logger = _log.CreateLogger("CL"), + }; - var serverConfig = new UdsServerConfig() - { - EndPoint = new UnixDomainSocketEndPoint(path), - }; - var clientConfig = new UdsClientConfig() - { - EndPoint = new UnixDomainSocketEndPoint(path), - }; + _client = ClientNexus.CreateClient(clientConfig, new ClientNexus()); + _server = ServerNexus.CreateServer(serverConfig, static () => new ServerNexus()); + await _server.StartAsync(); + await _client.ConnectAsync(); + } - _client = ClientNexus.CreateClient(clientConfig, new ClientNexus()); - _server = ServerNexus.CreateServer(serverConfig, static () => new ServerNexus()); - await _server.StartAsync(); - await _client.ConnectAsync(); - } + [GlobalCleanup] + public async Task GlobalCleanup() + { + await _client.DisconnectAsync(); + await _server.StopAsync(); + } - [GlobalCleanup] - public async Task GlobalCleanup() - { - await _client.DisconnectAsync(); - await _server.StopAsync(); - } + //[Benchmark] + public async ValueTask InvocationNoArgument() + { + await _client.Proxy.InvocationNoArgument(); + } - //[Benchmark] - public async ValueTask InvocationNoArgument() - { - await _client.Proxy.InvocationNoArgument(); - } + //[Benchmark] + public async ValueTask InvocationUnmanagedArgument() + { + await _client.Proxy.InvocationUnmanagedArgument(12345); + } - //[Benchmark] - public async ValueTask InvocationUnmanagedArgument() - { - await _client.Proxy.InvocationUnmanagedArgument(12345); - } + //[Benchmark] + public async ValueTask InvocationUnmanagedMultipleArguments() + { + await _client.Proxy.InvocationUnmanagedMultipleArguments(12345, 128475129847, 24812, 298471920875185871, 19818479124.12871924821d); + } - //[Benchmark] - public async ValueTask InvocationUnmanagedMultipleArguments() + //[Benchmark] + public async ValueTask InvocationNoArgumentWithResult() + { + await _client.Proxy.InvocationNoArgumentWithResult(); + } + + [Benchmark] + public async ValueTask InvocationWithDuplexPipe_Upload() + { + await using var pipe = _client.CreatePipe(); + await _client.Proxy.InvocationWithDuplexPipe_Upload(pipe); + try { - await _client.Proxy.InvocationUnmanagedMultipleArguments(12345, 128475129847, 24812, 298471920875185871, 19818479124.12871924821d); + await pipe.ReadyTask.WaitAsync(TimeSpan.FromSeconds(1)); } - - //[Benchmark] - public async ValueTask InvocationNoArgumentWithResult() + catch (Exception e) { - await _client.Proxy.InvocationNoArgumentWithResult(); - } - - [Benchmark] - public async ValueTask InvocationWithDuplexPipe_Upload() - { - await using var pipe = _client.CreatePipe(); - await _client.Proxy.InvocationWithDuplexPipe_Upload(pipe); - await pipe.ReadyTask; - await pipe.Output.WriteAsync(_uploadBuffer); + _log.Flush(Console.Out); + throw; } + + await pipe.Output.WriteAsync(_uploadBuffer); } } diff --git a/src/NexNetBenchmarks/NexNetBenchmarks.csproj b/src/NexNetBenchmarks/NexNetBenchmarks.csproj index e3ff6cdb..8915e40d 100644 --- a/src/NexNetBenchmarks/NexNetBenchmarks.csproj +++ b/src/NexNetBenchmarks/NexNetBenchmarks.csproj @@ -11,7 +11,11 @@ true Release false - + enable + + + + From 287f62309e24fb62f1642e55499e7c9618e90620 Mon Sep 17 00:00:00 2001 From: DJGosnell Date: Fri, 15 Sep 2023 18:31:00 -0400 Subject: [PATCH 07/15] Work on logging. --- src/NexNet.IntegrationTests/ConsoleLogger.cs | 2 +- .../Pipes/NexusServerTests_NexusDuplexPipe.cs | 4 ++- src/NexNet/INexusLogger.cs | 2 +- src/NexNet/Internals/NexusSession.cs | 2 +- src/NexNet/Pipes/NexusDuplexPipe.cs | 32 ++++++++++++++++--- src/NexNet/Pipes/NexusPipeManager.cs | 12 +++---- src/NexNet/Pipes/RentedNexusDuplexPipe.cs | 2 +- src/NexNetDemo/SampleLogger.cs | 2 +- 8 files changed, 40 insertions(+), 18 deletions(-) diff --git a/src/NexNet.IntegrationTests/ConsoleLogger.cs b/src/NexNet.IntegrationTests/ConsoleLogger.cs index c952b1c1..76bc5b9e 100644 --- a/src/NexNet.IntegrationTests/ConsoleLogger.cs +++ b/src/NexNet.IntegrationTests/ConsoleLogger.cs @@ -12,7 +12,7 @@ public class ConsoleLogger : INexusLogger private int _currentLineIndex = 0; private int _totalLinesWritten = 0; - public string? Category { get; } + public string? Category { get; set; } public bool LogEnabled { get; set; } = true; diff --git a/src/NexNet.IntegrationTests/Pipes/NexusServerTests_NexusDuplexPipe.cs b/src/NexNet.IntegrationTests/Pipes/NexusServerTests_NexusDuplexPipe.cs index 6699a2cf..e6216a0d 100644 --- a/src/NexNet.IntegrationTests/Pipes/NexusServerTests_NexusDuplexPipe.cs +++ b/src/NexNet.IntegrationTests/Pipes/NexusServerTests_NexusDuplexPipe.cs @@ -53,7 +53,7 @@ public async Task Server_PipeReaderReceivesDataMultipleTimesWithLargeData(Type t var count = 0; var largeData = new byte[1024 * 32]; // TODO: Review adding a test for increased iterations as this has been found to sometimes fail on CI. - const int iterations = 100000; + const int iterations = 1000; sNexus.ServerTaskValueWithDuplexPipeEvent = async (nexus, pipe) => { var result = await pipe.Input.ReadAsync().Timeout(1); @@ -71,6 +71,8 @@ public async Task Server_PipeReaderReceivesDataMultipleTimesWithLargeData(Type t } await tcs.Task.Timeout(1); + + Assert.Fail("Test is not complete"); } [TestCase(Type.Uds)] diff --git a/src/NexNet/INexusLogger.cs b/src/NexNet/INexusLogger.cs index 0b3786d6..f637908d 100644 --- a/src/NexNet/INexusLogger.cs +++ b/src/NexNet/INexusLogger.cs @@ -10,7 +10,7 @@ public interface INexusLogger /// /// Category for the log event. /// - public string? Category { get; } + public string? Category { get; set; } /// /// Level for the log event. diff --git a/src/NexNet/Internals/NexusSession.cs b/src/NexNet/Internals/NexusSession.cs index 8c958868..ec627163 100644 --- a/src/NexNet/Internals/NexusSession.cs +++ b/src/NexNet/Internals/NexusSession.cs @@ -104,7 +104,7 @@ public NexusSession(in NexusSessionConfigurations configurations ? new ServerSessionContext(this, _sessionManager!) : new ClientSessionContext(this); - Logger = configurations.Configs.Logger?.CreateLogger($"NexusSession [{Id}]"); + Logger = configurations.Configs.Logger?.CreateLogger($"NexusSession:S{Id}"); PipeManager = _cacheManager.PipeManagerCache.Rent(this); PipeManager.Setup(this); diff --git a/src/NexNet/Pipes/NexusDuplexPipe.cs b/src/NexNet/Pipes/NexusDuplexPipe.cs index 210d60d3..511ca3b5 100644 --- a/src/NexNet/Pipes/NexusDuplexPipe.cs +++ b/src/NexNet/Pipes/NexusDuplexPipe.cs @@ -78,12 +78,22 @@ internal enum State : byte /// /// True if this pipe is the pipe used for initiating the pipe connection. /// - internal bool InitiatingPipe; + internal readonly bool InitiatingPipe; /// /// Complete compiled ID containing the client bit and the server bit. /// - public ushort Id { get; set; } + public ushort Id + { + get => _id; + set + { + if(Logger != null) + Logger.Category = $"NexusDuplexPipe:S{_session!.Id}:P{value}"; + + _id = value; + } + } /// /// Initial ID of this side of the connection. @@ -115,15 +125,27 @@ internal enum State : byte /// protected INexusLogger? Logger; + private ushort _id; + NexusPipeWriter INexusDuplexPipe.WriterCore => _outputPipeWriter; NexusPipeReader INexusDuplexPipe.ReaderCore => _inputPipeReader; - internal NexusDuplexPipe(byte localId, INexusSession session) + internal NexusDuplexPipe(ushort fullId, byte localId, INexusSession session) { _readyTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); _session = session; - Logger = session.Logger?.CreateLogger(); + + if(fullId == 0) + Logger = session.Logger?.CreateLogger($"NexusDuplexPipe:S{session.Id}:L{localId:000}"); + else + { + Logger = session.Logger?.CreateLogger($"NexusDuplexPipe:S{session.Id}:P{fullId:00000}"); + } LocalId = localId; + _id = fullId; + + if(fullId == 0) + InitiatingPipe = true; _inputPipeReader = new NexusPipeReader( this, @@ -222,7 +244,7 @@ public bool UpdateState(State updatedState, bool remove = false) var isServer = _session.IsServer; - Logger?.LogTrace($"{Id}: Current State: {currentState}; Update State: {updatedState};"); + Logger?.LogTrace($"Current State: {currentState}; Update State: {updatedState};"); if (currentState == State.Unset) { diff --git a/src/NexNet/Pipes/NexusPipeManager.cs b/src/NexNet/Pipes/NexusPipeManager.cs index d9d2b9ce..9a228225 100644 --- a/src/NexNet/Pipes/NexusPipeManager.cs +++ b/src/NexNet/Pipes/NexusPipeManager.cs @@ -26,7 +26,7 @@ public void Setup(INexusSession session) _usedIds.SetAll(false); _isCanceled = false; _session = session; - _logger = session.Logger?.CreateLogger(); + _logger = session.Logger?.CreateLogger($"NexusPipeManager:S{session.Id}"); } public IRentedNexusDuplexPipe? RentPipe() @@ -38,7 +38,6 @@ public void Setup(INexusSession session) var partialId = GetPartialIdFromLocalId(localId); var pipe = new RentedNexusDuplexPipe(localId, _session) { - InitiatingPipe = true, Manager = this }; @@ -84,11 +83,7 @@ public async ValueTask RegisterPipe(byte otherId) var id = GetCompleteId(otherId, out var localId); - var pipe = new NexusDuplexPipe(localId, _session) - { - InitiatingPipe = false, - Id = id - }; + var pipe = new NexusDuplexPipe(id, localId, _session); if (!_activePipes.TryAdd(id, pipe)) throw new Exception("Could not add NexusDuplexPipe to the list of current pipes."); @@ -174,7 +169,10 @@ public DisconnectReason UpdateState(ushort id, NexusDuplexPipe.State state) // Move the pipe to the main active pipes. _activePipes.TryAdd(id, pipe); + + // Set the full ID of the pipe. pipe.Id = id; + pipe.UpdateState(state); _logger?.LogError($"Readies pipe {id}"); return DisconnectReason.None; diff --git a/src/NexNet/Pipes/RentedNexusDuplexPipe.cs b/src/NexNet/Pipes/RentedNexusDuplexPipe.cs index da929d3b..04dd83f0 100644 --- a/src/NexNet/Pipes/RentedNexusDuplexPipe.cs +++ b/src/NexNet/Pipes/RentedNexusDuplexPipe.cs @@ -7,7 +7,7 @@ namespace NexNet.Pipes; internal class RentedNexusDuplexPipe : NexusDuplexPipe, IRentedNexusDuplexPipe { public RentedNexusDuplexPipe(byte localId, INexusSession session) - : base(localId, session) + : base(0, localId, session) { } diff --git a/src/NexNetDemo/SampleLogger.cs b/src/NexNetDemo/SampleLogger.cs index 26a183f7..a5d03bba 100644 --- a/src/NexNetDemo/SampleLogger.cs +++ b/src/NexNetDemo/SampleLogger.cs @@ -6,7 +6,7 @@ class SampleLogger : INexusLogger { private readonly string _prefix; - public string? Category { get; } + public string? Category { get; set; } public SampleLogger(string prefix, string? category = null) { From 0e978c5152c73892af13f3b2298df0b4be327de8 Mon Sep 17 00:00:00 2001 From: DJGosnell Date: Fri, 15 Sep 2023 22:00:22 -0400 Subject: [PATCH 08/15] Updated max lines. Fixed fale failure on tests. --- src/NexNet.IntegrationTests/ConsoleLogger.cs | 2 +- .../Pipes/NexusServerTests_NexusDuplexPipe.cs | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/NexNet.IntegrationTests/ConsoleLogger.cs b/src/NexNet.IntegrationTests/ConsoleLogger.cs index 76bc5b9e..eb4b54f0 100644 --- a/src/NexNet.IntegrationTests/ConsoleLogger.cs +++ b/src/NexNet.IntegrationTests/ConsoleLogger.cs @@ -25,7 +25,7 @@ public int TotalLinesWritten get => _totalLinesWritten; } - public ConsoleLogger(int maxLines = 500) + public ConsoleLogger(int maxLines = 200) { _lines = new string[maxLines]; _baseLogger = this; diff --git a/src/NexNet.IntegrationTests/Pipes/NexusServerTests_NexusDuplexPipe.cs b/src/NexNet.IntegrationTests/Pipes/NexusServerTests_NexusDuplexPipe.cs index e6216a0d..9fdb6343 100644 --- a/src/NexNet.IntegrationTests/Pipes/NexusServerTests_NexusDuplexPipe.cs +++ b/src/NexNet.IntegrationTests/Pipes/NexusServerTests_NexusDuplexPipe.cs @@ -53,7 +53,7 @@ public async Task Server_PipeReaderReceivesDataMultipleTimesWithLargeData(Type t var count = 0; var largeData = new byte[1024 * 32]; // TODO: Review adding a test for increased iterations as this has been found to sometimes fail on CI. - const int iterations = 1000; + const int iterations = 10000; sNexus.ServerTaskValueWithDuplexPipeEvent = async (nexus, pipe) => { var result = await pipe.Input.ReadAsync().Timeout(1); @@ -71,8 +71,6 @@ public async Task Server_PipeReaderReceivesDataMultipleTimesWithLargeData(Type t } await tcs.Task.Timeout(1); - - Assert.Fail("Test is not complete"); } [TestCase(Type.Uds)] From 0c8750f30333cf37354c92acd4c616cb0a944b67 Mon Sep 17 00:00:00 2001 From: DJGosnell Date: Fri, 15 Sep 2023 22:23:09 -0400 Subject: [PATCH 09/15] Further logging cleanup --- src/NexNet/Pipes/NexusPipeReader.cs | 4 ++-- src/NexNetBenchmarks/InvocationBenchmarks.cs | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/NexNet/Pipes/NexusPipeReader.cs b/src/NexNet/Pipes/NexusPipeReader.cs index 9daeb5e0..2a4b485d 100644 --- a/src/NexNet/Pipes/NexusPipeReader.cs +++ b/src/NexNet/Pipes/NexusPipeReader.cs @@ -143,11 +143,11 @@ public async ValueTask BufferData(ReadOnlySequence if (_isCompleted) { - _logger?.LogTrace($"Pipe {_stateManager.Id} has is already completed. Cancelling."); + _logger?.LogTrace($"Already completed. Cancelling buffering."); return NexusPipeBufferResult.DataIgnored; } - _logger?.LogTrace($"Pipe {_stateManager.Id} has buffered {length} new bytes."); + _logger?.LogTrace($"Buffered {length} new bytes."); // Copy the data to the buffer. data.CopyTo(_buffer.GetSpan(length)); diff --git a/src/NexNetBenchmarks/InvocationBenchmarks.cs b/src/NexNetBenchmarks/InvocationBenchmarks.cs index d1d71a03..9f60c1e7 100644 --- a/src/NexNetBenchmarks/InvocationBenchmarks.cs +++ b/src/NexNetBenchmarks/InvocationBenchmarks.cs @@ -26,17 +26,17 @@ public async Task GlobalSetup() if (File.Exists(path)) File.Delete(path); - _log = new ConsoleLogger(500) { MinLogLevel = INexusLogger.LogLevel.Information }; + _log = new ConsoleLogger(200) { }; var serverConfig = new UdsServerConfig() { EndPoint = new UnixDomainSocketEndPoint(path), - //Logger = _log.CreateLogger("SV"), + Logger = _log.CreateLogger(null, "SV"), }; var clientConfig = new UdsClientConfig() { EndPoint = new UnixDomainSocketEndPoint(path), - //Logger = _log.CreateLogger("CL"), + Logger = _log.CreateLogger(null, "CL"), }; _client = ClientNexus.CreateClient(clientConfig, new ClientNexus()); From b87de586f901494f830b7e982c6b95008682f754 Mon Sep 17 00:00:00 2001 From: DJGosnell Date: Sat, 16 Sep 2023 00:13:02 -0400 Subject: [PATCH 10/15] Fixed issue with PipeReader not properly notifying the reader when it is completed. --- .../Pipes/NexusClientTests_NexusDuplexPipe.cs | 36 ++++++++++++++++--- .../Pipes/NexusServerTests_NexusDuplexPipe.cs | 3 +- .../Internals/NexusSession.Receiving.cs | 7 +++- src/NexNet/Internals/NexusSession.cs | 10 ++++-- src/NexNet/Internals/Utilities.cs | 4 +-- src/NexNet/Pipes/NexusPipeReader.cs | 19 +++++----- src/NexNetBenchmarks/InvocationBenchmarks.cs | 15 ++------ src/NexNetBenchmarks/Nexuses.cs | 2 +- 8 files changed, 65 insertions(+), 31 deletions(-) diff --git a/src/NexNet.IntegrationTests/Pipes/NexusClientTests_NexusDuplexPipe.cs b/src/NexNet.IntegrationTests/Pipes/NexusClientTests_NexusDuplexPipe.cs index 366c8455..89c07316 100644 --- a/src/NexNet.IntegrationTests/Pipes/NexusClientTests_NexusDuplexPipe.cs +++ b/src/NexNet.IntegrationTests/Pipes/NexusClientTests_NexusDuplexPipe.cs @@ -13,14 +13,11 @@ internal class NexusClientTests_NexusDuplexPipe : BasePipeTests [TestCase(Type.Quic)] public async Task Client_PipeReaderReceivesDataMultipleTimes(Type type) { - - //this.Logger.MinLogLevel = INexusLogger.LogLevel.Critical; - //BlockForClose = true; var (_, sNexus, _, cNexus, tcs) = await Setup(type); int count = 0; // TODO: Review adding a test for increased iterations as this has been found to sometimes fail on CI. - const int iterations = 10000; + const int iterations = 1000; cNexus.ClientTaskValueWithDuplexPipeEvent = async (nexus, pipe) => { var result = await pipe.Input.ReadAsync().Timeout(1); @@ -46,6 +43,37 @@ public async Task Client_PipeReaderReceivesDataMultipleTimes(Type type) await tcs.Task.Timeout(1); } + [TestCase(Type.Uds)] + [TestCase(Type.Tcp)] + [TestCase(Type.TcpTls)] + [TestCase(Type.Quic)] + public async Task Client_PipeReaderReceivesDataMultipleTimesWithLargeData(Type type) + { + var (_, sNexus, _, cNexus, tcs) = await Setup(type); + int count = 0; + var largeData = new byte[1024 * 32]; + // TODO: Review adding a test for increased iterations as this has been found to sometimes fail on CI. + const int iterations = 1000; + cNexus.ClientTaskValueWithDuplexPipeEvent = async (nexus, pipe) => + { + var result = await pipe.Input.ReadAsync().Timeout(1); + pipe.Input.AdvanceTo(result.Buffer.End); + + if (++count == iterations) + tcs.SetResult(); + }; + + for (int i = 0; i < iterations; i++) + { + await using var pipe = sNexus.Context.CreatePipe(); + await sNexus.Context.Clients.Caller.ClientTaskValueWithDuplexPipe(pipe).Timeout(1); + await pipe.ReadyTask.Timeout(1); + await pipe.Output.WriteAsync(largeData).Timeout(1); + } + + await tcs.Task.Timeout(1); + } + [TestCase(Type.Uds)] [TestCase(Type.Tcp)] [TestCase(Type.TcpTls)] diff --git a/src/NexNet.IntegrationTests/Pipes/NexusServerTests_NexusDuplexPipe.cs b/src/NexNet.IntegrationTests/Pipes/NexusServerTests_NexusDuplexPipe.cs index 9fdb6343..94601b54 100644 --- a/src/NexNet.IntegrationTests/Pipes/NexusServerTests_NexusDuplexPipe.cs +++ b/src/NexNet.IntegrationTests/Pipes/NexusServerTests_NexusDuplexPipe.cs @@ -53,10 +53,11 @@ public async Task Server_PipeReaderReceivesDataMultipleTimesWithLargeData(Type t var count = 0; var largeData = new byte[1024 * 32]; // TODO: Review adding a test for increased iterations as this has been found to sometimes fail on CI. - const int iterations = 10000; + const int iterations = 1000; sNexus.ServerTaskValueWithDuplexPipeEvent = async (nexus, pipe) => { var result = await pipe.Input.ReadAsync().Timeout(1); + pipe.Input.AdvanceTo(result.Buffer.End); if (++count == iterations) tcs.SetResult(); diff --git a/src/NexNet/Internals/NexusSession.Receiving.cs b/src/NexNet/Internals/NexusSession.Receiving.cs index 32e1cd6c..77488442 100644 --- a/src/NexNet/Internals/NexusSession.Receiving.cs +++ b/src/NexNet/Internals/NexusSession.Receiving.cs @@ -5,6 +5,7 @@ using System.Threading.Tasks; using System; using System.Runtime.CompilerServices; +using System.Threading; namespace NexNet.Internals; @@ -383,7 +384,10 @@ static async void InvokeOnConnected(object? sessionObj) { var invocationRequestMessage = message.As(); // Throttle invocations. - await _invocationSemaphore.WaitAsync().ConfigureAwait(false); + + Logger?.LogTrace($"Started Invoking method {invocationRequestMessage.MethodId}."); + + await _invocationSemaphore.WaitAsync(_disconnectionCts.Token).ConfigureAwait(false); if (!_invocationTaskArgumentsPool.TryTake(out var args)) args = new InvocationTaskArguments(); @@ -399,6 +403,7 @@ static async void InvocationTask(object? argumentsObj) var message = arguments.Message; try { + arguments.Session.Logger?.LogTrace($"Invoking method {message.MethodId}."); await session._nexus.InvokeMethod(message).ConfigureAwait(false); } catch (Exception e) diff --git a/src/NexNet/Internals/NexusSession.cs b/src/NexNet/Internals/NexusSession.cs index ec627163..86ca8367 100644 --- a/src/NexNet/Internals/NexusSession.cs +++ b/src/NexNet/Internals/NexusSession.cs @@ -49,8 +49,10 @@ private record struct ProcessResult( private readonly TaskCompletionSource? _readyTaskCompletionSource; private readonly TaskCompletionSource? _disconnectedTaskCompletionSource; - private volatile int _state; - + private volatile int _state; + + private readonly CancellationTokenSource _disconnectionCts; + public NexusPipeManager PipeManager { get; } public long Id { get; } @@ -80,6 +82,7 @@ public ConnectionState State } public Action? OnStateChanged; + public ConfigBase Config { get; } public bool IsServer { get; } @@ -100,6 +103,7 @@ public NexusSession(in NexusSessionConfigurations configurations _sessionManager = configurations.SessionManager; IsServer = configurations.IsServer; _nexus = configurations.Nexus; + _disconnectionCts = new CancellationTokenSource(); _nexus.SessionContext = configurations.IsServer ? new ServerSessionContext(this, _sessionManager!) : new ClientSessionContext(this); @@ -328,6 +332,8 @@ private async ValueTask DisconnectCore(DisconnectReason reason, bool sendDisconn } _state = ConnectionStateInternal.Disconnected; + + _disconnectionCts.Cancel(); OnStateChanged?.Invoke(State); // Cancel all pipe manager pipes and return to the cache. diff --git a/src/NexNet/Internals/Utilities.cs b/src/NexNet/Internals/Utilities.cs index d147dc18..365e0aef 100644 --- a/src/NexNet/Internals/Utilities.cs +++ b/src/NexNet/Internals/Utilities.cs @@ -10,9 +10,9 @@ internal static class Utilities /// /// The semaphore to release. [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static void TryReleaseSemaphore(SemaphoreSlim semaphore) + public static void TryReleaseSemaphore(SemaphoreSlim? semaphore) { - if (semaphore.CurrentCount == 0) + if (semaphore?.CurrentCount == 0) { try { diff --git a/src/NexNet/Pipes/NexusPipeReader.cs b/src/NexNet/Pipes/NexusPipeReader.cs index 2a4b485d..3dd51f3f 100644 --- a/src/NexNet/Pipes/NexusPipeReader.cs +++ b/src/NexNet/Pipes/NexusPipeReader.cs @@ -13,7 +13,7 @@ namespace NexNet.Pipes; internal class NexusPipeReader : PipeReader, IDisposable { //private readonly NexusDuplexPipe _nexusDuplexPipe; - private readonly SemaphoreSlim _readSemaphore = new SemaphoreSlim(0, 1); + private SemaphoreSlim? _readSemaphore = new SemaphoreSlim(0, 1); private readonly CancellationRegistrationArgs _cancelReadingArgs; private record CancellationRegistrationArgs(SemaphoreSlim Semaphore); @@ -177,12 +177,11 @@ public override ValueTask CompleteAsync(Exception? exception = null) if (_stateManager.UpdateState(_writingCompleteFlag)) { - Utilities.TryReleaseSemaphore(_readSemaphore); + var semaphore = Interlocked.Exchange(ref _readSemaphore, null); + Utilities.TryReleaseSemaphore(semaphore); return _stateManager.NotifyState(); } - Utilities.TryReleaseSemaphore(_readSemaphore); - return default; } @@ -194,8 +193,8 @@ public override void Complete(Exception? exception = null) public void CompleteNoNotify() { _isCompleted = true; - - Utilities.TryReleaseSemaphore(_readSemaphore); + var semaphore = Interlocked.Exchange(ref _readSemaphore, null); + Utilities.TryReleaseSemaphore(semaphore); } @@ -286,6 +285,9 @@ public override bool TryRead(out ReadResult result) // Check to see if we do in-fact have more data to read. If we do, then bypass the wait. do { + if (_readSemaphore == null) + return new ReadResult(_buffer.GetBuffer(), false, _isCompleted); + await _readSemaphore.WaitAsync(cancellationToken).ConfigureAwait(false); } while (_bufferTailPosition <= _examinedPosition && _isCanceled == false && _isCompleted == false); } @@ -306,7 +308,7 @@ public override bool TryRead(out ReadResult result) else { // ReSharper disable once MethodHasAsyncOverloadWithCancellation - _readSemaphore.Wait(0); + _readSemaphore?.Wait(0); } @@ -408,7 +410,8 @@ public override void CancelPendingRead() public void Dispose() { - _readSemaphore.Dispose(); + Interlocked.Exchange(ref _readSemaphore, null)?.Dispose(); + lock (_buffer) { _buffer.Reset(); diff --git a/src/NexNetBenchmarks/InvocationBenchmarks.cs b/src/NexNetBenchmarks/InvocationBenchmarks.cs index 9f60c1e7..beb6676c 100644 --- a/src/NexNetBenchmarks/InvocationBenchmarks.cs +++ b/src/NexNetBenchmarks/InvocationBenchmarks.cs @@ -31,12 +31,12 @@ public async Task GlobalSetup() var serverConfig = new UdsServerConfig() { EndPoint = new UnixDomainSocketEndPoint(path), - Logger = _log.CreateLogger(null, "SV"), + //Logger = _log.CreateLogger(null, "SV"), }; var clientConfig = new UdsClientConfig() { EndPoint = new UnixDomainSocketEndPoint(path), - Logger = _log.CreateLogger(null, "CL"), + //Logger = _log.CreateLogger(null, "CL"), }; _client = ClientNexus.CreateClient(clientConfig, new ClientNexus()); @@ -81,16 +81,7 @@ public async ValueTask InvocationWithDuplexPipe_Upload() { await using var pipe = _client.CreatePipe(); await _client.Proxy.InvocationWithDuplexPipe_Upload(pipe); - try - { - await pipe.ReadyTask.WaitAsync(TimeSpan.FromSeconds(1)); - } - catch (Exception e) - { - _log.Flush(Console.Out); - throw; - } - + await pipe.ReadyTask.WaitAsync(TimeSpan.FromSeconds(1)); await pipe.Output.WriteAsync(_uploadBuffer); } } diff --git a/src/NexNetBenchmarks/Nexuses.cs b/src/NexNetBenchmarks/Nexuses.cs index 51e41b01..271e766a 100644 --- a/src/NexNetBenchmarks/Nexuses.cs +++ b/src/NexNetBenchmarks/Nexuses.cs @@ -63,7 +63,7 @@ public async ValueTask InvocationWithDuplexPipe_Upload(INexusDuplexPipe duplexPi { var result = await reader.ReadAsync(); - if(result.IsCompleted) + if(result.IsCompleted || result.IsCanceled) break; reader.AdvanceTo(result.Buffer.End); From 1dfb08f97fdcdc2a2b301b9470810acce33a2eab Mon Sep 17 00:00:00 2001 From: DJGosnell Date: Sat, 16 Sep 2023 00:25:03 -0400 Subject: [PATCH 11/15] Added simple benchmark results. --- README.md | 19 +++++++++++++++++++ src/NexNetBenchmarks/BenchmarkConfig.cs | 4 ++-- src/NexNetBenchmarks/InvocationBenchmarks.cs | 8 ++++---- 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 437fa187..9a864c98 100644 --- a/README.md +++ b/README.md @@ -65,6 +65,25 @@ await client.ConnectAsync(); await client.Proxy.UpdateInfoAndWait(1, 2, "Custom Status"); ``` +## Benchmarks +``` +BenchmarkDotNet v0.13.8, Windows 11 (10.0.22621.2283/22H2/2022Update/SunValley2) +AMD Ryzen 9 3900X, 1 CPU, 24 logical and 12 physical cores +.NET SDK 8.0.100-rc.1.23455.8 + [Host] : .NET 7.0.10 (7.0.1023.36312), X64 RyuJIT AVX2 + Job-ASQQIE : .NET 7.0.10 (7.0.1023.36312), X64 RyuJIT AVX2 + +Platform=X64 Runtime=.NET 7.0 MaxIterationCount=5 +MaxWarmupIterationCount=3 MinIterationCount=3 MinWarmupIterationCount=1 +``` + +| Method | Mean | Error | StdDev | Op/s | Gen0 | Gen1 | Allocated | +|------------------------------------- |---------:|---------:|---------:|---------:|-------:|-------:|----------:| +| InvocationNoArgument | 38.66 us | 2.070 us | 0.537 us | 25,867.9 | 0.0610 | - | 681 B | +| InvocationUnmanagedArgument | 38.35 us | 2.496 us | 0.648 us | 26,078.6 | 0.0610 | - | 737 B | +| InvocationUnmanagedMultipleArguments | 38.98 us | 2.044 us | 0.531 us | 25,654.8 | 0.0610 | - | 785 B | +| InvocationNoArgumentWithResult | 38.22 us | 0.752 us | 0.195 us | 26,166.8 | 0.0610 | - | 721 B | +| InvocationWithDuplexPipe_Upload | 64.48 us | 2.690 us | 0.416 us | 15,509.1 | 2.0752 | 0.4883 | 14142 B | ## Method Invocation Table Some methods are handled differently based upon the arguments passed and there are limitations placed upon the types of arguments which can be used together. Most of these incompatibilities handled with Diagnostic Errors provided by the `NexNet.Generator`. Below is a table which shows valid combinations of arguments and return values. diff --git a/src/NexNetBenchmarks/BenchmarkConfig.cs b/src/NexNetBenchmarks/BenchmarkConfig.cs index 3186a09d..056f6494 100644 --- a/src/NexNetBenchmarks/BenchmarkConfig.cs +++ b/src/NexNetBenchmarks/BenchmarkConfig.cs @@ -27,8 +27,8 @@ public static IConfig Get() .WithPlatform(Platform.X64) .WithMinWarmupCount(1) .WithMaxWarmupCount(3) - .WithMinIterationCount(50) - .WithMaxIterationCount(51)) + .WithMinIterationCount(3) + .WithMaxIterationCount(5)) .AddDiagnoser(MemoryDiagnoser.Default) .AddColumnProvider(DefaultColumnProviders.Instance) .AddColumn(StatisticColumn.OperationsPerSecond) diff --git a/src/NexNetBenchmarks/InvocationBenchmarks.cs b/src/NexNetBenchmarks/InvocationBenchmarks.cs index beb6676c..66cce787 100644 --- a/src/NexNetBenchmarks/InvocationBenchmarks.cs +++ b/src/NexNetBenchmarks/InvocationBenchmarks.cs @@ -52,25 +52,25 @@ public async Task GlobalCleanup() await _server.StopAsync(); } - //[Benchmark] + [Benchmark] public async ValueTask InvocationNoArgument() { await _client.Proxy.InvocationNoArgument(); } - //[Benchmark] + [Benchmark] public async ValueTask InvocationUnmanagedArgument() { await _client.Proxy.InvocationUnmanagedArgument(12345); } - //[Benchmark] + [Benchmark] public async ValueTask InvocationUnmanagedMultipleArguments() { await _client.Proxy.InvocationUnmanagedMultipleArguments(12345, 128475129847, 24812, 298471920875185871, 19818479124.12871924821d); } - //[Benchmark] + [Benchmark] public async ValueTask InvocationNoArgumentWithResult() { await _client.Proxy.InvocationNoArgumentWithResult(); From 83b7f5e0e505008b5f3f65698125e343473859ca Mon Sep 17 00:00:00 2001 From: DJGosnell Date: Sat, 16 Sep 2023 00:33:44 -0400 Subject: [PATCH 12/15] Fix for warnings. --- .../Pipes/NexusServerTests_NexusDuplexPipe.cs | 4 +++- src/NexNetBenchmarks/InvocationBenchmarks.cs | 8 ++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/NexNet.IntegrationTests/Pipes/NexusServerTests_NexusDuplexPipe.cs b/src/NexNet.IntegrationTests/Pipes/NexusServerTests_NexusDuplexPipe.cs index 94601b54..1db6c992 100644 --- a/src/NexNet.IntegrationTests/Pipes/NexusServerTests_NexusDuplexPipe.cs +++ b/src/NexNet.IntegrationTests/Pipes/NexusServerTests_NexusDuplexPipe.cs @@ -85,10 +85,12 @@ public async Task Server_PipeReaderCreatesAndDestroysPipeMultipleTimes(Type type // TODO: Review adding a test for increased iterations as this has been found to sometimes fail on CI. const int iterations = 10000; - sNexus.ServerTaskValueWithDuplexPipeEvent = async (nexus, pipe) => + sNexus.ServerTaskValueWithDuplexPipeEvent = (nexus, pipe) => { if (++count == iterations) tcs.SetResult(); + + return ValueTask.CompletedTask; }; for (var i = 0; i < iterations; i++) diff --git a/src/NexNetBenchmarks/InvocationBenchmarks.cs b/src/NexNetBenchmarks/InvocationBenchmarks.cs index 66cce787..de41aa33 100644 --- a/src/NexNetBenchmarks/InvocationBenchmarks.cs +++ b/src/NexNetBenchmarks/InvocationBenchmarks.cs @@ -14,10 +14,10 @@ namespace NexNetBenchmarks; public class InvocationBenchmarks { - private NexusClient _client; - private NexusServer _server; + private NexusClient _client = null!; + private NexusServer _server = null!; private ReadOnlyMemory _uploadBuffer; - private ConsoleLogger _log; + private ConsoleLogger _log = null!; [GlobalSetup] public async Task GlobalSetup() { @@ -26,7 +26,7 @@ public async Task GlobalSetup() if (File.Exists(path)) File.Delete(path); - _log = new ConsoleLogger(200) { }; + _log = new ConsoleLogger() { }; var serverConfig = new UdsServerConfig() { From 94c46d4cd7ec1072875a679406fd331e3c7693ea Mon Sep 17 00:00:00 2001 From: DJGosnell Date: Sat, 16 Sep 2023 00:38:33 -0400 Subject: [PATCH 13/15] Version bump. --- src/NexNet.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/NexNet.props b/src/NexNet.props index a6e0677c..80c9fc25 100644 --- a/src/NexNet.props +++ b/src/NexNet.props @@ -1,6 +1,6 @@ - 0.3.2 + 0.3.3 enable latest net7.0 From fc930c43d79a7bbad88a51755b175cbb843a24c8 Mon Sep 17 00:00:00 2001 From: DJGosnell Date: Sun, 17 Sep 2023 00:07:23 -0400 Subject: [PATCH 14/15] Fixes for faulty tests. --- .../Pipes/NexusClientTests_NexusDuplexPipe.cs | 10 +++++----- .../Pipes/NexusServerTests_NexusDuplexPipe.cs | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/NexNet.IntegrationTests/Pipes/NexusClientTests_NexusDuplexPipe.cs b/src/NexNet.IntegrationTests/Pipes/NexusClientTests_NexusDuplexPipe.cs index 89c07316..dcc370f3 100644 --- a/src/NexNet.IntegrationTests/Pipes/NexusClientTests_NexusDuplexPipe.cs +++ b/src/NexNet.IntegrationTests/Pipes/NexusClientTests_NexusDuplexPipe.cs @@ -13,7 +13,7 @@ internal class NexusClientTests_NexusDuplexPipe : BasePipeTests [TestCase(Type.Quic)] public async Task Client_PipeReaderReceivesDataMultipleTimes(Type type) { - var (_, sNexus, _, cNexus, tcs) = await Setup(type); + var (_, sNexus, _, cNexus, tcs) = await Setup(type, true); int count = 0; // TODO: Review adding a test for increased iterations as this has been found to sometimes fail on CI. @@ -22,14 +22,14 @@ public async Task Client_PipeReaderReceivesDataMultipleTimes(Type type) { var result = await pipe.Input.ReadAsync().Timeout(1); + if (Interlocked.Increment(ref count) == iterations) + tcs.SetResult(); + // If the connection is still alive, the buffer should contain the data. if (!result.IsCompleted) { Assert.AreEqual(Data, result.Buffer.ToArray()); } - - if (++count == iterations) - tcs.SetResult(); }; for (int i = 0; i < iterations; i++) @@ -59,7 +59,7 @@ public async Task Client_PipeReaderReceivesDataMultipleTimesWithLargeData(Type t var result = await pipe.Input.ReadAsync().Timeout(1); pipe.Input.AdvanceTo(result.Buffer.End); - if (++count == iterations) + if (Interlocked.Increment(ref count) == iterations) tcs.SetResult(); }; diff --git a/src/NexNet.IntegrationTests/Pipes/NexusServerTests_NexusDuplexPipe.cs b/src/NexNet.IntegrationTests/Pipes/NexusServerTests_NexusDuplexPipe.cs index 1db6c992..a7df78b3 100644 --- a/src/NexNet.IntegrationTests/Pipes/NexusServerTests_NexusDuplexPipe.cs +++ b/src/NexNet.IntegrationTests/Pipes/NexusServerTests_NexusDuplexPipe.cs @@ -27,7 +27,7 @@ public async Task Server_PipeReaderReceivesDataMultipleTimes(Type type) Assert.AreEqual(Data, result.Buffer.ToArray()); } - if (++count == iterations) + if (Interlocked.Increment(ref count) == iterations) tcs.SetResult(); }; @@ -87,7 +87,7 @@ public async Task Server_PipeReaderCreatesAndDestroysPipeMultipleTimes(Type type const int iterations = 10000; sNexus.ServerTaskValueWithDuplexPipeEvent = (nexus, pipe) => { - if (++count == iterations) + if (Interlocked.Increment(ref count) == iterations) tcs.SetResult(); return ValueTask.CompletedTask; From e6a8040d1aeb9f720d196b75db9fdff7eb0c9ed5 Mon Sep 17 00:00:00 2001 From: DJGosnell Date: Sun, 17 Sep 2023 00:30:16 -0400 Subject: [PATCH 15/15] Fixed additional potential places where incrementing values could fail in concurrent situations. --- .../NexusServerTests_NexusInvocations.cs | 20 +++++++++---------- .../Pipes/NexusDuplexPipeWriterTests.cs | 2 +- .../Pipes/NexusServerTests_NexusDuplexPipe.cs | 2 +- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/NexNet.IntegrationTests/NexusServerTests_NexusInvocations.cs b/src/NexNet.IntegrationTests/NexusServerTests_NexusInvocations.cs index e7065c4e..e2b9bbfa 100644 --- a/src/NexNet.IntegrationTests/NexusServerTests_NexusInvocations.cs +++ b/src/NexNet.IntegrationTests/NexusServerTests_NexusInvocations.cs @@ -161,7 +161,7 @@ public async Task NexusInvokesOnAll(Type type) connectedNexus.OnConnectedEvent = async nexus => { // Second connection - if (++connectedCount == 2) + if (Interlocked.Increment(ref connectedCount) == 2) { await nexus.Context.Clients.All.ClientTask(); } @@ -199,7 +199,7 @@ public async Task NexusInvokesOnGroup(Type type) { nexus.Context.Groups.Add("group"); // Second connection - if (++connectedCount == 2) + if (Interlocked.Increment(ref connectedCount) == 2) { await nexus.Context.Clients.Group("group").ClientTask(); } @@ -234,7 +234,7 @@ public async Task NexusInvokesOnGroups(Type type) { connectedNexus.OnConnectedEvent = async nexus => { - if (++connectedCount == 1) { + if (Interlocked.Increment(ref connectedCount) == 1) { nexus.Context.Groups.Add("group"); } // Second connection @@ -276,7 +276,7 @@ public async Task NexusInvokesOnOthers(Type type) connectedNexus.OnConnectedEvent = async nexus => { // Second connection - if (++connectedCount == 2) + if (Interlocked.Increment(ref connectedCount) == 2) { await nexus.Context.Clients.Others.ClientTask(); await Task.Delay(10); @@ -288,8 +288,8 @@ public async Task NexusInvokesOnOthers(Type type) var (client1, clientNexus1) = CreateClient(CreateClientConfig(type)); var (client2, clientNexus2) = CreateClient(CreateClientConfig(type)); #pragma warning disable CS1998 - clientNexus1.ClientTaskEvent = async _ => invocationCount++; - clientNexus2.ClientTaskEvent = async _ => invocationCount++; + clientNexus1.ClientTaskEvent = async _ => Interlocked.Increment(ref invocationCount); + clientNexus2.ClientTaskEvent = async _ => Interlocked.Increment(ref invocationCount); #pragma warning restore CS1998 await server.StartAsync().Timeout(1); @@ -316,7 +316,7 @@ public async Task NexusInvokesOnClient(Type type) connectedNexus.OnConnectedEvent = async nexus => { // Second connection - if (++connectedCount == 2) + if (Interlocked.Increment(ref connectedCount) == 2) { await nexus.Context.Clients.Client(nexus.Context.Id).ClientTask(); await Task.Delay(10); @@ -328,8 +328,8 @@ public async Task NexusInvokesOnClient(Type type) var (client1, clientNexus1) = CreateClient(CreateClientConfig(type)); var (client2, clientNexus2) = CreateClient(CreateClientConfig(type)); #pragma warning disable CS1998 - clientNexus1.ClientTaskEvent = async _ => invocationCount++; - clientNexus2.ClientTaskEvent = async _ => invocationCount++; + clientNexus1.ClientTaskEvent = async _ => Interlocked.Increment(ref invocationCount); + clientNexus2.ClientTaskEvent = async _ => Interlocked.Increment(ref invocationCount); #pragma warning restore CS1998 await server.StartAsync().Timeout(1); @@ -356,7 +356,7 @@ public async Task NexusInvokesOnClients(Type type) connectedNexus.OnConnectedEvent = async nexus => { // Second connection - if (++connectedCount == 2) + if (Interlocked.Increment(ref connectedCount) == 2) { // ReSharper disable once AccessToModifiedClosure var clientIds = server!.GetContext().Clients.GetIds().ToArray(); diff --git a/src/NexNet.IntegrationTests/Pipes/NexusDuplexPipeWriterTests.cs b/src/NexNet.IntegrationTests/Pipes/NexusDuplexPipeWriterTests.cs index 214d3f62..0c9ff853 100644 --- a/src/NexNet.IntegrationTests/Pipes/NexusDuplexPipeWriterTests.cs +++ b/src/NexNet.IntegrationTests/Pipes/NexusDuplexPipeWriterTests.cs @@ -89,7 +89,7 @@ public async Task WriterChunks() messenger.OnSendCustomHeaderWithBody = (type, header, body, token) => { - if (++invocations == 2) + if (Interlocked.Increment(ref invocations) == 2) tcs.SetResult(); return default; diff --git a/src/NexNet.IntegrationTests/Pipes/NexusServerTests_NexusDuplexPipe.cs b/src/NexNet.IntegrationTests/Pipes/NexusServerTests_NexusDuplexPipe.cs index a7df78b3..909c2603 100644 --- a/src/NexNet.IntegrationTests/Pipes/NexusServerTests_NexusDuplexPipe.cs +++ b/src/NexNet.IntegrationTests/Pipes/NexusServerTests_NexusDuplexPipe.cs @@ -59,7 +59,7 @@ public async Task Server_PipeReaderReceivesDataMultipleTimesWithLargeData(Type t var result = await pipe.Input.ReadAsync().Timeout(1); pipe.Input.AdvanceTo(result.Buffer.End); - if (++count == iterations) + if (Interlocked.Increment(ref count) == iterations) tcs.SetResult(); };