From 4e3535f2c8fb68cb123ee1df4bc25bc7faa16029 Mon Sep 17 00:00:00 2001 From: guoling Date: Wed, 29 Jul 2026 14:56:48 +0800 Subject: [PATCH] Harden MMKV close lifecycle handling #1669 --- .../java/com/tencent/mmkv/MMKVTest.java | 3 + .../src/main/java/com/tencent/mmkv/MMKV.java | 11 +- CHANGELOG.md | 5 +- Core/MMKV.cpp | 4 + Core/MMKV.h | 5 +- Core/ThreadLock.cpp | 34 ++++- Core/ThreadLock.h | 5 + Core/ThreadLock_Win32.cpp | 10 ++ Core/cbridge/MMKVBridge.h | 5 +- KMP/README.md | 14 +- .../com/tencent/mmkv/kmp/MMKV.android.kt | 4 +- .../kotlin/com/tencent/mmkv/kmp/MMKV.kt | 10 +- .../com/tencent/mmkv/kmp/MMKVSmokeTest.kt | 19 ++- .../com/tencent/mmkv/kmp/MMKV.darwin.kt | 4 +- .../com/tencent/mmkv/kmp/MMKV.desktop.kt | 4 +- .../tencent/mmkv/kmp/MMKV.nativeDesktop.kt | 4 +- OpenHarmony/MMKV/src/main/ets/utils/MMKV.ets | 7 +- POSIX/CMakeLists.txt | 3 +- POSIX/demo/CMakeLists.txt | 15 +- POSIX/demo/TestThreadLock.cpp | 132 ++++++++++++++++++ POSIX/golang/mmkv.go | 5 +- Python/libmmkv_python.cpp | 6 +- flutter/mmkv/lib/mmkv.dart | 8 +- iOS/MMKV/MMKV/MMKV.h | 10 +- iOS/MMKV/MMKV/libMMKV.mm | 10 +- .../MMKVDemo.xcodeproj/project.pbxproj | 6 +- iOS/MMKVDemo/MMKVDemoTests/MMKVDemoTests.mm | 46 ++++++ 27 files changed, 316 insertions(+), 73 deletions(-) create mode 100644 POSIX/demo/TestThreadLock.cpp diff --git a/Android/MMKV/mmkv/src/androidTest/java/com/tencent/mmkv/MMKVTest.java b/Android/MMKV/mmkv/src/androidTest/java/com/tencent/mmkv/MMKVTest.java index e23a766b..1aec5c8a 100644 --- a/Android/MMKV/mmkv/src/androidTest/java/com/tencent/mmkv/MMKVTest.java +++ b/Android/MMKV/mmkv/src/androidTest/java/com/tencent/mmkv/MMKVTest.java @@ -290,6 +290,9 @@ public class MMKVTest { kv.close(); kv.close(); assertFalse(kv.encode("afterClose", true)); + // Two live wrappers backed by the same native instance are outside the + // supported close() contract. Discard the closed wrapper before reopen. + kv = null; MMKV reopened = MMKV.mmkvWithID(mmapID); assertTrue(reopened.decodeBool("value")); diff --git a/Android/MMKV/mmkv/src/main/java/com/tencent/mmkv/MMKV.java b/Android/MMKV/mmkv/src/main/java/com/tencent/mmkv/MMKV.java index 8f40439e..748fe792 100644 --- a/Android/MMKV/mmkv/src/main/java/com/tencent/mmkv/MMKV.java +++ b/Android/MMKV/mmkv/src/main/java/com/tencent/mmkv/MMKV.java @@ -50,7 +50,7 @@ import java.util.Set; * An highly efficient, reliable, multi-process key-value storage framework. * THE PERFECT drop-in replacement for SharedPreferences and MultiProcessSharedPreferences. */ -public class MMKV implements SharedPreferences, SharedPreferences.Editor, AutoCloseable { +public class MMKV implements SharedPreferences, SharedPreferences.Editor { private static final EnumMap recoverIndex; private static final EnumMap logLevel2Index; @@ -1331,12 +1331,11 @@ public class MMKV implements SharedPreferences, SharedPreferences.Editor, AutoCl /** * Permanently close the underlying native MMKV instance. *

