Skip to content

Close manifest cache DB before calling completion handlers #5881

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 7, 2022
Merged

Close manifest cache DB before calling completion handlers #5881

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 11 additions & 21 deletions Sources/PackageLoading/ManifestLoader.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,14 +333,17 @@ public final class ManifestLoader: ManifestLoaderProtocol {
)
}

var closeAfterRead = DelayableAction(target: cache) { cache in
let closingCompletion = { (result: Result<ManifestJSONParser.Result, Error>) in
do {
try cache.close()
try cache?.close()
} catch {
observabilityScope.emit(warning: "failed closing cache: \(error)")
}

callbackQueue.async {
completion(result)
}
}
defer { closeAfterRead.perform() }

let key : CacheKey
do {
Expand All @@ -353,9 +356,7 @@ public final class ManifestLoader: ManifestLoaderProtocol {
fileSystem: fileSystem
)
} catch {
return callbackQueue.async {
completion(.failure(error))
}
return closingCompletion(.failure(error))
}

do {
Expand All @@ -371,17 +372,12 @@ public final class ManifestLoader: ManifestLoaderProtocol {
fileSystem: fileSystem,
observabilityScope: observabilityScope
)
return callbackQueue.async {
completion(.success(parsedManifest))
}
return closingCompletion(.success(parsedManifest))
}
} catch {
observabilityScope.emit(warning: "failed loading cached manifest for '\(key.packageIdentity)': \(error)")
}

// delay closing cache until after write.
let closeAfterWrite = closeAfterRead.delay()

// shells out and compiles the manifest, finally output a JSON
observabilityScope.emit(debug: "evaluating manifest for '\(packageIdentity)' v. \(packageVersion?.description ?? "unknown")")
do {
Expand All @@ -396,8 +392,6 @@ public final class ManifestLoader: ManifestLoaderProtocol {
dispatchPrecondition(condition: .onQueue(callbackQueue))

do {
defer { closeAfterWrite.perform() }

let evaluationResult = try result.get()
// only cache successfully parsed manifests
let parseManifest = try self.parseManifest(
Expand All @@ -417,17 +411,13 @@ public final class ManifestLoader: ManifestLoaderProtocol {
observabilityScope.emit(warning: "failed storing manifest for '\(key.packageIdentity)' in cache: \(error)")
}

completion(.success(parseManifest))
return closingCompletion(.success(parseManifest))
} catch {
callbackQueue.async {
completion(.failure(error))
}
return closingCompletion(.failure(error))
}
}
} catch {
callbackQueue.async {
completion(.failure(error))
}
return closingCompletion(.failure(error))
}
}

Expand Down