Protect search from multi threading race condition.

libzim's search is not thread safe (mainly because xapian is not).
So we must protect our search objects from multi thread calls.

The best way to do this is to associate a mutex to the `zim::Searcher`
and lock the searcher each time we access object derivated from the
searcher (search, results, iterator, ...)
This commit is contained in:
Matthieu Gautier 2022-05-24 17:19:39 +02:00
parent e5ea210d2c
commit 1514661c26
3 changed files with 24 additions and 6 deletions

View File

@ -141,6 +141,22 @@ private: // functions
bool accept(const Book& book) const;
};
class ZimSearcher : public zim::Searcher
{
public:
explicit ZimSearcher(zim::Searcher&& searcher)
: zim::Searcher(searcher)
{}
std::unique_lock<std::mutex> getLock() {
return std::unique_lock<std::mutex>(m_mutex);
}
virtual ~ZimSearcher() = default;
private:
std::mutex m_mutex;
};
/**
* A Library store several books.
*/
@ -209,10 +225,10 @@ class Library
DEPRECATED std::shared_ptr<Reader> getReaderById(const std::string& id);
std::shared_ptr<zim::Archive> getArchiveById(const std::string& id);
std::shared_ptr<zim::Searcher> getSearcherById(const std::string& id) {
std::shared_ptr<ZimSearcher> getSearcherById(const std::string& id) {
return getSearcherByIds(BookIdSet{id});
}
std::shared_ptr<zim::Searcher> getSearcherByIds(const BookIdSet& ids);
std::shared_ptr<ZimSearcher> getSearcherByIds(const BookIdSet& ids);
/**
* Remove a book from the library.

View File

@ -93,7 +93,7 @@ struct Library::Impl
std::map<std::string, Entry> m_books;
using ArchiveCache = ConcurrentCache<std::string, std::shared_ptr<zim::Archive>>;
std::unique_ptr<ArchiveCache> mp_archiveCache;
using SearcherCache = MultiKeyCache<std::string, std::shared_ptr<zim::Searcher>>;
using SearcherCache = MultiKeyCache<std::string, std::shared_ptr<ZimSearcher>>;
std::unique_ptr<SearcherCache> mp_searcherCache;
std::vector<kiwix::Bookmark> m_bookmarks;
Xapian::WritableDatabase m_bookDB;
@ -304,7 +304,7 @@ std::shared_ptr<zim::Archive> Library::getArchiveById(const std::string& id)
}
}
std::shared_ptr<zim::Searcher> Library::getSearcherByIds(const BookIdSet& ids)
std::shared_ptr<ZimSearcher> Library::getSearcherByIds(const BookIdSet& ids)
{
assert(!ids.empty());
try {
@ -318,7 +318,7 @@ std::shared_ptr<zim::Searcher> Library::getSearcherByIds(const BookIdSet& ids)
}
archives.push_back(*archive);
}
return std::make_shared<zim::Searcher>(archives);
return std::make_shared<ZimSearcher>(zim::Searcher(archives));
});
} catch (std::invalid_argument&) {
return nullptr;

View File

@ -704,11 +704,13 @@ std::unique_ptr<Response> InternalServer::handle_search(const RequestContext& re
/* Make the search */
// Try to get a search from the searchInfo, else build it
auto searcher = mp_library->getSearcherByIds(bookIds);
auto lock(searcher->getLock());
std::shared_ptr<zim::Search> search;
try {
search = searchCache.getOrPut(searchInfo,
[=](){
auto searcher = mp_library->getSearcherByIds(bookIds);
return make_shared<zim::Search>(searcher->search(searchInfo.getZimQuery(m_verbose.load())));
}
);