Skip to content

Commit

Permalink
[PubGrub] Fix a potential deadlock when prefetching
Browse files Browse the repository at this point in the history
The prefetching logic had a bug which sometimes leads to deadlock if
prefetched container queries start calling the completion block while
the main resolution logic is trying to fetch a new container that is NOT
in the prefetching list (because it's new dependency). The main reason
for this is that the container provider is using a serial queue for
callbacks (which is questionable) but it's better to not make any
assumptions about the provider and never run potentially blocking code
in the compleition handler.

Unfortunately, this is proving hard to unit test because this
depends on network speed/latency etc.

<rdar://problem/58644686>
  • Loading branch information
ankitspd committed Jan 17, 2020
1 parent 11ae0a7 commit f9cf27e
Showing 1 changed file with 11 additions and 8 deletions.
19 changes: 11 additions & 8 deletions Sources/PackageGraph/Pubgrub.swift
Expand Up @@ -11,6 +11,7 @@
import struct TSCUtility.Version
import TSCBasic
import struct PackageModel.PackageReference
import Dispatch

/// A term represents a statement about a package that may be true or false.
public struct Term: Equatable, Hashable {
Expand Down Expand Up @@ -1845,15 +1846,17 @@ private final class ContainerProvider {
_prefetchingContainers.insert(identifier)

provider.getContainer(for: identifier, skipUpdate: skipUpdate) { container in
self.fetchCondition.whileLocked {
// Update the structures and signal any thread waiting
// on prefetching to finish.
let pubGrubContainer = container.map {
PubGrubPackageContainer($0, pinsMap: self.pinsMap)
DispatchQueue.global().async {
self.fetchCondition.whileLocked {
// Update the structures and signal any thread waiting
// on prefetching to finish.
let pubGrubContainer = container.map {
PubGrubPackageContainer($0, pinsMap: self.pinsMap)
}
self._fetchedContainers[identifier] = pubGrubContainer
self._prefetchingContainers.remove(identifier)
self.fetchCondition.signal()
}
self._fetchedContainers[identifier] = pubGrubContainer
self._prefetchingContainers.remove(identifier)
self.fetchCondition.signal()
}
}
}
Expand Down

0 comments on commit f9cf27e

Please sign in to comment.