5 Replies Latest reply: Dec 12, 2006 11:06 AM by 456795 RSS

    ConcurrentModificationException on a synchronized set

    807607
      Thanks to anyone who can help me here - I'm very confused.

      I have a class WebAppUtility that looks like this:

      private static Set<HttpSession> openSessions = Collections
                     .synchronizedSet(new HashSet<HttpSession>());
           
           public static void addSession(HttpSession session) {
                synchronized(openSessions){
                     openSessions.add(session);
                }
           }
           
           public static boolean removeSession(HttpSession session) {
                synchronized(openSessions){
                     return openSessions.remove(session);
                }
           }
           public static Set<String> getLoggedInUsers() throws Exception {
                Set<String> set = new HashSet<String>();
                synchronized (openSessions) {
                     for(Iterator i = openSessions.iterator(); i.hasNext();){
                          HttpSession session = (HttpSession)i.next();
                                    //grab attribute from session here and stick in "set"
                     }
                }
                return set;
           }
      Somehow when I have many users logging in and out I'm getting a ConcurrentModificationException under the getLoggedInUsers method:

      java.util.ConcurrentModificationException
      at java.util.HashMap$HashIterator.nextEntry(HashMap.java:787)
      at java.util.HashMap$KeyIterator.next(HashMap.java:823)
      at com.studentuniverse.utility.WebAppUtility.getLoggedInUsers(WebAppUtility.java:68)

      Line 68 is the one that looks like:
      HttpSession session = (HttpSession)i.next();
      Am I misusing the synchronizedSet in some way? Please guide me to proper threaded Set usage! Thank you very much.

      _kris                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                               
        • 1. Re: ConcurrentModificationException on a synchronized set
          807607
          While iterating through a so called fail-fast collection using an Iterator, you cannot remove elements using the collection's remove() method. You can only use the iterator's remove() method.

          The iterator checks in the next() method if the size of the collection has changed and throws a ConcurrentModificationException.
          • 2. Re: ConcurrentModificationException on a synchronized set
            807607
            But isn't that where the the synchronized(openSessions) block comes into play? The remove() method should never be called if the iterator is still going because all code blocks that access the set are synchronized on the same object. Please let me know if that assumption is wrong. Thanks.
            • 3. Re: ConcurrentModificationException on a synchronized set
              807607
              But isn't that where the the synchronized(openSessions) block comes
              into play?
              First of all you don't need the synchronize(openSessions) for the non-iterating methods as the synchronizedSet you have already synchronizes on the openSessions object.
              The remove() method should never be called if the
              iterator is still going because all code blocks that
              access the set are synchronized on the same object.
              Please let me know if that assumption is wrong.
              No - it's correct; you must be calling remove() or add() on the set from within your iterator block. ConcurrentModificationException is perhaps a misleading name in this instance.
              • 4. Re: ConcurrentModificationException on a synchronized set
                807607
                Just to close this out: I was definitely not modifying the set from within an iteration loop, here's the full code:
                               for(Iterator i = tempSet.iterator(); i.hasNext();){
                                    HttpSession session = (HttpSession)i.next();
                                    try {
                                         Principal p = (Principal) session.getAttribute("so_user");
                                         if (p != null) {
                                              set.add(p.getName());
                                         }
                                    } catch (Exception e) {
                                         log.warn("error when getting attribute from session");
                                    }
                               }
                The only solution I could come up with was to create a temporary copy set and iterate over that. That of course works fine (also disproving the modification of the set from within the iterator, which would occur on a copy as well.)

                Oh well, I'd really track this down if i had more time; thanks to everyone who replied!
                • 5. Re: ConcurrentModificationException on a synchronized set
                  456795
                  You need to synchronize around your iterator.
                      synchronized(tempSet) {
                                 for(Iterator i = tempSet.iterator(); i.hasNext();){
                                      HttpSession session = (HttpSession)i.next();
                                      try {
                                           Principal p = (Principal) session.getAttribute("so_user");
                                           if (p != null) {
                                                set.add(p.getName());
                                           }
                                      } catch (Exception e) {
                                           log.warn("error when getting attribute from session");
                                      }
                                 }
                      }
                  From [url http://java.sun.com/j2se/1.5.0/docs/api/java/util/Collections.html#synchronizedSet(java.util.Set)]Collections.synchronizedSet

                  Returns a synchronized (thread-safe) set backed by the specified set. In order to guarantee serial access, it is critical that all access to the backing set is accomplished through the returned set.
                  It is imperative that the user manually synchronize on the returned set when iterating over it:
                    Set s = Collections.synchronizedSet(new HashSet());
                        ...
                    synchronized(s) {
                        Iterator i = s.iterator(); // Must be in the synchronized block
                        while (i.hasNext())
                            foo(i.next());
                    }
                  Failure to follow this advice may result in non-deterministic behavior.