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:  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:  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.  ---------------------------------------- ---------------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":  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. --------------------------
