Uploaded image for project: 'Wicket'
  1. Wicket
  2. WICKET-4646

atomicity violation bugs of using concurrent collections

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.5.7
    • Fix Version/s: 1.5.8, 6.0.0
    • Component/s: wicket
    • Labels:

      Description

      My name is Yu Lin. I'm a Ph.D. student in the CS department at
      UIUC. I'm currently doing research on mining Java concurrent library
      misusages. I found some misusages of ConcurrentHashMap in wicket
      1.5.7, which may result in potential atomicity violation bugs.

      The code below is a snapshot of the code in file
      src/wicket-core/src/main/java/org/apache/wicket/request/resource/PackageResourceReference.java from line 152 to 157

      L152 UrlAttributes value = urlAttributesCacheMap.get(key);
      L153 if (value == null)
      L154

      { L155 value = getUrlAttributes(locale, style, variation); L156 urlAttributesCacheMap.put(key, value); L157 }

      In the code above, an atomicity violation may occur between lines <154
      and 156>. Suppose a thread T1 executes line 152 and finds out the
      concurrent hashmap does not contain the key "key". Before it gets to
      execute line 156, another thread T2 puts a pair <key, v> in the
      concurrent hashmap "urlAttributesCacheMap". Now thread T1 resumes
      execution and it will overwrite the value written by thread T2. Thus,
      the code no longer preserves the "put-if-absent" semantics. By using
      putIfAbsent method, we can eliminated this atomicity violation (see
      patch).

      I found some similar atomicity violations in other files:

      In
      src/wicket-core/src/main/java/org/apache/wicket/util/lang/PropertyResolver.java,
      atomicity violation may occur if thread T2 puts a value to the map
      between lines <385 and 522> or <234 and 260>. However, it seems map
      "getAndSetters" is not always a ConcurrentHashMap that the first
      atomicity violation at line 522 cannot be fixed by using putIfAbsent,
      so should we use synchronized?

      In
      src/wicket-core/src/main/java/org/apache/wicket/session/DefaultPageFactory.java,
      atomicity violation may occur if thread T2 puts a value to the map
      between lines <124 and 131> or <234 and 260>.

      In
      src/wicket-core/src/test/java/org/apache/wicket/versioning/InMemoryPageStore.java,
      atomicity violation may occur if thread T2 puts a value to the map
      between lines <95 and 97>.

      In
      src/wicket-spring/src/main/java/org/apache/wicket/spring/injection/annot/AnnotProxyFieldValueFactory.java,
      atomicity violation may occur if thread T2 puts a value to the map
      between lines <127 and 142> or <165 and 170>.

      In
      src/wicket-util/src/main/java/org/apache/wicket/util/convert/converter/AbstractIntegerConverter.java,
      atomicity violation may occur if thread T2 puts a value to the map
      between lines <47 and 51>.

      In
      src/wicket-util/src/main/java/org/apache/wicket/util/watch/ModificationWatcher.java,
      atomicity violation may occur if thread T2 puts a value to the map
      between lines <95 and 107>.

      Meanwhile, there may be an atomicty violation between lines <190 and 200> in
      src/wicket-core/src/main/java/org/apache/wicket/feedback/FeedbackMessages.java.
      Suppose thread T1 finds list "messages" is not empty at line
      187. Before T1 removes "toDelete" from "messages" at line 200, T1
      suspends and another thread T2 removes the same "toDelete" from "messages".
      When T1 resumes, it will remove nothing and "clear" will return the wrong value (in
      this case, "clear" method should return 0). Though I'm not sure whether this scenario
      will happen, I also provide a patch for this atomicty violation.

        Activity

        Hide
        jacklondongood Yu Lin added a comment -

        This is a patch that can fix the atomicity violation bugs.

        Show
        jacklondongood Yu Lin added a comment - This is a patch that can fix the atomicity violation bugs.
        Hide
        mgrigorov Martin Grigorov added a comment -

        Thank you!

        Show
        mgrigorov Martin Grigorov added a comment - Thank you!

          People

          • Assignee:
            mgrigorov Martin Grigorov
            Reporter:
            jacklondongood Yu Lin
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

              Estimated:
              Original Estimate - 504h
              504h
              Remaining:
              Remaining Estimate - 504h
              504h
              Logged:
              Time Spent - Not Specified
              Not Specified

                Development