GGraziadei commented on PR #8812:
URL: https://github.com/apache/storm/pull/8812#issuecomment-4826554064
Hi @reiabreu ,
I pulled your PR locally to verify the behavior. The fix is solid: I ran a
quick simulation and confirmed that without `WeakReference` the leak is
present, while your patch resolves it completely.
For reference, here is the quick unit test I used to reproduce and verify
the leak. Feel free to include it directly into the PR (I am not sure if you
have better alternatives to `forceGcAndAwaitCollection` method, I don't like
too much the `Thread.sleep ` which could introduce flakyness) , otherwise it is
LGTM for me.
```
@Test
public void testQueueIsCollectedAfterLongLivedProducerPublishes() throws
Exception {
Assertions.assertTimeoutPreemptively(Duration.ofSeconds(30), () -> {
ExecutorService producerPool = Executors.newSingleThreadExecutor();
try {
WeakReference<JCQueue> queueRef = publishFromPool(producerPool);
assertTrue(forceGcAndAwaitCollection(queueRef), "JCQueue was not
garbage collected");
} finally {
producerPool.shutdownNow();
}
});
}
private WeakReference<JCQueue> publishFromPool(ExecutorService pool) throws
Exception {
JCQueue queue = createQueue("leak", 100, 1024);
pool.submit(() -> {
for (long i = 0; i < 10; i++) {
queue.publish(i);
}
return null;
}).get();
return new WeakReference<>(queue);
}
private boolean forceGcAndAwaitCollection(WeakReference<JCQueue> ref) throws
InterruptedException {
for (int i = 0; i < 50 && ref.get() != null; i++) {
System.gc();
Thread.sleep(100);
}
return ref.get() == null;
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
