google-cloud-logging: LoggingImpl.pendingWrites remove before add (with suggested fix)

[spkrka]

In LoggingImpl, it is possible to leak memory in pendingWrites, if the future completes before the callback is added. If so, the callback runs before the call to pendingWrites.remove()

  @Test
  public void testMemoryLeak() {
    WriteLogEntriesRequest request =
            WriteLogEntriesRequest.newBuilder()
                    .addAllEntries(
                            Iterables.transform(
                                    ImmutableList.of(LOG_ENTRY1, LOG_ENTRY2), LogEntry.toPbFunction(PROJECT)))
                    .build();
    WriteLogEntriesResponse response = WriteLogEntriesResponse.newBuilder().build();
    EasyMock.expect(loggingRpcMock.write(request)).andStubReturn(ApiFutures.immediateFuture(response));
    EasyMock.replay(rpcFactoryMock, loggingRpcMock);
    logging = options.getService();
    LoggingImpl impl = (LoggingImpl) logging;
    assertEquals(0, impl.pendingWrites.size());
    for (int i = 0; i < 1000; i++) {
      logging.write(ImmutableList.of(LOG_ENTRY1, LOG_ENTRY2));
    }
    logging.flush();
    assertEquals(0, impl.pendingWrites.size());
  }

A proposed fix is to move this code to between creating the future and adding the callback.

        synchronized (writeLock) {
          pendingWrites.add(writeFuture);
        }

[spkrka]

PR: #4696

[ajaaym]

fixed with #4696