Bug 359851

Description DJ Houghton 2011-10-04 11:01:49 EDT

Cloned to track release into master.

+++ This bug was initially created as a clone of Bug #359698 +++

+++ This bug was initially created as a clone of Bug #333726 +++

Build ID: Eclipse 3.7.1

The change from bug 333726 introduces a "synchronized" block for the
EclipsePreferences#flush() method. Bug 359485 has a detailed stack trace and analysis why this change causes CDT to deadlock. 

The problem is that from inside the (now synchronized) flush() method, a number of protected methods are called. These "open calls" bear the risk of deadlock. In this case, ProjectPreferences#save() extends EclipsePreferences, and from inside its save() method may perform a Thread switch to update the workspace, which may in turn call resource listeners. Those Thread switches occur with the synchronized block still holding a lock, causing CDT (and potentially others) to deadlock.

I think there are 2 problems with the current situation:

1. Given that there are Open Calls from inside the flush() method, a
   "synchronized" statement on method level is not appropriate (as any 
   book on Java Concurrency will confirm). A different way should be found
   to fix the problem from bug 333726.

2. ProjectPreferences extends EclipsePreferences, thus violating the API 
   boundary and "hiding" the fact that there are Open Calls. I'm wondering
   whether we could either (a) avoid that kind of non-API usage or 
   (b) improve comments or tooling to make it more apparent that protected
   method calls from EclipsePreferences are in fact Open Calls.

   In fact, ProfilePreferences in equinox.p2 has the same problem -- it also
   schedules a SaveJob from inside its save() method thus causing Thread
   Switch and potential deadlock.