From 54adc5f9fb65f4b03bc28da5899d2413a84f66c2 Mon Sep 17 00:00:00 2001 From: riperiperi Date: Sun, 29 Aug 2021 20:03:41 +0100 Subject: [PATCH] Ensure that all threads wait for a read tracking action to complete. (#2597) * Lock around tracking action consume + execute. Not particularly fast. * Lock around preaction registration and use * Create a lock object * Nit --- Ryujinx.Memory/Tracking/RegionHandle.cs | 28 +++++++++++++++++-------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/Ryujinx.Memory/Tracking/RegionHandle.cs b/Ryujinx.Memory/Tracking/RegionHandle.cs index 2df02f1e3c..40deba9838 100644 --- a/Ryujinx.Memory/Tracking/RegionHandle.cs +++ b/Ryujinx.Memory/Tracking/RegionHandle.cs @@ -35,6 +35,7 @@ namespace Ryujinx.Memory.Tracking private event Action _onDirty; + private object _preActionLock = new object(); private RegionSignal _preAction; // Action to perform before a read or write. This will block the memory access. private readonly List _regions; private readonly MemoryTracking _tracking; @@ -115,17 +116,16 @@ namespace Ryujinx.Memory.Tracking /// Whether the region was written to or read internal void Signal(ulong address, ulong size, bool write, ref IList handleIterable) { - RegionSignal action = Interlocked.Exchange(ref _preAction, null); - // If this handle was already unmapped (even if just partially), // then we have nothing to do until it is mapped again. // The pre-action should be still consumed to avoid flushing on remap. if (Unmapped) { + Interlocked.Exchange(ref _preAction, null); return; } - if (action != null) + if (_preAction != null) { // Copy the handles list in case it changes when we're out of the lock. if (handleIterable is List) @@ -138,7 +138,12 @@ namespace Ryujinx.Memory.Tracking try { - action.Invoke(address, size); + lock (_preActionLock) + { + _preAction?.Invoke(address, size); + + _preAction = null; + } } finally { @@ -220,14 +225,19 @@ namespace Ryujinx.Memory.Tracking { ClearVolatile(); - RegionSignal lastAction = Interlocked.Exchange(ref _preAction, action); - if (lastAction == null && action != lastAction) + lock (_preActionLock) { - lock (_tracking.TrackingLock) + RegionSignal lastAction = _preAction; + _preAction = action; + + if (lastAction == null && action != lastAction) { - foreach (VirtualRegion region in _regions) + lock (_tracking.TrackingLock) { - region.UpdateProtection(); + foreach (VirtualRegion region in _regions) + { + region.UpdateProtection(); + } } } }