Skip to content

Commit 3bfe741

Browse files
committed
fix(batch): do not interrupt parent thread unnecessarily
1 parent 3eb5838 commit 3bfe741

2 files changed

Lines changed: 41 additions & 14 deletions

File tree

src/main/java/io/weaviate/client6/v1/api/collections/batch/BatchContext.java

Lines changed: 34 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -335,7 +335,21 @@ public TaskHandle retry(TaskHandle taskHandle) throws InterruptedException {
335335
@Override
336336
public void close() throws IOException {
337337
boolean closedBefore = closed;
338-
closed = true;
338+
339+
// Update the value atomically to make sure shutdownNow
340+
// does not unnecessarily interrupt this thread.
341+
synchronized (this) {
342+
closed = true;
343+
}
344+
345+
// If we'd been interrupted by shutdownNow, closing would've been
346+
// completed exceptionally prior to that. If that's not the case
347+
// but the current thread is interrupted, then we must propagate
348+
// the interrupt. But first, we should dispose of the services.
349+
if (Thread.interrupted() && !closing.isCompletedExceptionally()) {
350+
shutdownExecutors();
351+
Thread.currentThread().interrupt();
352+
}
339353

340354
log.atDebug()
341355
.addKeyValue("closed_before", closedBefore)
@@ -409,16 +423,26 @@ private void shutdownNow(Exception e) {
409423
send.cancel(true);
410424
}
411425

412-
if (!closed) {
413-
// Since shutdownNow is never triggered by the "main" thread,
414-
// it may be blocked on trying to add to the queue. While batch
415-
// context is active, we own this thread and may interrupt it.
416-
log.atDebug()
417-
.addKeyValue("thread", Thread::currentThread)
418-
.addKeyValue("closed", closed)
419-
.log("Interrupt parent thread");
420-
parent.interrupt();
426+
// Since shutdownNow is never triggered by the "main" thread,
427+
// it may be blocked on trying to add to the queue. While batch
428+
// context is active, we own this thread and may interrupt it.
429+
// We must be able to guarantee that shutdownNow never interrupts
430+
// an in-progress close and we also don't want to potentially block
431+
// the gRPC thread on which shutdownNow may be executing; we use
432+
// the doubly-checked locking pattern to helps us achieve that.
433+
if (closed) {
434+
return;
421435
}
436+
synchronized (this) {
437+
if (!closed) {
438+
log.atDebug()
439+
.addKeyValue("thread", Thread::currentThread)
440+
.addKeyValue("closed", closed)
441+
.log("Interrupt parent thread");
442+
parent.interrupt();
443+
}
444+
}
445+
422446
}
423447

424448
private void shutdownExecutors() {

src/test/java/io/weaviate/client6/v1/api/collections/batch/BatchContextTest.java

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -160,11 +160,14 @@ public void reset() throws Exception {
160160
// This resets the interrupted flag, allowing use to await the executors.
161161
Thread.interrupted();
162162

163-
backgroundThread.shutdownNow();
164-
backgroundThread.awaitTermination(10, TimeUnit.SECONDS);
163+
try {
164+
backgroundThread.shutdownNow();
165+
backgroundThread.awaitTermination(10, TimeUnit.SECONDS);
165166

166-
eventThread.shutdownNow();
167-
eventThread.awaitTermination(10, TimeUnit.SECONDS);
167+
eventThread.shutdownNow();
168+
eventThread.awaitTermination(10, TimeUnit.SECONDS);
169+
} catch (InterruptedException ignored) {
170+
}
168171

169172
REQUEST_QUEUE.clear();
170173

0 commit comments

Comments
 (0)