Skip to content

Commit 265c1ed

Browse files
yim-leeyim-lee
authored
[Collections] CLI 'remove' results in "database is locked" error (#3225)
Motivation: Using the `package-collection` CLI, the `remove` command results in "database is locked" error even though the collection is removed successfully. This is because a `SQLitePackageCollectionsStorage` is created at the beginning of each command run, and in the initializer the time-consuming `populateTargetTrie` is called. When the command finishes running, it triggers `SQLitePackageCollectionsStorage` to be closed, which in turn closes the SQLite connection, but this is done while `populateTargetTrie` is still running and thus causes the "database is locked" error. Modifications: - Initializing `PackageCollections` can be expensive and should only be done once in a command - Add and set `isShuttingDown` flag when `SQLitePackageCollectionsStorage.close` is called so `populateTargetTrie` knows that it should stop. - Try `db.close` in a second attempt after allowing time for database operations to react to `isShuttingDown` flag - `populateTargetTrie` should memoize its result Result: No "database is locked" error.
1 parent 86de647 commit 265c1ed

File tree

2 files changed

+169
-101
lines changed

2 files changed

+169
-101
lines changed

‎Sources/Commands/SwiftPackageCollectionsTool.swift

Lines changed: 78 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -134,12 +134,11 @@ public struct SwiftPackageCollectionsTool: ParsableCommand {
134134
}
135135

136136
let source = PackageCollectionsModel.CollectionSource(type: .json, url: collectionUrl)
137-
let collection = try with { collections in
138-
return try tsc_await { collections.getCollection(source, callback: $0) }
137+
try with { collections in
138+
let collection = try tsc_await { collections.getCollection(source, callback: $0) }
139+
_ = try tsc_await { collections.removeCollection(source, callback: $0) }
140+
print("Removed \"\(collection.name)\" from your package collections.")
139141
}
140-
141-
_ = try with { collections in try tsc_await { collections.removeCollection(source, callback: $0) } }
142-
print("Removed \"\(collection.name)\" from your package collections.")
143142
}
144143
}
145144

