River
  1. River
  2. RIVER-145

JoinManager synchronization on serviceItem should be reviewed, doc'd and fixed where appropriate

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: jtsk_2.1
    • Fix Version/s: None
    • Component/s: net_jini_lookup
    • Labels:
      None
    • Bugtraq ID:
      6219020

      Description

      Bugtraq ID 6219020

      The object JoinManager.serviceItem and its fields are accessed (read
      and written) in a number of threads/tasks in JoinManager. In most cases,
      that access is controlled through synchronization on either serviceItem
      or the JoinManager.joinSet object. But there are a few places where the
      access to serviceItem fields is not synchronized at all.

      The synchronization strategy and implementation for serviceItem should
      be reviewed and corrected where necessary.

      See the comments for a 'map' of serviceItem synchronization, and a
      description of the synchronization strategy regarding serviceItem.

      ( Comments note: }

      JoinManager {
        ServiceItem serviceItem
                      - serviceID
                      - service
                      - attributesSets
      }
      

      – Synchronization Strategy for serviceItem

      serviceItem is instantiated in createJoinManager() when the JoinManager is
      created and initialized. After instantiation, only the fields of serviceItem
      are accessed by the various threads. Access to those fields is controlled by
      synchronizing on either serviceItem itself, on joinSet, or on both.

      • Access to the serviceID field is controlled by synchronizing on
        serviceItem itself.
      • Access to the service and attributeSets fields is controlled by
        synchronizing on joinSet.

      The reason that joinSet is not used to control access to all the fields of
      serviceItem is because deadlock can occur if the sync on serviceItem in
      ProxyRegTask.runAfter() is changed to a sync on joinSet. This deadlock
      can occur because the TaskManager actually executes the runAfter() method
      inside of a sync block on the TaskManager itself. Thus,

      A. LUS-0 discovered 
      
      DiscMgrListener.discovered() -- LUS-0
        sync(joinSet)
          addTask(RegisterTask-0)
            sync(taskList)
              taskList.add(RegisterTask-0)
              sync(taskMgr)
                taskMgr.add(ProxyRegTask-0) ---------------------| 
              }//end sync                                        |
            }//end sync                                          |
        }//end sync                                              |
                                                                 |
                                             B.  TaskManager.run(ProxyRegTask-0)
                                               |-- sync(taskMgr) -- (lock-1)
      C. LUS-1 discovered                      |     ProxyRegTask-0.runAfter()
                                               |      .
      DiscMgrListener.discovered() -- LUS-1    |      .
        sync(joinSet) -- (lock-2) ----------|  |      .
          addTask(RegisterTask-1)           |-------- sync(joinSet) (wait on lock-2)
            sync(taskList)                     |      
              taskList.add(RegisterTask-1)     |
              sync(taskMgr) (wait on lock-1) --|       *** DEADLOCK ***
      

      Thus, to avoid the above deadlock, runAfter() and ProxyReg.register() both
      synchronize on serviceItem before accessing the serviceID field of
      serviceItem. For access to the other fields of serviceItem, synchronizing
      on joinSet seems to be sufficient; although, as the map below shows,
      there are places where no synchronization occurs, but should be
      considered.

      -- Synchronization Map --
      
      serviceItem field     access location           access type  synchronization
      ----------------- -----------------------      ----------- -----------------
      a.) serviceID     ProxyRegTask.runAfter()      read        sync(serviceItem)
                        ProxyReg.register()          read write  sync(serviceItem)
      
      b.) service       ProxyReg.register()          read write  sync(joinSet)
                                                                 & sync(serviceItem)
                        DiscMgrListener.discovered() read        sync(joinSet)
                        replaceRegistrationDo()           write  sync(joinSet)
          ***           createJoinManager()               write  NO SYNC
                        (when serviceItem instantiated)
      
      c.) attributeSets addAttributes()                   write  sync(joinSet)
                        setAttributes()                   write  sync(joinSet)
                        modifyAttributes()                write  sync(joinSet)
                        replaceRegistrationDo()           write  sync(joinSet)
          ***           createJoinManager()               write  NO SYNC
                        (when serviceItem instantiated)
      

      ( Suggested Fix note: )

      Currently, the only place in JoinManager where access to the fields of
      serviceItem are not synchronized is in createJoinManager(). In that
      method, the serviceItem is created (and its fields are written to) when
      the JoinManager itself is constructed and initialized. Because this occurs
      during construction/initialization, before any tasks or threads that access
      serviceItem have been started, it may not be necessary to synchronize
      access to that new serviceItem in createJoinManager.

      Thus, there may not currently be any sort of defect associated with
      synchronization on serviceItem in JoinManager. This bug report can
      then be viewed as a location to document any future defects or changes
      related to that synchronization.

        Activity

          People

          • Assignee:
            Unassigned
            Reporter:
            Peter Jones
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:

              Development