RFR: 7904105: intermittent test failure in CleanupDirTests.agentvm.conc

[Christian Stein]

On Tue, 28 Oct 2025 19:46:50 GMT, Vladimir Petko <vpetko at openjdk.org> wrote:

>> Hello Vladimir, we have seen intermittent failure in this test too, but i haven't had a chance to look deeper.
>> 
>> `TestStats` deals with counts in several different methods, so I'm not completely sure the proposed change is enough or the best way to address this. It may be that the code in `TestStats` itself might need to be addressed in some manner.
>
>> Hello Vladimir, we have seen intermittent failure in this test too, but i haven't had a chance to look deeper.
>> 
>> `TestStats` deals with counts in several different methods, so I'm not completely sure the proposed change is enough or the best way to address this. It may be that the code in `TestStats` itself might need to be addressed in some manner.
> 
> Hi, 
> 
> I have tried using atomics in TestStats and the implementation looked ugly / required a lot of refactoring. 
> 
> jtreg only updates counts using jtharness Observer callbacks. We already use `synchonzied` keyword in Observer callbacks[1] as the TestRunner spawns multiple threads if we specify concurrency > 1. I have used Intellij Idea to find all inheritors of BasicObserver and added synchronization. 
> 
> Maybe the better way would be to synchronize in jtharness itself to avoid  doing it in the client code? 
> Hmm, this is probably impossible unless we want to make a breaking change and remove `notifier` field from `Script` class. 
> 
> [1] https://github.com/openjdk/jtreg/blob/34ccc66ec89651637811678a17d3f810b4ac0780/src/share/classes/com/sun/javatest/regtest/report/ElapsedTimeHandler.java#L64

@vpa1977 do you want to issue an `/integrate` comment? I'd like to include this change in the upcoming 8.2 release of jtreg.

-------------

PR Comment: https://git.openjdk.org/jtreg/pull/297#issuecomment-3771843070