BulkWriter release unmanageable threads

[GregTreat]

I investigated thread leaks in my projects and found the issuer in Firestore.java API
Each usage of BulkWriter release at least one more unterminated thread.

We use driver version

com.google.cloud
spring-cloud-gcp-starter-data-firestore
3.2.1

I know that is legacy version, but still prefer to open the issue since did not found other reports about, as well as did not found release note dedicated to it.

[GregTreat]

Thread stack

Name: pool-1649-thread-1
State: WAITING on java.util.concurrent.locks.AbstractQueuedSynchronizer$ConditionObject@34ce65a4
Total blocked: 0 Total waited: 2

Stack trace:
java.base@17.0.12/jdk.internal.misc.Unsafe.park(Native Method)
java.base@17.0.12/java.util.concurrent.locks.LockSupport.park(LockSupport.java:341)
java.base@17.0.12/java.util.concurrent.locks.AbstractQueuedSynchronizer$ConditionNode.block(AbstractQueuedSynchronizer.java:506)
java.base@17.0.12/java.util.concurrent.ForkJoinPool.unmanagedBlock(ForkJoinPool.java:3465)
java.base@17.0.12/java.util.concurrent.ForkJoinPool.managedBlock(ForkJoinPool.java:3436)
java.base@17.0.12/java.util.concurrent.locks.AbstractQueuedSynchronizer$ConditionObject.await(AbstractQueuedSynchronizer.java:1623)
java.base@17.0.12/java.util.concurrent.ScheduledThreadPoolExecutor$DelayedWorkQueue.take(ScheduledThreadPoolExecutor.java:1170)
java.base@17.0.12/java.util.concurrent.ScheduledThreadPoolExecutor$DelayedWorkQueue.take(ScheduledThreadPoolExecutor.java:899)
java.base@17.0.12/java.util.concurrent.ThreadPoolExecutor.getTask(ThreadPoolExecutor.java:1062)
java.base@17.0.12/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1122)
java.base@17.0.12/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
java.base@17.0.12/java.lang.Thread.run(Thread.java:842)

Point on ScheduledThreadPoolExecutor issuer

[GregTreat]

My opinion, in FirestoreImpl.java exposed 2 APIs, zero configuration API become a source of the issue.

  @Nonnull
  public BulkWriter bulkWriter() {
    return new BulkWriter(this, BulkWriterOptions.builder().setThrottlingEnabled(true).build());
  }

  @Nonnull
  public BulkWriter bulkWriter(BulkWriterOptions options) {
    return new BulkWriter(this, options);
  }

Since constructor of BulkWriter do this

this.bulkWriterExecutor = options.getExecutor() != null ? options.getExecutor() : Executors.newSingleThreadScheduledExecutor();

My strong opinion, Executor in options can not be nullable. And must be managed as singleton in context

[zhumin8]

Thanks for reporting with us. We will triage and investigate as capacity allows.
In the meantime, if you have a support contract with Google, please create an issue in the support console instead of filing on GitHub. This will ensure a timely response.

Also, curious if this also happens with the newer 6.x versions? Can you provide a minimum reproducer code to demonstrate the problem you are encountering?

[GregTreat]

Pre requirements
java 17
spring-boot-starter-parent 2.6.6
spring-cloud-gcp-starter-data-firestore 3.2.1

Simple usage and easy to reproduce

BulkWriter bulkWriter = this.firestore.bulkWriter();
            int i = 1;
            for (Model models : model) {
                if (i % 300 == 0) {
                    bulkWriter.flush().get();
                    bulkWriter = this.firestore.bulkWriter();
                    i = 1;
                }
                bulkWriter.set(ref, model);
                ++i;
            }
            bulkWriter.flush().get();

And follow profiler, then you will see consistent threads increase on each usage.

About the version, I did not tried on latest. My hotfix was switch to BatchWriter instead.

[tom-andersen]

@GregTreat Thanks for the repro. I have created a fix, but you will need to handle the BulkWriter like any other AutoCloseable resource.

Instead of bulkWriter.flush().get(), call bulkWriter.close(). Not doing so will leak resources.

[tom-andersen]

Note, there is a workaround to provide your own executor as part of the BulkWriterOptions. This will allow you to manage the lifecycle of executor yourself.

[GregTreat]

@tom-andersen Thanks for pointed to possible solution. In my case BatchWriter was good enough, but I`m totally agree AutoCloseable was missed. I can suggest to use try with resource in FirestoreImpl.bulkWriter()
e.g.

`
@nonnull
public BulkWriter bulkWriter() {
try (BulkWriter writer = new BulkWriter(this, BulkWriterOptions.builder().setThrottlingEnabled(true).build())){
return writer;
}
}

`

[jinseopkim0]

googleapis/java-firestore#2202 was merged as a fix.