Skip to content

Commit 2c788ba

Browse files
committed
Don't copy Protobuf messages via assignment
This isn't safe, as the Protobuf struct can contain fields that are protected by locks. It's only safe to create new messages by copying fields individually, or by using proto.Merge().
1 parent 853626a commit 2c788ba

File tree

1 file changed

+10
-5
lines changed

1 file changed

+10
-5
lines changed

pkg/scheduler/in_memory_build_queue.go

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2508,19 +2508,24 @@ func (t *task) complete(bq *InMemoryBuildQueue, executeResponse *remoteexecution
25082508
// Already running too many background tasks.
25092509
backgroundInitialSizeClassLearner.Abandoned()
25102510
} else {
2511-
backgroundAction := *t.desiredState.Action
2512-
backgroundAction.DoNotCache = true
2513-
backgroundAction.Timeout = durationpb.New(backgroundTimeout)
25142511
backgroundTask := &task{
25152512
operations: map[*invocation]*operation{},
25162513
actionDigest: t.actionDigest,
2517-
desiredState: t.desiredState,
25182514
targetID: t.targetID,
25192515
expectedDuration: backgroundExpectedDuration,
25202516
initialSizeClassLearner: backgroundInitialSizeClassLearner,
25212517
stageChangeWakeup: make(chan struct{}),
25222518
}
2523-
backgroundTask.desiredState.Action = &backgroundAction
2519+
2520+
// Set do_not_cache to ensure that
2521+
// background learning doesn't
2522+
// overwrite the ActionResult obtained
2523+
// by the foreground task.
2524+
proto.Merge(&backgroundTask.desiredState, &t.desiredState)
2525+
backgroundAction := backgroundTask.desiredState.Action
2526+
backgroundAction.DoNotCache = true
2527+
backgroundAction.Timeout = durationpb.New(backgroundTimeout)
2528+
25242529
backgroundTask.newOperation(bq, pq.backgroundLearningOperationPriority, backgroundInvocation, true)
25252530
backgroundTask.schedule(bq)
25262531
}

0 commit comments

Comments
 (0)