(PSP/Vita/PS4) The room check could not tell a full ring from an empty one

write_pos and read_pos are both masked into the ring, so the writer
having filled every frame and the writer having written nothing both
read as a distance of zero, and the free-space computation answered
with the whole ring. A writer that caught up exactly lapped: it wrote
from read_pos forward, over the window the device was reading, while
the worker - reading the same distance - took the full ring for an
empty one and played silence.

Hold a frame back, so the two states differ.

Found with a threaded model of the mainloop against the write path
using the tree's own retro_atomic ops, under TSan: the race reproduces
with the frame returned and does not with it held back, and it is
independent of the read_pos publication order - each fix is needed and
neither covers the other.
This commit is contained in:
LibretroAdmin authored and libretroadmin committed 2026-09-20 15:54:45 +02:00
1 parent ba5339c419
commit d36404a114
3 files changed
+36 -18

No files matched your search

+12 -6
View File
@@ -71,6 +71,12 @@ typedef struct ps4_audio
#define AUDIO_BUFFER_SIZE (1u<<13u)
#define AUDIO_BUFFER_SIZE_MASK (AUDIO_BUFFER_SIZE-1)
/* What the writer may fill. One frame short of the ring, so that a
* full ring and an empty one do not both read as write_pos ==
* read_pos - which let the writer lap and overwrite the window the
* device was reading. */
#define AUDIO_BUFFER_USABLE (AUDIO_BUFFER_SIZE - 1u)
/* The silence sits past the ring. read_pos only ever advances by
* AUDIO_OUT_COUNT, which divides AUDIO_BUFFER_SIZE, so no window
* handed to the output syscall crosses into it. */
@@ -234,7 +240,7 @@ static ssize_t ps4_audio_write(void *data, const void *s, size_t len)
* frames here too. */
if (ps4->nonblock)
{
if (AUDIO_BUFFER_SIZE - ((uint16_t)(write_pos - (uint16_t)
if (AUDIO_BUFFER_USABLE - ((uint16_t)(write_pos - (uint16_t)
retro_atomic_load_acquire_int(&ps4->read_pos))
& AUDIO_BUFFER_SIZE_MASK) < sample_count)
return 0;
@@ -249,7 +255,7 @@ static ssize_t ps4_audio_write(void *data, const void *s, size_t len)
* are re-checked inside the eventcount's window, so a period
* freed between the check and the park costs nothing. */
int laps = PS4_AUDIO_WAIT_LAPS;
while (AUDIO_BUFFER_SIZE - ((uint16_t)(write_pos - (uint16_t)
while (AUDIO_BUFFER_USABLE - ((uint16_t)(write_pos - (uint16_t)
retro_atomic_load_acquire_int(&ps4->read_pos))
& AUDIO_BUFFER_SIZE_MASK) < sample_count)
{
@@ -257,7 +263,7 @@ static ssize_t ps4_audio_write(void *data, const void *s, size_t len)
if (--laps < 0 || !retro_atomic_load_acquire_int(&ps4->running))
return 0;
key = retro_eventcount_prepare_wait(&ps4->park);
if ( (AUDIO_BUFFER_SIZE - ((uint16_t)(write_pos - (uint16_t)
if ( (AUDIO_BUFFER_USABLE - ((uint16_t)(write_pos - (uint16_t)
retro_atomic_load_acquire_int(&ps4->read_pos))
& AUDIO_BUFFER_SIZE_MASK) >= sample_count)
|| !retro_atomic_load_acquire_int(&ps4->running))
@@ -348,7 +354,7 @@ static size_t ps4_write_avail(void *data)
if (!ps4 || !retro_atomic_load_acquire_int(&ps4->running))
return 0;
_len = AUDIO_BUFFER_SIZE - ((uint16_t)((uint16_t)
_len = AUDIO_BUFFER_USABLE - ((uint16_t)((uint16_t)
retro_atomic_load_relaxed_int(&ps4->write_pos) - (uint16_t)
retro_atomic_load_acquire_int(&ps4->read_pos))
& AUDIO_BUFFER_SIZE_MASK);
@@ -376,7 +382,7 @@ static size_t ps4_wait_writable(void *data, size_t len)
int key;
if (!retro_atomic_load_acquire_int(&ps4->running))
return 0;
avail = AUDIO_BUFFER_SIZE - ((uint16_t)((uint16_t)
avail = AUDIO_BUFFER_USABLE - ((uint16_t)((uint16_t)
retro_atomic_load_relaxed_int(&ps4->write_pos) - (uint16_t)
retro_atomic_load_acquire_int(&ps4->read_pos))
& AUDIO_BUFFER_SIZE_MASK);
@@ -388,7 +394,7 @@ static size_t ps4_wait_writable(void *data, size_t len)
if (--laps < 0)
return 0;
key = retro_eventcount_prepare_wait(&ps4->park);
if ((AUDIO_BUFFER_SIZE - ((uint16_t)((uint16_t)
if ((AUDIO_BUFFER_USABLE - ((uint16_t)((uint16_t)
retro_atomic_load_relaxed_int(&ps4->write_pos) - (uint16_t)
retro_atomic_load_acquire_int(&ps4->read_pos))
& AUDIO_BUFFER_SIZE_MASK)) >= want
+12 -6
View File
@@ -73,6 +73,12 @@ typedef struct psp2_audio
#define AUDIO_BUFFER_SIZE (1u<<13u)
#define AUDIO_BUFFER_SIZE_MASK (AUDIO_BUFFER_SIZE-1)
/* What the writer may fill. One frame short of the ring, so that a
* full ring and an empty one do not both read as write_pos ==
* read_pos - which let the writer lap and overwrite the window the
* device was reading. */
#define AUDIO_BUFFER_USABLE (AUDIO_BUFFER_SIZE - 1u)
/* The silence sits past the ring. read_pos only ever advances by
* AUDIO_OUT_COUNT, which divides AUDIO_BUFFER_SIZE, so no window
* handed to the output syscall crosses into it. */
@@ -233,7 +239,7 @@ static ssize_t psp2_audio_write(void *data, const void *s, size_t len)
* frames here too. */
if (psp->nonblock)
{
if (AUDIO_BUFFER_SIZE - ((uint16_t)(write_pos - (uint16_t)
if (AUDIO_BUFFER_USABLE - ((uint16_t)(write_pos - (uint16_t)
retro_atomic_load_acquire_int(&psp->read_pos))
& AUDIO_BUFFER_SIZE_MASK) < sample_count)
return 0;
@@ -248,7 +254,7 @@ static ssize_t psp2_audio_write(void *data, const void *s, size_t len)
* are re-checked inside the eventcount's window, so a period
* freed between the check and the park costs nothing. */
int laps = PSP2_AUDIO_WAIT_LAPS;
while (AUDIO_BUFFER_SIZE - ((uint16_t)(write_pos - (uint16_t)
while (AUDIO_BUFFER_USABLE - ((uint16_t)(write_pos - (uint16_t)
retro_atomic_load_acquire_int(&psp->read_pos))
& AUDIO_BUFFER_SIZE_MASK) < sample_count)
{
@@ -256,7 +262,7 @@ static ssize_t psp2_audio_write(void *data, const void *s, size_t len)
if (--laps < 0 || !retro_atomic_load_acquire_int(&psp->running))
return 0;
key = retro_eventcount_prepare_wait(&psp->park);
if ( (AUDIO_BUFFER_SIZE - ((uint16_t)(write_pos - (uint16_t)
if ( (AUDIO_BUFFER_USABLE - ((uint16_t)(write_pos - (uint16_t)
retro_atomic_load_acquire_int(&psp->read_pos))
& AUDIO_BUFFER_SIZE_MASK) >= sample_count)
|| !retro_atomic_load_acquire_int(&psp->running))
@@ -347,7 +353,7 @@ static size_t psp2_write_avail(void *data)
if (!psp || !retro_atomic_load_acquire_int(&psp->running))
return 0;
_len = AUDIO_BUFFER_SIZE - ((uint16_t)((uint16_t)
_len = AUDIO_BUFFER_USABLE - ((uint16_t)((uint16_t)
retro_atomic_load_relaxed_int(&psp->write_pos) - (uint16_t)
retro_atomic_load_acquire_int(&psp->read_pos))
& AUDIO_BUFFER_SIZE_MASK);
@@ -375,7 +381,7 @@ static size_t psp2_wait_writable(void *data, size_t len)
int key;
if (!retro_atomic_load_acquire_int(&psp->running))
return 0;
avail = AUDIO_BUFFER_SIZE - ((uint16_t)((uint16_t)
avail = AUDIO_BUFFER_USABLE - ((uint16_t)((uint16_t)
retro_atomic_load_relaxed_int(&psp->write_pos) - (uint16_t)
retro_atomic_load_acquire_int(&psp->read_pos))
& AUDIO_BUFFER_SIZE_MASK);
@@ -387,7 +393,7 @@ static size_t psp2_wait_writable(void *data, size_t len)
if (--laps < 0)
return 0;
key = retro_eventcount_prepare_wait(&psp->park);
if ((AUDIO_BUFFER_SIZE - ((uint16_t)((uint16_t)
if ((AUDIO_BUFFER_USABLE - ((uint16_t)((uint16_t)
retro_atomic_load_relaxed_int(&psp->write_pos) - (uint16_t)
retro_atomic_load_acquire_int(&psp->read_pos))
& AUDIO_BUFFER_SIZE_MASK)) >= want
+12 -6
View File
@@ -71,6 +71,12 @@ typedef struct psp_audio
#define AUDIO_BUFFER_SIZE (1u<<13u)
#define AUDIO_BUFFER_SIZE_MASK (AUDIO_BUFFER_SIZE-1)
/* What the writer may fill. One frame short of the ring, so that a
* full ring and an empty one do not both read as write_pos ==
* read_pos - which let the writer lap and overwrite the window the
* device was reading. */
#define AUDIO_BUFFER_USABLE (AUDIO_BUFFER_SIZE - 1u)
/* The silence sits past the ring. read_pos only ever advances by
* AUDIO_OUT_COUNT, which divides AUDIO_BUFFER_SIZE, so no window
* handed to the output syscall crosses into it. */
@@ -231,7 +237,7 @@ static ssize_t psp_audio_write(void *data, const void *s, size_t len)
* frames here too. */
if (psp->nonblock)
{
if (AUDIO_BUFFER_SIZE - ((uint16_t)(write_pos - (uint16_t)
if (AUDIO_BUFFER_USABLE - ((uint16_t)(write_pos - (uint16_t)
retro_atomic_load_acquire_int(&psp->read_pos))
& AUDIO_BUFFER_SIZE_MASK) < sample_count)
return 0;
@@ -246,7 +252,7 @@ static ssize_t psp_audio_write(void *data, const void *s, size_t len)
* are re-checked inside the eventcount's window, so a period
* freed between the check and the park costs nothing. */
int laps = PSP_AUDIO_WAIT_LAPS;
while (AUDIO_BUFFER_SIZE - ((uint16_t)(write_pos - (uint16_t)
while (AUDIO_BUFFER_USABLE - ((uint16_t)(write_pos - (uint16_t)
retro_atomic_load_acquire_int(&psp->read_pos))
& AUDIO_BUFFER_SIZE_MASK) < sample_count)
{
@@ -254,7 +260,7 @@ static ssize_t psp_audio_write(void *data, const void *s, size_t len)
if (--laps < 0 || !retro_atomic_load_acquire_int(&psp->running))
return 0;
key = retro_eventcount_prepare_wait(&psp->park);
if ( (AUDIO_BUFFER_SIZE - ((uint16_t)(write_pos - (uint16_t)
if ( (AUDIO_BUFFER_USABLE - ((uint16_t)(write_pos - (uint16_t)
retro_atomic_load_acquire_int(&psp->read_pos))
& AUDIO_BUFFER_SIZE_MASK) >= sample_count)
|| !retro_atomic_load_acquire_int(&psp->running))
@@ -345,7 +351,7 @@ static size_t psp_write_avail(void *data)
if (!psp || !retro_atomic_load_acquire_int(&psp->running))
return 0;
_len = AUDIO_BUFFER_SIZE - ((uint16_t)((uint16_t)
_len = AUDIO_BUFFER_USABLE - ((uint16_t)((uint16_t)
retro_atomic_load_relaxed_int(&psp->write_pos) - (uint16_t)
retro_atomic_load_acquire_int(&psp->read_pos))
& AUDIO_BUFFER_SIZE_MASK);
@@ -373,7 +379,7 @@ static size_t psp_wait_writable(void *data, size_t len)
int key;
if (!retro_atomic_load_acquire_int(&psp->running))
return 0;
avail = AUDIO_BUFFER_SIZE - ((uint16_t)((uint16_t)
avail = AUDIO_BUFFER_USABLE - ((uint16_t)((uint16_t)
retro_atomic_load_relaxed_int(&psp->write_pos) - (uint16_t)
retro_atomic_load_acquire_int(&psp->read_pos))
& AUDIO_BUFFER_SIZE_MASK);
@@ -385,7 +391,7 @@ static size_t psp_wait_writable(void *data, size_t len)
if (--laps < 0)
return 0;
key = retro_eventcount_prepare_wait(&psp->park);
if ((AUDIO_BUFFER_SIZE - ((uint16_t)((uint16_t)
if ((AUDIO_BUFFER_USABLE - ((uint16_t)((uint16_t)
retro_atomic_load_relaxed_int(&psp->write_pos) - (uint16_t)
retro_atomic_load_acquire_int(&psp->read_pos))
& AUDIO_BUFFER_SIZE_MASK)) >= want