Skip to content

Commit 62fef21

Browse files
authored
[Collections] Fix useSearchIndices race (#3529)
Motivation: rdar://78872613 `SQLitePackageCollectionsStorage.useSearchIndices` is set in `createSchemaIfNecessary`, which is invoked only along with a database operation. Since search methods (e.g., `searchPackages`) check `useSearchIndices` first thing, it's possible that `createSchemaIfNecessary` hasn't been called yet and as a result `useSearchIndices` defaults to `false`, causing a different code path to get executed. Modification: Make sure `createSchemaIfNecessary` is called before reading `useSearchIndices`.
1 parent 6e7bb87 commit 62fef21

File tree

1 file changed

+36
-8
lines changed

1 file changed

+36
-8
lines changed

Sources/PackageCollections/Storage/SQLitePackageCollectionsStorage.swift

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,14 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable
287287
func searchPackages(identifiers: [Model.CollectionIdentifier]? = nil,
288288
query: String,
289289
callback: @escaping (Result<Model.PackageSearchResult, Error>) -> Void) {
290-
if self.useSearchIndices.get() ?? false {
290+
let useSearchIndices: Bool
291+
do {
292+
useSearchIndices = try self.shouldUseSearchIndices()
293+
} catch {
294+
return callback(.failure(error))
295+
}
296+
297+
if useSearchIndices {
291298
var matches = [(collection: Model.CollectionIdentifier, package: PackageIdentity)]()
292299
var matchingCollections = Set<Model.CollectionIdentifier>()
293300

@@ -397,7 +404,14 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable
397404
func findPackage(identifier: PackageIdentity,
398405
collectionIdentifiers: [Model.CollectionIdentifier]?,
399406
callback: @escaping (Result<Model.PackageSearchResult.Item, Error>) -> Void) {
400-
if self.useSearchIndices.get() ?? false {
407+
let useSearchIndices: Bool
408+
do {
409+
useSearchIndices = try self.shouldUseSearchIndices()
410+
} catch {
411+
return callback(.failure(error))
412+
}
413+
414+
if useSearchIndices {
401415
var matches = [(collection: Model.CollectionIdentifier, package: PackageIdentity)]()
402416
var matchingCollections = Set<Model.CollectionIdentifier>()
403417

@@ -499,7 +513,14 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable
499513
callback(.success(result))
500514
}
501515

502-
if self.useSearchIndices.get() ?? false {
516+
let useSearchIndices: Bool
517+
do {
518+
useSearchIndices = try self.shouldUseSearchIndices()
519+
} catch {
520+
return callback(.failure(error))
521+
}
522+
523+
if useSearchIndices {
503524
var matches = [(collection: Model.CollectionIdentifier, package: PackageIdentity, targetName: String)]()
504525
var matchingCollections = Set<Model.CollectionIdentifier>()
505526

@@ -662,7 +683,7 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable
662683
}
663684

664685
private func insertToSearchIndices(collection: Model.Collection) throws {
665-
guard self.useSearchIndices.get() ?? false else { return }
686+
guard try self.shouldUseSearchIndices() else { return }
666687

667688
try self.ftsLock.withLock {
668689
// First delete existing data
@@ -730,7 +751,7 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable
730751
}
731752

732753
private func removeFromSearchIndices(identifier: Model.CollectionIdentifier) throws {
733-
guard self.useSearchIndices.get() ?? false else { return }
754+
guard try self.shouldUseSearchIndices() else { return }
734755

735756
let identifierBase64 = try self.encoder.encode(identifier.databaseKey()).base64EncodedString()
736757

@@ -760,6 +781,13 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable
760781
self.targetTrie.remove { $0.collection == identifier }
761782
}
762783

784+
private func shouldUseSearchIndices() throws -> Bool {
785+
// Make sure createSchemaIfNecessary is called and useSearchIndices is set before reading it
786+
try self.withDB { _ in
787+
self.useSearchIndices.get() ?? false
788+
}
789+
}
790+
763791
internal func populateTargetTrie(callback: @escaping (Result<Void, Error>) -> Void = { _ in }) {
764792
DispatchQueue.sharedConcurrent.async(group: nil, qos: .background, flags: .assignCurrentContext) {
765793
do {
@@ -893,7 +921,7 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable
893921
#if os(Android)
894922
// FTS queries for strings containing hyphens isn't working in SQLite on
895923
// Android, so disable for now.
896-
useSearchIndices.put(false)
924+
self.useSearchIndices.put(false)
897925
#else
898926
do {
899927
let ftsPackages = """
@@ -914,12 +942,12 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable
914942
"""
915943
try db.exec(query: ftsTargets)
916944

917-
useSearchIndices.put(true)
945+
self.useSearchIndices.put(true)
918946
} catch {
919947
// We can use FTS3 tables but queries yield different results when run on different
920948
// platforms. This could be because of SQLite version perhaps? But since we can't get
921949
// consistent results we will not fallback to FTS3 and just give up if FTS4 is not available.
922-
useSearchIndices.put(false)
950+
self.useSearchIndices.put(false)
923951
}
924952
#endif
925953

0 commit comments

Comments
 (0)