[patch] Possible race condition when placing samplers

0001-Update-JMeterTreeModel-in-a-threadsafe-and-timely-ma.patch (text/plain), 12.00 KB, created by Felix Schumacher on 2020-05-03 11:34:53 UTC

(hide)

Description: Possible race condition when placing samplers

Filename:

MIME Type:

Creator: Felix Schumacher

Created: 2020-05-03 11:34:53 UTC

Size: 12.00 KB

patch

obsolete

>From 80038c39d3dc4c5ef0fdf875318bda71113d5b59 Mon Sep 17 00:00:00 2001 >From: Felix Schumacher <felix.schumacher@internetallee.de> >Date: Sun, 3 May 2020 10:56:59 +0200 >Subject: [PATCH] Update JMeterTreeModel in a threadsafe and timely manner > >When recording multiple samples concurrently, we could get into a race >condition and place samples into wrong controllers. > >Now we try to get the current information about prefix and sampler as soon as possible >and delay the addition of the sampler to the recording controller by means of a queue. >--- > .../protocol/http/proxy/ProxyControl.java | 190 +++++++++++------- > 1 file changed, 119 insertions(+), 71 deletions(-) > >diff --git a/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/proxy/ProxyControl.java b/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/proxy/ProxyControl.java >index 99df08e80c..4780f3cf63 100644 >--- a/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/proxy/ProxyControl.java >+++ b/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/proxy/ProxyControl.java >@@ -17,11 +17,13 @@ > > package org.apache.jmeter.protocol.http.proxy; > >+import java.awt.event.ActionEvent; > import java.io.BufferedInputStream; > import java.io.File; > import java.io.FileInputStream; > import java.io.IOException; > import java.io.InputStream; >+import java.io.Serializable; > import java.net.MalformedURLException; > import java.nio.charset.StandardCharsets; > import java.security.GeneralSecurityException; >@@ -30,6 +32,7 @@ import java.security.UnrecoverableKeyException; > import java.security.cert.CertificateExpiredException; > import java.security.cert.CertificateNotYetValidException; > import java.security.cert.X509Certificate; >+import java.util.ArrayDeque; > import java.util.ArrayList; > import java.util.Arrays; > import java.util.Collection; >@@ -278,11 +281,17 @@ public class ProxyControl extends GenericController implements NonTestElement { > > private JMeterTreeModel nonGuiTreeModel; > >+ private ArrayDeque<SamplerInfo> sampleQueue = new ArrayDeque<>(); >+ >+ // accessed from Swing-Thread, only >+ private String oldPrefix = null; >+ > public ProxyControl() { > setPort(DEFAULT_PORT); > setExcludeList(new HashSet<>()); > setIncludeList(new HashSet<>()); > setCaptureHttpHeaders(true); // maintain original behaviour >+ new javax.swing.Timer(200, this::putSamplesIntoModel).start(); > } > > /** >@@ -608,7 +617,7 @@ public class ProxyControl extends GenericController implements NonTestElement { > if (authorization != null) { > setAuthorization(authorization, myTarget); > } >- placeSampler(sampler, testElements, myTarget); >+ sampleQueue.add(new SamplerInfo(sampler, testElements, myTarget, getPrefixHTTPSampleName(), groupingMode)); > } else { > if (log.isDebugEnabled()) { > log.debug( >@@ -1124,86 +1133,103 @@ public class ProxyControl extends GenericController implements NonTestElement { > return elements; > } > >- private void placeSampler( >- HTTPSamplerBase sampler, TestElement[] testElements, JMeterTreeNode myTarget) { >- try { >- final JMeterTreeModel treeModel = getJmeterTreeModel(); >- >- boolean firstInBatch = false; >- long now = System.currentTimeMillis(); >- long deltaT = now - lastTime; >- int cachedGroupingMode = groupingMode; >- if (deltaT > sampleGap) { >- String controllerName = StringUtils.isNotEmpty(getPrefixHTTPSampleName()) ? >- getPrefixHTTPSampleName() : sampler.getName(); >- if (!myTarget.isLeaf() && cachedGroupingMode == GROUPING_ADD_SEPARATORS) { >- addDivider(treeModel, myTarget); >- } >- if (cachedGroupingMode == GROUPING_IN_SIMPLE_CONTROLLERS) { >- addSimpleController(treeModel, myTarget, controllerName); >- } >- if (cachedGroupingMode == GROUPING_IN_TRANSACTION_CONTROLLERS) { >- addTransactionController(treeModel, myTarget, controllerName); >- } >- firstInBatch = true;// Remember this was first in its batch >- } >- if (lastTime == 0) { >- deltaT = 0; // Decent value for timers >- } >- lastTime = now; >- >- if (cachedGroupingMode == GROUPING_STORE_FIRST_ONLY) { >- if (!firstInBatch) { >- return; // Huh! don't store this one! >- } >+ private void putSamplesIntoModel(ActionEvent e) { >+ final JMeterTreeModel treeModel = getJmeterTreeModel(); >+ while (!sampleQueue.isEmpty()) { >+ SamplerInfo info = sampleQueue.poll(); >+ try { >+ log.info("Add sample {} into controller {}", info.sampler.getName(), info.prefix); >+ try { >+ long now = info.recordedAt; >+ long deltaT = now - lastTime; >+ boolean firstInBatch = prepareTree( treeModel, deltaT, info); >+ if (lastTime == 0) { >+ deltaT = 0; // Decent value for timers >+ } >+ lastTime = now; > >- // If we're not storing subsequent samplers, we'll need the >- // first sampler to do all the work...: >- sampler.setFollowRedirects(true); >- sampler.setImageParser(true); >- } >+ if (info.groupingMode == GROUPING_STORE_FIRST_ONLY) { >+ if (!firstInBatch) { >+ return; // Huh! don't store this one! >+ } > >- if (cachedGroupingMode == GROUPING_IN_SIMPLE_CONTROLLERS || >- cachedGroupingMode == GROUPING_IN_TRANSACTION_CONTROLLERS) { >- // Find the last controller in the target to store the >- // sampler there: >- for (int i = myTarget.getChildCount() - 1; i >= 0; i--) { >- JMeterTreeNode c = (JMeterTreeNode) myTarget.getChildAt(i); >- if (c.getTestElement() instanceof GenericController) { >- myTarget = c; >- break; >+ // If we're not storing subsequent samplers, we'll need the >+ // first sampler to do all the work...: >+ info.sampler.setFollowRedirects(true); >+ info.sampler.setImageParser(true); > } >- } >- } >- final long deltaTFinal = deltaT; >- final boolean firstInBatchFinal = firstInBatch; >- final JMeterTreeNode myTargetFinal = myTarget; >- JMeterUtils.runSafe(true, () -> { >- try { >- final JMeterTreeNode newNode = treeModel.addComponent(sampler, myTargetFinal); >- if (firstInBatchFinal) { >+ >+ final JMeterTreeNode targetNode = getTargetNode(info.target, info.groupingMode); >+ final JMeterTreeNode newNode = treeModel.addComponent(info.sampler, targetNode); >+ if (firstInBatch) { > if (addAssertions) { > addAssertion(treeModel, newNode); > } >- addTimers(treeModel, newNode, deltaTFinal); >+ addTimers(treeModel, newNode, deltaT); > } >+ addTestElements(treeModel, info.testElements, newNode); >+ } catch (IllegalUserActionException ex) { >+ log.error("Error placing sampler", ex); >+ JMeterUtils.reportErrorToUser(ex.getMessage()); >+ } >+ } catch (Exception ex) { >+ log.error("Error placing sampler", ex); >+ JMeterUtils.reportErrorToUser(ex.getMessage()); >+ } >+ } >+ } > >- if (testElements != null) { >- for (TestElement testElement : testElements) { >- if (isAddableTestElement(testElement)) { >- treeModel.addComponent(testElement, newNode); >- } >- } >- } >- } catch (IllegalUserActionException e) { >- log.error("Error placing sampler", e); >- JMeterUtils.reportErrorToUser(e.getMessage()); >+ private void addTestElements(final JMeterTreeModel treeModel, TestElement[] testElements, >+ final JMeterTreeNode newNode) throws IllegalUserActionException { >+ if (testElements == null) { >+ return; >+ } >+ for (TestElement testElement : testElements) { >+ if (isAddableTestElement(testElement)) { >+ treeModel.addComponent(testElement, newNode); >+ } >+ } >+ } >+ >+ private boolean prepareTree(final JMeterTreeModel treeModel, >+ long deltaT, SamplerInfo info) { >+ HTTPSamplerBase sampler = info.sampler; >+ JMeterTreeNode myTarget = info.target; >+ int cachedGroupingMode = info.groupingMode; >+ boolean prefixChanged = false; >+ if (oldPrefix == null || !oldPrefix.equals(info.prefix)) { >+ oldPrefix = info.prefix; >+ prefixChanged = true; >+ } >+ if (deltaT > sampleGap || prefixChanged) { >+ String controllerName = StringUtils.defaultString(getPrefixHTTPSampleName(), sampler.getName()); >+ if (!myTarget.isLeaf() && cachedGroupingMode == GROUPING_ADD_SEPARATORS) { >+ addDivider(treeModel, myTarget); >+ } >+ if (cachedGroupingMode == GROUPING_IN_SIMPLE_CONTROLLERS) { >+ addSimpleController(treeModel, myTarget, controllerName); >+ } >+ if (cachedGroupingMode == GROUPING_IN_TRANSACTION_CONTROLLERS) { >+ addTransactionController(treeModel, myTarget, controllerName); >+ } >+ return true;// Remember this was first in its batch >+ } >+ return false; >+ } >+ >+ private JMeterTreeNode getTargetNode(JMeterTreeNode origTarget, int cachedGroupingMode) { >+ if (cachedGroupingMode == GROUPING_IN_SIMPLE_CONTROLLERS || >+ cachedGroupingMode == GROUPING_IN_TRANSACTION_CONTROLLERS) { >+ // Find the last controller in the target to store the >+ // sampler there: >+ for (int i = origTarget.getChildCount() - 1; i >= 0; i--) { >+ JMeterTreeNode currentNode = (JMeterTreeNode) origTarget.getChildAt(i); >+ if (currentNode.getTestElement() instanceof GenericController) { >+ return currentNode; > } >- }); >- } catch (Exception e) { >- log.error("Error placing sampler", e); >- JMeterUtils.reportErrorToUser(e.getMessage()); >+ } > } >+ return origTarget; > } > > /** >@@ -1609,4 +1635,26 @@ public class ProxyControl extends GenericController implements NonTestElement { > return KEYSTORE_MODE == KeystoreMode.DYNAMIC_KEYSTORE; > } > >+ /** >+ * Holds information about a sampler at the time of recording by the HTTP proxy >+ */ >+ private static class SamplerInfo implements Serializable { >+ private static final long serialVersionUID = 1L; >+ private HTTPSamplerBase sampler; >+ private transient TestElement[] testElements; >+ private JMeterTreeNode target; >+ private String prefix; >+ private int groupingMode; >+ private long recordedAt; >+ >+ public SamplerInfo(HTTPSamplerBase sampler, TestElement[] testElements, JMeterTreeNode target, String prefix, >+ int groupingMode) { >+ this.sampler = sampler; >+ this.testElements = testElements; >+ this.target = target; >+ this.prefix = prefix; >+ this.groupingMode = groupingMode; >+ this.recordedAt = System.currentTimeMillis(); >+ } >+ } > } >-- >2.25.1 > View Attachment As Diff View Attachment As Raw

This is ASF Bugzilla: the Apache Software Foundation bug system. In case of problems with the functioning of ASF Bugzilla, please contact bugzilla-admin@apache.org. Please Note: this e-mail address is only for reporting problems with ASF Bugzilla. Mail about any other subject will be silently ignored.

• • Home
  • | New
  • | Browse
  • | Search
  • |
    [?]
  • | Reports
  • | Help
  • | New Account
  • | Log In
    Remember [x]
  • | Forgot Password
    Login: [x]