Skip to content

Commit 1c53ef5

Browse files
authored
Close manifest cache DB before calling completion handlers (#5881)
Previously, we were relying on `defer` to close the database, but that would mean we're closing it when exiting the scope which is typically *after* we are calling completion handlers. It seems as if we should instead call them before. Related to rdar://101956252
1 parent e0facda commit 1c53ef5

File tree

1 file changed

+11
-21
lines changed

1 file changed

+11
-21
lines changed

Sources/PackageLoading/ManifestLoader.swift

+11-21
Original file line numberDiff line numberDiff line change
@@ -333,14 +333,17 @@ public final class ManifestLoader: ManifestLoaderProtocol {
333333
)
334334
}
335335

336-
var closeAfterRead = DelayableAction(target: cache) { cache in
336+
let closingCompletion = { (result: Result<ManifestJSONParser.Result, Error>) in
337337
do {
338-
try cache.close()
338+
try cache?.close()
339339
} catch {
340340
observabilityScope.emit(warning: "failed closing cache: \(error)")
341341
}
342+
343+
callbackQueue.async {
344+
completion(result)
345+
}
342346
}
343-
defer { closeAfterRead.perform() }
344347

345348
let key : CacheKey
346349
do {
@@ -353,9 +356,7 @@ public final class ManifestLoader: ManifestLoaderProtocol {
353356
fileSystem: fileSystem
354357
)
355358
} catch {
356-
return callbackQueue.async {
357-
completion(.failure(error))
358-
}
359+
return closingCompletion(.failure(error))
359360
}
360361

361362
do {
@@ -371,17 +372,12 @@ public final class ManifestLoader: ManifestLoaderProtocol {
371372
fileSystem: fileSystem,
372373
observabilityScope: observabilityScope
373374
)
374-
return callbackQueue.async {
375-
completion(.success(parsedManifest))
376-
}
375+
return closingCompletion(.success(parsedManifest))
377376
}
378377
} catch {
379378
observabilityScope.emit(warning: "failed loading cached manifest for '\(key.packageIdentity)': \(error)")
380379
}
381380

382-
// delay closing cache until after write.
383-
let closeAfterWrite = closeAfterRead.delay()
384-
385381
// shells out and compiles the manifest, finally output a JSON
386382
observabilityScope.emit(debug: "evaluating manifest for '\(packageIdentity)' v. \(packageVersion?.description ?? "unknown")")
387383
do {
@@ -396,8 +392,6 @@ public final class ManifestLoader: ManifestLoaderProtocol {
396392
dispatchPrecondition(condition: .onQueue(callbackQueue))
397393

398394
do {
399-
defer { closeAfterWrite.perform() }
400-
401395
let evaluationResult = try result.get()
402396
// only cache successfully parsed manifests
403397
let parseManifest = try self.parseManifest(
@@ -417,17 +411,13 @@ public final class ManifestLoader: ManifestLoaderProtocol {
417411
observabilityScope.emit(warning: "failed storing manifest for '\(key.packageIdentity)' in cache: \(error)")
418412
}
419413

420-
completion(.success(parseManifest))
414+
return closingCompletion(.success(parseManifest))
421415
} catch {
422-
callbackQueue.async {
423-
completion(.failure(error))
424-
}
416+
return closingCompletion(.failure(error))
425417
}
426418
}
427419
} catch {
428-
callbackQueue.async {
429-
completion(.failure(error))
430-
}
420+
return closingCompletion(.failure(error))
431421
}
432422
}
433423

0 commit comments

Comments
 (0)