[PR #3956] [MERGED] Kernel.Vmm: Attempt to address race conditions involving ClampRangeSize, CopySparseMemory, and TryWriteBacking #3824

New issue

Closed

opened 2026-02-27 22:05:07 +03:00 by kerem · 0 comments

kerem commented

2026-02-27 22:05:07 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/shadps4-emu/shadPS4/pull/3956
Author: @StevenMiller123
Created: 1/24/2026
Status: ✅ Merged
Merged: 1/27/2026
Merged by: @georgemoralis

Base: main ← Head: attempt-2

📝 Commits (7)

5f4e9b8 no
619d2a6 Adjust locking strategy
e058391 Clang
e976445 Always GPU unmap
5881d95 Fixups
ca6f48b Update memory.cpp
ffef6d7 Rename mutex

📊 Changes

5 files changed (+145 additions, -98 deletions)

View changed files

📝 src/common/shared_first_mutex.h (+9 -0)
📝 src/core/address_space.cpp (+2 -6)
📝 src/core/address_space.h (+2 -1)
📝 src/core/memory.cpp (+127 -87)
📝 src/core/memory.h (+5 -4)

📄 Description

This PR makes ClampRangeSize, CopySparseMemory, and TryWriteBacking thread safe to fix some issues caused by #3954, and in doing so, makes a variety of changes to unmap-related logic to prevent deadlocking.

Specifically, I've adjusted all functions with GPU unmaps to perform their GPU unmaps as soon as possible, while locking a separate mutex I've added. When that operation is done, I attempt to lock for writing before these functions will modify vma_map.

The extra mutex is also now locked for all functions that modify vma_map, since we don't want vma_map modifications occurring while maps/unmaps are performing validity checks.

By using a separate mutex instead of locking reader locks, I can avoid race conditions with trying to transition from reader to writer locks, and ensure we don't have multiple threads trying to perform maps at the same time (and potentially invalidating the results of another thread's validity checks).

Also some smaller changes: I've cleaned up unmap code a bit, removing some unused variables that got left behind in my prior PRs, and I've added a try_lock to SharedFirstMutex to enable using some extra standard library lock types with it.

While I've tested locally with some of my heavily multi-threaded games, I'm willing to bet there are a plethora of bugs that need fixing here, so I'm opening this as a draft.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/shadps4-emu/shadPS4/pull/3956 **Author:** [@StevenMiller123](https://github.com/StevenMiller123) **Created:** 1/24/2026 **Status:** ✅ Merged **Merged:** 1/27/2026 **Merged by:** [@georgemoralis](https://github.com/georgemoralis) **Base:** `main` ← **Head:** `attempt-2` --- ### 📝 Commits (7) - [`5f4e9b8`](https://github.com/shadps4-emu/shadPS4/commit/5f4e9b85854cd5899141927d1bfd02b5cb4aa758) no - [`619d2a6`](https://github.com/shadps4-emu/shadPS4/commit/619d2a679b590f25fe26f9ce7eeeed965a2a17a3) Adjust locking strategy - [`e058391`](https://github.com/shadps4-emu/shadPS4/commit/e05839150d0c00e47bbea82bb5afc482fb31f2bf) Clang - [`e976445`](https://github.com/shadps4-emu/shadPS4/commit/e976445e11cc2aabc1ffdf12af605cb1feea4a96) Always GPU unmap - [`5881d95`](https://github.com/shadps4-emu/shadPS4/commit/5881d9595b1364e3f6b5a046d14d0f580331f0b9) Fixups - [`ca6f48b`](https://github.com/shadps4-emu/shadPS4/commit/ca6f48b498deb0bd0337bff8a9b4e31789750e4f) Update memory.cpp - [`ffef6d7`](https://github.com/shadps4-emu/shadPS4/commit/ffef6d763a7c321c303f2a14c55e362ad0011883) Rename mutex ### 📊 Changes **5 files changed** (+145 additions, -98 deletions) <details> <summary>View changed files</summary> 📝 `src/common/shared_first_mutex.h` (+9 -0) 📝 `src/core/address_space.cpp` (+2 -6) 📝 `src/core/address_space.h` (+2 -1) 📝 `src/core/memory.cpp` (+127 -87) 📝 `src/core/memory.h` (+5 -4) </details> ### 📄 Description This PR makes ClampRangeSize, CopySparseMemory, and TryWriteBacking thread safe to fix some issues caused by #3954, and in doing so, makes a variety of changes to unmap-related logic to prevent deadlocking. Specifically, I've adjusted all functions with GPU unmaps to perform their GPU unmaps as soon as possible, while locking a separate mutex I've added. When that operation is done, I attempt to lock for writing before these functions will modify vma_map. The extra mutex is also now locked for all functions that modify vma_map, since we don't want vma_map modifications occurring while maps/unmaps are performing validity checks. By using a separate mutex instead of locking reader locks, I can avoid race conditions with trying to transition from reader to writer locks, and ensure we don't have multiple threads trying to perform maps at the same time (and potentially invalidating the results of another thread's validity checks). Also some smaller changes: I've cleaned up unmap code a bit, removing some unused variables that got left behind in my prior PRs, and I've added a try_lock to SharedFirstMutex to enable using some extra standard library lock types with it. While I've tested locally with some of my heavily multi-threaded games, I'm willing to bet there are a plethora of bugs that need fixing here, so I'm opening this as a draft. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>