fix race condition when trie generation thread may be invoked after storage has been closed (#3505)

motivation: with trie genration now running on lower priority, a race was exposed where the operation may start after the storage already was closed

changes:
* in trie generation, guard on state before trying to execute a statement on the connection since it may be terminated
* in SQLite storage assert state does not got back frmo disconnected -> connected to help triage such issues
* adjust test to new assertion

Author: tomerd

Sources/PackageCollections/Storage/SQLitePackageCollectionsStorage.swift

@@ -763,6 +763,14 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable
         DispatchQueue.sharedConcurrent.async(group: nil, qos: .background, flags: .assignCurrentContext, execute: {
             self.targetTrieReady.memoize {
                 do {
+                    // since running on low priority thread make sure the database has not already gone away
+                    switch (try self.withStateLock { self.state }) {
+                    case .disconnected, .disconnecting:
+                        callback(.success(()))
+                        return false
+                    default:
+                        break
+                    }
                     // Use FTS to build the trie
                     let query = "SELECT collection_id_blob_base64, package_repository_url, name FROM \(Self.targetsFTSName);"
                     try self.executeStatement(query) { statement in
@@ -833,6 +841,8 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable
         let db = try self.withStateLock { () -> SQLite in
             let db: SQLite
             switch (self.location, self.state) {
+            case (_, .disconnecting), (_, .disconnected):
+                preconditionFailure("DB id disconnecting or disconnected")
             case (.path(let path), .connected(let database)):
                 if self.fileSystem.exists(path) {
                     db = database

Tests/PackageCollectionsTests/PackageCollectionsStorageTests.swift

@@ -124,16 +124,17 @@ class PackageCollectionsStorageTests: XCTestCase {
             }
 
             try storage.close()
-            storage.resetCache()
 
             XCTAssertTrue(storage.fileSystem.exists(storagePath), "expected file to exist at \(path)")
             try storage.fileSystem.writeFileContents(storagePath, bytes: ByteString("blah".utf8))
 
-            XCTAssertThrowsError(try tsc_await { callback in storage.get(identifier: mockCollections.first!.identifier, callback: callback) }, "expected error", { error in
+            let storage2 = SQLitePackageCollectionsStorage(path: path)
+            defer { XCTAssertNoThrow(try storage2.close()) }
+            XCTAssertThrowsError(try tsc_await { callback in storage2.get(identifier: mockCollections.first!.identifier, callback: callback) }, "expected error", { error in
                 XCTAssert("\(error)".contains("is not a database"), "Expected file is not a database error")
             })
 
-            XCTAssertThrowsError(try tsc_await { callback in storage.put(collection: mockCollections.first!, callback: callback) }, "expected error", { error in
+            XCTAssertThrowsError(try tsc_await { callback in storage2.put(collection: mockCollections.first!, callback: callback) }, "expected error", { error in
                 XCTAssert("\(error)".contains("is not a database"), "Expected file is not a database error")
             })
         }