From 68521f8c92d4d2b83c84cc61576d40739b5e2953 Mon Sep 17 00:00:00 2001 From: itsRevela Date: Wed, 1 Apr 2026 01:23:50 -0500 Subject: [PATCH] fix: async autosave thread safety and commit path Move StorageManager.AllocateSaveData() back under the save lock (called on the game thread before releasing). Background thread now only does pure zlib compression with no StorageManager calls. Add CommitPendingAsyncSave() to both the game thread tick and TickCoreSystems() to ensure saves commit during bootstrap, normal gameplay, and shutdown. --- Minecraft.Client/MinecraftServer.cpp | 5 ++ Minecraft.World/ConsoleSaveFileOriginal.cpp | 72 +++++++++------------ README.md | 5 +- 3 files changed, 40 insertions(+), 42 deletions(-) diff --git a/Minecraft.Client/MinecraftServer.cpp b/Minecraft.Client/MinecraftServer.cpp index 67b9b399..6ddbd580 100644 --- a/Minecraft.Client/MinecraftServer.cpp +++ b/Minecraft.Client/MinecraftServer.cpp @@ -2258,6 +2258,11 @@ void MinecraftServer::tick() PIXBeginNamedEvent(0,"Players tick"); players->tick(); PIXEndNamedEvent(); +#if defined(_WINDOWS64) + // Commit any pending async autosave on the game thread (same thread + // that called Flush/AllocateSaveData, which StorageManager requires). + ConsoleSaveFileOriginal::CommitPendingAsyncSave(); +#endif PIXBeginNamedEvent(0,"Connection tick"); connection->tick(); PIXEndNamedEvent(); diff --git a/Minecraft.World/ConsoleSaveFileOriginal.cpp b/Minecraft.World/ConsoleSaveFileOriginal.cpp index cbab89e6..1a5071e1 100644 --- a/Minecraft.World/ConsoleSaveFileOriginal.cpp +++ b/Minecraft.World/ConsoleSaveFileOriginal.cpp @@ -744,15 +744,24 @@ void ConsoleSaveFileOriginal::Flush(bool autosave, bool updateThumbnail ) StorageManager.GetSaveUniqueNumber(&saveOrCheckpointId); TelemetryManager->RecordLevelSaveOrCheckpoint(ProfileManager.GetPrimaryPad(), saveOrCheckpointId, fileSize); + // Allocate compression buffer while still on the game thread and + // holding the lock (StorageManager is not thread-safe). + byte *compData = static_cast(StorageManager.AllocateSaveData(fileSize + 8)); + if (compData == nullptr) + { + app.DebugPrintf("Async save: failed to allocate compression buffer, falling back to sync\n"); + delete[] snapshot; + goto sync_flush; + } + // Release the lock -- main thread and chunk trickle saves can resume ReleaseSaveAccess(); - // Compress and write on a background thread. - // Pack metadata into a heap struct so the lambda captures a single - // owning pointer instead of stack arrays that go out of scope. + // Pack context for the background thread struct AsyncSaveContext { byte *snapshot; + byte *compData; unsigned int fileSize; ConsoleSaveFile *self; PBYTE thumbData; @@ -763,6 +772,7 @@ void ConsoleSaveFileOriginal::Flush(bool autosave, bool updateThumbnail ) auto *ctx = new AsyncSaveContext(); ctx->snapshot = snapshot; + ctx->compData = compData; ctx->fileSize = fileSize; ctx->self = this; ctx->thumbData = pbThumbnailData; @@ -775,44 +785,26 @@ void ConsoleSaveFileOriginal::Flush(bool autosave, bool updateThumbnail ) std::thread([ctx]() { unsigned int compLength = ctx->fileSize + 8; - byte *compData = static_cast(StorageManager.AllocateSaveData(compLength)); - if (compData == nullptr) + Compression::getCompression()->Compress( + ctx->compData + 8, &compLength, ctx->snapshot, ctx->fileSize); + + ZeroMemory(ctx->compData, 8); + int saveVer = 0; + memcpy(ctx->compData, &saveVer, sizeof(int)); + unsigned int fs = ctx->fileSize; + memcpy(ctx->compData + 4, &fs, sizeof(int)); + + app.DebugPrintf("Async save: compressed %u -> %u bytes\n", ctx->fileSize, compLength); + + // Queue for the main thread to commit via StorageManager { - // Pre-calculate compressed size - compLength = 0; - Compression::getCompression()->Compress(nullptr, &compLength, ctx->snapshot, ctx->fileSize); - compLength += 8; - compData = static_cast(StorageManager.AllocateSaveData(compLength)); - } - - if (compData != nullptr) - { - Compression::getCompression()->Compress(compData + 8, &compLength, ctx->snapshot, ctx->fileSize); - - ZeroMemory(compData, 8); - int saveVer = 0; - memcpy(compData, &saveVer, sizeof(int)); - unsigned int fs = ctx->fileSize; - memcpy(compData + 4, &fs, sizeof(int)); - - app.DebugPrintf("Async save: compressed %u -> %u bytes\n", ctx->fileSize, compLength); - - // Queue for the main thread to commit via StorageManager - // (StorageManager requires main-thread calls + Tick() to flush) - { - std::lock_guard lock(s_pendingSaveMutex); - s_pendingSave.self = ctx->self; - s_pendingSave.thumbData = ctx->thumbData; - s_pendingSave.thumbSize = ctx->thumbSize; - memcpy(s_pendingSave.textMetadata, ctx->textMetadata, 88); - s_pendingSave.textMetadataBytes = ctx->textMetadataBytes; - s_pendingSave.ready = true; - } - } - else - { - app.DebugPrintf("Async save: failed to allocate compression buffer\n"); - s_asyncSaveInFlight.store(false, std::memory_order_release); + std::lock_guard lock(s_pendingSaveMutex); + s_pendingSave.self = ctx->self; + s_pendingSave.thumbData = ctx->thumbData; + s_pendingSave.thumbSize = ctx->thumbSize; + memcpy(s_pendingSave.textMetadata, ctx->textMetadata, 88); + s_pendingSave.textMetadataBytes = ctx->textMetadataBytes; + s_pendingSave.ready = true; } delete[] ctx->snapshot; diff --git a/README.md b/README.md index 9c3954b3..687e2718 100644 --- a/README.md +++ b/README.md @@ -17,8 +17,9 @@ This project is based on source code of Minecraft Legacy Console Edition v1.6.05 ### Async Autosave (Dedicated Server) - Autosave no longer freezes the server. Previously, every autosave compressed the entire world save file with zlib synchronously on the main thread, blocking all game ticks for 2-6 seconds depending on world size -- The save buffer is now snapshotted (memcpy) while holding the lock (~18ms), then compression runs on a background thread. The compressed data is committed back to StorageManager on the next main-thread tick -- Additionally, autosave on the dedicated server now only flushes entity data instead of re-saving all dirty chunks, matching the Xbox/Orbis behavior. Chunks are already saved continuously by the per-tick trickle save process +- The save buffer is now snapshotted (memcpy) while holding the save lock (~18ms), then compression runs on a detached background thread. Once compression finishes, the compressed data is committed back to StorageManager on the next game tick. If a previous async save is still in flight, it falls back to the original synchronous path to prevent unbounded queuing +- Autosave on the dedicated server now only flushes entity data instead of re-saving all dirty chunks, matching the Xbox/Orbis behavior. Chunks are already saved continuously by the per-tick trickle save process +- The async save commit runs from both the game thread tick and the ServerMain tick loop (via `TickCoreSystems`), ensuring saves complete during bootstrap, normal gameplay, and shutdown - Autosave timing breakdown is now logged to `server.log` for diagnostics (e.g. `autosave breakdown: players=0ms levels=0ms rules=0ms flush=18ms total=18ms`) ### Dedicated Server Entity Tracking Optimization