From 5e264333ba6b0b9c5cf00d6e2a771a3c8917086c Mon Sep 17 00:00:00 2001 From: Annie Fu Date: Tue, 9 Apr 2019 15:59:21 -0700 Subject: [PATCH] Fix XLG serialization crash when strong fingerprint computation fails (#114) During cache lookup strong fingerprint computation, if ObservedInputProcessingStatus is not "Success", the strong fingerprint cannot be computed and a cache miss is forced. In this case, make sure the strong fingerprint inputs retrieved from cache are logged to the XLG rather than some invalid defaults, even if the cache data could not be used to compute a strong fingerprint. Fixes AB#1504534 --- Public/Src/Engine/Scheduler/PipExecutor.cs | 26 +++++++++++++--------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/Public/Src/Engine/Scheduler/PipExecutor.cs b/Public/Src/Engine/Scheduler/PipExecutor.cs index 69f3fc629..7878a985d 100644 --- a/Public/Src/Engine/Scheduler/PipExecutor.cs +++ b/Public/Src/Engine/Scheduler/PipExecutor.cs @@ -1865,9 +1865,9 @@ namespace BuildXL.Scheduler } var pathSet = maybePathSet.Value; - (ObservedInputProcessingResult observedInputProcessingResult, StrongContentFingerprint? strongContentFingerPrint, ObservedPathSet pathSetUsed, ContentHash pathSetHashUsed) + (bool succeeded, ObservedInputProcessingResult observedInputProcessingResult, StrongContentFingerprint? strongContentFingerprint, ObservedPathSet pathSetUsed, ContentHash pathSetHashUsed) strongFingerprintComputationResult = - await ComputeStrongFingerprintBasedOnPriorObservedPathSetAsync( + await TryComputeStrongFingerprintBasedOnPriorObservedPathSetAsync( operationContext, environment, state, @@ -1876,10 +1876,16 @@ namespace BuildXL.Scheduler pathSet, entryRef.PathSetHash); - BoxRef strongFingerprintComputationData = new ProcessStrongFingerprintComputationData( - pathSet: strongFingerprintComputationResult.pathSetUsed, - pathSetHash: strongFingerprintComputationResult.pathSetHashUsed, - priorStrongFingerprints: new List(1) { entryRef.StrongFingerprint }); + // Record the most relevant strong fingerprint information, defaulting to information retrieved from cache + BoxRef strongFingerprintComputationData = strongFingerprintComputationResult.succeeded + ? new ProcessStrongFingerprintComputationData( + pathSet: strongFingerprintComputationResult.pathSetUsed, + pathSetHash: strongFingerprintComputationResult.pathSetHashUsed, + priorStrongFingerprints: new List(1) { strongFingerprintComputationResult.strongContentFingerprint.Value }) + : new ProcessStrongFingerprintComputationData( + pathSet: pathSet, + pathSetHash: entryRef.PathSetHash, + priorStrongFingerprints: new List(1) { entryRef.StrongFingerprint }); strongFingerprintComputationList.Add(strongFingerprintComputationData); @@ -1889,7 +1895,7 @@ namespace BuildXL.Scheduler switch (processingStatus) { case ObservedInputProcessingStatus.Success: - strongFingerprint = strongFingerprintComputationResult.Item2; + strongFingerprint = strongFingerprintComputationResult.strongContentFingerprint; Contract.Assume(strongFingerprint.HasValue); strongFingerprintComputationData.Value = strongFingerprintComputationData.Value.ToSuccessfulResult( @@ -3103,7 +3109,7 @@ namespace BuildXL.Scheduler /// Note that if the returned processing status is , then a failure has been logged and pip /// execution must fail. /// - private static async Task<(ObservedInputProcessingResult, StrongContentFingerprint?, ObservedPathSet, ContentHash)> ComputeStrongFingerprintBasedOnPriorObservedPathSetAsync( + private static async Task<(bool, ObservedInputProcessingResult, StrongContentFingerprint?, ObservedPathSet, ContentHash)> TryComputeStrongFingerprintBasedOnPriorObservedPathSetAsync( OperationContext operationContext, IPipExecutionEnvironment environment, PipExecutionState.PipScopeState state, @@ -3132,7 +3138,7 @@ namespace BuildXL.Scheduler // force cache miss if observed input processing result is not 'Success' if (validationResult.Status != ObservedInputProcessingStatus.Success) { - return (validationResult, default(StrongContentFingerprint?), default(ObservedPathSet), default(ContentHash)); + return (false, validationResult, default(StrongContentFingerprint?), default(ObservedPathSet), default(ContentHash)); } // check if now running with safer options than before (i.e., prior are not strictly safer than current) @@ -3165,7 +3171,7 @@ namespace BuildXL.Scheduler finalPathSetHash); } - return (validationResult, strongFingerprint, finalPathSet, finalPathSetHash); + return (true, validationResult, strongFingerprint, finalPathSet, finalPathSetHash); } }