Uploaded image for project: 'Sakai'
  1. Sakai
  2. SAK-24657

CLONE - ConcurrentModificationExceptions possible - Static Code review

    Details

    • Type: Bug
    • Status: CLOSED
    • Priority: Major
    • Resolution: Non-Issue
    • Affects Version/s: 2.8.0
    • Fix Version/s: 2.8.1
    • Component/s: Messages Tool
    • Labels:
      None
    • Environment:
      Compiled version of 2.8 rc 03 source code.
    • Previous Issue Keys:
      SAK-20336, MSGCNTR-497

      Description

      Using contributed bug pattern testing for Findbugs. Found 7 locations with the following issue:

      This exception may be thrown by methods that have detected concurrent modification of an object when such modification is not permissible.

      For example, it is not generally permissible for one thread to modify a Collection while another thread is iterating over it.

      Note that this exception does not always indicate that an object has been concurrently modified by a different thread. If a single thread issues a sequence of method invocations that violates the contract of an object, the object may throw this exception. For example, if a thread modifies a collection directly while it is iterating over the collection with a fail-fast iterator, the iterator will thow this exception.

      Example:

      for (Iterator it = myCollection.iterator(); it.hasNext()) {
      Object myObject = it.next();
      if (someConditionIsTrue) {
      // myCollection.remove(myObject); // wrong can throw ConcurrentModificationException
      it.remove(myObject); // right
      }
      }

      Locations:
      Pattern 1)
      Method deletes collection element while iterating
      This method removes items from a collection using the remove method of the collection, while at the same time iterating across the collection. Doing this will invalidate the iterator, and further use of it, will cause ConcurrentModificationExceptions to be thrown. To avoid this, the remove method of the iterator should be used.

      So this is only an issue if not returned in the if statement. However, even then the code is fragile to refactoring

      1) GradebookServiceHibernateImpl line 600
      catList.remove(cat_cat);

      2) SpreadsheetUploadBean line 1578 - Possibly ok, but fragile to refactoring
      gbGrades.remove(agr); // for efficiency

      3) SpreadsheetUploadBean line 1600 - Possibly ok, but fragile to refactoring
      assignList.remove(assignment);

      4) PrivateMessagesTool line 1150
      5) PrivateMessagesTool line 909

      Method modifies collection element while iterating
      This method modifies the contents of a collection using the collection api methods, while at the same time iterating across the collection. Doing this will invalidate the iterator, and further use of it, will cause ConcurrentModificationExceptions to be thrown.

      5) ItemAddListener - line 1141

      textSet.add(text);

      6) RosterBean - Line 384
      gradeRecords.add(agr);

        Gliffy Diagrams

          Zeplin

            Attachments

              Issue Links

                Activity

                  People

                  Assignee:
                  baholladay Bryan Holladay
                  Reporter:
                  a.m.berg@uva.nl Alan Berg
                  Votes:
                  0 Vote for this issue
                  Watchers:
                  0 Start watching this issue

                    Dates

                    Created:
                    Updated:
                    Resolved:

                      Git Integration