On Mon, 20 May 2024 14:49:13 GMT, Alan Bateman <[email protected]> wrote:
>> 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 with 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.
@AlanBateman Great suggestions, Alan. I've updated the PR to reflect your
improvements.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/19271#issuecomment-2120665477
