From c89272c8c2c74c199f0901fd579e51af995ab43b Mon Sep 17 00:00:00 2001
From: Roger Abelenda <roger.abelenda@abstracta.com.uy>
Date: Mon, 20 May 2024 14:17:32 -0300
Subject: [PATCH] Avoid interrupting threads when rpsThreadGroup ends execution

---
 .../core/threadgroups/RpsThreadGroup.java     | 45 ++++++++++++++++++-
 1 file changed, 43 insertions(+), 2 deletions(-)

diff --git a/jmeter-java-dsl/src/main/java/us/abstracta/jmeter/javadsl/core/threadgroups/RpsThreadGroup.java b/jmeter-java-dsl/src/main/java/us/abstracta/jmeter/javadsl/core/threadgroups/RpsThreadGroup.java
index 07f78198..e5fa4dba 100644
--- a/jmeter-java-dsl/src/main/java/us/abstracta/jmeter/javadsl/core/threadgroups/RpsThreadGroup.java
+++ b/jmeter-java-dsl/src/main/java/us/abstracta/jmeter/javadsl/core/threadgroups/RpsThreadGroup.java
@@ -1,6 +1,7 @@
 package us.abstracta.jmeter.javadsl.core.threadgroups;
 
 import com.blazemeter.jmeter.threads.AbstractDynamicThreadGroup;
+import com.blazemeter.jmeter.threads.DynamicThread;
 import com.blazemeter.jmeter.threads.concurrency.ConcurrencyThreadGroup;
 import com.blazemeter.jmeter.threads.concurrency.ConcurrencyThreadGroupGui;
 import java.time.Duration;
@@ -16,6 +17,7 @@
 import org.apache.jmeter.testelement.TestElement;
 import org.apache.jmeter.testelement.property.CollectionProperty;
 import org.apache.jmeter.threads.AbstractThreadGroup;
+import org.apache.jmeter.threads.JMeterContextService;
 import org.apache.jorphan.collections.HashTree;
 import us.abstracta.jmeter.javadsl.core.BuildTreeContext;
 import us.abstracta.jmeter.javadsl.core.util.JmeterFunction;
@@ -256,12 +258,28 @@ private TestElement buildTestAction() {
   }
 
   private TestElement buildTimer() {
-    VariableThroughputTimer ret = new VariableThroughputTimer();
+    VariableThroughputTimer ret = new NonInterruptingVariableThroughputTimer();
     ret.setData(buildTimerSchedulesData());
     configureTestElement(ret, buildTimerName(timerId++), VariableThroughputTimerGui.class);
     return ret;
   }
 
+  /**
+   * Always stops thread group gracefully, avoiding potential exceptions generated by
+   * {@link VariableThroughputTimer} when stopping a test, due to thread interruptions.
+   * <p>
+   * <a href="https://github.com/abstracta/jmeter-java-dsl/issues/257">Here</a> are more details on
+   * this issue.
+   */
+  public static class NonInterruptingVariableThroughputTimer extends VariableThroughputTimer {
+
+    @Override
+    protected void stopTest() {
+      JMeterContextService.getContext().getThreadGroup().stop();
+    }
+
+  }
+
   private String buildTimerName(int id) {
     return "rpsTimer" + id;
   }
@@ -276,7 +294,7 @@ private CollectionProperty buildTimerSchedulesData() {
 
   @Override
   protected AbstractThreadGroup buildThreadGroup() {
-    ConcurrencyThreadGroup ret = new ConcurrencyThreadGroup();
+    ConcurrencyThreadGroup ret = new ContractComplyingConcurrencyThreadGroup();
     ret.setTargetLevel(
         JmeterFunction.from("__tstFeedback", buildTimerName(timerId), initThreads, maxThreads,
             spareThreads));
@@ -285,6 +303,29 @@ protected AbstractThreadGroup buildThreadGroup() {
     return ret;
   }
 
+  /**
+   * This implementation of ConcurrencyThreadGroup complies with {@link AbstractThreadGroup}
+   * contract, and the original doesn't.
+   * <p>
+   * ConcurrencyThreadGroup stop should be graceful and is not, tellThreadsToStop should interrupt
+   * threads and is not.
+   */
+  public static class ContractComplyingConcurrencyThreadGroup extends ConcurrencyThreadGroup {
+
+    @Override
+    public void stop() {
+      running = false;
+      threadStarter.interrupt();
+      threads.forEach(DynamicThread::stop);
+    }
+
+    @Override
+    public void tellThreadsToStop() {
+      super.stop();
+    }
+
+  }
+
   @Override
   public LoadTimeLine buildLoadTimeline() {
     LoadTimeLine ret = new LoadTimeLine(name, counting.label + " per second");