Restore atomicity of sequence number generation and addition to the e…

…vent queue in the BES uploader.

The BES backends expect the sequence number and the order in the queue to coincide and the atomicity was removed in 7c79576 so we're restoring it now with this change.

Fixes #8160

PiperOrigin-RevId: 245416462

src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceUploader.java

@@ -173,19 +173,28 @@ void enqueueEvent(BuildEvent event) {
       }
       return;
     }
-    // BuildCompletingEvent marks the end of the build in the BEP event stream.
-    if (event instanceof BuildCompletingEvent) {
-      synchronized (lock) {
+
+    // The generation of the sequence number and the addition to the {@link #eventQueue} should be
+    // atomic since BES expects the events in that exact order.
+    // More details can be found in b/131393380.
+    // TODO(bazel-team): Consider relaxing this invariant by having a more relaxed order.
+    synchronized (lock) {
+      // BuildCompletingEvent marks the end of the build in the BEP event stream.
+      if (event instanceof BuildCompletingEvent) {
         this.buildStatus = extractBuildStatus((BuildCompletingEvent) event);
       }
+      ensureUploadThreadStarted();
+
+      // TODO(b/131393380): {@link #nextSeqNum} doesn't need to be an AtomicInteger if it's
+      //  always used under lock. It would be cleaner and more performant to update the sequence
+      //  number when we take the item off the queue.
+      eventQueue.addLast(
+          new SendRegularBuildEventCommand(
+              event,
+              localFileUploadFuture,
+              nextSeqNum.getAndIncrement(),
+              Timestamps.fromMillis(clock.currentTimeMillis())));
     }
-    ensureUploadThreadStarted();
-    eventQueue.addLast(
-        new SendRegularBuildEventCommand(
-            event,
-            localFileUploadFuture,
-            nextSeqNum.getAndIncrement(),
-            Timestamps.fromMillis(clock.currentTimeMillis())));
   }
 
   /**
@@ -201,8 +210,18 @@ public ListenableFuture<Void> close() {
 
     ensureUploadThreadStarted();
 
-    // Enqueue the last event which will terminate the upload.
-    eventQueue.addLast(new SendLastBuildEventCommand(nextSeqNum.getAndIncrement(), currentTime()));
+    // The generation of the sequence number and the addition to the {@link #eventQueue} should be
+    // atomic since BES expects the events in that exact order.
+    // More details can be found in b/131393380.
+    // TODO(bazel-team): Consider relaxing this invariant by having a more relaxed order.
+    synchronized (lock) {
+      // Enqueue the last event which will terminate the upload.
+      // TODO(b/131393380): {@link #nextSeqNum} doesn't need to be an AtomicInteger if it's
+      //  always used under lock. It would be cleaner and more performant to update the sequence
+      //  number when we take the item off the queue.
+      eventQueue.addLast(
+          new SendLastBuildEventCommand(nextSeqNum.getAndIncrement(), currentTime()));
+    }
 
     final SettableFuture<Void> finalCloseFuture = closeFuture;
     closeFuture.addListener(