Community
Participate
Working Groups
Build Identifier: M20100909-0800 org.eclipse.jst.jsf.context.AbstractDelegatingFactory, org.eclipse.jst.jsf.context.resolver.structureddocument.internal.impl.StructuredDocumentContextResolverFactory and org.eclipse.jst.jsf.context.structureddocument.internal.impl.StructuredDocumentContextFactory all access the _delegates protected member in AbstractDelegatingFactory. This member is a CopyOnwriteArrayList but still requires some synchronization due to a required sort after each add. The abstract class does not synchronize around a remove call, and the implementation classes synchronize too long when they only need to synchronize around the call to retrieve an iterator. Additionally the sync block in StructuredDocumentContextResolverFactory.delegateGetTaglibContextResolver holds the monitor while calling code getTaglibContextResolver() which can indirectly call JobManager.yeildRule() in ModelManagerImpl. This results in deadlock prone code as another job may acquire the rule and then attempt to access the resolver factory _delegates object. This is one of several deadlocks we have been experiencing due to calls to JobManager.yieldRule(). Reproducible: Sometimes
Here is an excerpted stack that shows the call to yield rule while inside of a synchronized block which starts in StructuredDocumentContextResolverFactory.delegateGetTaglibContextResolver at org.eclipse.core.internal.jobs.ThreadJob.waitForRun(ThreadJob.java:270) at org.eclipse.core.internal.jobs.ThreadJob.joinRun(ThreadJob.java:199) at org.eclipse.core.internal.jobs.JobManager.yieldRule(JobManager.java:1398) at org.eclipse.core.internal.jobs.InternalJob.yieldRule(InternalJob.java:600) at org.eclipse.core.runtime.jobs.Job.yieldRule(Job.java:709) at org.eclipse.wst.sse.core.internal.model.ModelManagerImpl$SharedObject.waitForLoadAttempt(ModelManagerImpl.java:139) at org.eclipse.wst.sse.core.internal.model.ModelManagerImpl.getExistingModel(ModelManagerImpl.java:1137) at org.eclipse.wst.sse.core.internal.model.ModelManagerImpl.getExistingModelForRead(ModelManagerImpl.java:1252) at org.eclipse.jst.jsf.context.resolver.structureddocument.internal.impl.WorkspaceContextResolver.getPath(WorkspaceContextResolver.java:104) at org.eclipse.jst.jsf.context.resolver.structureddocument.internal.impl.WorkspaceContextResolver.getProject(WorkspaceContextResolver.java:47) at org.eclipse.jst.jsf.context.resolver.structureddocument.internal.impl.WorkspaceContextResolver.getResource(WorkspaceContextResolver.java:80) at oracle.eclipse.tools.common.services.util.OEPEStructuredContextResolverFactory.getTaglibContextResolver(OEPEStructuredContextResolverFactory.java:63) at org.eclipse.jst.jsf.context.resolver.structureddocument.internal.impl.StructuredDocumentContextResolverFactory.delegateGetTaglibContextResolver(StructuredDocumentContextResolverFactory.java:263) at org.eclipse.jst.jsf.context.resolver.structureddocument.internal.impl.StructuredDocumentContextResolverFactory.getTaglibContextResolver(StructuredDocumentContextResolverFactory.java:222)
Created attachment 183435 [details] Suggested Patch
Thanks for the patch Andrew. Appreciate the contributions. Looks good. Raghu, can we request PMC approval for the new protected method AbstractDelegatingFactory.getDelegatesIterator(). It's highly unlikely any adopters subclassing AbstractDelegatingFactory would have this method but since it is a change, lets check that it is not an API issue for 3.2.3. Otherwise, we can release this for 3.3
* Explain why you believe this is a stop-ship defect. Or, if it is a "hotbug" (requested by an adopter) please document it as such. Requesting PMC approval for an API change in the maintenance release that we believe will have minimal impact on adopters. This change is essential to fix some of the deadlocks we have seen in the adopter product. This is a stop-ship and a hot-bug request. Adopter requesting the fix: Oracle. * Is there a work-around? If so, why do you believe the work-around is insufficient? No workaround * How has the fix been tested? Is there a test case attached to the bugzilla record? Has a JUnit Test been added? No Junit test. Manual testing. * Give a brief technical overview. Who has reviewed this fix? See description and comment 3. Reviewed by Carling Rogers. * What is the risk associated with this fix? Medium
The patch from Andrew McCulloch has been committed. Thanks for the contribution Andrew!
