Iteration over a view on a synchronized collection without obtaining a lock on the collection

**Bob Atkey** ([Bug 55827](https://bz.apache.org/bugzilla//show_bug.cgi?id=55827&redirect=false)):
We ran our static analysis tool ThreadSafe [1] on version 2.10 of JMeter, which appeared to uncover a couple of concurrency issues. One of the most interesting was the possibility of an unsynchronised iteration over a synchronised collection in the class 'AbstractTestElement'.

The relevant line is 499:
Iterator<Map.Entry<String, JMeterProperty>> iter = propMap.entrySet().iterator();

where the propMap field always contains a synchronised collection created at line 57 in the same file.

The JDK documentation for Collections.synchronizedMap states that:

> It is imperative that the user manually synchronize on the
> returned map when iterating over any of its collection views:
>
> Map m = Collections.synchronizedMap(new HashMap());
> ...
> Set s = m.keySet(); // Needn't be in synchronized block
> ...
> synchronized (m) { // Synchronizing on m, not s!
> Iterator i = s.iterator(); // Must be in synchronized block
> while (i.hasNext())
> foo(i.next());
> }
>
> Failure to follow this advice may result in non-deterministic behavior.

It appears that the code on line 499 does not correctly synchronize on propMap, leading to the possibility of the non-deterministic behaviour the JDK documentation warns about.

We're not sure that this can actually result in a user-visible bug, but we thought you'd like to know.

We are also planning to use this finding as an example of Android-related concurrency mistakes in an article about ThreadSafe. Obviously, if you, as the developers of JMeter, have any objections to our using this as an example, then we won't.

[1] ThreadSafe is a static analysis tool for Java concurrency, developed by Contemplate Ltd.:
http://www.contemplateltd.com/

Severity: major
OS: Linux

Read AbstractTestElement, starting at propMap's synchronized collection setup around line 57 and the iterator creation around line 499. Check the JDK synchronization requirements for collection views and inspect nearby code for existing concurrency tests. Done means iteration follows the required locking discipline without introducing regressions.

Written by the indexing model from the issue text.