Skip to content

Commit c9ca62f

Browse files
reiabreuclaude
andcommitted
utils: use WeakReference in BatchInserter to fix ThreadLocal retention of JCQueue
BatchInserter held a strong reference to its owning JCQueue, and the inserters live in instance-field ThreadLocals on the same JCQueue. This formed a cycle through ThreadLocalMap: value (BatchInserter) -> queue (JCQueue) -> thdLocalBatcher (ThreadLocal) = key Because the key was strongly reachable via the value, the weak-key expunge path in ThreadLocalMap never triggered, and the JCQueue (along with its metrics, recv/overflow queues and batch buffers) could not be GC'd for as long as any producer thread that ever published to it stayed alive. The fix stores the JCQueue as a WeakReference inside BatchInserter, cutting the value->key path. When the last external strong ref to the JCQueue is dropped, the ThreadLocal field it owns becomes weakly reachable, the ThreadLocalMap key can be expunged, and both the BatchInserter and the JCQueue are released. flush() and tryFlush() dereference the WeakReference once at entry and bail out cleanly if the queue has already been collected (dead topology in LocalCluster/embedded scenarios). publish() and tryPublish() are unchanged — they only manipulate currentBatch. Fixes #8810 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 39196b1 commit c9ca62f

1 file changed

Lines changed: 18 additions & 2 deletions

File tree

storm-client/src/jvm/org/apache/storm/utils/JCQueue.java

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
package org.apache.storm.utils;
2020

2121
import java.io.Closeable;
22+
import java.lang.ref.WeakReference;
2223
import java.util.ArrayList;
2324
import java.util.List;
2425
import org.apache.storm.metrics2.StormMetricRegistry;
@@ -325,11 +326,16 @@ public boolean tryFlush() {
325326
/* Not thread safe. Have one instance per producer thread or synchronize externally */
326327
private static class BatchInserter implements Inserter {
327328
private final int batchSz;
328-
private JCQueue queue;
329+
// WeakReference breaks the ThreadLocal retention cycle: thdLocalBatcher is an instance field
330+
// of JCQueue, so the ThreadLocalMap key (the ThreadLocal object) is kept strongly reachable
331+
// via value(BatchInserter) -> queue(JCQueue) -> field. A WeakReference here cuts that path,
332+
// allowing the key to become weakly-reachable and the entry to be expunged once the JCQueue
333+
// is no longer externally referenced.
334+
private final WeakReference<JCQueue> queueRef;
329335
private ArrayList<Object> currentBatch;
330336

331337
BatchInserter(JCQueue queue, int batchSz) {
332-
this.queue = queue;
338+
this.queueRef = new WeakReference<>(queue);
333339
this.batchSz = batchSz;
334340
this.currentBatch = new ArrayList<>(batchSz + 1);
335341
}
@@ -368,6 +374,11 @@ public void flush() throws InterruptedException {
368374
if (currentBatch.isEmpty()) {
369375
return;
370376
}
377+
JCQueue queue = queueRef.get();
378+
if (queue == null) {
379+
currentBatch.clear();
380+
return;
381+
}
371382
int publishCount = queue.tryPublishInternal(currentBatch);
372383
int retryCount = 0;
373384
while (publishCount == 0) { // retry till at least 1 element is drained
@@ -396,6 +407,11 @@ public boolean tryFlush() {
396407
if (currentBatch.isEmpty()) {
397408
return true;
398409
}
410+
JCQueue queue = queueRef.get();
411+
if (queue == null) {
412+
currentBatch.clear();
413+
return true;
414+
}
399415
int publishCount = queue.tryPublishInternal(currentBatch);
400416
if (publishCount == 0) {
401417
for (JCQueueMetrics jcQueueMetric : queue.jcqMetrics) {

0 commit comments

Comments
 (0)