Skip to content

Commit

Permalink
Fix litter deadlock with queuing data for tests
Browse files Browse the repository at this point in the history
  • Loading branch information
Tatskaari committed Nov 28, 2024
1 parent e8637eb commit b189652
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 18 deletions.
3 changes: 0 additions & 3 deletions src/build/build_step.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,6 @@ func Build(state *core.BuildState, target *core.BuildTarget, remote bool) {
}
// Mark the target as having finished building.
target.FinishBuild()
if target.IsTest() && state.NeedTests && state.IsOriginalTarget(target) {
state.QueueTestTarget(target)
}
}

func validateBuildTargetBeforeBuild(state *core.BuildState, target *core.BuildTarget) error {
Expand Down
4 changes: 2 additions & 2 deletions src/core/build_target.go
Original file line number Diff line number Diff line change
Expand Up @@ -712,8 +712,8 @@ func (target *BuildTarget) FinishBuild() {
}

// WaitForBuild blocks until this target has finished building.
func (target *BuildTarget) WaitForBuild() {
<-target.finishedBuilding
func (target *BuildTarget) WaitForBuild(dependant BuildLabel) {
waitOnChan(target.finishedBuilding, "Still waiting on (target %v).WaitForBuild(dependant %v)", target.Label, dependant)
}

// DeclaredOutputs returns the outputs from this target's original declaration.
Expand Down
4 changes: 2 additions & 2 deletions src/core/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -904,7 +904,7 @@ func (state *BuildState) WaitForBuiltTarget(l, dependent BuildLabel, mode ParseM
return make(chan struct{})
}); !inserted {
// Something's already registered for this, get on the train
waitOnChan(ch, "Still waiting on WaitForBuiltTarget(%v, %v, %v)", l, dependent, mode)
waitOnChan(ch, "Still waiting on WaitForBuiltTarget(label %v, dependant %v, ParseMode(%v))", l, dependent, mode)
return state.Graph.Target(l)
}
if err := state.queueTarget(l, dependent, mode.IsForSubinclude(), mode); err != nil {
Expand Down Expand Up @@ -1187,7 +1187,7 @@ func (state *BuildState) queueTargetAsync(target *BuildTarget, forceBuild, build
// Wait for these targets to actually build.
if building {
for _, t := range target.Dependencies() {
t.WaitForBuild()
t.WaitForBuild(target.Label)
if t.State() >= DependencyFailed { // Either the target failed or its dependencies failed
// Give up and set the original target as dependency failed
target.SetState(DependencyFailed)
Expand Down
44 changes: 33 additions & 11 deletions src/plz/plz.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,32 @@ func Run(targets, preTargets []core.BuildLabel, state *core.BuildState, config *
remoteLimiter := make(limiter, config.NumRemoteExecutors())
anyRemote := config.NumRemoteExecutors() > 0

completeAction := func(remote bool, task core.Task) {
if remote {
remoteLimiter.Release()
} else {
localLimiter.Release()
}

if task.Type != core.BuildTask {
state.TaskDone()
return
}

if state.NeedTests && task.Target.IsTest() && state.IsOriginalTarget(task.Target) {
state.QueueTestTarget(task.Target)
}
state.TaskDone()
}

startAction := func(remote bool) {
if remote {
remoteLimiter.Acquire()
} else {
localLimiter.Acquire()
}
}

// Start up all the build workers
var wg sync.WaitGroup
wg.Add(2)
Expand All @@ -64,21 +90,17 @@ func Run(targets, preTargets []core.BuildLabel, state *core.BuildState, config *
wg.Add(1)
go func(task core.Task) {
defer wg.Done()
remote := anyRemote && !task.Target.Local
if remote {
remoteLimiter.Acquire()
defer remoteLimiter.Release()
} else {
localLimiter.Acquire()
defer localLimiter.Release()
}

isRemote := anyRemote && !task.Target.Local
startAction(isRemote)
defer completeAction(isRemote, task)

switch task.Type {
case core.TestTask:
test.Test(state, task.Target, remote, int(task.Run))
test.Test(state, task.Target, isRemote, int(task.Run))
case core.BuildTask:
build.Build(state, task.Target, remote)
build.Build(state, task.Target, isRemote)
}
state.TaskDone()
}(task)
}
wg.Done()
Expand Down

0 comments on commit b189652

Please sign in to comment.