From 30f124fae4ad0541ef77e85e03bf65cadd59fdc5 Mon Sep 17 00:00:00 2001 From: Jeffrey Pfau Date: Tue, 9 Aug 2016 22:30:35 -0700 Subject: [PATCH] GBA Video: Fix threaded rendering race conditions --- src/gba/renderers/thread-proxy.c | 75 +++++++++++++++++--------------- src/util/ring-fifo.c | 23 +++++++--- 2 files changed, 56 insertions(+), 42 deletions(-) diff --git a/src/gba/renderers/thread-proxy.c b/src/gba/renderers/thread-proxy.c index fc9b38a92..733c6701d 100644 --- a/src/gba/renderers/thread-proxy.c +++ b/src/gba/renderers/thread-proxy.c @@ -70,7 +70,6 @@ void GBAVideoThreadProxyRendererInit(struct GBAVideoRenderer* renderer) { ConditionInit(&proxyRenderer->toThreadCond); MutexInit(&proxyRenderer->mutex); RingFIFOInit(&proxyRenderer->dirtyQueue, 0x40000, 0x1000); - proxyRenderer->threadState = PROXY_THREAD_STOPPED; proxyRenderer->vramProxy = anonymousMemoryMap(SIZE_VRAM); proxyRenderer->backend->palette = proxyRenderer->paletteProxy; @@ -230,10 +229,11 @@ void GBAVideoThreadProxyRendererFinishFrame(struct GBAVideoRenderer* renderer) { 0xDEADBEEF, }; RingFIFOWrite(&proxyRenderer->dirtyQueue, &dirty, sizeof(dirty)); - while (proxyRenderer->threadState == PROXY_THREAD_BUSY) { + do { + RingFIFOWrite(&proxyRenderer->dirtyQueue, &dirty, sizeof(dirty)); ConditionWake(&proxyRenderer->toThreadCond); ConditionWait(&proxyRenderer->fromThreadCond, &proxyRenderer->mutex); - } + } while (proxyRenderer->threadState == PROXY_THREAD_BUSY); proxyRenderer->backend->finishFrame(proxyRenderer->backend); proxyRenderer->vramDirtyBitmap = 0; MutexUnlock(&proxyRenderer->mutex); @@ -280,45 +280,48 @@ static THREAD_ENTRY _proxyThread(void* renderer) { ThreadSetName("Proxy Renderer Thread"); MutexLock(&proxyRenderer->mutex); + struct GBAVideoDirtyInfo item = {0}; while (proxyRenderer->threadState != PROXY_THREAD_STOPPED) { ConditionWait(&proxyRenderer->toThreadCond, &proxyRenderer->mutex); if (proxyRenderer->threadState == PROXY_THREAD_STOPPED) { break; } - proxyRenderer->threadState = PROXY_THREAD_BUSY; - - MutexUnlock(&proxyRenderer->mutex); - struct GBAVideoDirtyInfo item; - while (RingFIFORead(&proxyRenderer->dirtyQueue, &item, sizeof(item))) { - switch (item.type) { - case DIRTY_REGISTER: - proxyRenderer->backend->writeVideoRegister(proxyRenderer->backend, item.address, item.value); - break; - case DIRTY_PALETTE: - proxyRenderer->paletteProxy[item.address >> 1] = item.value; - proxyRenderer->backend->writePalette(proxyRenderer->backend, item.address, item.value); - break; - case DIRTY_OAM: - proxyRenderer->oamProxy.raw[item.address] = item.value; - proxyRenderer->backend->writeOAM(proxyRenderer->backend, item.address); - break; - case DIRTY_VRAM: - while (!RingFIFORead(&proxyRenderer->dirtyQueue, &proxyRenderer->vramProxy[item.address >> 1], 0x1000)); - proxyRenderer->backend->writeVRAM(proxyRenderer->backend, item.address); - break; - case DIRTY_SCANLINE: - proxyRenderer->backend->drawScanline(proxyRenderer->backend, item.address); - break; - case DIRTY_FLUSH: - // This is only here to ensure the queue gets flushed - break; - default: - // FIFO was corrupted - proxyRenderer->threadState = PROXY_THREAD_STOPPED; - break; - } + if (RingFIFORead(&proxyRenderer->dirtyQueue, &item, sizeof(item))) { + proxyRenderer->threadState = PROXY_THREAD_BUSY; + MutexUnlock(&proxyRenderer->mutex); + do { + switch (item.type) { + case DIRTY_REGISTER: + proxyRenderer->backend->writeVideoRegister(proxyRenderer->backend, item.address, item.value); + break; + case DIRTY_PALETTE: + proxyRenderer->paletteProxy[item.address >> 1] = item.value; + proxyRenderer->backend->writePalette(proxyRenderer->backend, item.address, item.value); + break; + case DIRTY_OAM: + proxyRenderer->oamProxy.raw[item.address] = item.value; + proxyRenderer->backend->writeOAM(proxyRenderer->backend, item.address); + break; + case DIRTY_VRAM: + while (!RingFIFORead(&proxyRenderer->dirtyQueue, &proxyRenderer->vramProxy[item.address >> 1], 0x1000)); + proxyRenderer->backend->writeVRAM(proxyRenderer->backend, item.address); + break; + case DIRTY_SCANLINE: + proxyRenderer->backend->drawScanline(proxyRenderer->backend, item.address); + break; + case DIRTY_FLUSH: + MutexLock(&proxyRenderer->mutex); + goto out; + default: + // FIFO was corrupted + MutexLock(&proxyRenderer->mutex); + proxyRenderer->threadState = PROXY_THREAD_STOPPED; + goto out; + } + } while (proxyRenderer->threadState == PROXY_THREAD_BUSY && RingFIFORead(&proxyRenderer->dirtyQueue, &item, sizeof(item))); + MutexLock(&proxyRenderer->mutex); } - MutexLock(&proxyRenderer->mutex); + out: ConditionWake(&proxyRenderer->fromThreadCond); if (proxyRenderer->threadState != PROXY_THREAD_STOPPED) { proxyRenderer->threadState = PROXY_THREAD_IDLE; diff --git a/src/util/ring-fifo.c b/src/util/ring-fifo.c index 1b8166dcd..f7387b60f 100644 --- a/src/util/ring-fifo.c +++ b/src/util/ring-fifo.c @@ -7,6 +7,15 @@ #include "util/memory.h" +#ifndef _MSC_VER +#define ATOMIC_STORE(DST, SRC) __atomic_store_n(&DST, SRC, __ATOMIC_RELEASE) +#define ATOMIC_LOAD(DST, SRC) DST = __atomic_load_n(&SRC, __ATOMIC_ACQUIRE) +#else +// TODO +#define ATOMIC_STORE(DST, SRC) DST = SRC +#define ATOMIC_LOAD(DST, SRC) DST = SRC +#endif + void RingFIFOInit(struct RingFIFO* buffer, size_t capacity, size_t maxalloc) { buffer->data = anonymousMemoryMap(capacity); buffer->capacity = capacity; @@ -24,13 +33,14 @@ size_t RingFIFOCapacity(const struct RingFIFO* buffer) { } void RingFIFOClear(struct RingFIFO* buffer) { - buffer->readPtr = buffer->data; - buffer->writePtr = buffer->data; + ATOMIC_STORE(buffer->readPtr, buffer->data); + ATOMIC_STORE(buffer->writePtr, buffer->data); } size_t RingFIFOWrite(struct RingFIFO* buffer, const void* value, size_t length) { void* data = buffer->writePtr; - void* end = buffer->readPtr; + void* end; + ATOMIC_LOAD(end, buffer->readPtr); size_t remaining; if ((intptr_t) data - (intptr_t) buffer->data + buffer->maxalloc >= buffer->capacity) { data = buffer->data; @@ -46,13 +56,14 @@ size_t RingFIFOWrite(struct RingFIFO* buffer, const void* value, size_t length) if (value) { memcpy(data, value, length); } - buffer->writePtr = (void*) ((intptr_t) data + length); + ATOMIC_STORE(buffer->writePtr, (void*) ((intptr_t) data + length)); return length; } size_t RingFIFORead(struct RingFIFO* buffer, void* output, size_t length) { void* data = buffer->readPtr; - void* end = buffer->writePtr; + void* end; + ATOMIC_LOAD(end, buffer->writePtr); size_t remaining; if ((intptr_t) data - (intptr_t) buffer->data + buffer->maxalloc >= buffer->capacity) { data = buffer->data; @@ -68,6 +79,6 @@ size_t RingFIFORead(struct RingFIFO* buffer, void* output, size_t length) { if (output) { memcpy(output, data, length); } - buffer->readPtr = (void*) ((intptr_t) data + length); + ATOMIC_STORE(buffer->readPtr, (void*) ((intptr_t) data + length)); return length; }