From 4368841f02792abb423255fa29cb1d5a352d0d41 Mon Sep 17 00:00:00 2001 From: erorcun Date: Tue, 16 Feb 2021 17:11:12 +0300 Subject: [PATCH 1/4] Fix rare stream deadlock on Windows --- src/core/CdStream.cpp | 41 +++++++++++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/src/core/CdStream.cpp b/src/core/CdStream.cpp index da85a238..977f16c2 100644 --- a/src/core/CdStream.cpp +++ b/src/core/CdStream.cpp @@ -14,9 +14,9 @@ struct CdReadInfo void *pBuffer; char field_C; bool bLocked; - bool bInUse; + bool bReading; int32 nStatus; - HANDLE hSemaphore; // used for CdStreamSync + HANDLE pDoneSemaphore; // used for CdStreamSync HANDLE hFile; OVERLAPPED Overlapped; }; @@ -53,9 +53,9 @@ CdStreamInitThread(void) { for ( int32 i = 0; i < gNumChannels; i++ ) { - gpReadInfo[i].hSemaphore = CreateSemaphore(nil, 0, 2, nil); + gpReadInfo[i].pDoneSemaphore = CreateSemaphore(nil, 0, 2, nil); - if ( gpReadInfo[i].hSemaphore == nil ) + if ( gpReadInfo[i].pDoneSemaphore == nil ) { printf("%s: failed to create sync semaphore\n", "cdvd_stream"); ASSERT(0); @@ -183,7 +183,7 @@ CdStreamShutdown(void) CloseHandle(_gCdStreamThread); for ( int32 i = 0; i < gNumChannels; i++ ) - CloseHandle(gpReadInfo[i].hSemaphore); + CloseHandle(gpReadInfo[i].pDoneSemaphore); } LocalFree(gpReadInfo); @@ -213,7 +213,7 @@ CdStreamRead(int32 channel, void *buffer, uint32 offset, uint32 size) if ( _gbCdStreamAsync ) { - if ( pChannel->nSectorsToRead != 0 || pChannel->bInUse ) + if ( pChannel->nSectorsToRead != 0 || pChannel->bReading ) return STREAM_NONE; pChannel->nStatus = STREAM_NONE; @@ -271,7 +271,7 @@ CdStreamGetStatus(int32 channel) if ( _gbCdStreamAsync ) { - if ( pChannel->bInUse ) + if ( pChannel->bReading ) return STREAM_READING; if ( pChannel->nSectorsToRead != 0 ) @@ -321,12 +321,21 @@ CdStreamSync(int32 channel) { pChannel->bLocked = true; - ASSERT( pChannel->hSemaphore != nil ); + ASSERT( pChannel->pDoneSemaphore != nil ); - WaitForSingleObject(pChannel->hSemaphore, INFINITE); + // Deadlock fix 1 +#ifdef FIX_BUGS + // This is while loop on Posix streamer, for spurious wakeups + if (pChannel->bLocked && pChannel->nSectorsToRead != 0){ + WaitForSingleObject(pChannel->pDoneSemaphore, INFINITE); + } + pChannel->bLocked = false; +#else + WaitForSingleObject(pChannel->pDoneSemaphore, INFINITE); +#endif } - pChannel->bInUse = false; + pChannel->bReading = false; return pChannel->nStatus; } @@ -398,7 +407,7 @@ WINAPI CdStreamThread(LPVOID lpThreadParameter) CdReadInfo *pChannel = &gpReadInfo[channel]; ASSERT( pChannel != nil ); - pChannel->bInUse = true; + pChannel->bReading = true; if ( pChannel->nStatus == STREAM_NONE ) { @@ -455,11 +464,15 @@ WINAPI CdStreamThread(LPVOID lpThreadParameter) if ( pChannel->bLocked ) { - ASSERT( pChannel->hSemaphore != nil ); - ReleaseSemaphore(pChannel->hSemaphore, 1, NULL); + ASSERT( pChannel->pDoneSemaphore != nil ); + // Deadlock fix 2 +#ifdef FIX_BUGS + pChannel->bLocked = 0; +#endif + ReleaseSemaphore(pChannel->pDoneSemaphore, 1, NULL); } - pChannel->bInUse = false; + pChannel->bReading = false; } } From 48227bd350ab894b11dc362f7f3954bae40776cc Mon Sep 17 00:00:00 2001 From: Adrian Graber Date: Sun, 24 Jan 2021 14:02:15 +0100 Subject: [PATCH 2/4] Add unnamed semaphore define toggle for CdStreamPosix --- src/core/CdStreamPosix.cpp | 77 +++++++++++++++++++++++++++++--------- 1 file changed, 60 insertions(+), 17 deletions(-) diff --git a/src/core/CdStreamPosix.cpp b/src/core/CdStreamPosix.cpp index e18280e5..09611fba 100644 --- a/src/core/CdStreamPosix.cpp +++ b/src/core/CdStreamPosix.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include "CdStream.h" @@ -25,6 +26,58 @@ bool flushStream[MAX_CDCHANNELS]; #endif +#ifdef USE_UNNAMED_SEM + +#define RE3_SEM_OPEN(name, ...) re3_sem_open() +sem_t* +re3_sem_open(void) +{ + sem_t* sem = (sem_t*)malloc(sizeof(sem_t)); + if (sem_init(sem, 0, 1) == -1) { + sem = SEM_FAILED; + } + + return sem; +} + +#define RE3_SEM_CLOSE(sem, format, ...) re3_sem_close(sem) +void +re3_sem_close(sem_t* sem) +{ + sem_destroy(sem); + free(sem); +} + +#else + +#define RE3_SEM_OPEN re3_sem_open +sem_t* +re3_sem_open(const char* format, ...) +{ + char semName[20]; + va_list va; + va_start(va, format); + vsprintf(semName, format, va); + + return sem_open(semName, O_CREAT, 0644, 1); +} + +#define RE3_SEM_CLOSE re3_sem_close +void +re3_sem_close(sem_t* sem, const char* format, ...) +{ + sem_close(sem); + + char semName[20]; + va_list va; + va_start(va, format); + vsprintf(semName, format, va); + + sem_unlink(semName); +} + +#endif + struct CdReadInfo { uint32 nSectorOffset; @@ -69,14 +122,13 @@ void CdStreamInitThread(void) { int status; - char semName[20]; #ifndef ONE_THREAD_PER_CHANNEL gChannelRequestQ.items = (int32 *)calloc(gNumChannels + 1, sizeof(int32)); gChannelRequestQ.head = 0; gChannelRequestQ.tail = 0; gChannelRequestQ.size = gNumChannels + 1; ASSERT(gChannelRequestQ.items != nil ); - gCdStreamSema = sem_open("/semaphore_cd_stream", O_CREAT, 0644, 0); + gCdStreamSema = RE3_SEM_OPEN("/semaphore_cd_stream"); if (gCdStreamSema == SEM_FAILED) { @@ -90,8 +142,7 @@ CdStreamInitThread(void) { for ( int32 i = 0; i < gNumChannels; i++ ) { - sprintf(semName,"/semaphore_done%d",i); - gpReadInfo[i].pDoneSemaphore = sem_open(semName, O_CREAT, 0644, 0); + gpReadInfo[i].pDoneSemaphore = RE3_SEM_OPEN("/semaphore_done%d", i); if (gpReadInfo[i].pDoneSemaphore == SEM_FAILED) { @@ -101,8 +152,7 @@ CdStreamInitThread(void) } #ifdef ONE_THREAD_PER_CHANNEL - sprintf(semName,"/semaphore_start%d",i); - gpReadInfo[i].pStartSemaphore = sem_open(semName, O_CREAT, 0644, 0); + gpReadInfo[i].pStartSemaphore = RE3_SEM_OPEN("/semaphore_start%d", i); if (gpReadInfo[i].pStartSemaphore == SEM_FAILED) { @@ -464,21 +514,14 @@ void *CdStreamThread(void *param) #ifndef ONE_THREAD_PER_CHANNEL for ( int32 i = 0; i < gNumChannels; i++ ) { - sem_close(gpReadInfo[i].pDoneSemaphore); - sprintf(semName,"/semaphore_done%d",i); - sem_unlink(semName); + RE3_SEM_CLOSE(gpReadInfo[i].pDoneSemaphore, "/semaphore_done%d", i); } - sem_close(gCdStreamSema); - sem_unlink("/semaphore_cd_stream"); + RE3_SEM_CLOSE(gCdStreamSema, "/semaphore_cd_stream"); free(gChannelRequestQ.items); #else - sem_close(gpReadInfo[channel].pStartSemaphore); - sprintf(semName,"/semaphore_start%d",channel); - sem_unlink(semName); + RE3_SEM_CLOSE(gpReadInfo[channel].pStartSemaphore, "/semaphore_start%d", channel); - sem_close(gpReadInfo[channel].pDoneSemaphore); - sprintf(semName,"/semaphore_done%d",channel); - sem_unlink(semName); + RE3_SEM_CLOSE(gpReadInfo[channel].pDoneSemaphore, "/semaphore_done%d", channel); #endif if (gpReadInfo) free(gpReadInfo); From ee287f61202c1684af730daa283ae18068ec8864 Mon Sep 17 00:00:00 2001 From: Adrian Graber Date: Sun, 24 Jan 2021 16:34:47 +0100 Subject: [PATCH 3/4] Only include sys/syscall.h when __linux__ is defined --- src/core/CdStreamPosix.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/core/CdStreamPosix.cpp b/src/core/CdStreamPosix.cpp index 09611fba..50d823d2 100644 --- a/src/core/CdStreamPosix.cpp +++ b/src/core/CdStreamPosix.cpp @@ -1,8 +1,8 @@ #ifndef _WIN32 #include "common.h" #include "crossplatform.h" -#include #include +#include #include #include #include @@ -13,7 +13,10 @@ #include #include #include + +#ifdef __linux__ #include +#endif #include "CdStream.h" #include "rwcore.h" From 1bfd7c034553d29b4b345b3f082662aafb5b55ef Mon Sep 17 00:00:00 2001 From: erorcun Date: Tue, 16 Feb 2021 18:26:44 +0300 Subject: [PATCH 4/4] Fix FindClose->closedir --- src/skel/crossplatform.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/skel/crossplatform.h b/src/skel/crossplatform.h index 6ea5b11e..9b43bcaf 100644 --- a/src/skel/crossplatform.h +++ b/src/skel/crossplatform.h @@ -136,7 +136,12 @@ void GetLocalTime_CP(SYSTEMTIME* out); typedef void* HANDLE; #define INVALID_HANDLE_VALUE NULL -#define FindClose(h) closedir((DIR*)h) +#define FindClose(h) \ + do { \ + if (h != nil) \ + closedir((DIR*)h); \ + } while(0) + #define LOCALE_USER_DEFAULT 0 #define DATE_SHORTDATE 0