FIXED - task 559: Deadlock between autosave thread and history code

http://code.google.com/p/google-refine/issues/detail?id=559

git-svn-id: http://google-refine.googlecode.com/svn/trunk@2538 7d457c2a-affb-35e4-300a-418c747d4874

main/src/com/google/refine/history/History.java

@@ -127,8 +127,13 @@ public class History implements Jsonizable {
      * Adding a new entry clears all currently held future histories
      * @param entry
      */
-    synchronized public void addEntry(HistoryEntry entry) {
+    public void addEntry(HistoryEntry entry) {
-        entry.apply(ProjectManager.singleton.getProject(_projectID));
+        Project project = ProjectManager.singleton.getProject(_projectID);
+        synchronized (project) {
+            // NOTE: project lock must be acquired *first* to prevent deadlocks, so we use a 
+            // synchronized block instead of synchronizing the entire method.
+            synchronized (this) {
+                entry.apply(project);
                 _pastEntries.add(entry);
 
                 setModified();
@@ -146,6 +151,8 @@ public class History implements Jsonizable {
                     }
                 }
             }
+        }
+    }
 
     protected void setModified() {
         ProjectManager.singleton.getProjectMetadata(_projectID).updateModified();
@@ -273,6 +280,12 @@ public class History implements Jsonizable {
         writer.endObject();
     }
 
+    /*
+     * NOTE: this method is called from the autosave thread with the Project
+     * lock already held, so no other synchronized method here can aquire that
+     * Project lock or a deadlock will result.be careful of thread synchronization to avoid
+     * deadlocks.
+     */
     synchronized public void save(Writer writer, Properties options) throws IOException {
         writer.write("pastEntryCount="); writer.write(Integer.toString(_pastEntries.size())); writer.write('\n');
         for (HistoryEntry entry : _pastEntries) {

main/src/com/google/refine/history/HistoryEntry.java

@@ -119,6 +119,12 @@ public class HistoryEntry implements Jsonizable {
         _manager.save(this, writer, options);
     }
 
+    /**
+     * Apply a change to a project.  In most cases you should already hold the Project lock
+     * before calling this method to prevent deadlocks.
+     * 
+     * @param project the project the change should be applied to
+     */
     public void apply(Project project) {
         if (getChange() == null) {
             ProjectManager.singleton.getHistoryEntryManager().loadChange(this);