Permission to use two minor synchronisation issues in JMeter for an article about the ThreadSafe static analysis tool

[Bob Atkey <bo...@contemplateltd.com>]

Hello,

I recently reported two minor synchronisation issues in version 2.10 of JMeter:
  https://issues.apache.org/bugzilla/show_bug.cgi?id=55826
  https://issues.apache.org/bugzilla/show_bug.cgi?id=55827

These issues were discovered by our static analysis tool ThreadSafe.
ThreadSafe is a commercial static analysis tool for Java concurrency
produced by Contemplate Ltd.:
  http://www.contemplateltd.com/

We'd like to use these examples for an article highlighting short
examples of issues that ThreadSafe has found in real-world open source
Java code.

We are not saying that these issues are particularly serious, merely
that they are the kinds of non-serious mistakes that are routinely
made while developing concurrent Java code. The kinds of mistakes that
developers would rather not have in their codebases, in case they lead
to more serious problems in the future.

Philippe Mouawad commented on the first issue, stating that it had been fixed:
  http://svn.apache.org/r1546654
and to ask the dev list for permission for us to use these examples. I
have included the text we have in mind below.

Thanks for your attention,
Bob Atkey


The following is the text we have in mind discussing the two issues
(in markdown format):

-----First snippet-------------------

Inconsistent synchronization on collections can be particularly
harmful to program behaviour. While incorrectly synchronizing accesses
to a field may "only" result in missed updates or stale information,
incorrectly synchronizing accesses to collections that have not been
designed for concurrent use can lead to violations of the collections'
internal invariants. Violation of a collection's internal invariants
may not immediately cause visible effects, but may cause odd
behaviour, including infinite loops or corrupted data, at a later
point in the program's execution.

An example of inconsistent use of synchronization when accessing a
shared collection is present in
[Apache JMeter](http://jmeter.apache.org/). Running ThreadSafe on
version 2.10 of Apache JMeter produces the following warning:

![Screenshot of the inconsistent synchronisation warning on the
collection stored in the field `RespTimeGraphVisualizer.internalList :
List<RespTimeGraphDataBean>`](jmeter-internallist-finding.png)

As before, we can ask ThreadSafe for more information on this report
by showing us the accesses to this field along with the locks that are
held:

![Screenshot of ThreadSafe's Accesses View investigating
`internalList`](jmeter-internallist-accesses.png)

Now we can see that there are three methods that access the collection
stored in the field `internalList`. One of these methods is
`actionPerformed`, which will be invoked by the Swing Gui framework on
the UI thread.

Another method that accesses the collection stored in `internalList`
is `add()`. Again, by investigating the possible callers of this
method, we can see that it is indeed called from the `run()` method of
a thread that is not the main UI Thread of the application, indicating
that synchronization ought to have been used.

![Screenshot of Eclipse's call hierarchy showing the `run()`
method](jmeter-internallist-ch.png)

----------------------------------------

---------------Second Snippet----------------------

For relatively simple scenarios, Java's built-in synchronized
collections facility can provide the right level of thread safety
without too much effort.

Synchronized collections wrap existing collections, providing the same
interface as the underlying collection, but synchronizing all accesses
on the synchronized collection instance. Synchronized collections are
created by using calls to special static methods similar to the
following:

    private List<X> threadSafeList =
             Collections.synchronizedList(new LinkedList<X>());

Synchronized collections are relatively easy to use compared to some
other thread-safe data structures, but there are still subtleties
involved in their use. A common mistake with synchronized collections
is to iterate over them without first synchronizing on the collection
itself. Since exclusive access to the collection has not been
enforced, it is possible for the collection to be modified by other
threads while iteration over its elements takes place. This may result
in `ConcurrentModificationException`s being thrown intermittently at
runtime, or non-deterministic behaviour, depending on the exact
scheduling of threads. The requirement to synchronize is clearly
documented in the
[JDK API documentation](http://docs.oracle.com/javase/7/docs/api/java/util/Collections.html#synchronizedList%28java.util.List%29):

> It is imperative that the user manually synchronize on the returned
> list when iterating over it:
>
>      List list = Collections.synchronizedList(new ArrayList());
>         ...
>      synchronized (list) {
>          Iterator i = list.iterator(); // Must be in synchronized block
>          while (i.hasNext())
>              foo(i.next());
>      }
>
> Failure to follow this advice may result in non-deterministic behavior.

Nevertheless, it is easy to forget to synchronize when iterating over
a synchronized collection, especially because they have exactly the
same interface as normal, unsynchronized, collections.

An example of this kind of mistake can be found in version 2.10 of
[Apache JMeter](http://jmeter.apache.org/). ThreadSafe reports the
following instance of "Unsafe iteration over a synchronized
collection":

![Screenshot of ThreadSafe's unsafe iteration
report](jmeter-unsafe-iteration.png)

The line reported by ThreadSafe contains the following code:

    Iterator<Map.Entry<String, JMeterProperty>> iter =
propMap.entrySet().iterator();

Here, the iteration is over a *view* on the synchronized collection,
created by the call to `entrySet()`. Since views on collections are
"live", this code has the same potential for non-deterministic
behaviour and `ConcurrentModificationException`s as discussed above.


--------------------------