diff --git a/core/libs/prx/libSceSaveData.native/Export.cpp b/core/libs/prx/libSceSaveData.native/Export.cpp index e1f14d1902..cd3f8fd5ad 100644 --- a/core/libs/prx/libSceSaveData.native/Export.cpp +++ b/core/libs/prx/libSceSaveData.native/Export.cpp @@ -122,7 +122,7 @@ void StartEventDelivery() { std::lock_guard lock(callbacks.mutex); if (callbacks.worker != nullptr) return; if (scePthreadCreate(&worker->thread, nullptr, DeliverEvents, worker.get(), "SceSaveDataEventCallback") != 0) { - throw std::runtime_error("sceSaveDataInitialize3: scePthreadCreate failed for the event callback thread"); + throw std::runtime_error("sceSaveDataRegisterEventCallback: scePthreadCreate failed for the event callback thread"); } callbacks.worker = worker.release(); } @@ -1011,7 +1011,6 @@ int APS5_VABI sceSaveDataGetSaveDataMemory2(SaveDataMemoryGet2* get_param) { int APS5_VABI sceSaveDataInitialize3(const void* init) { (void)init; std::lock_guard lock(g_slots_mutex); - if (g_initializations == 0) StartEventDelivery(); ++g_initializations; return SAVE_DATA_OK; } @@ -1770,6 +1769,7 @@ int APS5_VABI sceSaveDataRegisterEventCallback(SaveDataEventCallback callback, v if (callback == nullptr) { return SAVE_DATA_ERROR_PARAMETER; } + StartEventDelivery(); std::lock_guard lock(callbacks.mutex); callbacks.callback = callback; callbacks.userdata = userdata; diff --git a/core/libs/tests/GuestSaveDataEventCallback.cpp b/core/libs/tests/GuestSaveDataEventCallback.cpp index ea269616cb..ea1d0f208a 100644 --- a/core/libs/tests/GuestSaveDataEventCallback.cpp +++ b/core/libs/tests/GuestSaveDataEventCallback.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -134,6 +135,21 @@ private: std::thread thread; }; +#ifdef __linux__ +std::size_t TaskCount() { + return static_cast(std::distance(std::filesystem::directory_iterator("/proc/self/task"), std::filesystem::directory_iterator{})); +} + +bool WaitForTaskCount(std::size_t count) { + const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(5); + while (TaskCount() != count) { + if (std::chrono::steady_clock::now() > deadline) return false; + std::this_thread::sleep_for(std::chrono::milliseconds(1)); + } + return true; +} +#endif + void WriteText(const std::filesystem::path& path, const std::string& text) { std::ofstream file(path, std::ios::binary | std::ios::trunc); file << text; @@ -186,6 +202,43 @@ void CheckDelivered(std::size_t index, int callback, void* userdata, std::uint32 Check(SameEvent(polled, call.event), what + " is the event the poll returns"); } +void CheckSessionsWithoutRegistration() { +#ifdef __linux__ + const auto before = TaskCount(); +#endif + for (int session = 0; session < 2; ++session) { + const auto label = "session " + std::to_string(session) + " without a registration"; + Check(sceSaveDataInitialize3(nullptr) == 0, label + ": SaveData initializes"); +#ifdef __linux__ + Check(TaskCount() == before, label + ": Initialize3 starts no thread, " + std::to_string(before) + " threads before and " + std::to_string(TaskCount()) + " after"); +#endif + Check(sceSaveDataTerminate() == 0, label + ": SaveData terminates"); + } +} + +#ifdef __linux__ +void CheckDeliveryThreadStartsAtRegistration() { + const auto before = TaskCount(); + const auto counted = [&](const std::string& what) { return what + ", " + std::to_string(before) + " threads before and " + std::to_string(TaskCount()) + " now"; }; + int tag = 0; + Check(sceSaveDataInitialize3(nullptr) == 0, "SaveData initializes for the delivery thread check"); + Check(TaskCount() == before, counted("Initialize3 starts no delivery thread")); + Check(sceSaveDataRegisterEventCallback(nullptr, &tag) == SaveDataErrorParameter, "a null callback is PARAMETER in the delivery thread check"); + Check(TaskCount() == before, counted("a refused registration starts no delivery thread")); + Check(sceSaveDataRegisterEventCallback(First, &tag) == 0, "the first callback registers in the delivery thread check"); + Check(TaskCount() == before + 1, counted("the first registration starts one delivery thread")); + Check(sceSaveDataRegisterEventCallback(Second, &tag) == 0, "the second callback registers in the delivery thread check"); + Check(sceSaveDataUnregisterEventCallback() == 0, "the second callback unregisters in the delivery thread check"); + Check(sceSaveDataRegisterEventCallback(First, &tag) == 0, "the first callback registers again in the delivery thread check"); + Check(TaskCount() == before + 1, counted("a replacing registration and one after an unregister reuse the delivery thread")); + { + Watchdog watchdog("the terminate that joins the delivery thread"); + Check(sceSaveDataTerminate() == 0, "SaveData terminates after the delivery thread check"); + } + Check(WaitForTaskCount(before), counted("the final terminate joins the delivery thread within 5 s")); +} +#endif + void CheckRegistration() { int tag = 0; Check(sceSaveDataRegisterEventCallback(First, &tag) == SaveDataErrorNotInitialized, "register before initialize is NOT_INITIALIZED"); @@ -394,6 +447,10 @@ int main() { std::filesystem::current_path(root); int firstTag = 0; int secondTag = 0; + CheckSessionsWithoutRegistration(); +#ifdef __linux__ + CheckDeliveryThreadStartsAtRegistration(); +#endif CheckRegistration(); CheckBackupDelivery(&firstTag); CheckUmountWithBackupDelivery(&firstTag); diff --git a/docs/dev/debt/libSceSaveData.native.md b/docs/dev/debt/libSceSaveData.native.md index 16e2bfba66..e9b8d29929 100644 --- a/docs/dev/debt/libSceSaveData.native.md +++ b/docs/dev/debt/libSceSaveData.native.md @@ -32,7 +32,7 @@ - [sceSaveDataCheckBackupData, sceSaveDataRestoreBackupData](../../../core/libs/prx/libSceSaveData.native/Export.cpp) (libSceSaveData.native) - saves keep no icons, so the check never fills `icon`, as shadPS4 does without `icon0.png`, and another title's `titleId` returns `SAVE_DATA_ERROR_NOT_FOUND` because only the running title's saves are kept. Backups live in `_sd_backup/`, outside the save, and `sceSaveDataDelete` removes one with its save, as shadPS4's [backup](https://github.com/shadps4-emu/shadPS4/blob/ce74386283f559f2279a7c31416bcdc9359be0b3/src/core/libraries/save_data/save_backup.cpp#L93-L134) inside the save directory goes with it. On a PS4 Pro (firmware 10.71) another title's `titleId` also returns `SAVE_DATA_ERROR_NOT_FOUND`, and `sceSaveDataDelete` drops the backup too: after it the check and the restore return `SAVE_DATA_ERROR_NOT_FOUND` ([run](https://github.com/boykopovar/AnyPS5/pull/12748#issuecomment-6499581390)). - [sceSaveDataCheckBackupData](../../../core/libs/prx/libSceSaveData.native/Export.cpp) (libSceSaveData.native) - a `param` buffer that isn't zeroed is not refused here. On a PS4 Pro (firmware 10.71, the system `libSceSaveData.sprx`, SDK 0x05050001) a 1,328-byte `param` filled with 0xAA gave `SAVE_DATA_ERROR_PARAMETER` on every call, also with a backup present, while a zeroed or null one gave 0 ([run](https://github.com/boykopovar/AnyPS5/pull/12748#issuecomment-6499581390)). Which bytes the console checks wasn't measured, so the check fills `param` whatever it held. Not measured on a PS5. - [sceSaveDataRestoreBackupData](../../../core/libs/prx/libSceSaveData.native/Export.cpp) (libSceSaveData.native) - a crash during a restore is not recovered at the next start, and what the console leaves is not measured. A failed restore leaves the live save, its `.param` and its blocks file as they were: the backup is copied to `_sd_restore_tmp/` and checked first, and a failed move puts back what was already moved; only if that fails too does the old save stay in `_sd_restore_old/`. A crash during the copy leaves the live save untouched. One during the moves can leave parts of the live save in `_sd_restore_old/` and the copy in `_sd_restore_tmp/` or `_sd`; while the save's directory is missing from `_sd`, its restore returns `SAVE_DATA_ERROR_INTERNAL` and a creating mount makes an empty save. The next restore of the save first removes both directories. -- [sceSaveDataRegisterEventCallback, sceSaveDataUnregisterEventCallback](../../../core/libs/prx/libSceSaveData.native/Export.cpp) (libSceSaveData.native) - not measured on a console: the behaviour follows [shadPS4 #5226](https://github.com/shadps4-emu/shadPS4/pull/5226), and PS5's signatures are assumed to be PS4's (`void (*)(const SaveDataEvent*, void*)` and a userdata pointer). Unknown: which thread delivers, its priority and affinity, when it runs relative to the call, whether unregistering drops completions not yet delivered (here it does), and what a terminate from inside the callback does (here `SAVE_DATA_ERROR_BUSY`, as in shadPS4). Here `sceSaveDataInitialize3` starts the delivering thread with `scePthreadCreate`, the last `sceSaveDataTerminate` joins it, and each backup completion (type 1 or 2) is queued for `sceSaveDataGetEventResult` before it goes to the callback registered at that moment. +- [sceSaveDataRegisterEventCallback, sceSaveDataUnregisterEventCallback](../../../core/libs/prx/libSceSaveData.native/Export.cpp) (libSceSaveData.native) - not measured on a console: the behaviour follows [shadPS4 #5226](https://github.com/shadps4-emu/shadPS4/pull/5226), and PS5's signatures are assumed to be PS4's (`void (*)(const SaveDataEvent*, void*)` and a userdata pointer). Unknown: which thread delivers, its priority and affinity, when it runs relative to the call, whether unregistering drops completions not yet delivered (here it does), and what a terminate from inside the callback does (here `SAVE_DATA_ERROR_BUSY`, as in shadPS4). Here the first successful `sceSaveDataRegisterEventCallback` after `sceSaveDataInitialize3` starts the delivering thread with `scePthreadCreate`, the last `sceSaveDataTerminate` joins it if one was started, and each backup completion (type 1 or 2) is queued for `sceSaveDataGetEventResult` before it goes to the callback registered at that moment. - [sceSaveDataUmountWithBackup](../../../core/libs/prx/libSceSaveData.native/Export.cpp) (libSceSaveData.native) - a crash during a backup is not recovered at the next start either. A failed backup keeps the previous one in `_sd_backup/`. A crash between its two moves leaves the previous backup in `_sd_backup_old/`, where the check and the restore do not find it (`SAVE_DATA_ERROR_NOT_FOUND`); the next backup of the save removes it, and any partial copy in `_sd_backup_tmp/`, before it copies. - [sceSaveDataRestoreBackupData, sceSaveDataUmountWithBackup](../../../core/libs/prx/libSceSaveData.native/Export.cpp) (libSceSaveData.native) - on Windows nothing shows that a backup's or a restore's renames survive a crash or power loss. Every staged file is flushed before the first rename (`FlushFileBuffers` on Windows, `fsync` on POSIX, as libc does), and on POSIX the directories on both sides of each rename are flushed after it; Windows cannot flush directory entries, so there it rests on NTFS journaling. - [sceSaveDataBackup, sceSaveDataConvert, sceSaveDataDelete, sceSaveDataDirNameSearch, sceSaveDataDirNameSearchPs4, sceSaveDataGetEventResult, sceSaveDataGetMountInfo, sceSaveDataGetParam, sceSaveDataLoadIcon, sceSaveDataMount, sceSaveDataMount3, sceSaveDataSaveIcon, sceSaveDataSetParam, sceSaveDataUmount2](../../../core/libs/prx/libSceSaveData.native/Export.cpp) (libSceSaveData.native) - null pointer arguments return `SAVE_DATA_ERROR_PARAMETER` instead of throwing C++ exceptions, an implementation assumption matching guest error expectations not checked on hardware; unmounted targets return `SAVE_DATA_ERROR_NOT_MOUNTED`, and parameter type or buffer size violations return `SAVE_DATA_ERROR_PARAMETER` after slot lookup