[zennox1337]

When a test is stopped while some Arrivals Thread Group threads sit idle in the pool, verifyThreadsStopped() keeps returning false, JMeter logs that threads have not stopped, and with the default jmeterengine.stopfail.system.exit=true the process exits with code 1 even though the run was clean. We hit it with Free-Form Arrivals Thread Group stopped by a Flow Control Action "Stop Now" in non-GUI mode.

There are two causes in ArrivalsThreadGroup:

1. tellThreadsToStop() calls stopThread(name, true) for pool threads, but stopThread() only searches threads, so pool threads are never woken and stay in wait().
2. A pool thread woken by an interrupt (from stop() or testEnded()) returns from movedToPool() without removing itself from poolThreads. Only releasedPoolThread() removes entries, so verifyThreadsStopped() keeps seeing threads that have already finished.

Changes:

• tellThreadsToStop() stops pool threads directly and wakes them with notify() under the thread's monitor. No OS-level interrupt on the graceful path, so a thread being released from the pool at that moment does not get its in-flight sample aborted. stop() still interrupts.
• movedToPool() waits in a loop guarded by running, isStopping() and pool membership, which covers lost wakeups (release or stop before the thread reaches wait()) and spurious ones. The thread removes itself from poolThreads after the loop; after releasedPoolThread() the entry is already gone and this does nothing.
• verifyThreadsStopped() joins pool threads the same way the parent joins active ones.

The existing tests call verifyThreadsStopped() without checking its result, which is why this went unnoticed. Two new tests in ArrivalsThreadGroupTest stop a group while a thread is idle in the pool, one through tellThreadsToStop() and one through stop(), and assert verifyThreadsStopped(). Both fail without the fix. The full casutg suite passes (86 tests, JDK 11 as in pr.yml), and the new tests passed 10 runs in a row.

[undera]

Thanks for the thorough analysis. The diagnosis is right on both counts: stopThread() never reaches pool threads, and a thread woken by stop/interrupt never leaves poolThreads. The tests close exactly the gap that let this slip through.

The one thing I'd like changed is how idle threads are woken on the graceful path: the OS-level interrupt there can leak into a thread that is concurrently being released from the pool. Details inline. The cleaner way is a guarded wait in movedToPool() plus notify() on shutdown, which makes the interrupt unnecessary outside of stop().

The verifyThreadsStopped() join looks good as is. Your two new tests should keep passing unchanged with the suggested rework.

[undera]

tellThreadsToStop() is the graceful stop: in-flight samples are supposed to finish. Interrupting the OS thread here races with ArrivalsThreadStarter, which may be inside releasedPoolThread() at that moment (running = false doesn't stop a release already underway). The thread moves from poolThreads to threads, starts sampling, then receives this interrupt, which aborts the in-flight request.

Pool threads are parked on their own monitor, so a stop flag + notify() is enough to wake them, the same pattern releaseAllPoolThreads() uses. Combined with the guarded wait suggested below, this also covers a thread that hasn't reached wait() yet:

Suggested change

	thread.stop();
	thread.interruptOSThread();
	thread.stop();
	synchronized (thread) {
	thread.notify();
	}

The OS-level interrupt can stay in stop(), where aborting in-flight work is the intent.

[zennox1337]

Done in fa58827: stop() plus notify() under the monitor, the OS-level interrupt is left only in stop().

[undera]

This cleanup is correct, but it works around the real defect a few lines above: thread.wait() runs once with no condition and no loop, so it's exposed to both lost wakeups (notify arrives before the thread reaches wait()) and spurious wakeups (the thread returns while still in poolThreads, which is what this line patches over).

Could you make it a guarded wait?

synchronized (thread) {
    try {
        while (running && !thread.isStopping() && poolThreads.contains(thread)) {
            thread.wait();
        }
    } catch (InterruptedException e) {
        log.debug("Interrupted", e);
    }
}
poolThreads.remove(thread);

• releasedPoolThread() removes the thread from poolThreads before notifying, so a normal release exits the loop.
• A graceful stop sets the flags and then notifies under the same monitor, so the waiter sees them (the monitor gives the needed happens-before), even if it hadn't started waiting yet.
• stop() still breaks out via InterruptedException.

[zennox1337]

Done in fa58827: guarded wait exactly as suggested, with poolThreads.remove(thread) after the loop.

[zennox1337]

Thanks, reworked as suggested in fa58827: the graceful stop now sets the stop flag and notifies under the thread's monitor, movedToPool() uses the guarded wait, and the OS-level interrupt stays only in stop().

Since running is volatile and cleared first in super.tellThreadsToStop(), the guard also covers a thread caught between leaving threads and entering poolThreads while the stop loops run.

The two new tests pass unchanged (86/86 on JDK 11, the new ones 10 runs in a row). Dropping the notify() from tellThreadsToStop() makes testStopWithIdlePoolThread fail, so the tests cover the new path. I've also updated the PR description to match.

[codecov]

Codecov Report

❌ Patch coverage is 71.42857% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.06%. Comparing base (09d23ae) to head (fa58827).
⚠️ Report is 149 commits behind head on master.

Files with missing lines	Patch %	Lines
...r/jmeter/threads/arrivals/ArrivalsThreadGroup.java	71.42%	2 Missing and 2 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master     #900      +/-   ##
============================================
+ Coverage     68.90%   69.06%   +0.16%     
- Complexity     2629     2705      +76     
============================================
  Files           230      233       +3     
  Lines         15965    16411     +446     
  Branches       1638     1713      +75     
============================================
+ Hits          11000    11335     +335     
- Misses         4146     4205      +59     
- Partials        819      871      +52     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.