Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 312504

Summary: Deadlock in ResourceSetWorkbenchSynchronizer
Product: [WebTools] WTP Java EE Tools Reporter: Jason Sholl <jsholl>
Component: jst.j2eeAssignee: Jason Sholl <jsholl>
Status: RESOLVED FIXED QA Contact: Chuck Bridgham <cbridgha>
Severity: major    
Priority: P3 CC: neil.hauge
Version: 3.2Flags: jsholl: pmc_approved? (david_williams)
jsholl: pmc_approved? (raghunathan.srinivasan)
jsholl: pmc_approved? (naci.dai)
jsholl: pmc_approved? (deboer)
neil.hauge: pmc_approved+
jsholl: pmc_approved? (kaloyan)
cbridgha: review+
Target Milestone: 3.2 RC1   
Hardware: PC   
OS: Windows XP   
Whiteboard: PMC_approved
Attachments:
Description Flags
stack traces
none
patch none

Description Jason Sholl CLA 2010-05-11 16:27:03 EDT 
The deadlock is because of synchronization method around the addExtender() and notification methods.  I hit this in debug mode and then attempted to fix it while I was in debug mode (which is why the stacks are out of date).  The following two threads are attempting to load and unload the same resource at the same time.  In order for this to proceed, first the load needs to finish and then the unload needs to occur.

The ModalContext thread is performing the unload and is holding the ResourceSetWorkbenchSynchronizer lock while it waits for the ResourceSyncrhonizedIsLoadAdapter.loadingLock.  The Main thread is performing the load and has the ResourceSyncrhonizedIsLoadAdapter.loadingLock but needs the ResourceSetWorkbenchSynchronizer lock in order to complete the load.

The only reason a ResourceSetWorkbenchSynchronizer is necessary is to protect the set of extenders during notifications and adds.  The fix is to remove this lock and build the protection directly into the Set.  This protect works as follows:  before iterating for notification, the set is put in queuing mode.  While in queuing mode, modifications will not be pushed down to the set but instead held onto temporarily.  After iterating for notification, the set is taken out of queuing mode.  Any additional adds are then committed to the set and then iterated over in the same fashion (put the set in queuing mode, perform the iteration, and then check to see if anything else came in).

We decided to go with this new queuing mechanism rather than other constructs like listener lists or making copies during the notification because the notificaiton methods are called very frequently and a copy would be too slow.  Also, now with the synchronization removed from ResourceSetWorkbenchSynchronizer special care needs to be taken to ensure no event notifications are lost in the event that an add occurs during a notification.  This new mechanism will protect against these scenarios while still avoiding the deadlock.
Comment 1 Jason Sholl CLA 2010-05-11 16:27:51 EDT 
Created attachment 168029 [details]
stack traces
Comment 2 Jason Sholl CLA 2010-05-11 16:31:24 EDT 
Created attachment 168031 [details]
patch
Comment 3 Chuck Bridgham CLA 2010-05-12 12:59:19 EDT 
approved
Comment 4 Jason Sholl CLA 2010-05-12 15:56:47 EDT 
    * 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. 

Deadlock; need I say more?

    * Is there a work-around? If so, why do you believe the work-around is insufficient? 

No

    * How has the fix been tested? Is there a test case attached to the bugzilla record? Has a JUnit Test been added? 

This has been tested via UI in adopter product and via UI and JUnits in WTP; the entire WTP JUnit suite has run successfully with this fix.

    * Give a brief technical overview. Who has reviewed this fix? 

See initial comments

    * What is the risk associated with this fix? 

Minimal.
Comment 5 Jason Sholl CLA 2010-05-12 18:48:14 EDT 
code checked into head for wtp 32 rc1