diff --git a/src/dusk/asset/asset.c b/src/dusk/asset/asset.c index e1aa2ed7..2a0cc934 100644 --- a/src/dusk/asset/asset.c +++ b/src/dusk/asset/asset.c @@ -19,6 +19,8 @@ asset_t ASSET; errorret_t assetInit(void) { memoryZero(&ASSET, sizeof(asset_t)); + threadMutexInit(&ASSET.zipLock); + for(size_t i = 0; i < ASSET_LOADING_COUNT_MAX; i++) { threadMutexInit(&ASSET.loading[i].mutex); } @@ -37,9 +39,12 @@ errorret_t assetInit(void) { bool_t assetFileExists(const char_t *filename) { assertStrLenMax(filename, ASSET_FILE_NAME_MAX, "Filename too long."); - if(zip_name_locate(ASSET.zip, filename, 0) >= 0) return true; - if(zip_name_locate(ASSET.zipStored, filename, 0) >= 0) return true; - return false; + threadMutexLock(&ASSET.zipLock); + bool_t found = + zip_name_locate(ASSET.zip, filename, 0) >= 0 || + zip_name_locate(ASSET.zipStored, filename, 0) >= 0; + threadMutexUnlock(&ASSET.zipLock); + return found; } assetentry_t * assetGetEntry( @@ -453,5 +458,6 @@ errorret_t assetDispose(void) { } errorChain(assetDisposePlatform()); + threadMutexDispose(&ASSET.zipLock); errorOk(); } \ No newline at end of file diff --git a/src/dusk/asset/asset.h b/src/dusk/asset/asset.h index 0aa9ed72..15f3dfd0 100644 --- a/src/dusk/asset/asset.h +++ b/src/dusk/asset/asset.h @@ -39,6 +39,18 @@ typedef struct asset_s { assetplatform_t platform; + // Guards every libzip call against ASSET.zip/ASSET.zipStored (zip_fopen, + // zip_fread, zip_fclose, zip_fseek, zip_stat, zip_name_locate - see + // assetfile.c/assetFileExists). libzip is documented as not thread-safe, + // and this asset system genuinely calls it from multiple real threads at + // once (main thread, the background load thread below, and - on + // platforms that stream PCM from an asset on their own thread, like PSP + // - an audio feeder thread too). Confirmed necessary on real PSP + // hardware: without this lock, concurrent zip_fopen/zip_fread calls from + // different threads corrupted reads (EINVAL, then zlib data errors) once + // dusk.dsk stopped being read entirely into memory up front. + threadmutex_t zipLock; + // Background loading thread. thread_t loadThread; diff --git a/src/dusk/asset/assetfile.c b/src/dusk/asset/assetfile.c index 0ecf7ac0..06aaaa7f 100644 --- a/src/dusk/asset/assetfile.c +++ b/src/dusk/asset/assetfile.c @@ -28,11 +28,16 @@ errorret_t assetFileInit( // the stored one - remember which matched so assetFileOpen opens it from // the right archive. zip_stat_init(&file->stat); + threadMutexLock(&ASSET.zipLock); if(zip_stat(ASSET.zip, filename, 0, &file->stat) == 0) { file->sourceZip = ASSET.zip; } else if(zip_stat(ASSET.zipStored, filename, 0, &file->stat) == 0) { file->sourceZip = ASSET.zipStored; } else { + file->sourceZip = NULL; + } + threadMutexUnlock(&ASSET.zipLock); + if(file->sourceZip == NULL) { errorThrow("Failed to stat asset file: %s", filename); } @@ -60,8 +65,12 @@ errorret_t assetFileRewind(assetfile_t *file) { // skipping ranges of the decompressed content on some re-opens after // the first. A real seek avoids that failure mode entirely, with no // extra memory cost over the close+reopen fallback. - if(zip_file_is_seekable(file->zipFile)) { - if(zip_fseek(file->zipFile, 0, SEEK_SET) != 0) { + threadMutexLock(&ASSET.zipLock); + bool_t seekable = zip_file_is_seekable(file->zipFile); + int seekResult = seekable ? zip_fseek(file->zipFile, 0, SEEK_SET) : -1; + threadMutexUnlock(&ASSET.zipLock); + if(seekable) { + if(seekResult != 0) { errorThrow("Failed to seek asset file: %s", file->filename); } file->position = 0; @@ -79,7 +88,9 @@ errorret_t assetFileOpen(assetfile_t *file) { assertNotNull(file->sourceZip, "Asset file must be inited before opening."); assertNull(file->zipFile, "Asset file already open."); + threadMutexLock(&ASSET.zipLock); file->zipFile = zip_fopen(file->sourceZip, file->filename, 0); + threadMutexUnlock(&ASSET.zipLock); if(file->zipFile == NULL) { errorThrow("Failed to open asset file: %s", file->filename); } @@ -120,15 +131,27 @@ errorret_t assetFileRead( size_t chunkSize = mathMin( bufferSize - totalRead, ASSET_FILE_READ_CHUNK_MAX ); + threadMutexLock(&ASSET.zipLock); zip_int64_t bytesRead = zip_fread( file->zipFile, dest + totalRead, chunkSize ); + errorret_t readError = errorOkImpl(); if(bytesRead < 0) { - errorThrow( + // Built (not thrown) while still holding the lock, so + // zip_file_strerror() reads the just-failed zipFile's error state + // before another thread can touch it - errorThrow() itself isn't + // used here since it returns immediately, which would leave + // ASSET.zipLock held forever. + readError = errorThrowImpl( + &ERROR_STATE, ERROR_NOT_OK, __FILE__, __func__, __LINE__, "Failed to read from asset file: %s (%s)", file->filename, zip_file_strerror(file->zipFile) ); } + threadMutexUnlock(&ASSET.zipLock); + if(errorIsNotOk(readError)) { + errorChain(readError); + } if(bytesRead == 0) break; totalRead += (size_t)bytesRead; } @@ -141,7 +164,10 @@ errorret_t assetFileClose(assetfile_t *file) { assertNotNull(file, "Asset file cannot be NULL."); assertNotNull(file->zipFile, "Asset file must be opened before closing."); - if(zip_fclose(file->zipFile) != 0) { + threadMutexLock(&ASSET.zipLock); + int closeResult = zip_fclose(file->zipFile); + threadMutexUnlock(&ASSET.zipLock); + if(closeResult != 0) { errorThrow("Failed to close asset file: %s", file->filename); } file->zipFile = NULL;