Jetspeed 2
  1. Jetspeed 2
  2. JS2-761

ConcurrentModificationException in FileCache after jetspeed 2.1.2 installation

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.1.2
    • Fix Version/s: 2.2.0
    • Component/s: Components Core
    • Labels:
      None
    • Environment:
      java 1.5, SuSe Linux 10, tomcat 5.5.20

      Description

      I've downloaded and installed jetspeed portal. I've created several pages and have looked into jetspeed.log.
      There are some exceptions "org.apache.jetspeed.cache.file.FileCache - FileCache Scanner: Error in iteration...
      java.util.ConcurrentModificationException "

      Stack:
      2007-07-25 13:56:21,598 [Thread-2] ERROR org.apache.jetspeed.cache.file.FileCache - FileCache Scanner: Error in iteration...
      java.util.ConcurrentModificationException
      at java.util.HashMap$HashIterator.nextEntry(HashMap.java:841)
      at java.util.HashMap$ValueIterator.next(HashMap.java:871)

        Issue Links

          Activity

          Hide
          Woonsan Ko added a comment -

          After inspecting this issue, I came to the following conclusion:

          • The FileCacheScanner#run() method has a problematic synchronization issue.

          // line 402-407 of FileCache.java
          Collection values = Collections.synchronizedCollection(FileCache.this.cache.values());
          synchronized (values)
          {
          for (Iterator it = values.iterator(); it.hasNext(); )
          {
          FileCacheEntry entry = (FileCacheEntry) it.next();
          ...

          It synchronizes the 'values', entry collection of the cache object, but it cannot block entry additions of other threads because it does not synchronize the cache object.
          If the entry collection is modified, then the next() will sometimes throw a ConcurrentModificationException.

          By the way, I think we'd better defer fixing this issue to the 2.2 release because of the following reasons:

          • The problem may not be a critical problem in a production system because psml pages don't seem to be frequently added.
          • There's no critical problem even though this exception is logged. It will just scan again later.
          • I don't think it's a good idea that we improve this FileCache class. IMO, we'd better use good caching framework library such EhCache. This change could be bigger than we expect, so let's defer this issue.
          Show
          Woonsan Ko added a comment - After inspecting this issue, I came to the following conclusion: The FileCacheScanner#run() method has a problematic synchronization issue. // line 402-407 of FileCache.java Collection values = Collections.synchronizedCollection(FileCache.this.cache.values()); synchronized (values) { for (Iterator it = values.iterator(); it.hasNext(); ) { FileCacheEntry entry = (FileCacheEntry) it.next(); ... It synchronizes the 'values', entry collection of the cache object, but it cannot block entry additions of other threads because it does not synchronize the cache object. If the entry collection is modified, then the next() will sometimes throw a ConcurrentModificationException. By the way, I think we'd better defer fixing this issue to the 2.2 release because of the following reasons: The problem may not be a critical problem in a production system because psml pages don't seem to be frequently added. There's no critical problem even though this exception is logged. It will just scan again later. I don't think it's a good idea that we improve this FileCache class. IMO, we'd better use good caching framework library such EhCache. This change could be bigger than we expect, so let's defer this issue.
          Hide
          David Sean Taylor added a comment -

          Woonsan,

          Could we try catching the exception and retrying the refresh code?

          Show
          David Sean Taylor added a comment - Woonsan, Could we try catching the exception and retrying the refresh code?
          Hide
          Woonsan Ko added a comment -

          I think we should improve this by minimizing the locking.
          Catching exceptions and retrying the refresh is not enough because the synchronizing codes can reduce the performance significantly in case that there are a lot of pages.
          I'd like to improve this for the 2.2 release.

          Show
          Woonsan Ko added a comment - I think we should improve this by minimizing the locking. Catching exceptions and retrying the refresh is not enough because the synchronizing codes can reduce the performance significantly in case that there are a lot of pages. I'd like to improve this for the 2.2 release.
          Hide
          Woonsan Ko added a comment -

          Modified FileCache to use an injected JetspeedCache instance. It doesn't need to synchronize any object any more.

          Show
          Woonsan Ko added a comment - Modified FileCache to use an injected JetspeedCache instance. It doesn't need to synchronize any object any more.

            People

            • Assignee:
              Woonsan Ko
              Reporter:
              Vitaly Baranovsky
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development