Move Profiler.instance() closes before executing runtime.commandComplete().

PiperOrigin-RevId: 709850756
Change-Id: If4d4ccbfd716a2973dad4e0b5dcc2ecbf1db57fe

Author: yuyue730

src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java

@@ -745,6 +745,9 @@ private BlazeCommandResult execExclusively(
       return result;
     } finally {
       try {
+        // Profiler might still be running when an exception is thrown before BuildCompleteEvent is
+        // emitted or BlazeModule#completeCommand() is called. So we still need to try to stop the
+        // profiler here.
         Profiler.instance().stop();
         if (profilerStartedEvent.getProfile() instanceof LocalInstrumentationOutput profile) {
           profile.makeConvenienceLink();

src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java

@@ -765,6 +765,15 @@ public BlazeCommandResult afterCommand(
     }
     env.getBlazeWorkspace().clearEventBus();
 
+    // Some module's commandComplete() relies on the stoppage of profiler. And it is impossible the
+    // profiler is needed after all `BlazeModule.afterCommand`s are executed.
+    // See b/331203854#comment124 for more details.
+    try {
+      Profiler.instance().stop();
+    } catch (IOException e) {
+      env.getReporter().handle(Event.error("Error while writing profile file: " + e.getMessage()));
+    }
+
     for (BlazeModule module : blazeModules) {
       try (SilentCloseable closeable = Profiler.instance().profile(module + ".commandComplete")) {
         module.commandComplete();

src/main/java/com/google/devtools/build/lib/runtime/BuildSummaryStatsModule.java

@@ -192,9 +192,16 @@ public void buildComplete(BuildCompleteEvent event) {
         }
       }
       if (profileEvent != null && profileEvent.getProfile() != null) {
-        // This leads to missing the afterCommand profiles of the other modules in the profile.
-        // Since the BEP currently shuts down at the BuildCompleteEvent, we cannot just move posting
-        // the BuildToolLogs to afterCommand of this module.
+        // The profiler has to be stopped before `BuildEventServiceModule#afterCommand` is called,
+        // especially when it is a bep artifact. An unstopped bep artifact could lead to a deadlock
+        // in `BuildEventServiceModule#afterCommand`.
+        //
+        // We choose to stop profiler here instead of in `BuildSummaryStatsModule#afterCommand` so
+        // that no ordering between GoogleBuildSummaryStatsModule and BuildEventServiceModule's
+        // `afterCommand`s needs to be assumed. See b/253394502.
+        //
+        // Stopping the profiler here leads to missing the afterCommand profiles of the other
+        // modules in the profile, which is a compromise we are willing to make.
         try {
           Profiler.instance().stop();
           profileEvent.getProfile().publish(event.getResult().getBuildToolLogCollection());