Skip to content

Commit cbee42a

Browse files
authored
[Collections] PackageCollectionsTests.testUpdateAuthTokens sometimes causes signal 11 (#3534)
Motivation: rdar://78939220 `SQLitePackageCollectionsStorage.populateTargetTrie` is always called in the initializer and to be completed by a background thread. Looks like the crash could be caused by `populateTargetTrie` being run while `SQLitePackageCollectionsStorage` gets destroyed as part of `PackageCollections` deallocation. Modifications: Add option in `SQLitePackageCollectionsStorage.Configuration` to skip calling `populateTargetTrie` in tests to avoid the race between db destruction and `populateTargetTrie`. Not all tests need storage or make use of `populateTargetTrie`, and it's caused many crashes in CI already. We have added traps for db connection state but that doesn't seem to be enough. Another approach is probably to use `withExtendedLifetime` in `testUpdateAuthTokens`, but I think it is better/easier to have the config option.
1 parent 62fef21 commit cbee42a

File tree

3 files changed

+15
-7
lines changed

3 files changed

+15
-7
lines changed

Sources/PackageCollections/Storage/SQLitePackageCollectionsStorage.swift

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,9 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable
6161
self.encoder = JSONEncoder.makeWithDefaults()
6262
self.decoder = JSONDecoder.makeWithDefaults()
6363

64-
self.populateTargetTrie()
64+
if configuration.initializeTargetTrie {
65+
self.populateTargetTrie()
66+
}
6567
}
6668

6769
convenience init(path: AbsolutePath, diagnosticsEngine: DiagnosticsEngine? = nil) {
@@ -1018,12 +1020,15 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable
10181020

10191021
struct Configuration {
10201022
var batchSize: Int
1023+
var initializeTargetTrie: Bool
10211024

10221025
fileprivate var underlying: SQLite.Configuration
10231026

1024-
init() {
1025-
self.underlying = .init()
1027+
init(initializeTargetTrie: Bool = true) {
10261028
self.batchSize = 100
1029+
self.initializeTargetTrie = initializeTargetTrie
1030+
1031+
self.underlying = .init()
10271032
self.maxSizeInMegabytes = 100
10281033
// see https://www.sqlite.org/c3ref/busy_timeout.html
10291034
self.busyTimeoutMilliseconds = 1000

Tests/PackageCollectionsTests/PackageCollectionsTests.swift

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,13 @@ final class PackageCollectionsTests: XCTestCase {
2222
func testUpdateAuthTokens() throws {
2323
let authTokens = ThreadSafeKeyValueStore<AuthTokenType, String>()
2424
let configuration = PackageCollections.Configuration(authTokens: { authTokens.get() })
25-
let storage = makeMockStorage()
25+
26+
// This test doesn't use storage at all and finishes quickly so disable target trie to prevent race
27+
let storageConfig = SQLitePackageCollectionsStorage.Configuration(initializeTargetTrie: false)
28+
let storage = makeMockStorage(storageConfig)
2629
defer { XCTAssertNoThrow(try storage.close()) }
2730

28-
// disable cache for this test to avoid setup/cleanup
31+
// Disable cache for this test to avoid setup/cleanup
2932
let metadataProviderConfig = GitHubPackageMetadataProvider.Configuration(authTokens: configuration.authTokens, cacheTTLInSeconds: -1)
3033
let metadataProvider = GitHubPackageMetadataProvider(configuration: metadataProviderConfig)
3134
let packageCollections = PackageCollections(configuration: configuration, storage: storage, collectionProviders: [:], metadataProvider: metadataProvider)

Tests/PackageCollectionsTests/Utility.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,10 +121,10 @@ func makeMockPackageBasicMetadata() -> PackageCollectionsModel.PackageBasicMetad
121121
processedAt: Date())
122122
}
123123

124-
func makeMockStorage() -> PackageCollections.Storage {
124+
func makeMockStorage(_ collectionsStorageConfig: SQLitePackageCollectionsStorage.Configuration = .init()) -> PackageCollections.Storage {
125125
let mockFileSystem = InMemoryFileSystem()
126126
return .init(sources: FilePackageCollectionsSourcesStorage(fileSystem: mockFileSystem),
127-
collections: SQLitePackageCollectionsStorage(location: .memory))
127+
collections: SQLitePackageCollectionsStorage(location: .memory, configuration: collectionsStorageConfig))
128128
}
129129

130130
struct MockCollectionsProvider: PackageCollectionProvider {

0 commit comments

Comments
 (0)