- * This is a terminal, idempotent operation on this wrapper. It invalidates every Java - * wrapper/reference backed by the same native instance. Make sure no operation is running, - * discard all aliases, and only then reopen the same ID. Calls on this wrapper after close - * have no effect or return the method's empty/default value. + * All references backed by the same native instance become invalid immediately. + * The caller must ensure no operation is running and no reference is used afterward. + * Discard every reference before reopening the same ID. Repeated close on this Java + * wrapper is a no-op after its handle has been cleared. */ - @Override public void close() { synchronized (this) { long handle = nativeHandle; diff --git a/CHANGELOG.md b/CHANGELOG.md index aa211c22..b371d5f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,11 +1,12 @@ # MMKV Change Log -## v2.4.1 / 2026-07-28 +## v2.4.1 / 2026-07-29 This is a bugfix-oriented release based on v2.4.0. It focuses on data-safety fixes, encrypted-file compatibility, packaging/build cleanup, and ready-to-use Kotlin Multiplatform support for Android and iOS. ### Changes for All Platforms * **Fix:** Fixed an iOS/macOS Objective-C ownership handoff race where auto cleanup could release an MMKV wrapper before an ARC caller retained the returned instance. -* **Change:** Hardened and clarified `close()` as a terminal lifecycle operation across public APIs. Managed raw-handle wrappers now make repeated close safe and clear their handle; closing invalidates every alias backed by the same native instance, while raw C++/C/Python/Go handles explicitly require exclusive ownership. Android and KMP MMKV wrappers now implement `AutoCloseable`. +* **Fix:** Made native `ThreadLock` destruction release all recursive ownership held by the closing thread before destroying the POSIX mutex or Windows critical section. +* **Change:** Clarified `close()` as a destructive terminal lifecycle operation across public APIs. Closing invalidates every reference backed by the same native instance; callers must ensure no operation is running, discard all references, and only then reopen the same ID. Managed wrappers clear their own handle so repeated close on the same wrapper is harmless. * **Fix:** Fixed an oversized-key corruption bug where keys longer than the internal `uint16_t` holder limit could overflow key/value metadata and corrupt subsequent reads. * **Fix:** Fixed an `expireDuration` overflow bug ([#1665](https://github.com/tencent/mmkv/issues/1665)). Very large expiration durations are now clamped safely instead of wrapping around. * **Fix:** Improved encrypted MMKV random-IV upgrade behavior. Existing encrypted files using the older IV format now trigger a full writeback when possible so they are upgraded to the random-IV format. diff --git a/Core/MMKV.cpp b/Core/MMKV.cpp index daf12a1a..39dd3a73 100755 --- a/Core/MMKV.cpp +++ b/Core/MMKV.cpp @@ -375,6 +375,10 @@ void MMKV::close() { if (itr != g_instanceDic->end()) { g_instanceDic->erase(itr); } + // close() requires the caller to guarantee that no other operation or + // alias will use this instance. ThreadLock's destructor releases the + // recursive lock ownership held by this close path before destroying the + // underlying platform lock. delete this; } diff --git a/Core/MMKV.h b/Core/MMKV.h index b21ef085..5a87150f 100755 --- a/Core/MMKV.h +++ b/Core/MMKV.h @@ -609,8 +609,9 @@ public: size_t importFrom(MMKV *src); // Permanently close and destroy this instance. This is a terminal operation. - // All aliases to the pointer become invalid immediately after this call. - // The caller must ensure close() does not race with any other operation. + // All references backed by this native instance become invalid immediately. + // The caller must ensure close() does not race with any other operation and + // no reference is used afterward. void close(); // call this method if you are facing memory-warning diff --git a/Core/ThreadLock.cpp b/Core/ThreadLock.cpp index ab113e18..6c31f68b 100644 --- a/Core/ThreadLock.cpp +++ b/Core/ThreadLock.cpp @@ -38,28 +38,52 @@ ThreadLock::ThreadLock() : m_lock({}) { } ThreadLock::~ThreadLock() { - pthread_mutex_unlock(&m_lock); + while (m_lockCount > 0) { + auto ret = pthread_mutex_unlock(&m_lock); + if (ret != 0) { + MMKVError("fail to unlock %p while destroying, ret=%d, error=%s", + &m_lock, ret, strerror(ret)); + break; + } + --m_lockCount; + } - pthread_mutex_destroy(&m_lock); + auto ret = pthread_mutex_destroy(&m_lock); + if (ret != 0) { + MMKVError("fail to destroy %p, ret=%d, error=%s", &m_lock, ret, strerror(ret)); + } } void ThreadLock::lock() { auto ret = pthread_mutex_lock(&m_lock); if (ret != 0) { - MMKVError("fail to lock %p, ret=%d, errno=%s", &m_lock, ret, strerror(errno)); + MMKVError("fail to lock %p, ret=%d, error=%s", &m_lock, ret, strerror(ret)); + return; } + ++m_lockCount; } void ThreadLock::unlock() { + if (m_lockCount == 0) { + MMKVError("attempt to unlock unowned lock %p", &m_lock); + return; + } + --m_lockCount; auto ret = pthread_mutex_unlock(&m_lock); if (ret != 0) { - MMKVError("fail to unlock %p, ret=%d, errno=%s", &m_lock, ret, strerror(errno)); + ++m_lockCount; + MMKVError("fail to unlock %p, ret=%d, error=%s", &m_lock, ret, strerror(ret)); + return; } } bool ThreadLock::try_lock() { auto ret = pthread_mutex_trylock(&m_lock); - return (ret == 0); + if (ret == 0) { + ++m_lockCount; + return true; + } + return false; } void ThreadLock::initialize() { diff --git a/Core/ThreadLock.h b/Core/ThreadLock.h index b9a77965..24358e72 100755 --- a/Core/ThreadLock.h +++ b/Core/ThreadLock.h @@ -33,6 +33,7 @@ #ifndef MMKV_USING_PTHREAD # include #endif +#include namespace mmkv { @@ -50,6 +51,10 @@ class ThreadLock { #else CRITICAL_SECTION m_lock; #endif + // Tracks recursive ownership by the current thread. Destroying a lock + // owned by another thread remains unsupported; MMKV::close() requires the + // caller to ensure there are no racing operations. + uint32_t m_lockCount = 0; public: ThreadLock(); diff --git a/Core/ThreadLock_Win32.cpp b/Core/ThreadLock_Win32.cpp index 2f41f835..5634a8ff 100644 --- a/Core/ThreadLock_Win32.cpp +++ b/Core/ThreadLock_Win32.cpp @@ -32,6 +32,10 @@ ThreadLock::ThreadLock() : m_lock{0} { } ThreadLock::~ThreadLock() { + while (m_lockCount > 0) { + LeaveCriticalSection(&m_lock); + --m_lockCount; + } DeleteCriticalSection(&m_lock); } @@ -44,9 +48,15 @@ void ThreadLock::initialize() { void ThreadLock::lock() { EnterCriticalSection(&m_lock); + ++m_lockCount; } void ThreadLock::unlock() { + if (m_lockCount == 0) { + MMKVError("attempt to unlock unowned lock %p", &m_lock); + return; + } + --m_lockCount; LeaveCriticalSection(&m_lock); } diff --git a/Core/cbridge/MMKVBridge.h b/Core/cbridge/MMKVBridge.h index 86d95bc0..e160ae94 100644 --- a/Core/cbridge/MMKVBridge.h +++ b/Core/cbridge/MMKVBridge.h @@ -87,8 +87,9 @@ MMKVHandle_t mmkv_default(MMKVConfig_t config); MMKVHandle_t mmkv_with_id(const char *mmapID, MMKVConfig_t config); /* Returns internal pointer — valid for the lifetime of the instance. Do NOT free. */ const char *mmkv_mmap_id(MMKVHandle_t handle); -/* Permanently close an MMKV instance. All aliases to this handle become invalid. - This terminal operation must not race with any other operation. */ +/* Permanently close an MMKV instance. All references backed by the same native + instance become invalid immediately. This operation must not race with any + other operation, and no reference may be used afterward. */ void mmkv_close(MMKVHandle_t handle); /* Close an MMKV instance and clear the caller's handle. This is the preferred idempotent form for managed bindings. */ diff --git a/KMP/README.md b/KMP/README.md index ee95faee..ab3b1fcf 100644 --- a/KMP/README.md +++ b/KMP/README.md @@ -85,16 +85,10 @@ kv.encodeString("name", "MMKV") val name = kv.decodeString("name") ``` -`MMKV` implements `AutoCloseable`, so scoped ownership can use Kotlin's -`use {}` helper. `close()` is terminal and idempotent on that wrapper, but -it invalidates all wrappers backed by the same native instance. Do not race -it with another operation, and discard all aliases before reopening the ID. - -```kotlin -MMKV.mmkvWithID("scoped").use { kv -> - kv.encodeString("name", "MMKV") -} -``` +`close()` permanently destroys the native MMKV instance. All references backed +by the same native instance become invalid immediately. The caller must ensure +that no operation is running and no reference is used afterward. Discard every +reference before reopening the same ID. Android delegates to the native `com.tencent:mmkv:2.4.1` AAR. iOS embeds MMKV Core through the C bridge in the published native KLIBs, so consumers do not diff --git a/KMP/mmkv/src/androidMain/kotlin/com/tencent/mmkv/kmp/MMKV.android.kt b/KMP/mmkv/src/androidMain/kotlin/com/tencent/mmkv/kmp/MMKV.android.kt index 1c1df3b9..78de5184 100644 --- a/KMP/mmkv/src/androidMain/kotlin/com/tencent/mmkv/kmp/MMKV.android.kt +++ b/KMP/mmkv/src/androidMain/kotlin/com/tencent/mmkv/kmp/MMKV.android.kt @@ -71,7 +71,7 @@ fun MMKV.Companion.initialize( * Since the KMP class is now in `com.tencent.mmkv.kmp`, we can reference the Java * `com.tencent.mmkv.MMKV` directly via import alias without any naming collision. */ -actual class MMKV internal constructor(private val impl: AndroidMMKV) : AutoCloseable { +actual class MMKV internal constructor(private val impl: AndroidMMKV) { actual companion object { actual fun onExit() = AndroidMMKV.onExit() @@ -182,7 +182,7 @@ actual class MMKV internal constructor(private val impl: AndroidMMKV) : AutoClos actual fun sync() = impl.sync() actual fun async() = impl.async() actual fun trim() = impl.trim() - actual override fun close() = impl.close() + actual fun close() = impl.close() actual fun clearMemoryCache() = impl.clearMemoryCache() actual fun importFrom(source: MMKV): Long = impl.importFrom(source.impl) diff --git a/KMP/mmkv/src/commonMain/kotlin/com/tencent/mmkv/kmp/MMKV.kt b/KMP/mmkv/src/commonMain/kotlin/com/tencent/mmkv/kmp/MMKV.kt index 677c269c..587d41ed 100644 --- a/KMP/mmkv/src/commonMain/kotlin/com/tencent/mmkv/kmp/MMKV.kt +++ b/KMP/mmkv/src/commonMain/kotlin/com/tencent/mmkv/kmp/MMKV.kt @@ -28,7 +28,7 @@ package com.tencent.mmkv.kmp * on [MMKV.Companion] in each platform source set, since Android requires [Context] * while other platforms do not. */ -expect class MMKV : AutoCloseable { +expect class MMKV { companion object { /** @@ -275,11 +275,11 @@ expect class MMKV : AutoCloseable { /** * Permanently close the underlying native MMKV instance. - * This is terminal and idempotent on this wrapper, and it invalidates every - * wrapper backed by the same native instance. Do not race close with another - * operation. Discard all aliases before reopening the same ID. + * All references backed by the same native instance become invalid immediately. + * The caller must ensure no operation is running and no reference is used afterward. + * Discard every reference before reopening the same ID. */ - override fun close() + fun close() /** * Clear memory cache of the MMKV instance. diff --git a/KMP/mmkv/src/commonTest/kotlin/com/tencent/mmkv/kmp/MMKVSmokeTest.kt b/KMP/mmkv/src/commonTest/kotlin/com/tencent/mmkv/kmp/MMKVSmokeTest.kt index eaf2207b..a1a5454a 100644 --- a/KMP/mmkv/src/commonTest/kotlin/com/tencent/mmkv/kmp/MMKVSmokeTest.kt +++ b/KMP/mmkv/src/commonTest/kotlin/com/tencent/mmkv/kmp/MMKVSmokeTest.kt @@ -123,16 +123,20 @@ class MMKVSmokeTest { fun closeIsIdempotentAndTerminal() { MMKVTestEnv.initialize() val id = MMKVTestEnv.uniqueID("close") - val kv = MMKV.mmkvWithID(id) - kv.clearAll() - assertTrue(kv.encodeString("key", "value")) + run { + val kv = MMKV.mmkvWithID(id) + kv.clearAll() + assertTrue(kv.encodeString("key", "value")) - kv.close() - kv.close() - assertFailsWith { - kv.decodeString("key") + kv.close() + kv.close() + assertFailsWith { + kv.decodeString("key") + } } + // Two live wrappers backed by the same native instance are outside the + // supported close() contract. The closed wrapper is scoped away before reopen. val reopened = MMKV.mmkvWithID(id) try { assertEquals("value", reopened.decodeString("key")) @@ -141,4 +145,5 @@ class MMKVSmokeTest { reopened.close() } } + } diff --git a/KMP/mmkv/src/darwinMain/kotlin/com/tencent/mmkv/kmp/MMKV.darwin.kt b/KMP/mmkv/src/darwinMain/kotlin/com/tencent/mmkv/kmp/MMKV.darwin.kt index 1ac017c1..0efde2e6 100644 --- a/KMP/mmkv/src/darwinMain/kotlin/com/tencent/mmkv/kmp/MMKV.darwin.kt +++ b/KMP/mmkv/src/darwinMain/kotlin/com/tencent/mmkv/kmp/MMKV.darwin.kt @@ -90,7 +90,7 @@ fun MMKV.Companion.initialize( // endregion @OptIn(ExperimentalForeignApi::class) -actual class MMKV internal constructor(handle: COpaquePointer) : AutoCloseable { +actual class MMKV internal constructor(handle: COpaquePointer) { private var nativeHandle: COpaquePointer? = handle private val handle: COpaquePointer @@ -363,7 +363,7 @@ actual class MMKV internal constructor(handle: COpaquePointer) : AutoCloseable { actual fun sync() = mmkv_sync(handle, true) actual fun async() = mmkv_sync(handle, false) actual fun trim() = mmkv_trim(handle) - actual override fun close() { + actual fun close() { val current = nativeHandle ?: return memScoped { val handleVar = alloc() diff --git a/KMP/mmkv/src/desktopMain/kotlin/com/tencent/mmkv/kmp/MMKV.desktop.kt b/KMP/mmkv/src/desktopMain/kotlin/com/tencent/mmkv/kmp/MMKV.desktop.kt index 51e87bfe..61d8cc08 100644 --- a/KMP/mmkv/src/desktopMain/kotlin/com/tencent/mmkv/kmp/MMKV.desktop.kt +++ b/KMP/mmkv/src/desktopMain/kotlin/com/tencent/mmkv/kmp/MMKV.desktop.kt @@ -175,7 +175,7 @@ fun MMKV.Companion.initialize( // endregion -actual class MMKV internal constructor(handle: Pointer) : AutoCloseable { +actual class MMKV internal constructor(handle: Pointer) { private var nativeHandle: Pointer? = handle private val handle: Pointer @@ -351,7 +351,7 @@ actual class MMKV internal constructor(handle: Pointer) : AutoCloseable { actual fun sync() = lib.mmkv_sync(handle, true) actual fun async() = lib.mmkv_sync(handle, false) actual fun trim() = lib.mmkv_trim(handle) - actual override fun close() { + actual fun close() { val current = nativeHandle ?: return val handleRef = PointerByReference(current) lib.mmkv_close_handle(handleRef) diff --git a/KMP/mmkv/src/nativeDesktopMain/kotlin/com/tencent/mmkv/kmp/MMKV.nativeDesktop.kt b/KMP/mmkv/src/nativeDesktopMain/kotlin/com/tencent/mmkv/kmp/MMKV.nativeDesktop.kt index 16e770a6..bd0d26b5 100644 --- a/KMP/mmkv/src/nativeDesktopMain/kotlin/com/tencent/mmkv/kmp/MMKV.nativeDesktop.kt +++ b/KMP/mmkv/src/nativeDesktopMain/kotlin/com/tencent/mmkv/kmp/MMKV.nativeDesktop.kt @@ -58,7 +58,7 @@ fun MMKV.Companion.initialize( // endregion @OptIn(ExperimentalForeignApi::class) -actual class MMKV internal constructor(handle: COpaquePointer) : AutoCloseable { +actual class MMKV internal constructor(handle: COpaquePointer) { private var nativeHandle: COpaquePointer? = handle private val handle: COpaquePointer @@ -329,7 +329,7 @@ actual class MMKV internal constructor(handle: COpaquePointer) : AutoCloseable { actual fun sync() = mmkv_sync(handle, true) actual fun async() = mmkv_sync(handle, false) actual fun trim() = mmkv_trim(handle) - actual override fun close() { + actual fun close() { val current = nativeHandle ?: return memScoped { val handleVar = alloc() diff --git a/OpenHarmony/MMKV/src/main/ets/utils/MMKV.ets b/OpenHarmony/MMKV/src/main/ets/utils/MMKV.ets index b4b7d59d..cdaec919 100644 --- a/OpenHarmony/MMKV/src/main/ets/utils/MMKV.ets +++ b/OpenHarmony/MMKV/src/main/ets/utils/MMKV.ets @@ -682,9 +682,10 @@ export class MMKV { /** * Permanently close the underlying native MMKV instance. - * This is terminal and idempotent on this wrapper, and it invalidates every - * wrapper backed by the same native instance. Do not race close with another - * operation. Discard all aliases before reopening the same ID. + * All references backed by the same native instance become invalid immediately. + * The caller must ensure no operation is running and no reference is used afterward. + * Discard every reference before reopening the same ID. Repeated close on this + * ArkTS wrapper is a no-op after its handle has been cleared. */ public close(): void { const handle = this.nativeHandle; diff --git a/POSIX/CMakeLists.txt b/POSIX/CMakeLists.txt index 2a539088..36bc51e0 100644 --- a/POSIX/CMakeLists.txt +++ b/POSIX/CMakeLists.txt @@ -22,6 +22,7 @@ cmake_minimum_required(VERSION 3.10.0) project(MMKV) +include(CTest) + add_subdirectory(src) add_subdirectory(demo) - diff --git a/POSIX/demo/CMakeLists.txt b/POSIX/demo/CMakeLists.txt index 7f39ada3..7c046bff 100644 --- a/POSIX/demo/CMakeLists.txt +++ b/POSIX/demo/CMakeLists.txt @@ -58,6 +58,19 @@ set_target_properties(UnitTest PROPERTIES CXX_STANDARD 20 ) +add_executable(TestThreadLock + TestThreadLock.cpp) +target_include_directories(TestThreadLock PRIVATE + ../../Core) +target_link_libraries(TestThreadLock + mmkv) +set_target_properties(TestThreadLock PROPERTIES + CXX_STANDARD 17 + ) +if(BUILD_TESTING) + add_test(NAME TestThreadLock COMMAND TestThreadLock) +endif() + add_executable(demo_c demo_c.c) target_link_libraries(demo_c @@ -69,6 +82,6 @@ set_target_properties(demo_c PROPERTIES add_dependencies(demo process TestInterProcessLock + TestThreadLock UnitTest demo_c) - diff --git a/POSIX/demo/TestThreadLock.cpp b/POSIX/demo/TestThreadLock.cpp new file mode 100644 index 00000000..e0908ee3 --- /dev/null +++ b/POSIX/demo/TestThreadLock.cpp @@ -0,0 +1,132 @@ +/* + * Tencent is pleased to support the open source community by making + * MMKV available. + * + * Copyright (C) 2026 THL A29 Limited, a Tencent company. + * All rights reserved. + * + * Licensed under the BSD 3-Clause License (the "License"); you may not use + * this file except in compliance with the License. You may obtain a copy of + * the License at + * + * https://opensource.org/licenses/BSD-3-Clause + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "ThreadLock.h" +#include "MMKVHandler.h" +#include "MMKVLog.h" + +#include +#include +#include +#include +#include + +using namespace mmkv; + +class TestLogHandler final : public MMKVHandler { +public: + std::atomic errorCount{0}; + + void mmkvLog(MMKVLogLevel level, + const char *, + int, + const char *, + MMKVLog_t) override { + if (level == MMKVLogError) { + ++errorCount; + } + } +}; + +static void require(bool condition, const char *message) { + if (!condition) { + std::fprintf(stderr, "test ThreadLock destruction: %s\n", message); + std::exit(EXIT_FAILURE); + } +} + +static void destroyUnlockedLock() { + auto lock = new ThreadLock(); + delete lock; +} + +static void destroyLockedOnce() { + auto lock = new ThreadLock(); + lock->lock(); + delete lock; +} + +static void destroyRecursivelyLocked() { + auto lock = new ThreadLock(); + lock->lock(); + lock->lock(); + lock->lock(); + delete lock; +} + +static void tryLockTracksOwnership() { + auto lock = new ThreadLock(); +#ifndef MMKV_WIN32 + require(lock->try_lock(), "first try_lock failed"); + require(lock->try_lock(), "recursive try_lock failed"); +#else + lock->lock(); + lock->lock(); +#endif + delete lock; +} + +static void normalLockStillWorks() { + ThreadLock lock; + lock.lock(); + lock.lock(); + lock.unlock(); + lock.unlock(); +} + +static void concurrentLockingTracksOwnership() { + constexpr uint32_t threadCount = 8; + constexpr uint32_t iterationCount = 10000; + ThreadLock lock; + uint32_t value = 0; + std::vector threads; + threads.reserve(threadCount); + + for (uint32_t thread = 0; thread < threadCount; ++thread) { + threads.emplace_back([&lock, &value] { + for (uint32_t iteration = 0; iteration < iterationCount; ++iteration) { + lock.lock(); + ++value; + lock.unlock(); + } + }); + } + for (auto &thread : threads) { + thread.join(); + } + require(value == threadCount * iterationCount, "concurrent lock lost an update"); +} + +int main() { + TestLogHandler logHandler; + g_handler = &logHandler; + + destroyUnlockedLock(); + destroyLockedOnce(); + destroyRecursivelyLocked(); + tryLockTracksOwnership(); + normalLockStillWorks(); + concurrentLockingTracksOwnership(); + + g_handler = nullptr; + require(logHandler.errorCount == 0, "lock lifecycle logged an error"); + std::puts("test ThreadLock destruction: passed"); + return 0; +} diff --git a/POSIX/golang/mmkv.go b/POSIX/golang/mmkv.go index 902789cb..d8a08e02 100644 --- a/POSIX/golang/mmkv.go +++ b/POSIX/golang/mmkv.go @@ -281,8 +281,9 @@ type MMKV interface { Trim() // Close permanently destroys the native instance. - // The copied Go handle and all of its aliases become invalid immediately. - // Close must not race with another operation on the same instance. + // All Go values and other bindings backed by the same native instance become + // invalid immediately. Close must not race with another operation, and the + // value-receiver API means repeated Close calls are invalid. Close() /* ReKey Change encryption key for the MMKV instance. diff --git a/Python/libmmkv_python.cpp b/Python/libmmkv_python.cpp index ed93dc09..562caef7 100644 --- a/Python/libmmkv_python.cpp +++ b/Python/libmmkv_python.cpp @@ -398,9 +398,9 @@ PYBIND11_MODULE(mmkv, m) { clsMMKV.def("isReadOnly", &MMKV::isReadOnly, "check read-only mode"); clsMMKV.def("close", &MMKV::close, - "Permanently close the instance. This is terminal: all Python aliases " - "to the same native instance become invalid, and close must not race " - "with another operation."); + "Permanently close the native instance. All Python objects and other " + "bindings backed by the same instance become invalid immediately. " + "Close must not race with another operation and must not be repeated."); clsMMKV.def_static("rootDir", &MMKV::getRootDir, "get the root directory of MMKV"); diff --git a/flutter/mmkv/lib/mmkv.dart b/flutter/mmkv/lib/mmkv.dart index 6835e940..c7643616 100644 --- a/flutter/mmkv/lib/mmkv.dart +++ b/flutter/mmkv/lib/mmkv.dart @@ -723,9 +723,11 @@ class MMKV { /// Permanently close the underlying native MMKV instance. /// - /// This is terminal and idempotent on this wrapper, and it invalidates every - /// wrapper backed by the same native instance. Do not race close with another - /// operation. Discard all aliases before reopening the same ID. + /// All references backed by the same native instance become invalid + /// immediately. The caller must ensure no operation is running and no + /// reference is used afterward. Discard every reference before reopening the + /// same ID. Repeated close on this Dart wrapper is a no-op after its handle + /// has been cleared. void close() { final handle = _handle; if (handle == nullptr) { diff --git a/iOS/MMKV/MMKV/MMKV.h b/iOS/MMKV/MMKV/MMKV.h index 2bafea6c..b16637f0 100644 --- a/iOS/MMKV/MMKV/MMKV.h +++ b/iOS/MMKV/MMKV/MMKV.h @@ -401,11 +401,11 @@ class MMKV; // return count of items imported - (size_t)importFrom:(MMKV *)src; -/// Release MMKV's cache ownership of this wrapper. -/// This is a terminal, idempotent operation on this wrapper. It must not race -/// with another operation. Discard all strong references to this wrapper before -/// reopening the same ID; reopening while the old wrapper is alive is unsupported. -/// Any use of this wrapper after close is a programming error. +/// Permanently close the underlying native MMKV instance. +/// All references backed by the same native instance become invalid immediately. +/// The caller must ensure no operation is running and no reference is used +/// afterward. Discard every reference before reopening the same ID. Repeated +/// close on this Objective-C wrapper is harmless after its handle is cleared. - (void)close; /// call this method if you are facing memory-warning diff --git a/iOS/MMKV/MMKV/libMMKV.mm b/iOS/MMKV/MMKV/libMMKV.mm index 0f3002e3..c2cef87b 100644 --- a/iOS/MMKV/MMKV/libMMKV.mm +++ b/iOS/MMKV/MMKV/libMMKV.mm @@ -447,6 +447,7 @@ static BOOL g_hasCalledInitializeMMKV = NO; - (void)close { // Keep the wrapper alive while removing the dictionary's retain. [self retain]; + mmkv::MMKV *kv = nullptr; { SCOPED_LOCK(g_lock); MMKVInfo("closing %@", m_mmapID); @@ -454,10 +455,11 @@ static BOOL g_hasCalledInitializeMMKV = NO; if ([g_instanceDic objectForKey:m_mmapKey] == self) { [g_instanceDic removeObjectForKey:m_mmapKey]; } - - // Actual Core destruction remains tied to wrapper deallocation so - // existing external references are not turned into nullptr wrappers. - // A second close on this wrapper is therefore harmless. + kv = m_mmkv; + m_mmkv = nullptr; + } + if (kv) { + kv->close(); } [self release]; } diff --git a/iOS/MMKVDemo/MMKVDemo.xcodeproj/project.pbxproj b/iOS/MMKVDemo/MMKVDemo.xcodeproj/project.pbxproj index c752853d..54bebbd3 100644 --- a/iOS/MMKVDemo/MMKVDemo.xcodeproj/project.pbxproj +++ b/iOS/MMKVDemo/MMKVDemo.xcodeproj/project.pbxproj @@ -35,6 +35,7 @@ CB3167172A4C6E450003221B /* MMKV.framework in CopyFiles */ = {isa = PBXBuildFile; fileRef = CB36F25420F8A20C009AF46F /* MMKV.framework */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; }; CB36F24C20F8A20C009AF46F /* MMKVDemoTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = CB36F24B20F8A20C009AF46F /* MMKVDemoTests.mm */; }; CB36F25A20F8D728009AF46F /* MMKVPerformanceTest.mm in Sources */ = {isa = PBXBuildFile; fileRef = CB36F25920F8D728009AF46F /* MMKVPerformanceTest.mm */; }; + CB36F25B20F8D728009AF46F /* MMKV.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = CB36F25420F8A20C009AF46F /* MMKV.framework */; }; CB467FBC2431EE3000FD7421 /* MMKVAppExtension.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = CB467FBB2431EE2D00FD7421 /* MMKVAppExtension.framework */; }; CB5C469724AB87F3001B25B8 /* MMKVWatchExtension.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = CBF1907C243D7116001C82ED /* MMKVWatchExtension.framework */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; }; CB6180EA26E904AC003FF912 /* MMKVAppExtension.framework in CopyFiles */ = {isa = PBXBuildFile; fileRef = CB467FBB2431EE2D00FD7421 /* MMKVAppExtension.framework */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; }; @@ -373,6 +374,7 @@ isa = PBXFrameworksBuildPhase; buildActionMask = 2147483647; files = ( + CB36F25B20F8D728009AF46F /* MMKV.framework in Frameworks */, ); runOnlyForDeploymentPostprocessing = 0; }; @@ -1514,7 +1516,6 @@ isa = XCBuildConfiguration; buildSettings = { ALWAYS_EMBED_SWIFT_STANDARD_LIBRARIES = YES; - BUNDLE_LOADER = "$(TEST_HOST)"; CLANG_ENABLE_OBJC_WEAK = YES; CLANG_WARN_DEPRECATED_OBJC_IMPLEMENTATIONS = YES; CLANG_WARN_OBJC_IMPLICIT_RETAIN_SELF = YES; @@ -1529,7 +1530,6 @@ PRODUCT_BUNDLE_IDENTIFIER = com.tencent.mmkvdemoTests; PRODUCT_NAME = "$(TARGET_NAME)"; TARGETED_DEVICE_FAMILY = "1,2"; - TEST_HOST = "$(BUILT_PRODUCTS_DIR)/MMKVDemo.app/MMKVDemo"; }; name = Debug; }; @@ -1537,7 +1537,6 @@ isa = XCBuildConfiguration; buildSettings = { ALWAYS_EMBED_SWIFT_STANDARD_LIBRARIES = YES; - BUNDLE_LOADER = "$(TEST_HOST)"; CLANG_ENABLE_OBJC_WEAK = YES; CLANG_WARN_DEPRECATED_OBJC_IMPLEMENTATIONS = YES; CLANG_WARN_OBJC_IMPLICIT_RETAIN_SELF = YES; @@ -1552,7 +1551,6 @@ PRODUCT_BUNDLE_IDENTIFIER = com.tencent.mmkvdemoTests; PRODUCT_NAME = "$(TARGET_NAME)"; TARGETED_DEVICE_FAMILY = "1,2"; - TEST_HOST = "$(BUILT_PRODUCTS_DIR)/MMKVDemo.app/MMKVDemo"; }; name = Release; }; diff --git a/iOS/MMKVDemo/MMKVDemoTests/MMKVDemoTests.mm b/iOS/MMKVDemo/MMKVDemoTests/MMKVDemoTests.mm index 4cb78968..af5514af 100644 --- a/iOS/MMKVDemo/MMKVDemoTests/MMKVDemoTests.mm +++ b/iOS/MMKVDemo/MMKVDemoTests/MMKVDemoTests.mm @@ -26,6 +26,10 @@ using namespace std; #define KeyNotExist @"KeyNotExist" +@interface MMKV (AutoCleanupTesting) ++ (void)tryAutoCleanUpInstances; +@end + @interface MockNSCoding : NSObject @property NSString *string1; @property NSString *string2; @@ -77,6 +81,11 @@ using namespace std; MMKV *mmkv; } ++ (void)setUp { + [super setUp]; + [MMKV initializeMMKV:nil]; +} + - (void)setUp { [super setUp]; @@ -448,6 +457,8 @@ using namespace std; [kv close]; [kv close]; + // Two live wrappers backed by the same native instance are outside the + // supported close contract. Discard the closed wrapper before reopen. kv = nil; MMKV *reopened = [MMKV mmkvWithID:mmapID]; @@ -456,6 +467,41 @@ using namespace std; [reopened close]; } +- (void)testConcurrentGetAndAutoCleanupOwnershipHandoff { + NSString *mmapID = @"auto_cleanup_handoff_test"; + @autoreleasepool { + MMKV *seed = [MMKV mmkvWithID:mmapID]; + XCTAssertTrue([seed setBool:YES forKey:@"value"]); + } + + dispatch_group_t group = dispatch_group_create(); + dispatch_queue_t queue = dispatch_get_global_queue(QOS_CLASS_USER_INITIATED, 0); + + for (NSUInteger worker = 0; worker < 8; ++worker) { + dispatch_group_async(group, queue, ^{ + for (NSUInteger index = 0; index < 2000; ++index) { + @autoreleasepool { + MMKV *kv = [MMKV mmkvWithID:mmapID]; + XCTAssertTrue([kv setBool:((index & 1) != 0) forKey:@"value"]); + } + } + }); + } + for (NSUInteger cleaner = 0; cleaner < 2; ++cleaner) { + dispatch_group_async(group, queue, ^{ + for (NSUInteger index = 0; index < 2000; ++index) { + @autoreleasepool { + [MMKV tryAutoCleanUpInstances]; + } + } + }); + } + + XCTAssertEqual(dispatch_group_wait(group, dispatch_time(DISPATCH_TIME_NOW, 30 * NSEC_PER_SEC)), 0); + MMKV *kv = [MMKV mmkvWithID:mmapID]; + [kv clearAll]; +} + - (void)compareDate:(NSDate *)date withDate:(NSDate *)other { XCTAssertEqualWithAccuracy(date.timeIntervalSince1970, other.timeIntervalSince1970, 0.001); }