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

Always use inDatabase: in FMDatabaseQueue. Respect crashOnErrors for open #649

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

benasher44
Copy link
Contributor

In our code, we use a similar dispatch_get_specific technique to guard against deadlock. When I realized this, I was hoping I could just use what's built into FMDB instead. However, It appears not all calls go through inDatabase:, which means those other calls could deadlock un-knowlingly (or at least without making much noise).

In doing that, I also realized that you could conceivably call close on an FMDatabaseQueue and then call inDatabase: and have the [self database] fail to re-open the database and call the block with a nil db, which would break the nullability guarantee of nonnull as described in FMDatabaseQueue.h. This isn't a big deal in Obj-C, but in Swift it'll cause crash. I fixed it to bail out early and not call the block if the re-open fails. I also added crashOnErrors support to the open methods in FMDatabase to make this scenario hard to miss in debug.

After writing this, it occurs to me that there could have been some reason for not re-using inDatabase: everywhere. If that's the case, I'm open to refactoring this in a different way to accomplish the same goal. Thanks!

@cacaosteve
Copy link

Would this help a crash I am getting using the database queue from my main thread? It only happens sometimes. It's a background thread that is crashing. I know I could use a dispatch queue from the main thread, but also that blocking should work fine.

libobjc.A.dylib
objc_release
libobjc.A.dylib
object_cxxDestructFromClass(objc_object*, objc_class*)
libobjc.A.dylib
objc_destructInstance
libobjc.A.dylib
_objc_rootDealloc
Foundation
-[NSOperation dealloc]
libsystem_blocks.dylib
bool HelperBase::disposeCapture<(HelperBase::BlockCaptureKind)3>(unsigned int, unsigned char*)
libsystem_blocks.dylib
HelperBase::destroyBlock(Block_layout*, bool, unsigned char*)
libsystem_blocks.dylib
_call_dispose_helpers_excp
libsystem_blocks.dylib
_Block_release
libdispatch.dylib
__destroy_helper_block_8_32c35_ZTS29dispatch_block_private_data_s
libsystem_blocks.dylib
_call_dispose_helpers_excp
libsystem_blocks.dylib
_Block_release
libdispatch.dylib
_dispatch_client_callout
libdispatch.dylib
_dispatch_continuation_pop
libdispatch.dylib
_dispatch_async_redirect_invoke
libdispatch.dylib
_dispatch_root_queue_drain
libdispatch.dylib
_dispatch_worker_thread2
libsystem_pthread.dylib
_pthread_wqthread
libsystem_pthread.dylib
start_wqthread

@lzcuriosity
Copy link

lzcuriosity commented Dec 3, 2022 via email

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

Successfully merging this pull request may close these issues.

3 participants