[PR #3186] [MERGED] buffer_cache: Fix various thread races on data upload and invalidation #3292

New issue

Closed

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

kerem commented

2026-02-27 22:03:09 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/shadps4-emu/shadPS4/pull/3186
Author: @raphaelthegreat
Created: 7/3/2025
Status: ✅ Merged
Merged: 7/3/2025
Merged by: @LNDF

Base: main ← Head: read-races

📝 Commits (3)

212e37b buffer_cache: Fix various thread races on data upload and invalidation
70b4b98 memory_tracker: Improve locking more on invalidation
1273155 Merge branch 'main' into read-races

📊 Changes

5 files changed (+84 additions, -84 deletions)

View changed files

📝 src/video_core/buffer_cache/buffer_cache.cpp (+26 -54)
📝 src/video_core/buffer_cache/buffer_cache.h (+4 -3)
📝 src/video_core/buffer_cache/memory_tracker.h (+37 -11)
📝 src/video_core/buffer_cache/region_manager.h (+13 -13)
📝 src/video_core/page_manager.cpp (+4 -3)

📄 Description

With this PR the address space critical log spam with readbacks enabled should be gone.

On x64 in any operating system having pages marked with write-only protection is not allowed and it depends on the OS how its handled. On Linux it appears to silently make the page noaccess, while on windows it throws an error like mentioned.

To avoid this the backend always first drops the write access by syncing CPU data and then marks the area as GPU modified which drops the read access. On page invalidation the opposite order is used. However because these actions are separate, when multiple threads attempt to access the same page together with GPU thread, those protect actions can become interleaved and result in invalid protection state

To resolve this, whenever 2 protect actions are necessary they are fused under a single function call and mutex lock, so page state is always in a known state before each one. I've also fixed another race that could occur during CPU data upload. It could be possible while the memcpy to staging buffer performed in SynchronizeBuffer, that another thread invalidates that page and writes new data. Unless that page is invalidated again that data would not get synced. Now the memcpy is performed inside the ForEachUploadRange function with a mutex lock and protection is moved to be before the memcpy again, so that if another page attempts to write there it will have to wait first

_{🔄 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/3186 **Author:** [@raphaelthegreat](https://github.com/raphaelthegreat) **Created:** 7/3/2025 **Status:** ✅ Merged **Merged:** 7/3/2025 **Merged by:** [@LNDF](https://github.com/LNDF) **Base:** `main` ← **Head:** `read-races` --- ### 📝 Commits (3) - [`212e37b`](https://github.com/shadps4-emu/shadPS4/commit/212e37b8ecdccb737ca9bf2519e0b0fa33cbf9d0) buffer_cache: Fix various thread races on data upload and invalidation - [`70b4b98`](https://github.com/shadps4-emu/shadPS4/commit/70b4b981dff15d7535308846bbb9beb86dcc09b5) memory_tracker: Improve locking more on invalidation - [`1273155`](https://github.com/shadps4-emu/shadPS4/commit/1273155cb38fede12c3d716d1af7d78b85c196ed) Merge branch 'main' into read-races ### 📊 Changes **5 files changed** (+84 additions, -84 deletions) <details> <summary>View changed files</summary> 📝 `src/video_core/buffer_cache/buffer_cache.cpp` (+26 -54) 📝 `src/video_core/buffer_cache/buffer_cache.h` (+4 -3) 📝 `src/video_core/buffer_cache/memory_tracker.h` (+37 -11) 📝 `src/video_core/buffer_cache/region_manager.h` (+13 -13) 📝 `src/video_core/page_manager.cpp` (+4 -3) </details> ### 📄 Description With this PR the address space critical log spam with readbacks enabled should be gone. On x64 in any operating system having pages marked with write-only protection is not allowed and it depends on the OS how its handled. On Linux it appears to silently make the page noaccess, while on windows it throws an error like mentioned. To avoid this the backend always first drops the write access by syncing CPU data and then marks the area as GPU modified which drops the read access. On page invalidation the opposite order is used. However because these actions are separate, when multiple threads attempt to access the same page together with GPU thread, those protect actions can become interleaved and result in invalid protection state To resolve this, whenever 2 protect actions are necessary they are fused under a single function call and mutex lock, so page state is always in a known state before each one. I've also fixed another race that could occur during CPU data upload. It could be possible while the memcpy to staging buffer performed in SynchronizeBuffer, that another thread invalidates that page and writes new data. Unless that page is invalidated again that data would not get synced. Now the memcpy is performed inside the ForEachUploadRange function with a mutex lock and protection is moved to be before the memcpy again, so that if another page attempts to write there it will have to wait first --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>