diff --git a/android/cpp-adapter.cpp b/android/cpp-adapter.cpp index 5912d7b1..5d2b5bd8 100644 --- a/android/cpp-adapter.cpp +++ b/android/cpp-adapter.cpp @@ -1,9 +1,11 @@ #include "OPSqlite.hpp" #include "logs.h" #include +#include #include #include #include +#include #include namespace jsi = facebook::jsi; @@ -24,7 +26,15 @@ struct OPSQLiteBridge : jni::JavaClass { } private: - static void installNativeJsi( + // OPSQLiteBridge is a Kotlin singleton shared by every generation (see + // OPSQLiteBridge.kt), so it can't carry per-generation identity itself. + // installNativeJsi() heap-allocates a shared_ptr and hands the caller back + // a raw handle to it; OPSQLiteModule (which IS recreated per generation) + // holds that handle and passes it back to clearStateNativeJsi(), which is + // what lets invalidate() flip THIS generation's liveness flag rather than + // a process-global a newer, overlapping generation may have already + // reassigned. Released by clearStateNativeJsi(). + static jlong installNativeJsi( jni::alias_ref thiz, jlong jsiRuntimePtr, jni::alias_ref jsCallInvokerHolder, jni::alias_ref dbPath) { @@ -32,12 +42,24 @@ struct OPSQLiteBridge : jni::JavaClass { auto jsCallInvoker = jsCallInvokerHolder->cthis()->getCallInvoker(); std::string dbPathStr = dbPath->toStdString(); - opsqlite::install(*jsiRuntime, jsCallInvoker, dbPathStr.c_str(), - "libcrsqlite", "libsqlite_vec"); + auto generation_alive = opsqlite::install( + *jsiRuntime, jsCallInvoker, dbPathStr.c_str(), "libcrsqlite", + "libsqlite_vec"); + + auto *handle = new std::shared_ptr>(generation_alive); + return reinterpret_cast(handle); } - static void clearStateNativeJsi(jni::alias_ref thiz) { - opsqlite::invalidate(); + static void clearStateNativeJsi(jni::alias_ref thiz, + jlong handlePtr) { + auto *handle = + reinterpret_cast> *>(handlePtr); + if (handle == nullptr) { + return; + } + + opsqlite::invalidate(*handle); + delete handle; } }; diff --git a/android/src/main/java/com/op/sqlite/OPSQLiteBridge.kt b/android/src/main/java/com/op/sqlite/OPSQLiteBridge.kt index 44f86df6..115ae377 100644 --- a/android/src/main/java/com/op/sqlite/OPSQLiteBridge.kt +++ b/android/src/main/java/com/op/sqlite/OPSQLiteBridge.kt @@ -6,29 +6,34 @@ import com.facebook.react.common.annotations.FrameworkAPI @OptIn(FrameworkAPI::class) class OPSQLiteBridge { + // Returns an opaque handle to the native generation-liveness flag created + // for this call. This object is a singleton shared by every JS runtime + // generation, so it can't hold per-generation state itself -- the caller + // (OPSQLiteModule, which IS recreated per generation) must hold onto the + // handle and pass it back to invalidate(). private external fun installNativeJsi( jsContextNativePointer: Long, jsCallInvokerHolder: CallInvokerHolderImpl, docPath: String - ) - private external fun clearStateNativeJsi() + ): Long + private external fun clearStateNativeJsi(handle: Long) - fun install(context: ReactContext) { + fun install(context: ReactContext): Long { val jsContextPointer = context.javaScriptContextHolder!!.get() val jsCallInvokerHolder = context.catalystInstance.jsCallInvokerHolder as CallInvokerHolderImpl // Trick to get the base database path val dbPath = context.getDatabasePath("defaultDatabase").absolutePath.replace("defaultDatabase", "") - installNativeJsi( + return installNativeJsi( jsContextPointer, jsCallInvokerHolder, dbPath ) } - fun invalidate() { - clearStateNativeJsi() + fun invalidate(handle: Long) { + clearStateNativeJsi(handle) } companion object { diff --git a/android/src/main/java/com/op/sqlite/OPSQLiteModule.kt b/android/src/main/java/com/op/sqlite/OPSQLiteModule.kt index 8d341b72..3cbb7ef8 100644 --- a/android/src/main/java/com/op/sqlite/OPSQLiteModule.kt +++ b/android/src/main/java/com/op/sqlite/OPSQLiteModule.kt @@ -13,6 +13,13 @@ import java.io.OutputStream import com.facebook.react.util.RNLog; internal class OPSQLiteModule(context: ReactApplicationContext?) : ReactContextBaseJavaModule(context) { + // Handle for THIS generation's liveness flag, returned by install() and + // handed back to invalidate(). OPSQLiteModule is recreated per bridge/ + // generation by OPSQLitePackage, unlike the OPSQLiteBridge singleton it + // calls into, so this instance field -- not that singleton -- is what + // carries per-generation identity across the JNI boundary. + private var generationAliveHandle: Long = 0 + override fun getName(): String { return NAME } @@ -41,7 +48,7 @@ internal class OPSQLiteModule(context: ReactApplicationContext?) : ReactContextB @ReactMethod(isBlockingSynchronousMethod = true) fun install(): Boolean { return try { - OPSQLiteBridge.instance.install(reactApplicationContext) + generationAliveHandle = OPSQLiteBridge.instance.install(reactApplicationContext) true } catch (exception: Exception) { Log.e(NAME, "Install exception: $exception") @@ -105,7 +112,7 @@ internal class OPSQLiteModule(context: ReactApplicationContext?) : ReactContextB override fun invalidate() { super.invalidate() - OPSQLiteBridge.instance.invalidate() + OPSQLiteBridge.instance.invalidate(generationAliveHandle) } companion object { diff --git a/cpp/OPSqlite.cpp b/cpp/OPSqlite.cpp index 79f228f2..6ef2685f 100644 --- a/cpp/OPSqlite.cpp +++ b/cpp/OPSqlite.cpp @@ -12,7 +12,6 @@ #include "utils.hpp" #include #include -#include #include #include #include @@ -25,56 +24,39 @@ namespace react = facebook::react; std::string _base_path; std::string _crsqlite_path; std::string _sqlite_vec_path; -std::vector> dbs; -// Guards `dbs`. Two JS runtime generations overlap during a bridgeless reload, -// so open() and invalidate() can touch this vector from different threads at -// the same time. -std::mutex dbs_mutex; -bool invalidated = false; std::shared_ptr invoker; std::shared_ptr> generation_alive; // React native will try to clean the module on JS context invalidation -// (CodePush/Hot Reload) The clearState function is called -void invalidate() { - // Global flag used by the threads to stop work - invalidated = true; - - // Mark THIS generation dead. Work queued by it holds a copy of the flag, so - // it drops its completions instead of resolving into a runtime that is being - // torn down. +// (CodePush/Hot Reload) The clearState function is called. Each currently +// open DBHostObject cleans itself up independently -- ~DBHostObject() +// already calls invalidate() (interrupt + drain + close) whenever the JS +// runtime destroys it, so there's no registry to walk here. All this needs +// to do is mark THIS generation dead so in-flight async work drops its +// result instead of resolving into whichever runtime replaces it. +void invalidate(const std::shared_ptr> &generation_alive) { if (generation_alive != nullptr) { generation_alive->store(false); } - - // Take ownership of the registry under the lock before touching it. This runs - // on the outgoing generation's TurboModule queue, while the incoming - // generation's open() may already be emplacing into `dbs` on its own JS - // thread: RCTHost constructs the new RCTInstance without waiting for the old - // one to finish invalidating. Iterating the vector directly can therefore run - // off a reallocated buffer. - std::vector> closing; - { - std::lock_guard g(dbs_mutex); - closing.swap(dbs); - } - - for (const auto &db : closing) { - db->invalidate(); - } } -void install(jsi::Runtime &rt, - const std::shared_ptr &_invoker, - const char *base_path, const char *crsqlite_path, - const char *sqlite_vec_path) { +std::shared_ptr> +install(jsi::Runtime &rt, const std::shared_ptr &_invoker, + const char *base_path, const char *crsqlite_path, + const char *sqlite_vec_path) { _base_path = std::string(base_path); _crsqlite_path = std::string(crsqlite_path); _sqlite_vec_path = std::string(sqlite_vec_path); opsqlite::invoker = _invoker; - opsqlite::invalidated = false; - opsqlite::generation_alive = std::make_shared>(true); + + // Also returned to the caller: DBs opened by this generation shadow-copy + // the global at construction time (see DBHostObject.hpp), while + // invalidate() needs the shared_ptr handed back directly so it flips THIS + // generation's flag even if a newer, overlapping generation's install() + // has already reassigned the global. + auto local_generation_alive = std::make_shared>(true); + opsqlite::generation_alive = local_generation_alive; auto open = HFN0 { jsi::Object options = args[0].asObject(rt); @@ -114,10 +96,6 @@ void install(jsi::Runtime &rt, std::shared_ptr db = std::make_shared( rt, path, name, path, readOnly, failOnCreate, encryption_key); - { - std::lock_guard g(dbs_mutex); - dbs.emplace_back(db); - } return jsi::Object::createFromHostObject(rt, db); }); @@ -171,11 +149,6 @@ void install(jsi::Runtime &rt, std::make_shared(rt, url, auth_token, path); #endif - { - std::lock_guard g(dbs_mutex); - dbs.emplace_back(db); - } - return jsi::Object::createFromHostObject(rt, db); }); @@ -236,11 +209,6 @@ void install(jsi::Runtime &rt, rt, name, path, url, auth_token, remote_encryption_key); #endif - { - std::lock_guard g(dbs_mutex); - dbs.emplace_back(db); - } - return jsi::Object::createFromHostObject(rt, db); }); #endif @@ -257,6 +225,8 @@ void install(jsi::Runtime &rt, #endif rt.global().setProperty(rt, "__OPSQLiteProxy", std::move(module)); + + return local_generation_alive; } void expoUpdatesWorkaround(const char *base_path) { diff --git a/cpp/OPSqlite.hpp b/cpp/OPSqlite.hpp index 91511ab8..63bd550d 100644 --- a/cpp/OPSqlite.hpp +++ b/cpp/OPSqlite.hpp @@ -1,19 +1,25 @@ #pragma once #include +#include #include #include +#include namespace opsqlite { namespace jsi = facebook::jsi; namespace react = facebook::react; -void install(jsi::Runtime &rt, - const std::shared_ptr &invoker, - const char *base_path, const char *crsqlite_path, - const char *sqlite_vec_path); -void invalidate(); +// Returns this generation's liveness flag. Platform glue holds onto it +// (independent of the JS runtime) so it can be handed back to invalidate() +// when this SAME generation tears down, without needing a process-global to +// look it up -- see the comment on opsqlite::generation_alive in types.hpp. +std::shared_ptr> +install(jsi::Runtime &rt, const std::shared_ptr &invoker, + const char *base_path, const char *crsqlite_path, + const char *sqlite_vec_path); +void invalidate(const std::shared_ptr> &generation_alive); void expoUpdatesWorkaround(const char *base_path); } // namespace opsqlite diff --git a/cpp/types.hpp b/cpp/types.hpp index 5844e615..853483ca 100644 --- a/cpp/types.hpp +++ b/cpp/types.hpp @@ -11,18 +11,18 @@ namespace opsqlite { extern std::shared_ptr invoker; -extern bool invalidated; // Liveness of the current JS runtime generation. Replaced by install() and // cleared by invalidate(), so each generation gets its own flag rather than -// sharing the process-global `invalidated` bool. +// sharing one process-global bool. // // Whoever queues work copies the shared_ptr when the work is created, so it -// always observes ITS OWN generation's liveness. Checking a process-global -// instead is wrong in both directions during a bridgeless reload, where two -// generations overlap: an outgoing generation clearing it would suppress the -// incoming generation's callbacks, and an incoming generation setting it would -// re-enable the outgoing generation's. +// always observes ITS OWN generation's liveness. install() also returns this +// same shared_ptr so invalidate() can be handed it back directly, rather +// than reading this global -- which, during a bridgeless reload where two +// generations briefly overlap, might already have been reassigned to the +// incoming generation's flag by the time the outgoing generation's +// invalidate() runs. extern std::shared_ptr> generation_alive; struct ArrayBuffer { diff --git a/example/ios/Podfile.lock b/example/ios/Podfile.lock index b0670900..60d01ce4 100644 --- a/example/ios/Podfile.lock +++ b/example/ios/Podfile.lock @@ -2161,7 +2161,7 @@ EXTERNAL SOURCES: SPEC CHECKSUMS: FBLazyVector: b3e7ad108f0d882e30445c5527d774e3fd432f3d hermes-engine: d9c82b910641f3bb8d520dbd9d5f193582515958 - op-sqlite: 2863e0409a4782b557388b6893c34ebe5068e889 + op-sqlite: ab6c039f6cb78ae6ab32d0c320631b037db72326 RCTDeprecation: 2a74a2c57675e64419bd89078efde81f7c1de90b RCTRequired: 30451112e6fef4e6f31b4e7eee0845156e35e4b0 RCTSwiftUI: 5aaf0b07e747ba749dc6acc94d8bd41eea4b570f diff --git a/ios/OPSQLite.mm b/ios/OPSQLite.mm index e4e65e0e..b1542296 100644 --- a/ios/OPSQLite.mm +++ b/ios/OPSQLite.mm @@ -6,6 +6,16 @@ #import #import +@interface OPSQLite () { + // This generation's liveness flag. Set by install(), consumed by + // invalidate() -- kept as a plain ivar (not routed through the JS + // runtime) because invalidate() runs with no Runtime reference at all, + // and this module instance is itself per-generation: RN constructs a + // fresh one for every new RCTInstance. + std::shared_ptr> _generationAlive; +} +@end + @implementation OPSQLite @synthesize bridge = _bridge; @@ -95,8 +105,9 @@ - (NSDictionary *)getConstants { NSString *sqlite_vec_path = @""; #endif - opsqlite::install(runtime, callInvoker, [documentPath UTF8String], - [crsqlite_path UTF8String], [sqlite_vec_path UTF8String]); + _generationAlive = + opsqlite::install(runtime, callInvoker, [documentPath UTF8String], + [crsqlite_path UTF8String], [sqlite_vec_path UTF8String]); return @true; } @@ -151,7 +162,8 @@ - (NSDictionary *)getConstants { } - (void)invalidate { - opsqlite::invalidate(); + opsqlite::invalidate(_generationAlive); + _generationAlive = nullptr; } + (void)expoUpdatesWorkaround {