bep: introduce BuildEventArtifactUploaderFactory

There can be multiple BuildEventTransports active at
the same time and we need to ensure that each transport
gets its own BuildEventArtifactUploader as these transports
might have different lifecycles.

We do that by introducing another level of indirection via
the BuildEventArtifactUploaderFactory. BlazeModules now
register a factory object instead of an uploader.

In addition, the BuildEventArtifactUploader gets a shutdown()
method that allows to free any resources associated with it.

PiperOrigin-RevId: 203752092

15 files changed, 248 insertions, 139 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceModule.java b/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceModule.java
index 115808a66a..b12f398f3f 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceModule.java
+++ b/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceModule.java
@@ -27,7 +27,7 @@ import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions;
 import com.google.devtools.build.lib.buildeventservice.client.BuildEventServiceClient;
 import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploader;
-import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploaderMap;
+import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploaderFactoryMap;
 import com.google.devtools.build.lib.buildeventstream.BuildEventProtocolOptions;
 import com.google.devtools.build.lib.buildeventstream.BuildEventTransport;
 import com.google.devtools.build.lib.buildeventstream.transports.BuildEventStreamOptions;
@@ -88,7 +88,7 @@ public abstract class BuildEventServiceModule<T extends BuildEventServiceOptions
             commandEnvironment.getReporter(),
             commandEnvironment.getBlazeModuleEnvironment(),
             commandEnvironment.getRuntime().getClock(),
-            commandEnvironment.getRuntime().getBuildEventArtifactUploaders(),
+            commandEnvironment.getRuntime().getBuildEventArtifactUploaderFactoryMap(),
             commandEnvironment.getReporter(),
             commandEnvironment.getBuildRequestId().toString(),
             commandEnvironment.getCommandId().toString(),
@@ -139,12 +139,12 @@ public abstract class BuildEventServiceModule<T extends BuildEventServiceOptions
       EventHandler commandLineReporter,
       ModuleEnvironment moduleEnvironment,
       Clock clock,
