Endless wait by adding Synchronizing Timer

Here is one bug found in JMeter on Synchronizing Timer, they have adopted in their source code right now :) https://issues.apache.org/bugzilla/show_bug.cgi?id=48901

All the threads will perform endless wait by adding Synchronizing Timer.

This issue is much easier reproduced under a heavy load. I was using 100
threads to a simple HTTP request sampler with adding a Synchronizing Timer,
Groupsize set to 100 as well.

please see the attachment which is the stack trace of 100 threads doing a
endless wait.

Some initial invstigation for this issue:

Here is the current source code:

public long delay() {
synchronized (sync) {
timerCounter[0]++;
final int groupSz = getGroupSize();
final int count = timerCounter[0];
if (
(groupSz == 0 && count >=
JMeterContextService.getNumberOfThreads())
||
(groupSz > 0 && count >= groupSz)
) {
sync.notifyAll();
} else {
try {
sync.wait();
} catch (InterruptedException e) {
log.warn(e.getLocalizedMessage());
}
}
timerCounter[0]=0; // Reset for next time
}
return 0;
}

The main problem from my perspective is after one thread call sync.notifyAll();
,which means after the lock has been released,it will execute
timerCounter[0]=0;(correct me if i am wrong)

After chaning the code as following(move "timerCounter[0]=0;" in front of
"sync.notifyAll();", it does not impact any business impact, but make more
sense to me), and the issue disappear:
public long delay() {
synchronized (sync) {
timerCounter[0]++;
final int groupSz = getGroupSize();
final int count = timerCounter[0];
if (
(groupSz == 0 && count >=
JMeterContextService.getNumberOfThreads())
||
(groupSz > 0 && count >= groupSz)
) {
timerCounter[0]=0; // Reset for next time
sync.notifyAll();
} else {
try {
sync.wait();
} catch (InterruptedException e) {
log.warn(e.getLocalizedMessage());
}
}
}
return 0;
}