Jetty
  1. Jetty
  2. JETTY-948

ConcurrentModificationException in terracotta session scavenger

    Details

    • Type: Bug Bug
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 6.1.16
    • Component/s: None
    • Labels:
      None
    • Number of attachments :
      0

      Description

      JETTY-947 unfortunately makes this very hard to see but it is occurring once you get some load on the system

      java.util.ConcurrentModificationException
      at java.util.Hashtable$Enumerator.next(Hashtable.java:1031)
      at java.util.Hashtable$EntriesIterator.nextEntry(Hashtable.java:955)
      at java.util.Hashtable$EntriesIterator.next(Hashtable.java:943)
      at org.mortbay.terracotta.servlet.TerracottaSessionManager.scavenge(TerracottaSessionManager.java:484)
      at org.mortbay.terracotta.servlet.TerracottaSessionManager.run(TerracottaSessionManager.java:166)
      at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:441)
      at java.util.concurrent.FutureTask$Sync.innerRunAndReset(FutureTask.java:292)
      at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:58)
      at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$101(ScheduledThreadPoolExecutor.java:98)
      at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.runPeriodic(ScheduledThreadPoolExecutor.java:181)
      at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:205)
      at java.util.concurrent.ThreadPoolExecutor$Worker.runTask(ThreadPoolExecutor.java:886)
      at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:908)
      at java.lang.Thread.run(Thread.java:619)

      This is an unfortunate behavior of clustered Hashtable instances. The local synchronization performed on the _sessionExpirations while collecting the candidate is not sufficient since updates from other nodes in the cluster can modify it. Without using some terracotta APIs, the only way to safely traverse the keys of the Hashtable is to use the keys() Enumeration.

        Activity

        Hide
        Tim Eck added a comment -

        Those line numbers are slightly off (I was testing some local changes). This should match the 6.1.15 tagged sources

        java.util.ConcurrentModificationException
        at java.util.Hashtable$Enumerator.next(Hashtable.java:1031)
        at java.util.Hashtable$EntriesIterator.nextEntry(Hashtable.java:955)
        at java.util.Hashtable$EntriesIterator.next(Hashtable.java:943)
        at org.mortbay.terracotta.servlet.TerracottaSessionManager.scavenge(TerracottaSessionManager.java:482)
        at org.mortbay.terracotta.servlet.TerracottaSessionManager.run(TerracottaSessionManager.java:164)
        at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:441)
        at java.util.concurrent.FutureTask$Sync.innerRunAndReset(FutureTask.java:292)
        at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:58)
        at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$101(ScheduledThreadPoolExecutor.java:98)
        at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.runPeriodic(ScheduledThreadPoolExecutor.java:181)
        at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:205)
        at java.util.concurrent.ThreadPoolExecutor$Worker.runTask(ThreadPoolExecutor.java:886)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:908)
        at java.lang.Thread.run(Thread.java:619)

        Show
        Tim Eck added a comment - Those line numbers are slightly off (I was testing some local changes). This should match the 6.1.15 tagged sources java.util.ConcurrentModificationException at java.util.Hashtable$Enumerator.next(Hashtable.java:1031) at java.util.Hashtable$EntriesIterator.nextEntry(Hashtable.java:955) at java.util.Hashtable$EntriesIterator.next(Hashtable.java:943) at org.mortbay.terracotta.servlet.TerracottaSessionManager.scavenge(TerracottaSessionManager.java:482) at org.mortbay.terracotta.servlet.TerracottaSessionManager.run(TerracottaSessionManager.java:164) at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:441) at java.util.concurrent.FutureTask$Sync.innerRunAndReset(FutureTask.java:292) at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:58) at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$101(ScheduledThreadPoolExecutor.java:98) at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.runPeriodic(ScheduledThreadPoolExecutor.java:181) at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:205) at java.util.concurrent.ThreadPoolExecutor$Worker.runTask(ThreadPoolExecutor.java:886) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:908) at java.lang.Thread.run(Thread.java:619)
        Hide
        Simone Bordet added a comment - - edited

        Tim,

        I fail to see how iterating over the keys makes any difference with respect to the ConcurrentModificationException, as we're adding/removing keys while iterating over them, unless this is a feature of the clustered Hashtable.
        If you confirm this feature, then I can just modify the code in this way to make it work properly ?

        synchronized (_sessions)
        {
            synchronized (_sessionExpirations)
            {
                for (String sessionId : _sessionExpirations.keySet())
                {
                    MutableLong value = _sessionExpirations.get(sessionId);
                    if (value != null)
                    {
                        long expirationTime = value.value;
                        Log.debug("Estimated expiration time {} for session {}", expirationTime, sessionId);
                        if (expirationTime > 0 && expirationTime < now) candidates.add(sessionId);
                    }
                }
        
                _sessions.keySet().retainAll(_sessionExpirations.keySet());
            }
        }
        

        Note that I am using keySet() and not keys() as you suggest; does this make any difference ?

        Thanks !

        Show
        Simone Bordet added a comment - - edited Tim, I fail to see how iterating over the keys makes any difference with respect to the ConcurrentModificationException, as we're adding/removing keys while iterating over them, unless this is a feature of the clustered Hashtable. If you confirm this feature, then I can just modify the code in this way to make it work properly ? synchronized (_sessions) { synchronized (_sessionExpirations) { for ( String sessionId : _sessionExpirations.keySet()) { MutableLong value = _sessionExpirations.get(sessionId); if (value != null ) { long expirationTime = value.value; Log.debug( "Estimated expiration time {} for session {}" , expirationTime, sessionId); if (expirationTime > 0 && expirationTime < now) candidates.add(sessionId); } } _sessions.keySet().retainAll(_sessionExpirations.keySet()); } } Note that I am using keySet() and not keys() as you suggest; does this make any difference ? Thanks !
        Hide
        Tim Eck added a comment -

        The suggestion for using keys() instead of keySet() was deliberate. Enumeration (what keys() returns) does not throw ConcurrentModificationException like the iterator from keySet() can. I'm glad you added the null check after the get() since that can happen now.

        Show
        Tim Eck added a comment - The suggestion for using keys() instead of keySet() was deliberate. Enumeration (what keys() returns) does not throw ConcurrentModificationException like the iterator from keySet() can. I'm glad you added the null check after the get() since that can happen now.
        Hide
        Simone Bordet added a comment -

        Fixed as suggested, using Enumeration. Also added a catch block to avoid exit of the scavenger thread.

        Show
        Simone Bordet added a comment - Fixed as suggested, using Enumeration. Also added a catch block to avoid exit of the scavenger thread.

          People

          • Assignee:
            Simone Bordet
            Reporter:
            Tim Eck
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: