From b6614c6ad5d7d19594b80f4917df27bf476e8f03 Mon Sep 17 00:00:00 2001 From: Mary-nyan Date: Sun, 1 Jan 2023 17:35:29 +0100 Subject: [PATCH] chore: Update tests dependencies (#3978) * chore: Update tests dependencies * Apply TSR Berry suggestion to add a GC.SuppressFinalize in MemoryBlock.cs * Ensure we wait for the test thread to be dead on PartialUnmap * Use platform attribute for os specific tests * Make P/Invoke methods private * Downgrade NUnit3TestAdapter to 4.1.0 * test: Disable warning about platform compat for ThreadLocalMap() Co-authored-by: TSR Berry <20988865+TSRBerry@users.noreply.github.com> --- Directory.Packages.props | 2 +- .../Memory/PartialUnmaps/PartialUnmapState.cs | 6 +-- Ryujinx.Memory.Tests/Tests.cs | 24 +++------ Ryujinx.Memory/MemoryBlock.cs | 7 ++- Ryujinx.Tests/Memory/PartialUnmaps.cs | 51 +++++++------------ 5 files changed, 34 insertions(+), 56 deletions(-) diff --git a/Directory.Packages.props b/Directory.Packages.props index 5f434bf17b..c02c1ae564 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -25,7 +25,7 @@ - + diff --git a/Ryujinx.Common/Memory/PartialUnmaps/PartialUnmapState.cs b/Ryujinx.Common/Memory/PartialUnmaps/PartialUnmapState.cs index 3463d06cff..5b0bc07ec0 100644 --- a/Ryujinx.Common/Memory/PartialUnmaps/PartialUnmapState.cs +++ b/Ryujinx.Common/Memory/PartialUnmaps/PartialUnmapState.cs @@ -27,7 +27,7 @@ namespace Ryujinx.Common.Memory.PartialUnmaps [SupportedOSPlatform("windows")] [LibraryImport("kernel32.dll")] - public static partial int GetCurrentThreadId(); + private static partial int GetCurrentThreadId(); [SupportedOSPlatform("windows")] [LibraryImport("kernel32.dll", SetLastError = true)] @@ -36,7 +36,7 @@ namespace Ryujinx.Common.Memory.PartialUnmaps [SupportedOSPlatform("windows")] [LibraryImport("kernel32.dll", SetLastError = true)] [return: MarshalAs (UnmanagedType.Bool)] - public static partial bool CloseHandle(IntPtr hObject); + private static partial bool CloseHandle(IntPtr hObject); [SupportedOSPlatform("windows")] [LibraryImport("kernel32.dll", SetLastError = true)] @@ -160,4 +160,4 @@ namespace Ryujinx.Common.Memory.PartialUnmaps } } } -} +} \ No newline at end of file diff --git a/Ryujinx.Memory.Tests/Tests.cs b/Ryujinx.Memory.Tests/Tests.cs index c5a7842ec2..2717b76ad7 100644 --- a/Ryujinx.Memory.Tests/Tests.cs +++ b/Ryujinx.Memory.Tests/Tests.cs @@ -39,14 +39,10 @@ namespace Ryujinx.Memory.Tests } [Test] + // Memory aliasing tests fail on CI at the moment. + [Platform(Exclude = "MacOsX")] public void Test_Alias() { - if (OperatingSystem.IsMacOS()) - { - // Memory aliasing tests fail on CI at the moment. - return; - } - using MemoryBlock backing = new MemoryBlock(0x10000, MemoryAllocationFlags.Mirrorable); using MemoryBlock toAlias = new MemoryBlock(0x10000, MemoryAllocationFlags.Reserve | MemoryAllocationFlags.ViewCompatible); @@ -58,14 +54,10 @@ namespace Ryujinx.Memory.Tests } [Test] + // Memory aliasing tests fail on CI at the moment. + [Platform(Exclude = "MacOsX")] public void Test_AliasRandom() { - if (OperatingSystem.IsMacOS()) - { - // Memory aliasing tests fail on CI at the moment. - return; - } - using MemoryBlock backing = new MemoryBlock(0x80000, MemoryAllocationFlags.Mirrorable); using MemoryBlock toAlias = new MemoryBlock(0x80000, MemoryAllocationFlags.Reserve | MemoryAllocationFlags.ViewCompatible); @@ -94,14 +86,10 @@ namespace Ryujinx.Memory.Tests } [Test] + // Memory aliasing tests fail on CI at the moment. + [Platform(Exclude = "MacOsX")] public void Test_AliasMapLeak() { - if (OperatingSystem.IsMacOS()) - { - // Memory aliasing tests fail on CI at the moment. - return; - } - ulong pageSize = 4096; ulong size = 100000 * pageSize; // The mappings limit on Linux is usually around 65K, so let's make sure we are above that. diff --git a/Ryujinx.Memory/MemoryBlock.cs b/Ryujinx.Memory/MemoryBlock.cs index 41e6224bbf..6b9d852de3 100644 --- a/Ryujinx.Memory/MemoryBlock.cs +++ b/Ryujinx.Memory/MemoryBlock.cs @@ -379,7 +379,12 @@ namespace Ryujinx.Memory /// /// It's an error to use the memory block after disposal. /// - public void Dispose() => FreeMemory(); + public void Dispose() + { + FreeMemory(); + + GC.SuppressFinalize(this); + } ~MemoryBlock() => FreeMemory(); diff --git a/Ryujinx.Tests/Memory/PartialUnmaps.cs b/Ryujinx.Tests/Memory/PartialUnmaps.cs index 1088b52c4a..b805969d3d 100644 --- a/Ryujinx.Tests/Memory/PartialUnmaps.cs +++ b/Ryujinx.Tests/Memory/PartialUnmaps.cs @@ -9,6 +9,7 @@ using Ryujinx.Memory.Tests; using Ryujinx.Memory.Tracking; using System; using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Threading; @@ -57,14 +58,10 @@ namespace Ryujinx.Tests.Memory } [Test] + // Memory aliasing tests fail on CI at the moment. + [Platform(Exclude = "MacOsX")] public void PartialUnmap([Values] bool readOnly) { - if (OperatingSystem.IsMacOS()) - { - // Memory aliasing tests fail on CI at the moment. - return; - } - // Set up an address space to test partial unmapping. // Should register the signal handler to deal with this on Windows. ulong vaSize = 0x100000; @@ -78,11 +75,13 @@ namespace Ryujinx.Tests.Memory ref var state = ref PartialUnmapState.GetRef(); + Thread testThread = null; + bool shouldAccess = true; + try { // Globally reset the struct for handling partial unmap races. PartialUnmapState.Reset(); - bool shouldAccess = true; bool error = false; // Create a large mapping. @@ -93,8 +92,6 @@ namespace Ryujinx.Tests.Memory memory.Reprotect(0, vaSize, MemoryPermission.Read); } - Thread testThread; - if (readOnly) { // Write a value to the physical memory, then try to read it repeately from virtual. @@ -193,6 +190,10 @@ namespace Ryujinx.Tests.Memory } finally { + // In case something failed, we want to ensure the test thread is dead before disposing of the memory. + shouldAccess = false; + testThread?.Join(); + exceptionHandler.Dispose(); unusedMainMemory.Dispose(); memory.Dispose(); @@ -201,13 +202,10 @@ namespace Ryujinx.Tests.Memory } [Test] + // Memory aliasing tests fail on CI at the moment. + [Platform(Exclude = "MacOsX")] public unsafe void PartialUnmapNative() { - if (OperatingSystem.IsMacOS()) - { - // Memory aliasing tests fail on CI at the moment. - return; - } // Set up an address space to test partial unmapping. // Should register the signal handler to deal with this on Windows. @@ -284,26 +282,17 @@ namespace Ryujinx.Tests.Memory } [Test] + // Only test in Windows, as this is only used on Windows and uses Windows APIs for trimming. + [Platform("Win")] + [SuppressMessage("Interoperability", "CA1416:Validate platform compatibility")] public void ThreadLocalMap() { - if (!OperatingSystem.IsWindows()) - { - // Only test in Windows, as this is only used on Windows and uses Windows APIs for trimming. - return; - } - PartialUnmapState.Reset(); ref var state = ref PartialUnmapState.GetRef(); bool running = true; var testThread = new Thread(() => { - if (!OperatingSystem.IsWindows()) - { - // Need this here to avoid a warning. - return; - } - PartialUnmapState.GetRef().RetryFromAccessViolation(); while (running) { @@ -330,14 +319,10 @@ namespace Ryujinx.Tests.Memory } [Test] + // Only test in Windows, as this is only used on Windows and uses Windows APIs for trimming. + [Platform("Win")] public unsafe void ThreadLocalMapNative() { - if (!OperatingSystem.IsWindows()) - { - // Only test in Windows, as this is only used on Windows and uses Windows APIs for trimming. - return; - } - EnsureTranslator(); PartialUnmapState.Reset(); @@ -481,4 +466,4 @@ namespace Ryujinx.Tests.Memory Assert.False(error); } } -} +} \ No newline at end of file