On Fri, 17 May 2024 13:19:19 GMT, Viktor Klang <vkl...@openjdk.org> wrote:
>> Local testing seems to indicate that this fix (which mirrors what's done in
>> the FIFO mode) addresses the problem. Regression test added for JDK20+
>
> Viktor Klang has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Moving the memory leak test for SynchronousQueue into its own test and runs
> only for JDK20+, using VirtualThreads
Looks okay, I just wonder how reliable assertDoesntLeak will concurrent agents,
debug builds, and VM options such as a Xcomp that will challenge the
await(10s). Minimally the timed awaits should be untimed. Another idea is try
an ES like this:
try (var executor = Executors.newVirtualThreadPerTaskExecutor()) {
for (int i = 0; i < NUMBER_OF_ITEMS; i++) {
executor.submit( .. producer .. );
executor.submit(.. consumer ..);
}
}
No need for the count down latches or the need to retry on
InterruptedException. The close method waits for the 400 virtual threads to
finish so you can assert that the queue is empty.
The loop that waits for survivors to empty can be unbounded too. If there is a
leak then the test will timeout.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/19271#issuecomment-2120616967
