On 20 November 2011 10:04, <[email protected]> wrote:
> Author: pmouawad
> Date: Sun Nov 20 10:04:02 2011
> New Revision: 1204144
>
> URL: http://svn.apache.org/viewvc?rev=1204144&view=rev
> Log:
> Bug 52215 - Confusing synchronization in StatVisualizer, SummaryReport
> ,Summariser and issue in StatGraphVisualizer
>
> Modified:
>
> jmeter/trunk/src/components/org/apache/jmeter/visualizers/StatGraphVisualizer.java
>
> jmeter/trunk/src/components/org/apache/jmeter/visualizers/StatVisualizer.java
>
> jmeter/trunk/src/components/org/apache/jmeter/visualizers/SummaryReport.java
> jmeter/trunk/src/core/org/apache/jmeter/reporters/Summariser.java
> jmeter/trunk/xdocs/changes.xml
>
> Modified:
> jmeter/trunk/src/components/org/apache/jmeter/visualizers/StatGraphVisualizer.java
> URL:
> http://svn.apache.org/viewvc/jmeter/trunk/src/components/org/apache/jmeter/visualizers/StatGraphVisualizer.java?rev=1204144&r1=1204143&r2=1204144&view=diff
> ==============================================================================
> ---
> jmeter/trunk/src/components/org/apache/jmeter/visualizers/StatGraphVisualizer.java
> (original)
> +++
> jmeter/trunk/src/components/org/apache/jmeter/visualizers/StatGraphVisualizer.java
> Sun Nov 20 10:04:02 2011
> @@ -104,6 +104,11 @@ public class StatGraphVisualizer extends
>
> private transient ObjectTableModel model;
>
> + /**
> + * Lock used to protect tableRows update + model update
> + */
> + private transient Object lock = new Object();
> +
Lock objects should be final to prevent accidental change - and to
document the fact.
Mutable locks don't work (in general).
> private final Map<String, SamplingStatCalculator> tableRows =
> new ConcurrentHashMap<String, SamplingStatCalculator>();
>
> @@ -203,7 +208,7 @@ public class StatGraphVisualizer extends
> public void add(SampleResult res) {
> SamplingStatCalculator row = null;
> final String sampleLabel = res.getSampleLabel();
> - synchronized (tableRows) {
> + synchronized (lock) {
> row = tableRows.get(sampleLabel);
> if (row == null) {
> row = new SamplingStatCalculator(sampleLabel);
> @@ -220,10 +225,12 @@ public class StatGraphVisualizer extends
> * Clears this visualizer and its model, and forces a repaint of the
> table.
> */
> public void clearData() {
> - model.clearData();
> - tableRows.clear();
> - tableRows.put(TOTAL_ROW_LABEL, new
> SamplingStatCalculator(TOTAL_ROW_LABEL));
> - model.addRow(tableRows.get(TOTAL_ROW_LABEL));
> + synchronized (lock) {
> + model.clearData();
> + tableRows.clear();
> + tableRows.put(TOTAL_ROW_LABEL, new
> SamplingStatCalculator(TOTAL_ROW_LABEL));
> + model.addRow(tableRows.get(TOTAL_ROW_LABEL));
> + }
> }
>
> /**
>
> Modified:
> jmeter/trunk/src/components/org/apache/jmeter/visualizers/StatVisualizer.java
> URL:
> http://svn.apache.org/viewvc/jmeter/trunk/src/components/org/apache/jmeter/visualizers/StatVisualizer.java?rev=1204144&r1=1204143&r2=1204144&view=diff
> ==============================================================================
> ---
> jmeter/trunk/src/components/org/apache/jmeter/visualizers/StatVisualizer.java
> (original)
> +++
> jmeter/trunk/src/components/org/apache/jmeter/visualizers/StatVisualizer.java
> Sun Nov 20 10:04:02 2011
> @@ -105,6 +105,11 @@ public class StatVisualizer extends Abst
>
> private transient ObjectTableModel model;
>
> + /**
> + * Lock used to protect tableRows update + model update
> + */
> + private transient Object lock = new Object();
> +
> private final Map<String, SamplingStatCalculator> tableRows =
> new ConcurrentHashMap<String, SamplingStatCalculator>();
>
> @@ -161,7 +166,7 @@ public class StatVisualizer extends Abst
> public void add(SampleResult res) {
> SamplingStatCalculator row = null;
> final String sampleLabel =
> res.getSampleLabel(useGroupName.isSelected());
> - synchronized (tableRows) {
> + synchronized (lock) {
> row = tableRows.get(sampleLabel);
> if (row == null) {
> row = new SamplingStatCalculator(sampleLabel);
> @@ -186,7 +191,7 @@ public class StatVisualizer extends Abst
> * Clears this visualizer and its model, and forces a repaint of the
> table.
> */
> public void clearData() {
> - synchronized (tableRows) {
> + synchronized (lock) {
> model.clearData();
> tableRows.clear();
> tableRows.put(TOTAL_ROW_LABEL, new
> SamplingStatCalculator(TOTAL_ROW_LABEL));
>
> Modified:
> jmeter/trunk/src/components/org/apache/jmeter/visualizers/SummaryReport.java
> URL:
> http://svn.apache.org/viewvc/jmeter/trunk/src/components/org/apache/jmeter/visualizers/SummaryReport.java?rev=1204144&r1=1204143&r2=1204144&view=diff
> ==============================================================================
> ---
> jmeter/trunk/src/components/org/apache/jmeter/visualizers/SummaryReport.java
> (original)
> +++
> jmeter/trunk/src/components/org/apache/jmeter/visualizers/SummaryReport.java
> Sun Nov 20 10:04:02 2011
> @@ -102,6 +102,11 @@ public class SummaryReport extends Abstr
>
> private transient ObjectTableModel model;
>
> + /**
> + * Lock used to protect tableRows update + model update
> + */
> + private transient Object lock = new Object();
> +
> private final Map<String, Calculator> tableRows =
> new ConcurrentHashMap<String, Calculator>();
>
> @@ -157,7 +162,7 @@ public class SummaryReport extends Abstr
> public void add(SampleResult res) {
> Calculator row = null;
> final String sampleLabel =
> res.getSampleLabel(useGroupName.isSelected());
> - synchronized (tableRows) {
> + synchronized (lock) {
> row = tableRows.get(sampleLabel);
> if (row == null) {
> row = new Calculator(sampleLabel);
> @@ -182,7 +187,8 @@ public class SummaryReport extends Abstr
> * Clears this visualizer and its model, and forces a repaint of the
> table.
> */
> public void clearData() {
> - synchronized (tableRows) {
> + //Synch is needed because a clear can occur while add occurs
> + synchronized (lock) {
> model.clearData();
> tableRows.clear();
> tableRows.put(TOTAL_ROW_LABEL, new Calculator(TOTAL_ROW_LABEL));
>
> Modified: jmeter/trunk/src/core/org/apache/jmeter/reporters/Summariser.java
> URL:
> http://svn.apache.org/viewvc/jmeter/trunk/src/core/org/apache/jmeter/reporters/Summariser.java?rev=1204144&r1=1204143&r2=1204144&view=diff
> ==============================================================================
> --- jmeter/trunk/src/core/org/apache/jmeter/reporters/Summariser.java
> (original)
> +++ jmeter/trunk/src/core/org/apache/jmeter/reporters/Summariser.java Sun Nov
> 20 10:04:02 2011
> @@ -21,8 +21,8 @@ package org.apache.jmeter.reporters;
> import java.io.Serializable;
> import java.text.DecimalFormat;
> import java.util.Map;
> -import java.util.Set;
> import java.util.Map.Entry;
> +import java.util.Set;
> import java.util.concurrent.ConcurrentHashMap;
>
> import org.apache.jmeter.engine.event.LoopIterationEvent;
> @@ -86,6 +86,11 @@ public class Summariser extends Abstract
> */
> private static final int INTERVAL_WINDOW = 5; // in seconds
>
> + /**
> + * Lock used to protect accumulators update + instanceCount update
> + */
> + private transient Object lock = new Object();
> +
> /*
> * This map allows summarisers with the same name to contribute to the
> same totals.
> */
> @@ -116,7 +121,7 @@ public class Summariser extends Abstract
> */
> public Summariser() {
> super();
> - synchronized (accumulators) {
> + synchronized (lock) {
> accumulators.clear();
> instanceCount=0;
> }
> @@ -308,7 +313,7 @@ public class Summariser extends Abstract
> * {@inheritDoc}
> */
> public void testStarted(String host) {
> - synchronized (accumulators) {
> + synchronized (lock) {
> myName = getName();
> myTotals = accumulators.get(myName);
> if (myTotals == null){
> @@ -327,7 +332,7 @@ public class Summariser extends Abstract
> */
> public void testEnded(String host) {
> Set<Entry<String, Totals>> totals = null;
> - synchronized (accumulators) {
> + synchronized (lock) {
> instanceCount--;
> if (instanceCount <= 0){
> totals = accumulators.entrySet();
>
> Modified: jmeter/trunk/xdocs/changes.xml
> URL:
> http://svn.apache.org/viewvc/jmeter/trunk/xdocs/changes.xml?rev=1204144&r1=1204143&r2=1204144&view=diff
> ==============================================================================
> --- jmeter/trunk/xdocs/changes.xml (original)
> +++ jmeter/trunk/xdocs/changes.xml Sun Nov 20 10:04:02 2011
> @@ -147,6 +147,7 @@ This behaviour can be changed with prope
> <li>Bug 51733 - SyncTimer is messed up if you a interrupt a test plan</li>
> <li>Bug 52118 - New toolbar : shutdown and stop buttons not disabled when no
> test is running</li>
> <li>Bug 52125 - StatCalculator.addAll(StatCalculator calc) joins incorrect
> if there are more samples with the same response time in one of the
> TreeMap</li>
> +<li>Bug 52215 - Confusing synchronization in StatVisualizer, SummaryReport
> ,Summariser and issue in StatGraphVisualizer</li>
> </ul>
>
> <!-- =================== Improvements =================== -->
>
>
>
