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

Crashes (possibly) due to concurrent access to bulkUpdateIndex and search #15

Closed
lkoskela opened this issue Mar 10, 2016 · 6 comments
Closed
Labels

Comments

@lkoskela
Copy link

Hi,

I'm getting crashes (EXC_CRASH with either SIGABRT or SIGSEGV or an EXC_BAD_ACCESS if the Xcode debugger is connected) from BRFullTextSearch inside lucene::search::PrefixQuery::rewrite(lucene::index::IndexReader *) or lucene::index::SegmentTermDocs::SegmentTermDocs(lucene::index::SegmentReader const *) and I have a hunch that it might relate to threads and concurrent modification of the searcher/query objects while a search is being made.

Here's what's happening:

  • The "us.bluerocket.CLucene.IndexWrite" queue is executing [CLuceneSearchService bulkUpdateIndex:queue:finished:] (apparently just having finished modifying the index and thus triggering a call to resetSearcher in the UI thread)
  • My own "search queue" is executing [CLuceneSearchService search:] (apparently trying to read the index with a SegmentReader)
  • The UI thread is executing [CLuceneSearchService resetSearcher] (apparently closing the same SegmentReader my search queue is using)

The examples in README.md don't feature the use of NSOperationQueue or dispatch_async and CLuceneSearchService.h says that it "supports concurrent updates and queries across different threads", which makes me believe I should be fine doing what I'm doing, i.e. calling bulkUpdateIndexAndWait:error: and search: from an NSOperationQueue but it would be really nice to get confirmation whether I'm trying to use CLuceneSearchService in a way that's not at all intended.

@lkoskela lkoskela changed the title Concurrency Crashes (possibly) due to concurrent access to bulkUpdateIndex and search Mar 10, 2016
@lkoskela
Copy link
Author

To clarify, resetSearcher is not being executed every time these crashes happen. Sometimes it's just the search: and bulkUpdateIndex:queue:finished:.

I've also tried switching between bulkUpdateIndex:queue:finished: and bulkUpdateIndexAndWait:error: but the crashes seem to be almost identical in each case.

@msqr
Copy link
Contributor

msqr commented Mar 10, 2016

It sounds like you have a slightly different use case than tested for, with searches executing outside the main thread. I'd have to look a bit more closely at the code to be sure, but I'm guessing that CLuceneSearchService might be expecting searches to be executing on the main thread while updates run from any other thread. That is how the example apps are set up, and how I've always used it on other projects.

Is there any way for you to share a sample project that demonstrates the crash you're seeing? You seem to be able to reproduce it easily, is that true? It might be easy to fix the crash for your use case if I could reproduce what you're seeing.

@lkoskela
Copy link
Author

That does sound plausible. I'll put together a simplified project that exhibits the crash.

@lkoskela
Copy link
Author

Here's what I came up with: https://github.com/lkoskela/BRFullTextSearch-ThreadingSample

It's not as simple as I hoped but it does eventually trigger the crash if you start the app in iOS Simulator and let it run for a minute or two. It generates random additions, removals and searches to an index. You can also tap on the view to trigger a search.

msqr added a commit that referenced this issue Mar 11, 2016
…ause any concurrently executing searchers will have their IndexReader closed from under them. Instead rely on shared_ptr deallocation of the Searcher, which then calls close() in its destructor. See #15.
@lkoskela
Copy link
Author

Commits bd9733c and 9c65e85 seem to fix the crash for me! I can't reproduce it anymore and previously I managed to reproduce it within seconds. Thanks!

@msqr
Copy link
Contributor

msqr commented Mar 11, 2016

Glad to hear! I've merged the fixes in for the next release.

@msqr msqr closed this as completed Mar 11, 2016
@msqr msqr added the bug label Mar 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants