diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationContext.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationContext.java index 74002a724aa770..86b7391811ff96 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationContext.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationContext.java @@ -581,8 +581,8 @@ void addAdditionalInputs(NestedSetBuilder builder) { } } - /** @return modules maps from direct dependencies. */ - public Iterable getDirectModuleMaps() { + /** Returns modules maps from direct dependencies. */ + public ImmutableList getDirectModuleMaps() { return directModuleMaps; } @@ -595,8 +595,8 @@ DerivedArtifact getSeparateHeaderModule(boolean usePic) { } /** - * @return all declared headers of the current module if the current target is compiled as a - * module. + * Returns all declared headers of the current module if the current target is compiled as a + * module. */ ImmutableList getHeaderModuleSrcs(boolean separateModule) { if (separateModule) { @@ -651,7 +651,7 @@ public static CcCompilationContext createWithExtraHeaderTokens( headerTokens.build()); } - /** @return the C++ module map of the owner. */ + /** Returns the C++ module map of the owner. */ public CppModuleMap getCppModuleMap() { return cppModuleMap; } @@ -1154,7 +1154,7 @@ public ImmutableList getMergedResult() { * Gathers data about the PIC and no-PIC .pcm files belonging to this context and the associated * information about the headers, e.g. modular vs. textual headers and pre-grepped header files. * - *

This also implements a data structure very similar to NestedSet, but chosing slightly + *

This also implements a data structure very similar to NestedSet, but choosing slightly * different trade-offs to account for the specific data stored in here, specifically, we know * that there is going to be a single entry in every node of the DAG. Contrary to NestedSet, we * reuse memoization data from dependencies to conserve both runtime and memory. Experiments have diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcStarlarkInternal.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcStarlarkInternal.java index 09d7f140a7838a..2c9a4736be3feb 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcStarlarkInternal.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcStarlarkInternal.java @@ -22,7 +22,9 @@ import com.google.devtools.build.docgen.annot.DocCategory; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.CommandLineExpansionException; +import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.actions.ActionConstructionContext; +import com.google.devtools.build.lib.analysis.actions.PathMappers; import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue; import com.google.devtools.build.lib.analysis.starlark.StarlarkActionFactory; import com.google.devtools.build.lib.analysis.starlark.StarlarkRuleContext; @@ -590,6 +592,7 @@ public void createModuleMapAction( Boolean generateSubmodules, Boolean withoutExternDependencies) throws EvalException { + RuleContext ruleContext = actions.getRuleContext(); ActionConstructionContext actionConstructionContext = actions.getActionConstructionContext(); actions .asActionRegistry(actions) @@ -609,6 +612,10 @@ public void createModuleMapAction( compiledModule, moduleMapHomeIsCwd, generateSubmodules, - withoutExternDependencies)); + withoutExternDependencies, + PathMappers.getOutputPathsMode(ruleContext.getConfiguration()), + ruleContext + .getConfiguration() + .modifiedExecutionInfo(ImmutableMap.of(), CppModuleMapAction.MNEMONIC))); } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainVariables.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainVariables.java index d041711e74b0aa..5701e7497cf37f 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainVariables.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainVariables.java @@ -1159,6 +1159,54 @@ public int hashCode() { } } + @Immutable + private static final class ArtifactSetSequence extends VariableValueAdapter { + private final NestedSet values; + + private ArtifactSetSequence(NestedSet values) { + Preconditions.checkNotNull(values); + this.values = values; + } + + @Override + public ImmutableList getSequenceValue( + String variableName, PathMapper pathMapper) { + ImmutableList valuesList = values.toList(); + ImmutableList.Builder sequences = + ImmutableList.builderWithExpectedSize(valuesList.size()); + for (Artifact value : valuesList) { + sequences.add(new StringValue(pathMapper.getMappedExecPathString(value))); + } + return sequences.build(); + } + + @Override + public String getVariableTypeName() { + return Sequence.SEQUENCE_VARIABLE_TYPE_NAME; + } + + @Override + public boolean isTruthy() { + return !values.isEmpty(); + } + + @Override + public boolean equals(Object other) { + if (this == other) { + return true; + } + if (!(other instanceof ArtifactSetSequence otherArtifacts)) { + return false; + } + return values.shallowEquals(otherArtifacts.values); + } + + @Override + public int hashCode() { + return values.shallowHashCode(); + } + } + /** * Single structure value. Be careful not to create sequences of single structures, as the memory * overhead is prohibitively big. @@ -1470,6 +1518,19 @@ public Builder addPathFragmentSequenceVariable(String name, NestedSetAccepts values as NestedSet. Nested set is stored directly, not cloned, not flattened. + */ + @CanIgnoreReturnValue + public Builder addArtifactSequenceVariable(String name, NestedSet values) { + checkVariableNotPresentAlready(name); + Preconditions.checkNotNull(values, "Cannot set null as a value for variable '%s'", name); + variablesMap.put(name, new ArtifactSetSequence(values)); + return this; + } + /** * Add a variable built using {@code VariableValueBuilder} api that expands {@code name} to the * value returned by the {@code builder}. diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileBuildVariables.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileBuildVariables.java index 97b9f90fa71cb4..77f5da2683856c 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileBuildVariables.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileBuildVariables.java @@ -163,7 +163,7 @@ public static CcToolchainVariables setupVariablesOrReportRuleError( Artifact diagnosticsFile, ImmutableList variablesExtensions, ImmutableMap additionalBuildVariables, - Iterable directModuleMaps, + ImmutableList directModuleMaps, NestedSet includeDirs, NestedSet quoteIncludeDirs, NestedSet systemIncludeDirs, @@ -237,7 +237,7 @@ public static CcToolchainVariables setupVariablesOrThrowEvalException( Artifact diagnosticsFile, ImmutableList variablesExtensions, ImmutableMap additionalBuildVariables, - Iterable directModuleMaps, + ImmutableList directModuleMaps, NestedSet includeDirs, NestedSet quoteIncludeDirs, NestedSet systemIncludeDirs, @@ -303,7 +303,7 @@ private static CcToolchainVariables setupVariables( Artifact diagnosticsFile, ImmutableList variablesExtensions, ImmutableMap additionalBuildVariables, - Iterable directModuleMaps, + ImmutableList directModuleMaps, NestedSet includeDirs, NestedSet quoteIncludeDirs, NestedSet systemIncludeDirs, @@ -475,7 +475,7 @@ public static void setupCommonVariables( String fdoStamp, List variablesExtensions, Map additionalBuildVariables, - Iterable directModuleMaps, + ImmutableList directModuleMaps, ImmutableList includeDirs, ImmutableList quoteIncludeDirs, ImmutableList systemIncludeDirs, @@ -509,7 +509,7 @@ private static void setupCommonVariablesInternal( String fdoStamp, List variablesExtensions, Map additionalBuildVariables, - Iterable directModuleMaps, + ImmutableList directModuleMaps, NestedSet includeDirs, NestedSet quoteIncludeDirs, NestedSet systemIncludeDirs, @@ -528,9 +528,9 @@ private static void setupCommonVariablesInternal( buildVariables.addStringVariable(MODULE_NAME.getVariableName(), cppModuleMap.getName()); buildVariables.addArtifactVariable( MODULE_MAP_FILE.getVariableName(), cppModuleMap.getArtifact()); - buildVariables.addStringSequenceVariable( + buildVariables.addArtifactSequenceVariable( DEPENDENT_MODULE_MAP_FILES.getVariableName(), - Iterables.transform(directModuleMaps, Artifact::getExecPathString)); + NestedSetBuilder.wrap(Order.STABLE_ORDER, directModuleMaps)); } if (featureConfiguration.isEnabled(CppRuleClasses.USE_HEADER_MODULES)) { // Module inputs will be set later when the action is executed. diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModuleMapAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModuleMapAction.java index 9b6edb86d8d125..62a26fe0409c8f 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModuleMapAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModuleMapAction.java @@ -16,15 +16,26 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Optional; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSortedMap; +import com.google.common.collect.Interner; import com.google.common.collect.Iterables; +import com.google.common.collect.Maps; import com.google.devtools.build.lib.actions.ActionExecutionContext; import com.google.devtools.build.lib.actions.ActionKeyContext; import com.google.devtools.build.lib.actions.ActionOwner; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander; +import com.google.devtools.build.lib.actions.CommandLineExpansionException; +import com.google.devtools.build.lib.actions.ExecutionRequirements; +import com.google.devtools.build.lib.actions.PathMapper; import com.google.devtools.build.lib.analysis.actions.AbstractFileWriteAction; import com.google.devtools.build.lib.analysis.actions.DeterministicWriter; +import com.google.devtools.build.lib.analysis.actions.PathMappers; +import com.google.devtools.build.lib.analysis.config.CoreOptions.OutputPathsMode; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; +import com.google.devtools.build.lib.collect.nestedset.Order; +import com.google.devtools.build.lib.concurrent.BlazeInterners; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.util.Fingerprint; import com.google.devtools.build.lib.vfs.PathFragment; @@ -35,6 +46,8 @@ import java.util.Collection; import java.util.HashSet; import java.util.List; +import java.util.Map; +import java.util.Set; import javax.annotation.Nullable; /** @@ -43,8 +56,11 @@ */ @Immutable public final class CppModuleMapAction extends AbstractFileWriteAction { + public static final String MNEMONIC = "CppModuleMap"; private static final String GUID = "4f407081-1951-40c1-befc-d6b4daff5de3"; + private static final Interner> executionInfoInterner = + BlazeInterners.newWeakInterner(); // C++ module map of the current target private final CppModuleMap cppModuleMap; @@ -65,6 +81,7 @@ public final class CppModuleMapAction extends AbstractFileWriteAction { private final boolean compiledModule; private final boolean generateSubmodules; private final boolean externDependencies; + private final ImmutableSortedMap executionInfo; public CppModuleMapAction( ActionOwner owner, @@ -77,7 +94,9 @@ public CppModuleMapAction( boolean compiledModule, boolean moduleMapHomeIsCwd, boolean generateSubmodules, - boolean externDependencies) { + boolean externDependencies, + OutputPathsMode outputPathsMode, + ImmutableMap executionInfo) { super( owner, NestedSetBuilder.stableOrder() @@ -95,6 +114,22 @@ public CppModuleMapAction( this.compiledModule = compiledModule; this.generateSubmodules = generateSubmodules; this.externDependencies = externDependencies; + // Save memory by storing outputPathsMode implicitly via the presence of + // ExecutionRequirements.SUPPORTS_PATH_MAPPING in the key set. Path mapping is only effectively + // enabled if the key is present *and* the mode is set to STRIP, so if the latter is not the + // case, we can safely not store the key. + Map storedExecutionInfo; + if (outputPathsMode == OutputPathsMode.STRIP) { + storedExecutionInfo = executionInfo; + } else { + storedExecutionInfo = + Maps.filterKeys( + executionInfo, k -> !k.equals(ExecutionRequirements.SUPPORTS_PATH_MAPPING)); + } + this.executionInfo = + storedExecutionInfo.isEmpty() + ? ImmutableSortedMap.of() + : executionInfoInterner.intern(ImmutableSortedMap.copyOf(storedExecutionInfo)); } @Override @@ -110,9 +145,16 @@ public boolean makeExecutable() { @Override public DeterministicWriter newDeterministicWriter(ActionExecutionContext ctx) { final ArtifactExpander artifactExpander = ctx.getArtifactExpander(); + // TODO: It is possible that compile actions consuming the module map have path mapping disabled + // due to inputs conflicting across configurations. Since these inputs aren't inputs of the + // module map action, the generated map still contains mapped paths, which then results in + // compilation failures. This should be very rare as #include doesn't allow to disambiguate + // between headers from different configurations but with identical root-relative paths. + final PathMapper pathMapper = + PathMappers.create(this, getOutputPathsMode(), /* isStarlarkAction= */ false); return out -> { OutputStreamWriter content = new OutputStreamWriter(out, StandardCharsets.ISO_8859_1); - PathFragment fragment = cppModuleMap.getArtifact().getExecPath(); + PathFragment fragment = pathMapper.map(cppModuleMap.getArtifact().getExecPath()); int segmentsToExecPath = fragment.segmentCount() - 1; Optional umbrellaHeader = cppModuleMap.getUmbrellaHeader(); String leadingPeriods = moduleMapHomeIsCwd ? "" : "../".repeat(segmentsToExecPath); @@ -134,7 +176,8 @@ public DeterministicWriter newDeterministicWriter(ActionExecutionContext ctx) { leadingPeriods, /* canCompile= */ false, deduper, - /*isUmbrellaHeader*/ true); + /*isUmbrellaHeader*/ true, + pathMapper); } else { for (Artifact artifact : expandedHeaders(artifactExpander, publicHeaders)) { appendHeader( @@ -144,7 +187,8 @@ public DeterministicWriter newDeterministicWriter(ActionExecutionContext ctx) { leadingPeriods, /* canCompile= */ true, deduper, - /*isUmbrellaHeader*/ false); + /*isUmbrellaHeader*/ false, + pathMapper); } for (Artifact artifact : expandedHeaders(artifactExpander, privateHeaders)) { appendHeader( @@ -154,7 +198,8 @@ public DeterministicWriter newDeterministicWriter(ActionExecutionContext ctx) { leadingPeriods, /* canCompile= */ true, deduper, - /*isUmbrellaHeader*/ false); + /*isUmbrellaHeader*/ false, + pathMapper); } for (Artifact artifact : separateModuleHdrs) { appendHeader( @@ -164,7 +209,8 @@ public DeterministicWriter newDeterministicWriter(ActionExecutionContext ctx) { leadingPeriods, /* canCompile= */ false, deduper, - /*isUmbrellaHeader*/ false); + /*isUmbrellaHeader*/ false, + pathMapper); } for (PathFragment additionalExportedHeader : additionalExportedHeaders) { appendHeader( @@ -174,7 +220,8 @@ public DeterministicWriter newDeterministicWriter(ActionExecutionContext ctx) { leadingPeriods, /*canCompile*/ false, deduper, - /*isUmbrellaHeader*/ false); + /*isUmbrellaHeader*/ false, + pathMapper); } } for (CppModuleMap dep : dependencies) { @@ -196,7 +243,8 @@ public DeterministicWriter newDeterministicWriter(ActionExecutionContext ctx) { leadingPeriods, /* canCompile= */ true, deduper, - /*isUmbrellaHeader*/ false); + /*isUmbrellaHeader*/ false, + pathMapper); } for (CppModuleMap dep : dependencies) { content.append(" use \"").append(dep.getName()).append("\"\n"); @@ -211,7 +259,7 @@ public DeterministicWriter newDeterministicWriter(ActionExecutionContext ctx) { .append(dep.getName()) .append("\" \"") .append(leadingPeriods) - .append(dep.getArtifact().getExecPathString()) + .append(pathMapper.getMappedExecPathString(dep.getArtifact())) .append("\""); } } @@ -219,8 +267,8 @@ public DeterministicWriter newDeterministicWriter(ActionExecutionContext ctx) { }; } - private static Iterable expandedHeaders(ArtifactExpander artifactExpander, - Iterable unexpandedHeaders) { + private static ImmutableList expandedHeaders( + ArtifactExpander artifactExpander, Iterable unexpandedHeaders) { List expandedHeaders = new ArrayList<>(); for (Artifact unexpandedHeader : unexpandedHeaders) { if (unexpandedHeader.isTreeArtifact()) { @@ -233,9 +281,17 @@ private static Iterable expandedHeaders(ArtifactExpander artifactExpan return ImmutableList.copyOf(expandedHeaders); } - private void appendHeader(Appendable content, String visibilitySpecifier, - PathFragment path, String leadingPeriods, boolean canCompile, HashSet deduper, - boolean isUmbrellaHeader) throws IOException { + private void appendHeader( + Appendable content, + String visibilitySpecifier, + PathFragment unmappedPath, + String leadingPeriods, + boolean canCompile, + Set deduper, + boolean isUmbrellaHeader, + PathMapper pathMapper) + throws IOException { + PathFragment path = pathMapper.map(unmappedPath); if (deduper.contains(path)) { return; } @@ -268,14 +324,15 @@ private boolean shouldCompileHeader(PathFragment path) { @Override public String getMnemonic() { - return "CppModuleMap"; + return MNEMONIC; } @Override protected void computeKey( ActionKeyContext actionKeyContext, @Nullable Artifact.ArtifactExpander artifactExpander, - Fingerprint fp) { + Fingerprint fp) + throws CommandLineExpansionException, InterruptedException { fp.addString(GUID); fp.addInt(privateHeaders.size()); for (Artifact artifact : privateHeaders) { @@ -308,6 +365,25 @@ protected void computeKey( fp.addBoolean(compiledModule); fp.addBoolean(generateSubmodules); fp.addBoolean(externDependencies); + PathMappers.addToFingerprint( + getMnemonic(), + getExecutionInfo(), + NestedSetBuilder.emptySet(Order.STABLE_ORDER), + actionKeyContext, + getOutputPathsMode(), + fp); + } + + @Override + public ImmutableMap getExecutionInfo() { + return executionInfo; + } + + private OutputPathsMode getOutputPathsMode() { + // See comment in the constructor for how outputPathsMode is stored implicitly. + return executionInfo.containsKey(ExecutionRequirements.SUPPORTS_PATH_MAPPING) + ? OutputPathsMode.STRIP + : OutputPathsMode.OFF; } @VisibleForTesting diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/BUILD b/src/test/java/com/google/devtools/build/lib/rules/cpp/BUILD index 97231ee9e395be..6976200361ed93 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/BUILD +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/BUILD @@ -479,6 +479,7 @@ java_test( deps = [ "//src/main/java/com/google/devtools/build/lib/actions", "//src/main/java/com/google/devtools/build/lib/actions:artifacts", + "//src/main/java/com/google/devtools/build/lib/analysis:config/core_options", "//src/main/java/com/google/devtools/build/lib/rules/cpp", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", "//src/test/java/com/google/devtools/build/lib/actions/util", diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CppModuleMapActionTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CppModuleMapActionTest.java index 4d53fbb8c2ef4d..5762fde909991b 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CppModuleMapActionTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CppModuleMapActionTest.java @@ -16,11 +16,13 @@ import static com.google.common.truth.Truth.assertThat; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.actions.ActionKeyContext; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.ArtifactRoot; import com.google.devtools.build.lib.actions.ArtifactRoot.RootType; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; +import com.google.devtools.build.lib.analysis.config.CoreOptions; import com.google.devtools.build.lib.testutil.Scratch; import com.google.devtools.build.lib.vfs.PathFragment; import java.io.IOException; @@ -60,15 +62,17 @@ private static CppModuleMapAction createCppModuleMapAction( return new CppModuleMapAction( ActionsTestUtil.NULL_ACTION_OWNER, cppModuleMap, - /*privateHeaders=*/ ImmutableList.of(), - /*publicHeaders=*/ ImmutableList.of(), + /* privateHeaders= */ ImmutableList.of(), + /* publicHeaders= */ ImmutableList.of(), ImmutableList.copyOf(dependencies), - /*additionalExportedHeaders=*/ ImmutableList.of(), - /*separateModuleHeaders=*/ ImmutableList.of(), - /*compiledModule=*/ false, - /*moduleMapHomeIsCwd=*/ false, - /*generateSubmodules=*/ false, - /*externDependencies=*/ false); + /* additionalExportedHeaders= */ ImmutableList.of(), + /* separateModuleHeaders= */ ImmutableList.of(), + /* compiledModule= */ false, + /* moduleMapHomeIsCwd= */ false, + /* generateSubmodules= */ false, + /* externDependencies= */ false, + CoreOptions.OutputPathsMode.OFF, + /* executionInfo= */ ImmutableMap.of()); } private Artifact createOutputArtifact(String rootRelativePath) { diff --git a/src/test/shell/bazel/path_mapping_test.sh b/src/test/shell/bazel/path_mapping_test.sh index d16a38c5e42c8d..239d6e9db4792f 100755 --- a/src/test/shell/bazel/path_mapping_test.sh +++ b/src/test/shell/bazel/path_mapping_test.sh @@ -623,9 +623,9 @@ EOF bazel run \ --verbose_failures \ --experimental_output_paths=strip \ - --modify_execution_info=CppCompile=+supports-path-mapping \ + --modify_execution_info=CppCompile=+supports-path-mapping,CppModuleMap=+supports-path-mapping \ --remote_executor=grpc://localhost:${worker_port} \ - --features=-module_maps \ + --features=layering_check \ "//$pkg:main" &>"$TEST_log" || fail "Expected success" expect_log 'Hello, lib1!' @@ -636,9 +636,9 @@ EOF bazel run \ --verbose_failures \ --experimental_output_paths=strip \ - --modify_execution_info=CppCompile=+supports-path-mapping \ + --modify_execution_info=CppCompile=+supports-path-mapping,CppModuleMap=+supports-path-mapping \ --remote_executor=grpc://localhost:${worker_port} \ - --features=-module_maps \ + --features=layering_check \ "//$pkg:transitioned_main" &>"$TEST_log" || fail "Expected success" expect_log 'Hi there, lib1!' diff --git a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ExecutionServer.java b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ExecutionServer.java index fd7ea0460877ee..9d73fdd2ad3215 100644 --- a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ExecutionServer.java +++ b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ExecutionServer.java @@ -172,11 +172,7 @@ public void waitExecution(WaitExecutionRequest wr, StreamObserver res return; } ((ServerCallStreamObserver) responseObserver) - .setOnCancelHandler( - () -> { - future.cancel(true); - operationsCache.remove(opName); - }); + .setOnCancelHandler(() -> operationsCache.remove(opName)); waitExecution(opName, future, responseObserver); } @@ -238,11 +234,7 @@ public void execute(ExecuteRequest request, StreamObserver responseOb executorService.submit(() -> execute(context, request, opName)); operationsCache.put(opName, future); ((ServerCallStreamObserver) responseObserver) - .setOnCancelHandler( - () -> { - future.cancel(true); - operationsCache.remove(opName); - }); + .setOnCancelHandler(() -> operationsCache.remove(opName)); // Send the first operation. responseObserver.onNext(Operation.newBuilder().setName(opName).build()); // When the operation completes, send the result.