Skip to content

Commit e8129be

Browse files
yim-leeyim-lee
committed
[Collections] CLI 'remove' results in "database is locked" error
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 d9fefd7 commit e8129be

File tree

2 files changed

+127
-101
lines changed

2 files changed

+127
-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: 49 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,9 +80,34 @@ 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+
8186
try self.stateLock.withLock {
8287
if case .connected(let db) = self.state {
83-
try db.close()
88+
do {
89+
try db.close()
90+
} catch {
91+
// This could be because another operation is still running. Wait a bit then try again.
92+
let semaphore = DispatchSemaphore(value: 0)
93+
let callback = { (result: Result<Void, Error>) in
94+
// If second attempt fails as well, semaphore will timeout in which case we will throw error.
95+
if case .success = result {
96+
semaphore.signal()
97+
}
98+
}
99+
self.queue.asyncAfter(deadline: .now() + .milliseconds(200)) {
100+
do {
101+
try db.close()
102+
callback(.success(()))
103+
} catch {
104+
callback(.failure(error))
105+
}
106+
}
107+
guard case .success = semaphore.wait(timeout: .now() + .milliseconds(300)) else {
108+
throw StringError("Failed to close database")
109+
}
110+
}
84111
}
85112
self.state = .disconnected
86113
}
@@ -756,25 +783,29 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable
756783

757784
func populateTargetTrie(callback: @escaping (Result<Void, Error>) -> Void = { _ in }) {
758785
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)
786+
self.targetTrieReady.memoize {
787+
do {
788+
// Use FTS to build the trie
789+
let query = "SELECT collection_id_blob_base64, package_repository_url, name FROM \(Self.targetsFTSName);"
790+
try self.executeStatement(query) { statement in
791+
while let row = try statement.step() {
792+
if self.isShuttingdown.get() ?? false { return }
793+
794+
let targetName = row.string(at: 2)
795+
796+
if let collectionData = Data(base64Encoded: row.string(at: 0)),
797+
let collection = try? self.decoder.decode(Model.CollectionIdentifier.self, from: collectionData) {
798+
let collectionPackage = CollectionPackage(collection: collection, package: PackageIdentity(url: row.string(at: 1)))
799+
self.targetTrie.insert(word: targetName.lowercased(), foundIn: collectionPackage)
800+
}
770801
}
771802
}
803+
callback(.success(()))
804+
return true
805+
} catch {
806+
callback(.failure(error))
807+
return false
772808
}
773-
self.targetTrieReady.put(true)
774-
callback(.success(()))
775-
} catch {
776-
self.targetTrieReady.put(false)
777-
callback(.failure(error))
778809
}
779810
}
780811
}

0 commit comments

Comments
 (0)