Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clearState doesn't take care of pending threads #32

Open
savv opened this issue Nov 14, 2023 · 3 comments
Open

clearState doesn't take care of pending threads #32

savv opened this issue Nov 14, 2023 · 3 comments

Comments

@savv
Copy link
Contributor

savv commented Nov 14, 2023

I've noticed that the sqlite lib crashes when codepush restarts the app, if there are a pending async queries.

The reason is that it tries to use a runtime that doesn't exist anymore, crashing here.

I had previously tried to mitigate this issue with https://github.com/margelo/react-native-quick-sqlite/pull/16. While correct, it still allows pending executions to complete, causing these crashes.

One way to fix this is to kill ThreadPool's threads from osp::clearState().

@ospfranco any thoughts?

Full stack trace

facebook::jsi::Object::Object (jsi.h:658)
createSequelQueryExecutionResult (JSIHelper.cpp:135)
const::lambda::operator() (bindings.cpp:240)
std::__1::__invoke<T> (type_traits:3918)
std::__1::__invoke_void_return_wrapper<T>::__call<T> (invoke.h:61)
std::__1::__function::__alloc_func<T>::operator() (function.h:178)
std::__1::__function::__func<T>::operator() (function.h:352)
std::__1::__function::__value_func<T>::operator() (function.h:505)
std::__1::function<T>::operator() (function.h:1182)
facebook::react::Instance::JSCallInvoker::scheduleAsync::lambda::operator() (Instance.cpp:295)
std::__1::__invoke<T> (type_traits:3918)
std::__1::__invoke_void_return_wrapper<T>::__call<T> (invoke.h:61)
std::__1::__function::__alloc_func<T>::operator() (function.h:178)
std::__1::__function::__func<T>::operator() (function.h:352)
std::__1::__function::__value_func<T>::operator() (function.h:505)
std::__1::function<T>::operator() (function.h:1182)
facebook::react::NativeToJsBridge::runOnExecutorQueue::lambda::operator() (NativeToJsBridge.cpp:310)
std::__1::__invoke<T> (type_traits:3918)
std::__1::__invoke_void_return_wrapper<T>::__call<T> (invoke.h:61)
std::__1::__function::__alloc_func<T>::operator() (function.h:178)
std::__1::__function::__func<T>::operator() (function.h:352)
std::__1::__function::__value_func<T>::operator() (function.h:505)
std::__1::function<T>::operator() (function.h:1182)
facebook::react::tryAndReturnError (RCTCxxUtils.mm:74)
facebook::react::RCTMessageThread::tryFunc (RCTMessageThread.mm:69)
std::__1::__function::__value_func<T>::operator() (function.h:505)
std::__1::function<T>::operator() (function.h:1182)
facebook::react::RCTMessageThread::runAsync (RCTMessageThread.mm:45)
CoreFoundation
+0x0355c4
<redacted>
CoreFoundation
+0x033ee8
<redacted>
CoreFoundation
+0x031de8
<redacted>
CoreFoundation
+0x031a04
CFRunLoopRunSpecific
+[RCTCxxBridge runRunLoop] (RCTCxxBridge.mm:337)
Foundation
+0x0a805c
<redacted>
libsystem_pthread
+0x00218c
_pthread_start
@savv
Copy link
Contributor Author

savv commented Nov 14, 2023

Yet another mitigation could be to sql_interrupt, prior to calling close_v2 from osp::clearState(). If I understood correctly, this should make all pending transactions terminate synchronously, and before the runtime disappears.

@ospfranco
Copy link

I took a bit more, but managed to fix the crash when reloading the bundle via react-native-restart. You can give it a try with Codepush and let me know if it works. For now, I will stop spamming margelo since quick-sqlite belongs to them now.

https://github.com/OP-Engineering/op-sqlite/releases/tag/1.0.11

@savv
Copy link
Contributor Author

savv commented Nov 16, 2023

Hi Franco, many thanks for addressing this issue! That's a very thorough fix. Just one suggestion.

This comment is no longer accurate in my opinion:

    // In certain cases, this will return SQLITE_OK, mark the database connection as an unusable "zombie",
    // and deallocate the connection later.

It could be replace with sth like:
"Calling sqlite3_interrupt will cause all pending (async) execution threads to immediately throw SQLITE_INTERRUPT and sqlite3_close_v2 to return immediately with SQLITE_OK."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants