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

Bugtraq ID6219112

For each lookup service that is discovered, the JoinManager creates a
new instance of ProxyReg that wraps the discovered lookup service's
proxy. Each such proxyReg object contains a list of tasks that are
executed. For each proxyReg, there is a special wrapper task named,
ProxyRegTask that is queued with the TaskManager.

Each ProxyRegTask is executed in parallel by the TaskManager.
Within each ProxyRegTask, the tasks contained in the correpsonding proxyReg's
taskList are each run in FIFO order.

For any given proxyReg object, the proxyReg object itself, as well as the
fields of that object, are accessed (read or written) in the various tasks
and threads (including the main thread) executed by the JoinManager. In
most cases, that access is controlled through synchronization on various
objects (different objects are used to provide more "granularity" to the
access). There are a few places where the access to a proxyReg object or
its fields is not synchronized at all.

The synchronization strategy and implementation for each proxyReg object and
its fields should be reviewed and corrected where appropriate.

See the comments for a 'map' of serviceItem synchronization, and a
discription of the synchronization strategy regarding each proxyReg and
its fields.

Comments:
JoinManager {
ProxyRegTask extends RetryTask {
ProxyReg proxyReg
tryOnce() {}
runAfter() {}
stopTrying() {}
}
ProxyReg

{ ProxyRegTask proxyRegTask ServiceRegistrar proxy ServiceRegistration srvcRegistration Lease serviceLease List taskList DiscLeaseListener dListener }

}

– Synchronization Strategy for serviceItem –

Because each new proxyReg is placed in the joinSet, and because the tasks
corresponding to a particular proxyReg (stored in that proxyReg's taskList)
are unrelated (and run in parallel) to the tasks of the other proxyRegs,
access to a particular proxyReg instance is controlled primarily through synchronization on either joinSet or the proxyReg's taskList. This is intended
to provide better granularity between the accesses that occur across all
proxyReg tasks (access to joinSet) and accesses that are specific to a
particular proxyReg (access to a particular proxyReg's taskList). Additionally,
there is one special where in addtion to syncing on joinSet, there is a
nested sync on serviceItem. This is necessary because in addition to accessing
a field of a proxyReg (requiring a sync on joinSet), there is also an
access to serviceItem.serviceID which requires a sync on serviceItem (see
bug 6219020).

Thus, for a given proxyReg:

– Access to the taskList of the proxyReg is controlled by synchronizing
on the taskList itself.
– Access to the other fields of the proxyReg is controlled by
synchronizing on joinSet.

– Synchronization Map –

proxyReg field access location access type synchronization
----------------- ----------------------- ----------- -----------------
a.) proxyRegTask ProxyRegTask.tryOnce() write sync(taskList)
ProxyReg.addTask() read write sync(taskList)
removeTasks() read write sync(taskList)

b.) proxy ProxyReg.constructor() write NO SYNC
[*** Note: constructor is only place proxy field is written to ***]
getJoinSet() read sync(joinSet)

• • • findReg() read NO SYNC
    • proxyReg.equals() read NO SYNC

c.) srvcRegistration ProxyReg.register() read write sync(joinSet)
sync(joinSet)
&sync(serviceItem)

• • • ProxyReg.addAttributes() read NO SYNC
    • ProxyReg.modifyAttributes() read NO SYNC
    • ProxyReg.setAttributes() read NO SYNC
      getJoinSet() read sync(joinSet)

d.) serviceLease DiscardLeaseListener.notify() read sync(joinSet)

• • • DiscardProxyTask.run() read NO SYNC
    • ProxyReg.register() write NO SYNC
      DiscMgrListener.discarded() read sync(joinSet)
      replaceRegistrationDo() read sync(joinSet)

e.) taskList ProxyRegTask.tryOnce() read write sync(taskList)
ProxyReg.addTask() write sync(taskList)
removeTasks() write sync(taskList)

f.) dListener ProxyReg.register() read NO SYNC
[*** Note: dListener is instantiated when each proxyReg instance is
created. No need then to synchronize access to dListener? ***]

Suggested Fix
Each of the items labelled with a "NO SYNC" in the synchronization map
documented in "Comments" section should be analyzed and the appropriate
synchronization should be added.