-      BuildEventArtifactUploaderMap artifactUploaders,
+      BuildEventArtifactUploaderFactoryMap buildEventArtifactUploaderFactoryMap,
       Reporter reporter,
       String buildRequestId,
       String invocationId,
       String commandName) {
-    Preconditions.checkNotNull(artifactUploaders);
+    Preconditions.checkNotNull(buildEventArtifactUploaderFactoryMap);
 
     try {
       T besOptions =
@@ -152,14 +152,17 @@ public abstract class BuildEventServiceModule<T extends BuildEventServiceOptions
               optionsProvider.getOptions(optionsClass()),
               "Could not get BuildEventServiceOptions.");
       AuthAndTLSOptions authTlsOptions =
-          checkNotNull(optionsProvider.getOptions(AuthAndTLSOptions.class),
+          checkNotNull(
+              optionsProvider.getOptions(AuthAndTLSOptions.class),
               "Could not get AuthAndTLSOptions.");
       BuildEventStreamOptions bepOptions =
-          checkNotNull(optionsProvider.getOptions(BuildEventStreamOptions.class),
-          "Could not get BuildEventStreamOptions.");
+          checkNotNull(
+              optionsProvider.getOptions(BuildEventStreamOptions.class),
+              "Could not get BuildEventStreamOptions.");
       BuildEventProtocolOptions protocolOptions =
-          checkNotNull(optionsProvider.getOptions(BuildEventProtocolOptions.class),
-          "Could not get BuildEventProtocolOptions.");
+          checkNotNull(
+              optionsProvider.getOptions(BuildEventProtocolOptions.class),
+              "Could not get BuildEventProtocolOptions.");
 
       BuildEventTransport besTransport = null;
       try {
@@ -173,7 +176,7 @@ public abstract class BuildEventServiceModule<T extends BuildEventServiceOptions
                 moduleEnvironment,
                 clock,
                 protocolOptions,
-                artifactUploaders,
+                buildEventArtifactUploaderFactoryMap,
                 commandLineReporter,
                 startupOptionsProvider);
       } catch (Exception e) {
@@ -186,7 +189,10 @@ public abstract class BuildEventServiceModule<T extends BuildEventServiceOptions
 
       ImmutableSet<BuildEventTransport> bepTransports =
           BuildEventTransportFactory.createFromOptions(
-              bepOptions, protocolOptions, artifactUploaders, moduleEnvironment::exit);
+              bepOptions,
+              protocolOptions,
+              buildEventArtifactUploaderFactoryMap,
+              moduleEnvironment::exit);
 
       ImmutableSet.Builder<BuildEventTransport> transportsBuilder =
           ImmutableSet.<BuildEventTransport>builder().addAll(bepTransports);
@@ -216,7 +222,7 @@ public abstract class BuildEventServiceModule<T extends BuildEventServiceOptions
       ModuleEnvironment moduleEnvironment,
       Clock clock,
       BuildEventProtocolOptions protocolOptions,
-      BuildEventArtifactUploaderMap artifactUploaders,
+      BuildEventArtifactUploaderFactoryMap buildEventArtifactUploaderFactoryMap,
       EventHandler commandLineReporter,
       OptionsProvider startupOptionsProvider)
       throws IOException, OptionsParsingException {
@@ -224,8 +230,9 @@ public abstract class BuildEventServiceModule<T extends BuildEventServiceOptions
       logger.fine("BuildEventServiceTransport is disabled.");
       return null;
     } else {
-      logger.fine(format("Will create BuildEventServiceTransport streaming to '%s'",
-          besOptions.besBackend));
+      logger.fine(
+          format(
+              "Will create BuildEventServiceTransport streaming to '%s'", besOptions.besBackend));
 
       final String besResultsUrl;
       if (!Strings.isNullOrEmpty(besOptions.besResultsUrl)) {
@@ -247,7 +254,9 @@ public abstract class BuildEventServiceModule<T extends BuildEventServiceOptions
 
       BuildEventServiceClient client = createBesClient(besOptions, authTlsOptions);
       BuildEventArtifactUploader artifactUploader =
-          artifactUploaders.select(protocolOptions.buildEventUploadStrategy);
+          buildEventArtifactUploaderFactoryMap
+              .select(protocolOptions.buildEventUploadStrategy)
+              .create();
 
       BuildEventTransport besTransport =
           new BuildEventServiceTransport(
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceTransport.java b/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceTransport.java
index cb02ae3693..cd7492e0e2 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceTransport.java
+++ b/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceTransport.java
@@ -404,7 +404,11 @@ public class BuildEventServiceTransport implements BuildEventTransport {
           publishBuildFinishedEvent(result);
         }
       } finally {
-        besClient.shutdown();
+        try {
+          besClient.shutdown();
+        } finally {
+          artifactUploader.shutdown();
+        }
       }
       return null;
     }
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventArtifactUploader.java b/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventArtifactUploader.java
index 3f536325db..284ce6de67 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventArtifactUploader.java
+++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventArtifactUploader.java
@@ -31,6 +31,11 @@ public interface BuildEventArtifactUploader {
         public ListenableFuture<PathConverter> upload(Map<Path, LocalFile> files) {
           return completedPathConverter;
         }
+
+        @Override
+        public void shutdown() {
+          // Intentionally left empty.
+        }
       };
 
   /**
@@ -43,4 +48,9 @@ public interface BuildEventArtifactUploader {
    * as it should appear in the BEP.
    */
   ListenableFuture<PathConverter> upload(Map<Path, LocalFile> files);
+
+  /**
+   * Shutdown any resources associated with the uploader.
+   */
+  void shutdown();
 }
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventArtifactUploaderFactory.java b/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventArtifactUploaderFactory.java
new file mode 100644
index 0000000000..bee7c40433
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventArtifactUploaderFactory.java
@@ -0,0 +1,28 @@
+// 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.buildeventstream;
+
+import static com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploader.LOCAL_FILES_UPLOADER;
+
+/** A factory for {@link BuildEventArtifactUploader}. */
+public interface BuildEventArtifactUploaderFactory {
+
+  BuildEventArtifactUploaderFactory LOCAL_FILES_UPLOADER_FACTORY = () -> LOCAL_FILES_UPLOADER;
+
+  /**
+   * Returns a new instance of a {@link BuildEventArtifactUploader}. The call is responsible for
+   * calling {@link BuildEventArtifactUploader#shutdown()} on the returned instance.
+   */
+  BuildEventArtifactUploader create();
+}
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventArtifactUploaderMap.java b/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventArtifactUploaderFactoryMap.java
index e9f611ab9a..64c677341d 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventArtifactUploaderMap.java
+++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildEventArtifactUploaderFactoryMap.java
@@ -21,15 +21,15 @@ import javax.annotation.concurrent.ThreadSafe;
 
 /** Selects between multiple available upload strategies. */
 @ThreadSafe
