From 2eb4050acb20fa3ab23394bb4734a176d6f247f6 Mon Sep 17 00:00:00 2001 From: Marco Bubke Date: Wed, 10 Jul 2019 14:03:38 +0200 Subject: [PATCH] ClangRefactoring: Make the indexer more robust We check now if the database is busy. This should not be happen but better be careful. Change-Id: I8b667ff183368977991974ea1fe7fcde837e968a Reviewed-by: Tim Jenssen --- .../source/symbolindexer.cpp | 58 +++++++++++-------- tests/unit/unittest/symbolindexer-test.cpp | 43 +++++++------- 2 files changed, 57 insertions(+), 44 deletions(-) diff --git a/src/tools/clangrefactoringbackend/source/symbolindexer.cpp b/src/tools/clangrefactoringbackend/source/symbolindexer.cpp index 4d042b3fd33..c6909a07321 100644 --- a/src/tools/clangrefactoringbackend/source/symbolindexer.cpp +++ b/src/tools/clangrefactoringbackend/source/symbolindexer.cpp @@ -91,12 +91,31 @@ void SymbolIndexer::updateProjectParts(ProjectPartContainers &&projectParts) updateProjectPart(std::move(projectPart)); } +namespace { +void store(SymbolStorageInterface &symbolStorage, + BuildDependenciesStorageInterface &buildDependencyStorage, + Sqlite::TransactionInterface &transactionInterface, + SymbolsCollectorInterface &symbolsCollector) +{ + try { + Sqlite::ImmediateTransaction transaction{transactionInterface}; + buildDependencyStorage.insertOrUpdateIndexingTimeStamps(symbolsCollector.fileStatuses()); + symbolStorage.addSymbolsAndSourceLocations(symbolsCollector.symbols(), + symbolsCollector.sourceLocations()); + transaction.commit(); + } catch (const Sqlite::StatementIsBusy &) { + store(symbolStorage, buildDependencyStorage, transactionInterface, symbolsCollector); + } +} +} // namespace + void SymbolIndexer::updateProjectPart(ProjectPartContainer &&projectPart) { ProjectPartId projectPartId = projectPart.projectPartId; std::vector symbolIndexerTask; symbolIndexerTask.reserve(projectPart.sourcePathIds.size()); + for (FilePathId sourcePathId : projectPart.sourcePathIds) { SourceTimeStamps dependentTimeStamps = m_buildDependencyStorage.fetchIncludedIndexingTimeStamps( sourcePathId); @@ -117,27 +136,28 @@ void SymbolIndexer::updateProjectPart(ProjectPartContainer &&projectPart) preIncludeSearchPath}; symbolsCollector.setFile(sourcePathId, commandLineBuilder.commandLine); - return symbolsCollector.collectSymbols(); - }; - auto store = [&] { - Sqlite::ImmediateTransaction transaction{m_transactionInterface}; - m_buildDependencyStorage.insertOrUpdateIndexingTimeStamps( - symbolsCollector.fileStatuses()); - m_symbolStorage.addSymbolsAndSourceLocations(symbolsCollector.symbols(), - symbolsCollector.sourceLocations()); - transaction.commit(); + return symbolsCollector.collectSymbols(); }; const PchPaths pchPaths = m_precompiledHeaderStorage.fetchPrecompiledHeaders( projectPart.projectPartId); if (pchPaths.projectPchPath.size() && collect(pchPaths.projectPchPath)) { - store(); + store(m_symbolStorage, + m_buildDependencyStorage, + m_transactionInterface, + symbolsCollector); } else if (pchPaths.systemPchPath.size() && collect(pchPaths.systemPchPath)) { - store(); + store(m_symbolStorage, + m_buildDependencyStorage, + m_transactionInterface, + symbolsCollector); } else if (collect({})) { - store(); + store(m_symbolStorage, + m_buildDependencyStorage, + m_transactionInterface, + symbolsCollector); } }; @@ -196,23 +216,15 @@ void SymbolIndexer::updateChangedPath(FilePathId filePathId, return symbolsCollector.collectSymbols(); }; - auto store = [&] { - Sqlite::ImmediateTransaction transaction{m_transactionInterface}; - m_buildDependencyStorage.insertOrUpdateIndexingTimeStamps(symbolsCollector.fileStatuses()); - m_symbolStorage.addSymbolsAndSourceLocations(symbolsCollector.symbols(), - symbolsCollector.sourceLocations()); - transaction.commit(); - }; - const PchPaths pchPaths = m_precompiledHeaderStorage.fetchPrecompiledHeaders( optionalArtefact->projectPartId); if (pchPaths.projectPchPath.size() && collect(pchPaths.projectPchPath)) { - store(); + store(m_symbolStorage, m_buildDependencyStorage, m_transactionInterface, symbolsCollector); } else if (pchPaths.systemPchPath.size() && collect(pchPaths.systemPchPath)) { - store(); + store(m_symbolStorage, m_buildDependencyStorage, m_transactionInterface, symbolsCollector); } else if (collect({})) { - store(); + store(m_symbolStorage, m_buildDependencyStorage, m_transactionInterface, symbolsCollector); } }; diff --git a/tests/unit/unittest/symbolindexer-test.cpp b/tests/unit/unittest/symbolindexer-test.cpp index 3e0359cd8d6..86d0072c904 100644 --- a/tests/unit/unittest/symbolindexer-test.cpp +++ b/tests/unit/unittest/symbolindexer-test.cpp @@ -39,9 +39,10 @@ #include #include +#include #include #include -#include +#include #include #include #include @@ -868,29 +869,29 @@ TEST_F(SymbolIndexer, UpdateProjectPartsCallsGetsNoPchPathsAndHasErrors) TEST_F(SymbolIndexer, UpdateProjectPartsFetchIncludedIndexingTimeStamps) { InSequence s; - ProjectPartContainer projectPart{1, - {"-Wno-pragma-once-outside-header"}, - {{"BAR", "1", 1}, {"FOO", "1", 2}}, - Utils::clone(systemIncludeSearchPaths), - Utils::clone(projectIncludeSearchPaths), - {header1PathId}, - {main1PathId, main2PathId}, - Utils::Language::Cxx, - Utils::LanguageVersion::CXX14, - Utils::LanguageExtension::None}; - EXPECT_CALL(mockBuildDependenciesStorage, fetchIncludedIndexingTimeStamps(Eq(main1PathId))) - .WillOnce(Return(dependentSourceTimeStamps1)); - EXPECT_CALL(mockModifiedTimeChecker, isUpToDate(dependentSourceTimeStamps1)); - EXPECT_CALL(mockBuildDependenciesStorage, fetchIncludedIndexingTimeStamps(Eq(main2PathId))) - .WillOnce(Return(dependentSourceTimeStamps2)); - EXPECT_CALL(mockModifiedTimeChecker, isUpToDate(dependentSourceTimeStamps2)); + EXPECT_CALL(mockSqliteTransactionBackend, immediateBegin()); EXPECT_CALL(mockCollector, fileStatuses()).WillRepeatedly(ReturnRef(fileStatuses1)); - EXPECT_CALL(mockBuildDependenciesStorage, insertOrUpdateIndexingTimeStamps(fileStatuses1)); - EXPECT_CALL(mockCollector, fileStatuses()).WillRepeatedly(ReturnRef(fileStatuses2)); - EXPECT_CALL(mockBuildDependenciesStorage, insertOrUpdateIndexingTimeStamps(fileStatuses2)); + EXPECT_CALL(mockBuildDependenciesStorage, insertOrUpdateIndexingTimeStamps(_)); + EXPECT_CALL(mockSymbolStorage, addSymbolsAndSourceLocations(_, _)); + EXPECT_CALL(mockSqliteTransactionBackend, commit()); - indexer.updateProjectParts({projectPart}); + indexer.updateProjectParts({projectPart1}); +} + +TEST_F(SymbolIndexer, UpdateProjectPartsIsBusyInStoringData) +{ + InSequence s; + + EXPECT_CALL(mockSqliteTransactionBackend, immediateBegin()) + .WillOnce(Throw(Sqlite::StatementIsBusy{""})); + EXPECT_CALL(mockSqliteTransactionBackend, immediateBegin()); + EXPECT_CALL(mockCollector, fileStatuses()).WillRepeatedly(ReturnRef(fileStatuses1)); + EXPECT_CALL(mockBuildDependenciesStorage, insertOrUpdateIndexingTimeStamps(_)); + EXPECT_CALL(mockSymbolStorage, addSymbolsAndSourceLocations(_, _)); + EXPECT_CALL(mockSqliteTransactionBackend, commit()); + + indexer.updateProjectParts({projectPart1}); } TEST_F(SymbolIndexer, DependentSourceAreNotUpToDate)