2

This is may be duplicate question but I am a bit confuse in ConcurrentModificationException. I gone through some other questions on stack Overflow also some articles related to How to avoid ConcurrentModificationException. I come to know that this exception occurs while looping on collection AND modifying the same (most common issue with remove).

But in my case I am just looping (foreach) on an java.util.Set then am I m getting this exception.

Moreover I am not getting this exception always, When I do load testing of my web application using Jmeter (150 users in 5 seconds) in this scenario I m getting exception

Here is my code where I am getting exception according to stack trace from log file

public static Map<String, String> getMessageMap(Locale locale) {
    Properties properties;
    Set<Object> objkeys;
    Map<String, String> messageMap = null;

    if (messageMap == null) {
        messageMap = new HashMap<String, String>();
        // 1 Get properties file.
        properties = Utility.loadPropertiesFile(locale); // this method return properties from static veriable

        // 2 Get all keys of properties file.
        objkeys = properties.keySet();

        // 3 Add all key values into map.
        for (Object key : objkeys) { caught exception here
            String keyName = key.toString();
            if (keyName.contains("global.")) {
                messageMap.put(keyName, properties.getProperty(keyName));
            }
        }

    }
    return messageMap;
}

According to log file ConcurrentModificationException is occurred at line for (Object key : objkeys)

Here is some lines from stack trace

java.util.ConcurrentModificationException
at java.util.Hashtable$Enumerator.next(Unknown Source)
at com.utilities.MKCLUtility.getMessageMap(Utility.java:164)
at com.utilities.MKCLUtility.addMessage(Utility.java:49)
at com.controllers.LoginController.loginPost(LoginController.java:132)

What can I do to avoid this? and why this exception is occurring though I am not modifying the set.

Updated Code

Iterator<Object> iterator = objkeys.iterator();
while (iterator.hasNext()) 
{
    String keyName = iterator.next().toString();
    if (keyName.contains("global.")) {
        messageMap.put(keyName, properties.getProperty(keyName));
    }
}
almightyGOSU
  • 3,731
  • 6
  • 31
  • 41
Amogh
  • 4,453
  • 11
  • 45
  • 106
  • 2
    Use the `Set`s iterator instead – MadProgrammer Jun 23 '14 at 09:11
  • You shouldn't change the structure of a Collection within a loop, You will end up with ConcurrentModificationException. The "how to fix this?" part is already mentioned by @MadProgrammer – TheLostMind Jun 23 '14 at 09:12
  • @TheLostMind Sorry, But I am not getting you. – Amogh Jun 23 '14 at 09:13
  • @MadProgrammer I updated code in question with `iterator` is it correct? – Amogh Jun 23 '14 at 09:16
  • 1
    Maybe you have indeed multiple threads calling this code or another code that modifies the properties object? – Seelenvirtuose Jun 23 '14 at 09:21
  • Without compiling or running it, that looks more correct – MadProgrammer Jun 23 '14 at 09:21
  • @MadProgrammer The iterator is also used in the for-each-loop variant. This does not seem to be the problem. – Seelenvirtuose Jun 23 '14 at 09:22
  • 1
    @Amogh - What is the relation between messageMap and properties? – TheLostMind Jun 23 '14 at 09:25
  • 1
    Where does `messageMap` come from? – Wundwin Born Jun 23 '14 at 09:25
  • Would be explained when `properties` and `messageMap` were the same objects. Maybe show (look at) a bit more of getMessage/addMessage. – Joop Eggen Jun 23 '14 at 09:26
  • code which I updated in question still throws same exception. – Amogh Jun 23 '14 at 10:01
  • @TheLostMind, `properties` (java.util.Properties) is the static variable containing property list loaded from `.properties` file and I want to make an `messageMap` (Map) (non-static) which contains messages from `properties` which has "global." in key part. – Amogh Jun 23 '14 at 10:06
  • I updated question with full method. – Amogh Jun 23 '14 at 10:12
  • `getMessageMap()` get called for every user means in my case if I load test with 250 users in 10 sec. i.e 25 users per sec. so these many times this method will execute. Is this prob.? – Amogh Jun 23 '14 at 10:19
  • @Amogh - You shouldn't get this exception.. Do a deep copy of "objkeys " and try with the new objkeys Set. – TheLostMind Jun 23 '14 at 10:21
  • @TheLostMind, Deep Copy?? You mean to say serialize the `objkeys `?? – Amogh Jun 23 '14 at 10:32
  • Your first code bit is extremely confusing. First off, what is the `if()` check on `messageMap` doing? You just set it to null, now you are checking if it's null? Second, why are you using a `Set`? According to the APIs, `Properties` is basically a `Map`, but you get the keys as `Objects` then do a conversion back to `String` using `toString()`? There seems to be a lot of unnecessary code going on. Also, flags went off in my head when you mentioned this method is called 25 times per second, but is a Static method. This makes me think there is a thread safety issue. – CodeChimp Jun 23 '14 at 11:13

2 Answers2

0

By the guidance from comments on question I changed by code to :

    public static Map<String, String> getMessageMap(Locale locale) {
    Properties properties;
    Set<Object> objkeys;


    if (messageMap == null) {
        messageMap = new HashMap<String, String>();
        // 1 Get properties file.
        properties = MKCLUtility.loadPropertiesFile(locale);

        // 2 Get all keys of properties file.
        objkeys = properties.keySet();

        // 3 Add all key values into map.
        Iterator<Object> iterator = objkeys.iterator();
        while (iterator.hasNext()) 
        {
            String keyName = iterator.next().toString();
            if (keyName.contains("global.")) {
                messageMap.put(keyName, properties.getProperty(keyName));
            }
        }
    }
    return messageMap;
}

I made messageMap as static so, this function on first request will check for null if it is null then messageMap get filled then for next request it will directly return messageMap.

By this ConcurrentModificationException is resolved.

But still If their some suggestion you want to give then Plzz I would like to know.

Amogh
  • 4,453
  • 11
  • 45
  • 106
-2

You modify the messageMap map while iterating over it's keyset. That's the reason you're getting the message. Just collect the matching keys inside a ArrayList object, then iterate over that and modify the messageMap map .

EDIT :

the exception is actually referring to the messageMap and not properties . Are these 2 related in any way (common keys) . Is the code not shown here iterating over messageMap too ?

omu_negru
  • 4,642
  • 4
  • 27
  • 38
  • But I don't think I am modifying `properties` map. At which line I am modifying it. As you said I am taking all its key set in another `set` and iterating on it `objkeys=properties.keySet();` – Amogh Jun 23 '14 at 09:21
  • Clarification about messageMap and not properties is added in comment to question and I am not iterating over messageMap I am just returning it. – Amogh Jun 23 '14 at 10:08
  • Does the new code using an iterator also throw the CME ? – omu_negru Jun 23 '14 at 10:42