You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@river.apache.org by "Peter Firmstone (JIRA)" <ji...@apache.org> on 2015/12/05 09:05:10 UTC
[jira] [Resolved] (RIVER-145) JoinManager synchronization on
serviceItem should be reviewed, doc'd and fixed where appropriate
[ https://issues.apache.org/jira/browse/RIVER-145?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Peter Firmstone resolved RIVER-145.
-----------------------------------
Resolution: Fixed
Fix Version/s: River_3.0.0
The synchronization in JoinManager has been reviewed and fixed.
> JoinManager synchronization on serviceItem should be reviewed, doc'd and fixed where appropriate
> ------------------------------------------------------------------------------------------------
>
> Key: RIVER-145
> URL: https://issues.apache.org/jira/browse/RIVER-145
> Project: River
> Issue Type: Bug
> Components: net_jini_lookup
> Affects Versions: jtsk_2.1
> Reporter: Peter Jones
> Priority: Minor
> Fix For: River_3.0.0
>
>
> Bugtraq ID [6219020|http://bugs.sun.com/bugdatabase/view_bug.do?bug_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}}.
> h4. ( Comments note: }
> {noformat}
> JoinManager {
> ServiceItem serviceItem
> - serviceID
> - service
> - attributesSets
> }
> {noformat}
> -- 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,
> {noformat}
> 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 ***
> {noformat}
> 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.
> {noformat}
> -- 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)
> {noformat}
> h4. ( 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.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)