@@ -163,31 +162,29 @@ public struct SwiftPackageCollectionsTool: ParsableCommand {
163162
var searchQuery: String
164163

165164
mutating func run() throws {
166-
switch searchMethod {
167-
case .keywords:
168-
let results = try with { collections in
169-
return try tsc_await { collections.findPackages(searchQuery, collections: nil, callback: $0) }
170-
}
171-
172-
if jsonOptions.json {
173-
try JSONEncoder.makeWithDefaults().print(results.items)
174-
} else {
175-
results.items.forEach {
176-
print("\($0.package.repository.url): \($0.package.summary ?? "")")
165+
try with { collections in
166+
switch searchMethod {
167+
case .keywords:
168+
let results = try tsc_await { collections.findPackages(searchQuery, collections: nil, callback: $0) }
169+
170+
if jsonOptions.json {
171+
try JSONEncoder.makeWithDefaults().print(results.items)
172+
} else {
173+
results.items.forEach {
174+
print("\($0.package.repository.url): \($0.package.summary ?? "")")
175+
}
177176
}
178-
}
179177

180-
case .module:
181-
let results = try with { collections in
182-
return try tsc_await { collections.findTargets(searchQuery, searchType: .exactMatch, collections: nil, callback: $0) }
183-
}
178+
case .module:
179+
let results = try tsc_await { collections.findTargets(searchQuery, searchType: .exactMatch, collections: nil, callback: $0) }
184180

185-
let packages = Set(results.items.flatMap { $0.packages })
186-
if jsonOptions.json {
187-
try JSONEncoder.makeWithDefaults().print(packages)
188-
} else {
189-
packages.forEach {
190-
print("\($0.repository.url): \($0.summary ?? "")")
181+
let packages = Set(results.items.flatMap { $0.packages })
182+
if jsonOptions.json {
183+
try JSONEncoder.makeWithDefaults().print(packages)
184+
} else {
185+
packages.forEach {
186+
print("\($0.repository.url): \($0.summary ?? "")")
187+
}
191188
}
192189
}
193190
}
@@ -229,71 +226,69 @@ public struct SwiftPackageCollectionsTool: ParsableCommand {
229226
}
230227

231228
mutating func run() throws {
232-
let identity = PackageIdentity(url: packageUrl)
233-
let reference = PackageReference(identity: identity, path: packageUrl)
234-
235-
do { // assume URL is for a package
236-
let result = try with { collections in
237-
return try tsc_await { collections.getPackageMetadata(reference, callback: $0) }
238-
}
229+
try with { collections in
230+
let identity = PackageIdentity(url: packageUrl)
231+
let reference = PackageReference(identity: identity, path: packageUrl)
239232

240-
if let versionString = version {
241-
guard let version = TSCUtility.Version(string: versionString), let result = result.package.versions.first(where: { $0.version == version }), let printedResult = printVersion(result) else {
242-
throw CollectionsError.invalidVersionString(versionString)
243-
}
233+
do { // assume URL is for a package
234+
let result = try tsc_await { collections.getPackageMetadata(reference, callback: $0) }
244235

245-
if jsonOptions.json {
246-
try JSONEncoder.makeWithDefaults().print(result)
236+
if let versionString = version {
237+
guard let version = TSCUtility.Version(string: versionString), let result = result.package.versions.first(where: { $0.version == version }), let printedResult = printVersion(result) else {
238+
throw CollectionsError.invalidVersionString(versionString)
239+
}
240+
241+
if jsonOptions.json {
242+
try JSONEncoder.makeWithDefaults().print(result)
243+
} else {
244+
print("\(indent())Version: \(printedResult)")
245+
}
247246
} else {
248-
print("\(indent())Version: \(printedResult)")
247+
let description = optionalRow("Description", result.package.summary)
248+
let versions = result.package.versions.map { "\($0.version)" }.joined(separator: ", ")
249+
let watchers = optionalRow("Watchers", result.package.watchersCount?.description)
250+
let readme = optionalRow("Readme", result.package.readmeURL?.absoluteString)
251+
let authors = optionalRow("Authors", result.package.authors?.map { $0.username }.joined(separator: ", "))
252+
let latestVersion = optionalRow("\(String(repeating: "-", count: 60))\n\(indent())Latest Version", printVersion(result.package.latestVersion))
253+
254+
if jsonOptions.json {
255+
try JSONEncoder.makeWithDefaults().print(result.package)
256+
} else {
257+
print("""
258+
\(description)
259+
Available Versions: \(versions)\(watchers)\(readme)\(authors)\(latestVersion)
260+
""")
261+
}
249262
}
250-
} else {
251-
let description = optionalRow("Description", result.package.summary)
252-
let versions = result.package.versions.map { "\($0.version)" }.joined(separator: ", ")
253-
let watchers = optionalRow("Watchers", result.package.watchersCount?.description)
254-
let readme = optionalRow("Readme", result.package.readmeURL?.absoluteString)
255-
let authors = optionalRow("Authors", result.package.authors?.map { $0.username }.joined(separator: ", "))
256-
let latestVersion = optionalRow("\(String(repeating: "-", count: 60))\n\(indent())Latest Version", printVersion(result.package.latestVersion))
257-
263+
} catch { // assume URL is for a collection
264+
// If a version argument was given, we do not perform the fallback.
265+
if version != nil {
266+
throw error
267+
}
268+
269+
guard let collectionUrl = URL(string: packageUrl) else {
270+
throw CollectionsError.invalidArgument("collectionUrl")
271+
}
272+
273+
let source = PackageCollectionsModel.CollectionSource(type: .json, url: collectionUrl)
274+
let collection = try tsc_await { collections.getCollection(source, callback: $0) }
275+
276+
let description = optionalRow("Description", collection.overview)
277+
let keywords = optionalRow("Keywords", collection.keywords?.joined(separator: ", "))
278+
let createdAt = optionalRow("Created At", DateFormatter().string(from: collection.createdAt))
279+
let packages = collection.packages.map { "\($0.repository.url)" }.joined(separator: "\n\(indent(levels: 2))")
280+
258281
if jsonOptions.json {
259-
try JSONEncoder.makeWithDefaults().print(result.package)
282+
try JSONEncoder.makeWithDefaults().print(collection)
260283
} else {
261284
print("""
262-
\(description)
263-
Available Versions: \(versions)\(watchers)\(readme)\(authors)\(latestVersion)
264-
""")
285+
Name: \(collection.name)
286+
Source: \(collection.source.url)\(description)\(keywords)\(createdAt)
287+
Packages:
288+
\(packages)
289+
""")
265290
}
266291
}
267-
} catch { // assume URL is for a collection
268-
// If a version argument was given, we do not perform the fallback.
269-
if version != nil {
270-
throw error
271-
}
272-
273-
guard let collectionUrl = URL(string: packageUrl) else {
274-
throw CollectionsError.invalidArgument("collectionUrl")
275-
}
276-
277-
let source = PackageCollectionsModel.CollectionSource(type: .json, url: collectionUrl)
278-
let collection = try with { collections in
279-
try tsc_await { collections.getCollection(source, callback: $0) }
280-
}
281-
282-
let description = optionalRow("Description", collection.overview)
283-
let keywords = optionalRow("Keywords", collection.keywords?.joined(separator: ", "))
284-
let createdAt = optionalRow("Created At", DateFormatter().string(from: collection.createdAt))
285-
let packages = collection.packages.map { "\($0.repository.url)" }.joined(separator: "\n\(indent(levels: 2))")
286-
287-
if jsonOptions.json {
288-
try JSONEncoder.makeWithDefaults().print(collection)
289-
} else {
290-
print("""
291-
Name: \(collection.name)
292-
Source: \(collection.source.url)\(description)\(keywords)\(createdAt)
293-
Packages:
294-
\(packages)
295-
""")
296-
}
297292
}
298293
}
299294
}

‎Sources/PackageCollections/Storage/SQLitePackageCollectionsStorage.swift

Lines changed: 91 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable
4141
private let cache = ThreadSafeKeyValueStore<Model.CollectionIdentifier, Model.Collection>()
4242
private let cacheLock = Lock()
4343

44+
private let isShuttingdown = ThreadSafeBox<Bool>()
45+
4446
// Lock helps prevent concurrency errors with transaction statements during e.g. `refreshCollections`,
4547
// since only one transaction is allowed per SQLite connection. We need transactions to speed up bulk updates.
4648
// TODO: we could potentially optimize this with db connection pool
@@ -50,7 +52,7 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable
5052

5153
// Targets have in-memory trie in addition to SQLite FTS as optimization
5254
private let targetTrie = Trie<CollectionPackage>()
53-
private var targetTrieReady = ThreadSafeBox<Bool>(false)
55+
private var targetTrieReady = ThreadSafeBox<Bool>()
5456

5557
init(location: SQLite.Location? = nil, diagnosticsEngine: DiagnosticsEngine? = nil) {
5658
self.location = location ?? .path(localFileSystem.swiftPMCacheDirectory.appending(components: "package-collection.db"))
@@ -78,14 +80,81 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable
7880
}
7981

8082
func close() throws {
83+
// Signal long-running operation (e.g., populateTargetTrie) to stop
84+
self.isShuttingdown.put(true)
85+
86+
func retryClose(db: SQLite, exponentialBackoff: inout ExponentialBackoff) throws {
87+
let semaphore = DispatchSemaphore(value: 0)
88+
let callback = { (result: Result<Void, Error>) in
89+
// If it has failed, the semaphore will timeout in which case we will retry
90+
if case .success = result {
91+
semaphore.signal()
92+
}
93+
}
94+
95+
// This throws error if we have exhausted our attempts
96+
let delay = try exponentialBackoff.nextDelay()
97+
self.queue.asyncAfter(deadline: .now() + delay) {
98+
do {
99+
try db.close()
100+
callback(.success(()))
101+
} catch {
102+
callback(.failure(error))
103+
}
104+
}
105+
// Add some buffer to allow `asyncAfter` to run
106+
guard case .success = semaphore.wait(timeout: .now() + delay + .milliseconds(50)) else {
107+
return try retryClose(db: db, exponentialBackoff: &exponentialBackoff)
108+
}
109+
}
110+
81111
try self.stateLock.withLock {
82112
if case .connected(let db) = self.state {
83-
try db.close()
113+
do {
114+
try db.close()
115+
} catch {
116+
var exponentialBackoff = ExponentialBackoff()
117+
do {
118+
try retryClose(db: db, exponentialBackoff: &exponentialBackoff)
119+
} catch {
120+
throw StringError("Failed to close database")
121+
}
122+
}
84123
}
85124
self.state = .disconnected
86125
}
87126
}
88127

128+
private struct ExponentialBackoff {
129+
let intervalInMilliseconds: Int
130+
let randomizationFactor: Int
131+
let maximumAttempts: Int
132+
133+
var attempts: Int = 0
134+
var multipler: Int = 1
135+
136+
var canRetry: Bool {
137+
self.attempts < self.maximumAttempts
138+
}
139+
140+
init(intervalInMilliseconds: Int = 100, randomizationFactor: Int = 100, maximumAttempts: Int = 3) {
141+
self.intervalInMilliseconds = intervalInMilliseconds
142+
self.randomizationFactor = randomizationFactor
143+
self.maximumAttempts = maximumAttempts
144+
}
145+
146+
mutating func nextDelay() throws -> DispatchTimeInterval {
147+
guard self.canRetry else {
148+
throw StringError("Maximum attempts reached")
149+
}
150+
let delay = self.multipler * intervalInMilliseconds
151+
let jitter = Int.random(in: 0 ... self.randomizationFactor)
152+
self.attempts += 1
153+
self.multipler *= 2
154+
return .milliseconds(delay + jitter)
155+
}
156+
}
157+
89158
func put(collection: Model.Collection,
90159
callback: @escaping (Result<Model.Collection, Error>) -> Void) {
91160
self.queue.async {
@@ -756,25 +825,29 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable
756825

757826
func populateTargetTrie(callback: @escaping (Result<Void, Error>) -> Void = { _ in }) {
758827
self.queue.async {
759-
do {
760-
// Use FTS to build the trie
761-
let query = "SELECT collection_id_blob_base64, package_repository_url, name FROM \(Self.targetsFTSName);"
762-
try self.executeStatement(query) { statement in
763-
while let row = try statement.step() {
764-
let targetName = row.string(at: 2)
765-
766-
if let collectionData = Data(base64Encoded: row.string(at: 0)),
767-
let collection = try? self.decoder.decode(Model.CollectionIdentifier.self, from: collectionData) {
768-
let collectionPackage = CollectionPackage(collection: collection, package: PackageIdentity(url: row.string(at: 1)))
769-
self.targetTrie.insert(word: targetName.lowercased(), foundIn: collectionPackage)
828+
self.targetTrieReady.memoize {
829+
do {
830+
// Use FTS to build the trie
831+
let query = "SELECT collection_id_blob_base64, package_repository_url, name FROM \(Self.targetsFTSName);"
832+
try self.executeStatement(query) { statement in
833+
while let row = try statement.step() {
834+
if self.isShuttingdown.get() ?? false { return }
835+
836+
let targetName = row.string(at: 2)
837+
838+
if let collectionData = Data(base64Encoded: row.string(at: 0)),
839+
let collection = try? self.decoder.decode(Model.CollectionIdentifier.self, from: collectionData) {
840+
let collectionPackage = CollectionPackage(collection: collection, package: PackageIdentity(url: row.string(at: 1)))
841+
self.targetTrie.insert(word: targetName.lowercased(), foundIn: collectionPackage)
842+
}
770843
}
771844
}
845+
callback(.success(()))
846+
return true
847+
} catch {
848+
callback(.failure(error))
849+
return false
772850
}
773-
self.targetTrieReady.put(true)
774-
callback(.success(()))
775-
} catch {
776-
self.targetTrieReady.put(false)
777-
callback(.failure(error))
778851
}
779852
}
780853
}

0 commit comments

Comments
 (0)