-public class BuildEventArtifactUploaderMap {
-  private final ImmutableMap<String, BuildEventArtifactUploader> uploaders;
+public class BuildEventArtifactUploaderFactoryMap {
+  private final ImmutableMap<String, BuildEventArtifactUploaderFactory> uploaders;
 
-  private BuildEventArtifactUploaderMap(
-      ImmutableMap<String, BuildEventArtifactUploader> uploaders) {
+  private BuildEventArtifactUploaderFactoryMap(
+      ImmutableMap<String, BuildEventArtifactUploaderFactory> uploaders) {
     this.uploaders = uploaders;
   }
 
-  public BuildEventArtifactUploader select(@Nullable String name) {
+  public BuildEventArtifactUploaderFactory select(@Nullable String name) {
     if (name == null && !uploaders.values().isEmpty()) {
       // TODO(b/110235226): We currently choose the strategy with alphabetically first strategy,
       // which happens to be backwards-compatible; we need to set
@@ -37,23 +37,23 @@ public class BuildEventArtifactUploaderMap {
       // make it an error to pass null.
       return uploaders.values().iterator().next();
     }
-    return uploaders.getOrDefault(name, BuildEventArtifactUploader.LOCAL_FILES_UPLOADER);
+    return uploaders.getOrDefault(
+        name, BuildEventArtifactUploaderFactory.LOCAL_FILES_UPLOADER_FACTORY);
   }
 
-  /** Builder class for {@link BuildEventArtifactUploaderMap}. */
+  /** Builder class for {@link BuildEventArtifactUploaderFactoryMap}. */
   public static class Builder {
-    private final SortedMap<String, BuildEventArtifactUploader> uploaders = new TreeMap<>();
+    private final SortedMap<String, BuildEventArtifactUploaderFactory> uploaders = new TreeMap<>();
 
-    public Builder() {
-    }
+    public Builder() {}
 
-    public Builder add(String name, BuildEventArtifactUploader uploader) {
+    public Builder add(String name, BuildEventArtifactUploaderFactory uploader) {
       uploaders.put(name, uploader);
       return this;
     }
 
-    public BuildEventArtifactUploaderMap build() {
-      return new BuildEventArtifactUploaderMap(ImmutableMap.copyOf(uploaders));
+    public BuildEventArtifactUploaderFactoryMap build() {
+      return new BuildEventArtifactUploaderFactoryMap(ImmutableMap.copyOf(uploaders));
     }
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/BuildEventTransportFactory.java b/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/BuildEventTransportFactory.java
index eef7583451..6cc539418d 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/BuildEventTransportFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/BuildEventTransportFactory.java
@@ -18,7 +18,7 @@ import static com.google.common.base.Strings.isNullOrEmpty;
 
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploader;
-import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploaderMap;
+import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploaderFactoryMap;
 import com.google.devtools.build.lib.buildeventstream.BuildEventProtocolOptions;
 import com.google.devtools.build.lib.buildeventstream.BuildEventTransport;
 import com.google.devtools.build.lib.util.AbruptExitException;
@@ -107,14 +107,14 @@ public enum BuildEventTransportFactory {
   public static ImmutableSet<BuildEventTransport> createFromOptions(
       BuildEventStreamOptions options,
       BuildEventProtocolOptions protocolOptions,
-      BuildEventArtifactUploaderMap artifactUploaders,
+      BuildEventArtifactUploaderFactoryMap artifactUploaders,
       Consumer<AbruptExitException> exitFunc)
       throws IOException {
     ImmutableSet.Builder<BuildEventTransport> buildEventTransportsBuilder = ImmutableSet.builder();
     for (BuildEventTransportFactory transportFactory : BuildEventTransportFactory.values()) {
       if (transportFactory.enabled(options)) {
         BuildEventArtifactUploader uploader = transportFactory.usePathConverter(options)
-            ? artifactUploaders.select(protocolOptions.buildEventUploadStrategy)
+            ? artifactUploaders.select(protocolOptions.buildEventUploadStrategy).create()
             : BuildEventArtifactUploader.LOCAL_FILES_UPLOADER;
         buildEventTransportsBuilder.add(
             transportFactory.create(options, protocolOptions, uploader, exitFunc));
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/FileTransport.java b/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/FileTransport.java
index 186a6a5f0c..de29f4b7d4 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/FileTransport.java
+++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/transports/FileTransport.java
@@ -76,7 +76,7 @@ abstract class FileTransport implements BuildEventTransport {
     this.uploader = uploader;
     this.options = options;
     this.exitFunc = exitFunc;
-    this.writer = new SequentialWriter(path, this::serializeEvent, exitFunc);
+    this.writer = new SequentialWriter(path, this::serializeEvent, exitFunc, uploader);
   }
 
   @ThreadSafe
@@ -90,6 +90,7 @@ abstract class FileTransport implements BuildEventTransport {
     @VisibleForTesting OutputStream out;
     private final Function<BuildEventStreamProtos.BuildEvent, byte[]> serializeFunc;
     private final Consumer<AbruptExitException> exitFunc;
+    private final BuildEventArtifactUploader uploader;
 
     @VisibleForTesting
     final BlockingQueue<ListenableFuture<BuildEventStreamProtos.BuildEvent>> pendingWrites =
@@ -100,7 +101,8 @@ abstract class FileTransport implements BuildEventTransport {
     SequentialWriter(
         String path,
         Function<BuildEventStreamProtos.BuildEvent, byte[]> serializeFunc,
-        Consumer<AbruptExitException> exitFunc) {
+        Consumer<AbruptExitException> exitFunc,
+        BuildEventArtifactUploader uploader) {
       try {
         this.out = new BufferedOutputStream(new FileOutputStream(path));
       } catch (FileNotFoundException e) {
@@ -115,6 +117,7 @@ abstract class FileTransport implements BuildEventTransport {
       this.writerThread = new Thread(this);
       this.serializeFunc = serializeFunc;
       this.exitFunc = exitFunc;
+      this.uploader = uploader;
       writerThread.start();
     }
 
@@ -135,8 +138,12 @@ abstract class FileTransport implements BuildEventTransport {
         logger.log(Level.SEVERE, "Failed to write BEP events to file.", e);
       } finally {
         try {
-          out.flush();
-          out.close();
+          try {
+            out.flush();
+            out.close();
+          } finally {
+            uploader.shutdown();
+          }
         } catch (IOException e) {
           logger.log(Level.SEVERE, "Failed to close BEP file output stream.", e);
         }
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
index 3a726bf6ca..58a62480d9 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
@@ -17,10 +17,13 @@ package com.google.devtools.build.lib.remote;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableList;
 import com.google.common.util.concurrent.Futures;
+import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.ListeningScheduledExecutorService;
 import com.google.common.util.concurrent.MoreExecutors;
 import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions;
 import com.google.devtools.build.lib.authandtls.GoogleAuthUtils;
+import com.google.devtools.build.lib.buildeventstream.BuildEvent.LocalFile;
+import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploader;
 import com.google.devtools.build.lib.buildeventstream.PathConverter;
 import com.google.devtools.build.lib.buildtool.BuildRequest;
 import com.google.devtools.build.lib.events.Event;
@@ -43,6 +46,7 @@ import com.google.devtools.remoteexecution.v1test.Digest;
 import io.grpc.Channel;
 import io.grpc.ClientInterceptors;
 import java.io.IOException;
+import java.util.Map;
 import java.util.concurrent.Executors;
 import java.util.logging.Logger;
 
@@ -74,16 +78,10 @@ public final class RemoteModule extends BlazeModule {
         Digest digest = digestUtil.compute(path);
         return remoteInstanceName.isEmpty()
             ? String.format(
-                "bytestream://%s/blobs/%s/%d",
-                server,
-                digest.getHash(),
-                digest.getSizeBytes())
+                "bytestream://%s/blobs/%s/%d", server, digest.getHash(), digest.getSizeBytes())
             : String.format(
                 "bytestream://%s/%s/blobs/%s/%d",
-                server,
-                remoteInstanceName,
-                digest.getHash(),
-                digest.getSizeBytes());
+                server, remoteInstanceName, digest.getHash(), digest.getSizeBytes());
       } catch (IOException e) {
         // TODO(ulfjack): Don't fail silently!
         return fallbackConverter.apply(path);
@@ -98,11 +96,21 @@ public final class RemoteModule extends BlazeModule {
 
   @Override
   public void serverInit(OptionsProvider startupOptions, ServerBuilder builder) {
-    builder.addBuildEventArtifactUploader(
-        files -> {
-          // TODO(ulfjack): Actually hook up upload here.
-          return Futures.immediateFuture(converter);
-        },
+    builder.addBuildEventArtifactUploaderFactory(
+        () ->
+            new BuildEventArtifactUploader() {
+
+              @Override
+              public ListenableFuture<PathConverter> upload(Map<Path, LocalFile> files) {
+                // TODO(ulfjack): Actually hook up upload here.
+                return Futures.immediateFuture(converter);
+              }
+
+              @Override
+              public void shutdown() {
+                // Intentionally left empty.
+              }
+            },
         "remote");
   }
 
@@ -225,8 +233,10 @@ public final class RemoteModule extends BlazeModule {
           new RemoteActionContextProvider(env, cache, executor, digestUtil, logDir);
     } catch (IOException e) {
       env.getReporter().handle(Event.error(e.getMessage()));
-      env.getBlazeModuleEnvironment().exit(new AbruptExitException(
-          "Error initializing RemoteModule", ExitCode.COMMAND_LINE_ERROR));
+      env.getBlazeModuleEnvironment()
+          .exit(
+              new AbruptExitException(
+                  "Error initializing RemoteModule", ExitCode.COMMAND_LINE_ERROR));
     }
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
index 5a1708225d..a5336fa920 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
@@ -31,7 +31,7 @@ import com.google.devtools.build.lib.analysis.ServerDirectories;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
 import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory;
 import com.google.devtools.build.lib.analysis.test.CoverageReportActionFactory;
-import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploaderMap;
+import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploaderFactoryMap;
 import com.google.devtools.build.lib.clock.BlazeClock;
 import com.google.devtools.build.lib.clock.Clock;
 import com.google.devtools.build.lib.events.Event;
@@ -155,7 +155,7 @@ public final class BlazeRuntime {
   private final String defaultsPackageContent;
   private final SubscriberExceptionHandler eventBusExceptionHandler;
   private final String productName;
-  private final BuildEventArtifactUploaderMap buildEventArtifactUploaders;
+  private final BuildEventArtifactUploaderFactoryMap buildEventArtifactUploaderFactoryMap;
   private final ActionKeyContext actionKeyContext;
 
   // Workspace state (currently exactly one workspace per server)
@@ -180,7 +180,7 @@ public final class BlazeRuntime {
       InvocationPolicy moduleInvocationPolicy,
       Iterable<BlazeCommand> commands,
       String productName,
-      BuildEventArtifactUploaderMap buildEventArtifactUploaders) {
+      BuildEventArtifactUploaderFactoryMap buildEventArtifactUploaderFactoryMap) {
     // Server state
     this.fileSystem = fileSystem;
     this.blazeModules = blazeModules;
@@ -207,7 +207,7 @@ public final class BlazeRuntime {
     CommandNameCache.CommandNameCacheInstance.INSTANCE.setCommandNameCache(
         new CommandNameCacheImpl(getCommandMap()));
     this.productName = productName;
-    this.buildEventArtifactUploaders = buildEventArtifactUploaders;
+    this.buildEventArtifactUploaderFactoryMap = buildEventArtifactUploaderFactoryMap;
   }
 
   public BlazeWorkspace initWorkspace(BlazeDirectories directories, BinTools binTools)
@@ -1253,8 +1253,8 @@ public final class BlazeRuntime {
     return productName;
   }
 
-  public BuildEventArtifactUploaderMap getBuildEventArtifactUploaders() {
-    return buildEventArtifactUploaders;
+  public BuildEventArtifactUploaderFactoryMap getBuildEventArtifactUploaderFactoryMap() {
+    return buildEventArtifactUploaderFactoryMap;
   }
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/ServerBuilder.java b/src/main/java/com/google/devtools/build/lib/runtime/ServerBuilder.java
index 73bb27e67a..b88d487cdd 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/ServerBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/ServerBuilder.java
@@ -18,8 +18,8 @@ import com.google.common.base.Function;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
-import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploader;
-import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploaderMap;
+import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploaderFactory;
+import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploaderFactoryMap;
 import com.google.devtools.build.lib.packages.AttributeContainer;
 import com.google.devtools.build.lib.packages.PackageFactory;
 import com.google.devtools.build.lib.packages.RuleClass;
@@ -45,8 +45,8 @@ public final class ServerBuilder {
       ImmutableList.builder();
   private final ImmutableList.Builder<PackageFactory.EnvironmentExtension> environmentExtensions =
       ImmutableList.builder();
-  private final BuildEventArtifactUploaderMap.Builder buildEventArtifactUploaders =
-      new BuildEventArtifactUploaderMap.Builder();
+  private final BuildEventArtifactUploaderFactoryMap.Builder buildEventArtifactUploaderFactories =
+      new BuildEventArtifactUploaderFactoryMap.Builder();
 
   @VisibleForTesting
   public ServerBuilder() {}
@@ -87,8 +87,8 @@ public final class ServerBuilder {
     return commands.build();
   }
 
-  public BuildEventArtifactUploaderMap getBuildEventArtifactUploaderMap() {
-    return buildEventArtifactUploaders.build();
+  public BuildEventArtifactUploaderFactoryMap getBuildEventArtifactUploaderMap() {
+    return buildEventArtifactUploaderFactories.build();
   }
 
   /**
@@ -178,9 +178,9 @@ public final class ServerBuilder {
     return this;
   }
 
-  public ServerBuilder addBuildEventArtifactUploader(
-      BuildEventArtifactUploader uploader, String name) {
-    buildEventArtifactUploaders.add(name, uploader);
+  public ServerBuilder addBuildEventArtifactUploaderFactory(
+      BuildEventArtifactUploaderFactory uploaderFactory, String name) {
+    buildEventArtifactUploaderFactories.add(name, uploaderFactory);
     return this;
   }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/buildeventservice/BazelBuildEventServiceModuleTest.java b/src/test/java/com/google/devtools/build/lib/buildeventservice/BazelBuildEventServiceModuleTest.java
index 37ac8f6c95..cda7069033 100644
--- a/src/test/java/com/google/devtools/build/lib/buildeventservice/BazelBuildEventServiceModuleTest.java
+++ b/src/test/java/com/google/devtools/build/lib/buildeventservice/BazelBuildEventServiceModuleTest.java
@@ -26,8 +26,8 @@ import com.google.devtools.build.lib.actions.ActionExecutedEvent;
 import com.google.devtools.build.lib.actions.ActionExecutedEvent.ErrorTiming;
 import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
 import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions;
-import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploader;
-import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploaderMap;
+import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploaderFactory;
+import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploaderFactoryMap;
 import com.google.devtools.build.lib.buildeventstream.BuildEventProtocolOptions;
 import com.google.devtools.build.lib.buildeventstream.transports.BinaryFormatFileTransport;
 import com.google.devtools.build.lib.buildeventstream.transports.BuildEventStreamOptions;
@@ -138,8 +138,8 @@ public class BazelBuildEventServiceModuleTest {
         commandLineReporter,
         moduleEnvironment,
         clock,
-        new BuildEventArtifactUploaderMap.Builder()
-            .add("", BuildEventArtifactUploader.LOCAL_FILES_UPLOADER)
+        new BuildEventArtifactUploaderFactoryMap.Builder()
+            .add("", BuildEventArtifactUploaderFactory.LOCAL_FILES_UPLOADER_FACTORY)
             .build(),
         reporter,
         /* buildRequestId= */ "foo",
diff --git a/src/test/java/com/google/devtools/build/lib/buildeventstream/BuildEventArtifactUploaderFactoryMapTest.java b/src/test/java/com/google/devtools/build/lib/buildeventstream/BuildEventArtifactUploaderFactoryMapTest.java
new file mode 100644
index 0000000000..8bf867ae6a
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/buildeventstream/BuildEventArtifactUploaderFactoryMapTest.java
@@ -0,0 +1,75 @@
+// 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.buildeventstream;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.util.concurrent.Futures;
+import com.google.common.util.concurrent.ListenableFuture;
+import com.google.devtools.build.lib.buildeventstream.BuildEvent.LocalFile;
+import com.google.devtools.build.lib.vfs.Path;
+import java.util.Map;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** Tests for {@link BuildEventArtifactUploaderFactoryMap}. */
+@RunWith(JUnit4.class)
+public final class BuildEventArtifactUploaderFactoryMapTest {
+  private BuildEventArtifactUploaderFactoryMap uploaderFactories;
+  private BuildEventArtifactUploaderFactory noConversionUploaderFactory;
+
+  @Before
+  public void setUp() {
+    noConversionUploaderFactory =
+        () ->
+            new BuildEventArtifactUploader() {
+              @Override
+              public ListenableFuture<PathConverter> upload(Map<Path, LocalFile> files) {
+                return Futures.immediateFuture(PathConverter.NO_CONVERSION);
+              }
+
+              @Override
+              public void shutdown() {
+                // Intentionally left empty.
+              }
+            };
+    uploaderFactories =
+        new BuildEventArtifactUploaderFactoryMap.Builder()
+            .add("a", BuildEventArtifactUploaderFactory.LOCAL_FILES_UPLOADER_FACTORY)
+            .add("b", noConversionUploaderFactory)
+            .build();
+  }
+
+  @Test
+  public void testEmptyUploaders() throws Exception {
+    BuildEventArtifactUploaderFactoryMap emptyUploader =
+        new BuildEventArtifactUploaderFactoryMap.Builder().build();
+    assertThat(emptyUploader.select(null).create())
+        .isEqualTo(BuildEventArtifactUploader.LOCAL_FILES_UPLOADER);
+  }
+
+  @Test
+  public void testAlphabeticalOrder() {
+    assertThat(uploaderFactories.select(null).create())
+        .isEqualTo(BuildEventArtifactUploader.LOCAL_FILES_UPLOADER);
+  }
+
+  @Test
+  public void testSelectByName() throws Exception {
+    assertThat(uploaderFactories.select("b"))
+        .isEqualTo(noConversionUploaderFactory);
+  }
+}
diff --git a/src/test/java/com/google/devtools/build/lib/buildeventstream/BuildEventArtifactUploaderMapTest.java b/src/test/java/com/google/devtools/build/lib/buildeventstream/BuildEventArtifactUploaderMapTest.java
deleted file mode 100644
index 848fdf3a30..0000000000
--- a/src/test/java/com/google/devtools/build/lib/buildeventstream/BuildEventArtifactUploaderMapTest.java
+++ /dev/null
@@ -1,57 +0,0 @@
-// 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.buildeventstream;
-
-import static com.google.common.truth.Truth.assertThat;
-
-import com.google.common.util.concurrent.Futures;
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
-
-/** Tests for {@link BuildEventArtifactUploaderMap}. */
-@RunWith(JUnit4.class)
-public final class BuildEventArtifactUploaderMapTest {
-  private BuildEventArtifactUploaderMap uploader;
-  private BuildEventArtifactUploader noConversionUploader;
-
-  @Before
-  public void setUp() {
-    noConversionUploader = files -> Futures.immediateFuture(PathConverter.NO_CONVERSION);
-    uploader =
-        new BuildEventArtifactUploaderMap.Builder()
-            .add("a", BuildEventArtifactUploader.LOCAL_FILES_UPLOADER)
-            .add("b", noConversionUploader)
-            .build();
-  }
-
-  @Test
-  public void testEmptyUploaders() throws Exception {
-    BuildEventArtifactUploaderMap emptyUploader =
-        new BuildEventArtifactUploaderMap.Builder().build();
-    assertThat(emptyUploader.select(null))
-        .isEqualTo(BuildEventArtifactUploader.LOCAL_FILES_UPLOADER);
-  }
-
-  @Test
-  public void testAlphabeticalOrder() throws Exception {
-    assertThat(uploader.select(null)).isEqualTo(BuildEventArtifactUploader.LOCAL_FILES_UPLOADER);
-  }
-
-  @Test
-  public void testSelectByName() throws Exception {
-    assertThat(uploader.select("b")).isEqualTo(noConversionUploader);
-  }
-}
diff --git a/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/BinaryFormatFileTransportTest.java b/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/BinaryFormatFileTransportTest.java
index a20345de27..7bc077e8ac 100644
--- a/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/BinaryFormatFileTransportTest.java
+++ b/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/BinaryFormatFileTransportTest.java
@@ -16,6 +16,7 @@ package com.google.devtools.build.lib.buildeventstream.transports;
 
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploader.LOCAL_FILES_UPLOADER;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 import com.google.common.collect.ImmutableList;
@@ -24,6 +25,7 @@ import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.SettableFuture;
 import com.google.devtools.build.lib.buildeventstream.ArtifactGroupNamer;
 import com.google.devtools.build.lib.buildeventstream.BuildEvent;
+import com.google.devtools.build.lib.buildeventstream.BuildEvent.LocalFile;
 import com.google.devtools.build.lib.buildeventstream.BuildEvent.LocalFile.LocalFileType;
 import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploader;
 import com.google.devtools.build.lib.buildeventstream.BuildEventContext;
@@ -42,6 +44,7 @@ import java.io.FileInputStream;
 import java.io.InputStream;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.Map;
 import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.locks.LockSupport;
@@ -206,14 +209,20 @@ public class BinaryFormatFileTransportTest {
     BuildEvent event1 = new WithLocalFileEvent(file1);
     BuildEvent event2 = new WithLocalFileEvent(file2);
 
-    BuildEventArtifactUploader uploader =
-        files -> {
-          if (files.containsKey(file1)) {
-            LockSupport.parkNanos(TimeUnit.MILLISECONDS.toNanos(200));
-          }
-          return Futures.immediateFuture(new FileUriPathConverter());
-        };
-
+    BuildEventArtifactUploader uploader = Mockito.spy(new BuildEventArtifactUploader() {
+      @Override
+      public ListenableFuture<PathConverter> upload(Map<Path, LocalFile> files) {
+        if (files.containsKey(file1)) {
+          LockSupport.parkNanos(TimeUnit.MILLISECONDS.toNanos(200));
+        }
+        return Futures.immediateFuture(new FileUriPathConverter());
+      }
+
+      @Override
+      public void shutdown() {
+        // Intentionally left empty.
+      }
+    });
     File output = tmp.newFile();
     BinaryFormatFileTransport transport =
         new BinaryFormatFileTransport(output.getAbsolutePath(), defaultOpts, uploader, (e) -> {});
@@ -230,6 +239,8 @@ public class BinaryFormatFileTransportTest {
           .isEqualTo(event2.asStreamProto(null));
       assertThat(in.available()).isEqualTo(0);
     }
+
+    verify(uploader).shutdown();
   }
 
   @Test
@@ -241,7 +252,17 @@ public class BinaryFormatFileTransportTest {
     BuildEvent event = new WithLocalFileEvent(file1);
 
     SettableFuture<PathConverter> upload = SettableFuture.create();
-    BuildEventArtifactUploader uploader = files -> upload;
+    BuildEventArtifactUploader uploader = Mockito.spy(new BuildEventArtifactUploader() {
+      @Override
+      public ListenableFuture<PathConverter> upload(Map<Path, LocalFile> files) {
+        return upload;
+      }
+
+      @Override
+      public void shutdown() {
+        // Intentionally left empty.
+      }
+    });
 
     File output = tmp.newFile();
     BinaryFormatFileTransport transport =
@@ -258,6 +279,8 @@ public class BinaryFormatFileTransportTest {
           .isEqualTo(event.asStreamProto(null));
       assertThat(in.available()).isEqualTo(0);
     }
+
+    verify(uploader).shutdown();
   }
 
   private static class WithLocalFileEvent implements BuildEvent {
diff --git a/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/BuildEventTransportFactoryTest.java b/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/BuildEventTransportFactoryTest.java
index 5494684b3f..fe5f916206 100644
--- a/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/BuildEventTransportFactoryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/BuildEventTransportFactoryTest.java
@@ -22,7 +22,7 @@ import com.google.common.collect.FluentIterable;
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.buildeventstream.ArtifactGroupNamer;
 import com.google.devtools.build.lib.buildeventstream.BuildEvent;
-import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploaderMap;
+import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploaderFactoryMap;
 import com.google.devtools.build.lib.buildeventstream.BuildEventContext;
 import com.google.devtools.build.lib.buildeventstream.BuildEventProtocolOptions;
 import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos;
@@ -86,8 +86,8 @@ public class BuildEventTransportFactoryTest {
     Mockito.validateMockitoUsage();
   }
 
-  private BuildEventArtifactUploaderMap localFilesOnly() {
-    return new BuildEventArtifactUploaderMap.Builder().build();
+  private BuildEventArtifactUploaderFactoryMap localFilesOnly() {
+    return new BuildEventArtifactUploaderFactoryMap.Builder().build();
   }
 
   @Test