Community
Participate
Working Groups
+++ 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.
For clarification I'd like to rephrase the issue: (1) A thread notifying a resource change can hold the lock on preferences: CModelManager.resourceChanged(IResourceChangeEvent) line: 892 NotificationManager$1.run() line: 291 SafeRunner.run(ISafeRunnable) line: 42 NotificationManager.notify(...) line: 285 NotificationManager.broadcastChanges(...) line: 149 Workspace.broadcastPostChange() line: 395 Workspace.endOperation(...) line: 1530 Workspace.run(...) line: 2353 ProjectPreferences.save() line: 628 ProjectPreferences(EclipsePreferences).flush() line: 353 ProjectPreferences.flush() line: 380 (2) The same lock is required when accessing a node in the preferences: ProjectPreferences(EclipsePreferences).getChild(...) line: 400 ProjectPreferences(EclipsePreferences).internalNode(...) line: 542 ProjectPreferences(EclipsePreferences).node(String) line: 670 With that CDT is lured into a deadlock: (3) We use a lock X within the resource notification. (4) At a different place when owning X we access a node in the preferences. (1)+(3): Locks preferences, locks X. (4)+(2): Locks X, locks preferences. ==> With a bad timing this dead-locks.
Fixed in 3.7.x branch. http://git.eclipse.org/c/equinox/rt.equinox.bundles.git/commit/?id=2607f050eb20c418a2b56437cdb388a397ebc207
CQ:WIND00309400 When importing the tree from the git R3_7_maintenance branch, I noticed that the bundle version has not been updated yet. It would be good to get the version updated for upcoming M-builds towards 3.7.2.
Yep, there are a couple Equinox and p2 bundles that need incrementing. We had some issues with releasing things this week so I'm going to increment all the bundles at the same time once we get an accurate comparator result from the tests.
Hi, it looks like the patch is NOT in 3.7.2, see bug 375601 comment 4
Commit f81da88c2918758cb73695b3ebd3931670fcfb7a added the synchronize back and some subclasses call the internalFlush to prevent the dead lock but InstancePreferences can still deadlock.
Ah, sorry, I mixed left/right sides.
