Repository: bazelbuild/bazel
Issue: Windows:  failed to delete output files before executing action (.dll)
Labels: type: bug
P2
area-Windows
team-OSS

### Description of the problem / feature request:

When compiling on Windows, sometimes .dll cannot be deleted.
The typical error message is: " xx.dll: failed to delete output files before executing action" 

I am using remote_cache with remote_download_minimal, and it seems those items that actually are left in the local cache,  sometimes cannot be deleted.

Manually you can delete the .dlls just fine, so a workaround for now is to search the local cache for .dlls and delete them before server side compilation.

I have previously seen problems when defender or anti-virus is temporarily locking files. However this failure is permanent: Bazel cannot delete the files, the user can. 

### Feature requests: what underlying problem are you trying to solve with this feature?

1. Get a better error message when this error occurs (permission, read error etc).
2. Create a more stable way to delete files on windows (retry?, unlocking?)


### Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

It is sporadic, but probably connected to remote_cache

### What operating system are you running Bazel on?

Windows 10

### What's the output of `bazel info release`?

1.2.0

###  Have you found anything relevant by searching the web?

Nothing

### Any other information, logs, or outputs that you want to share?

No.


Issue comments:
We saw such bugs before; we refined the file deletion logic to deal with ever more situations, alas it's still incomplete apparently.

Do you know anything else about those files? Are they marked read-only?

@buchgr -- do you think those files might be held open?
Well, on the server this is quite frequent. It affects all binaries (.exe and .dll), and my best guess  is that it is connected to anti-virus or Windows defender is scanning the file, and therefore locking it.

When the server is entered, there is no problem to remove the binary, and nothing that is locking it.
I have some more info, most likely it is Windows Defender that locks the files, and the locking can last from minutes up to 30+ min. (so the unlocking period is not permanent, it is only very long)

Only specific files are affected, so far: .dll, .exe, .bin, .a and maybe .exp

Most of the time the locking  period is short, and then it only affects repeated jobs on the server side.
However, when the locking period is long, also local users are affected.

"Unlocker" can unlock the files in all cases, so the current solution is to walk through the output tree, and delete any locked binaries before the build (which takes quite some time on Windows.)
Is there any update on this issue? I'm still facing the same issue on Bazel 4.1.0 on Windows 10.
Having this issue sometimes. It'd be great if it printed out the path for the file so I could use some other way to delete it.

edit: I found that `bazel clean` will list the file, but I'll have trouble deleting it normally even with sudo. doing `sudo rm -rf /home/$USER/.cache/bazel` seems to delete them on wsl, but also nukes bazel completely, requiring it to rebuild and reacquire everything.
I'm seeing this a lot when building android targets with `bazel mobile-install`.  It's incredibly irritating because it doesn't tell me which files to remove and in order to continue I have to `bazel clean`, which then forces me to rebuild *everything* (and re-sync IntelliJ) even though I suspect only some outputs related to the `android_binary` are the problem.  It would be nice if there was a way to `bazel clean` a single target.
I just encountered this bug on a Windows machine in GitHub Actions while building a jar file that does not contain any native code.
I debugged another instance in which this happend and traced it back to being caused by an attempted rebuild of the rules_kotlin worker while it was still running due to a previous build request.

@larsrc-google Is the case of using non-prebuilt workers fully supported? It looks like all worker processes would need to be shut down before the worker dependencies can be rebuilt - at least on Windows, where deleting a file that is held open by a process isn't possible.
That does make sense. That is a Windows-specific problem, and I don't have a good immediate solution. 
@larsrc-google A workaround that shouldn't be too costly could be to restart all workers on Windows when the analysis cache is discarded. What do you think?

Pull request review comments:

Patch that resolved the issue:
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionException.java b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionException.java
index 613a15187a6d47..d81adfd419c5de 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionException.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionException.java
@@ -57,7 +57,7 @@ public ActionExecutionException(
       ActionAnalysisMetadata action,
       boolean catastrophe,
       DetailedExitCode detailedExitCode) {
-    super(message, cause);
+    super(combineMessages(message, cause), cause);
     this.action = action;
     this.catastrophe = catastrophe;
     this.detailedExitCode = checkNotNull(detailedExitCode);
@@ -96,7 +96,7 @@ public ActionExecutionException(
       NestedSet<Cause> rootCauses,
       boolean catastrophe,
       DetailedExitCode detailedExitCode) {
-    super(message, cause);
+    super(combineMessages(message, cause), cause);
     this.action = action;
     this.rootCauses = rootCauses;
     this.catastrophe = catastrophe;
@@ -203,4 +203,12 @@ public DetailedExitCode getDetailedExitCode() {
   public boolean showError() {
     return getMessage() != null;
   }
+
+  @Nullable
+  private static String combineMessages(String message, @Nullable Throwable cause) {
+    if (cause == null || cause.getMessage() == null) {
+      return message;
+    }
+    return message + ": " + cause.getMessage();
+  }
 }


Test patch:
diff --git a/src/test/java/com/google/devtools/build/lib/actions/ActionExecutionExceptionTest.java b/src/test/java/com/google/devtools/build/lib/actions/ActionExecutionExceptionTest.java
new file mode 100644
index 00000000000000..8fc50ff9e2f20b
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/actions/ActionExecutionExceptionTest.java
@@ -0,0 +1,45 @@
+// Copyright 2018 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.actions;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
+import com.google.devtools.build.lib.actions.util.TestAction.DummyAction;
+import com.google.devtools.build.lib.server.FailureDetails.Execution;
+import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
+import com.google.devtools.build.lib.util.DetailedExitCode;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** {@link ActionExecutionException}Test */
+@RunWith(JUnit4.class)
+public final class ActionExecutionExceptionTest {
+
+  @Test
+  public void containsCauseMessage() {
+    Exception e =
+        new ActionExecutionException(
+            "message",
+            new Exception("cause"),
+            new DummyAction(ActionsTestUtil.DUMMY_ARTIFACT, ActionsTestUtil.DUMMY_ARTIFACT),
+            false,
+            DetailedExitCode.of(
+                FailureDetail.newBuilder().setExecution(Execution.getDefaultInstance()).build()));
+    assertThat(e).hasMessageThat().contains("message");
+    assertThat(e).hasMessageThat().contains("cause");
+  }
+}
diff --git a/src/test/java/com/google/devtools/build/lib/actions/BUILD b/src/test/java/com/google/devtools/build/lib/actions/BUILD
index 0cabee71ee39ea..ac8d1b48587ef4 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/actions/BUILD
@@ -63,6 +63,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/testutils",
         "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/testutils:depsutils",
         "//src/main/java/com/google/devtools/build/lib/util",
+        "//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
         "//src/main/java/com/google/devtools/build/lib/util:filetype",
         "//src/main/java/com/google/devtools/build/lib/util:string",
         "//src/main/java/com/google/devtools/build/lib/vfs",