[406352] fix deadlock in JpaProjectPropertiesPage - change

JpaFacetVersionChangeDataModelProvider to get default values from
preferences (not from the JPA project)

7 files changed, 108 insertions, 82 deletions

diff --git a/common/plugins/org.eclipse.jpt.common.ui/src/org/eclipse/jpt/common/ui/internal/properties/JptProjectPropertiesPage.java b/common/plugins/org.eclipse.jpt.common.ui/src/org/eclipse/jpt/common/ui/internal/properties/JptProjectPropertiesPage.java
index bb85486bf1..373833d6a6 100644
--- a/common/plugins/org.eclipse.jpt.common.ui/src/org/eclipse/jpt/common/ui/internal/properties/JptProjectPropertiesPage.java
+++ b/common/plugins/org.eclipse.jpt.common.ui/src/org/eclipse/jpt/common/ui/internal/properties/JptProjectPropertiesPage.java
@@ -55,7 +55,14 @@ import org.eclipse.wst.common.project.facet.ui.internal.FacetsPropertyPage;
 
 
 public abstract class JptProjectPropertiesPage
-		extends LibraryFacetPropertyPage {
+	extends LibraryFacetPropertyPage
+{
+	/**
+	 * It's not clear what this page's lifecycle is (e.g. when {@link
+	 * #createPageContents(Composite)} is called and how often it can be called);
+	 * so we must maintain this flag.
+	 */
+	protected volatile boolean engaged = false;
 
 	protected final ModifiablePropertyValueModel<IProject> projectModel;
 	protected final BufferedModifiablePropertyValueModel.Trigger trigger;
@@ -117,9 +124,7 @@ public abstract class JptProjectPropertiesPage
 
 	@Override
 	protected Control createPageContents(Composite parent) {
-		if (this.projectModel.getValue() != null) {
-			this.disengageListeners();
-		}
+		this.disengageListeners(); // not sure why we do this here...
 
 		this.projectModel.setValue(this.getProject());
 
@@ -146,11 +151,25 @@ public abstract class JptProjectPropertiesPage
 	 */
 	protected abstract void createWidgets(Composite parent);
 
-	protected void engageListeners() {
+	protected final void engageListeners() {
+		if ( ! this.engaged) {
+			this.engageListeners_();
+			this.engaged = true;
+		}
+	}
+
+	protected void engageListeners_() {
 		this.engageValidationListener();
 	}
 
-	protected void disengageListeners() {
+	protected final void disengageListeners() {
+		if (this.engaged) {
+			this.disengageListeners_();
+			this.engaged = false;
+		}
+	}
+
+	protected void disengageListeners_() {
 		this.disengageValidationListener();
 	}
 
diff --git a/jaxb/plugins/org.eclipse.jpt.jaxb.core/src/org/eclipse/jpt/jaxb/core/internal/InternalJaxbProjectManager.java b/jaxb/plugins/org.eclipse.jpt.jaxb.core/src/org/eclipse/jpt/jaxb/core/internal/InternalJaxbProjectManager.java
index 8de102fc48..056cc46df2 100644
--- a/jaxb/plugins/org.eclipse.jpt.jaxb.core/src/org/eclipse/jpt/jaxb/core/internal/InternalJaxbProjectManager.java
+++ b/jaxb/plugins/org.eclipse.jpt.jaxb.core/src/org/eclipse/jpt/jaxb/core/internal/InternalJaxbProjectManager.java
@@ -234,7 +234,8 @@ public class InternalJaxbProjectManager
 	// ********** plug-in controlled life-cycle **********
 
 	/**
-	 * Internal: called by {@link JptJaxbCorePlugin Dali plug-in}.
+	 * Internal: called by the {@link InternalJaxbWorkspace JAXB workspace}
+	 * constructor.
 	 */
 	void start() {
 		try {
diff --git a/jaxb/plugins/org.eclipse.jpt.jaxb.ui/src/org/eclipse/jpt/jaxb/ui/internal/properties/JaxbProjectPropertiesPage.java b/jaxb/plugins/org.eclipse.jpt.jaxb.ui/src/org/eclipse/jpt/jaxb/ui/internal/properties/JaxbProjectPropertiesPage.java
index 3877d544f2..ef65a1aa6e 100644
--- a/jaxb/plugins/org.eclipse.jpt.jaxb.ui/src/org/eclipse/jpt/jaxb/ui/internal/properties/JaxbProjectPropertiesPage.java
+++ b/jaxb/plugins/org.eclipse.jpt.jaxb.ui/src/org/eclipse/jpt/jaxb/ui/internal/properties/JaxbProjectPropertiesPage.java
@@ -29,6 +29,7 @@ import org.eclipse.jpt.common.utility.internal.model.value.SortedListValueModelA
 import org.eclipse.jpt.common.utility.internal.model.value.StaticCollectionValueModel;
 import org.eclipse.jpt.common.utility.model.Model;
 import org.eclipse.jpt.common.utility.model.event.PropertyChangeEvent;
+import org.eclipse.jpt.common.utility.model.listener.PropertyChangeAdapter;
 import org.eclipse.jpt.common.utility.model.listener.PropertyChangeListener;
 import org.eclipse.jpt.common.utility.model.value.CollectionValueModel;
 import org.eclipse.jpt.common.utility.model.value.ListValueModel;
@@ -94,14 +95,19 @@ public class JaxbProjectPropertiesPage
 	}
 	
 	private PropertyChangeListener buildPlatformListener(){
-		return new PropertyChangeListener() {
-			public void propertyChanged(PropertyChangeEvent event) {
-				JaxbProjectPropertiesPage.this.platformChanged((JaxbPlatformConfig) event.getNewValue());
-			}
-		};
+		return new PlatformListener();
+	}
+
+	/* CU private */ class PlatformListener
+		extends PropertyChangeAdapter
+	{
+		@Override
+		public void propertyChanged(PropertyChangeEvent event) {
+			JaxbProjectPropertiesPage.this.platformChanged();
+		}
 	}
 	
-	void platformChanged(JaxbPlatformConfig newPlatform) {
+	void platformChanged() {
 		if ( ! this.getControl().isDisposed()) {
 			// handle null, in the case the jpa facet is changed via the facets page,
 			// the library install delegate is temporarily null
@@ -168,15 +174,15 @@ public class JaxbProjectPropertiesPage
 	}
 	
 	@Override
-	protected void engageListeners() {
-		super.engageListeners();
+	protected void engageListeners_() {
+		super.engageListeners_();
 		this.platformModel.addPropertyChangeListener(PropertyValueModel.VALUE, this.platformListener);
 	}
 	
 	@Override
-	public void disengageListeners() {
+	public void disengageListeners_() {
 		this.platformModel.removePropertyChangeListener(PropertyValueModel.VALUE, this.platformListener);
-		super.disengageListeners();
+		super.disengageListeners_();
 	}
 	
 	
@@ -287,7 +293,7 @@ public class JaxbProjectPropertiesPage
 	@Override
 	protected Model[] buildValidationModels() {
 		return new Model[] {
-			platformModel
+			this.platformModel
 		};
 	}
 	
diff --git a/jpa/plugins/org.eclipse.jpt.jpa.core/src/org/eclipse/jpt/jpa/core/internal/facet/JpaFacetDataModelProvider.java b/jpa/plugins/org.eclipse.jpt.jpa.core/src/org/eclipse/jpt/jpa/core/internal/facet/JpaFacetDataModelProvider.java
index 61c92069de..03788c2111 100644
--- a/jpa/plugins/org.eclipse.jpt.jpa.core/src/org/eclipse/jpt/jpa/core/internal/facet/JpaFacetDataModelProvider.java
+++ b/jpa/plugins/org.eclipse.jpt.jpa.core/src/org/eclipse/jpt/jpa/core/internal/facet/JpaFacetDataModelProvider.java
@@ -174,7 +174,7 @@ public abstract class JpaFacetDataModelProvider
 			return getDefaultLibraryProvider();
 		}
 		if (propertyName.equals(CONNECTION)) {
-			return getDefaultConnection();
+			return getDefaultConnectionName();
 		}
 		if (propertyName.equals(CONNECTION_ACTIVE)) {
 			return Boolean.valueOf(this.connectionIsActive());
@@ -252,7 +252,7 @@ public abstract class JpaFacetDataModelProvider
 			};
 	}
 	
-	protected abstract String getDefaultConnection();
+	protected abstract String getDefaultConnectionName();
 	
 	protected abstract Boolean getDefaultUserWantsToOverrideDefaultCatalog();
 	
diff --git a/jpa/plugins/org.eclipse.jpt.jpa.core/src/org/eclipse/jpt/jpa/core/internal/facet/JpaFacetInstallDataModelProvider.java b/jpa/plugins/org.eclipse.jpt.jpa.core/src/org/eclipse/jpt/jpa/core/internal/facet/JpaFacetInstallDataModelProvider.java
index 2de8e27947..a799546839 100644
--- a/jpa/plugins/org.eclipse.jpt.jpa.core/src/org/eclipse/jpt/jpa/core/internal/facet/JpaFacetInstallDataModelProvider.java
+++ b/jpa/plugins/org.eclipse.jpt.jpa.core/src/org/eclipse/jpt/jpa/core/internal/facet/JpaFacetInstallDataModelProvider.java
@@ -90,7 +90,7 @@ public class JpaFacetInstallDataModelProvider
 	}
 	
 	@Override
-	protected String getDefaultConnection() {
+	protected String getDefaultConnectionName() {
 		return null;
 	}
 	
diff --git a/jpa/plugins/org.eclipse.jpt.jpa.core/src/org/eclipse/jpt/jpa/core/internal/facet/JpaFacetVersionChangeDataModelProvider.java b/jpa/plugins/org.eclipse.jpt.jpa.core/src/org/eclipse/jpt/jpa/core/internal/facet/JpaFacetVersionChangeDataModelProvider.java
index 7908414e9a..c27d56238b 100644
--- a/jpa/plugins/org.eclipse.jpt.jpa.core/src/org/eclipse/jpt/jpa/core/internal/facet/JpaFacetVersionChangeDataModelProvider.java
+++ b/jpa/plugins/org.eclipse.jpt.jpa.core/src/org/eclipse/jpt/jpa/core/internal/facet/JpaFacetVersionChangeDataModelProvider.java
@@ -14,119 +14,119 @@ import org.eclipse.core.resources.ResourcesPlugin;
 import org.eclipse.core.runtime.IStatus;
 import org.eclipse.jpt.common.utility.internal.iterable.IterableTools;
 import org.eclipse.jpt.jpa.core.JpaPlatform;
-import org.eclipse.jpt.jpa.core.JpaProject;
+import org.eclipse.jpt.jpa.core.JpaPreferences;
 import org.eclipse.jpt.jpa.core.JptJpaCoreMessages;
 import org.eclipse.jpt.jpa.core.internal.plugin.JptJpaCorePlugin;
+import org.eclipse.jpt.jpa.core.platform.JpaPlatformManager;
 
+/**
+ * See <code>org.eclipse.jpt.jpa.core/plugin.xml:org.eclipse.wst.common.project.facet.core.facets</code>.
+ * <p>
+ * <strong>NB:</strong> We could get the default values from the JPA project
+ * (instead of from the preferences);
+ * but that exposes us to deadlocks (see bug 406352): We have a lock on the
+ * faceted project here and would be waiting for the JPA project to be built;
+ * meanwhile, the JPA project manager is building the JPA project and could be
+ * waiting on a lock on the faceted project....
+ */
 public class JpaFacetVersionChangeDataModelProvider
 	extends JpaFacetDataModelProvider
 {
 	/**
-	 * required default constructor
+	 * default constructor used by extension point
 	 */
 	public JpaFacetVersionChangeDataModelProvider() {
 		super();
 	}
-	
-	
-	protected String getProjectName() {
-		return getStringProperty(FACET_PROJECT_NAME);
-	}
-	
-	protected JpaProject getJpaProject() {
-		try {
-			JpaProject.Reference ref = this.getJpaProjectReference();
-			return (ref == null) ? null : ref.getValue();
-		} catch (InterruptedException ex) {
-			Thread.currentThread().interrupt();
-			return null;
-		}
-	}
 
-	protected JpaProject.Reference getJpaProjectReference() {
-		return (JpaProject.Reference) this.getProject().getAdapter(JpaProject.Reference.class);
-	}
 
-	protected IProject getProject() {
-		return ResourcesPlugin.getWorkspace().getRoot().getProject(this.getProjectName());
-	}
-	
-	
-	// **************** defaults **********************************************
-	
+	// ********** default values **********
+
 	@Override
 	protected JpaPlatform.Config getDefaultPlatformConfig() {
-		return getJpaProject().getJpaPlatform().getConfig();
+		String id = JpaPreferences.getJpaPlatformID(this.getProject());
+		return (id == null) ? null : this.getPlatformConfig(id);
 	}
-	
+
+	protected JpaPlatform.Config getPlatformConfig(String id) {
+		JpaPlatformManager jpaPlatformManager = this.getJpaPlatformManager();
+		return (jpaPlatformManager == null) ? null : jpaPlatformManager.getJpaPlatformConfig(id);
+	}
+
 	@Override
-	protected String getDefaultConnection() {
-		return getJpaProject().getDataSource().getConnectionProfileName();
+	protected String getDefaultConnectionName() {
+		return JpaPreferences.getConnectionProfileName(this.getProject());
 	}
-	
+
 	@Override
 	protected Boolean getDefaultUserWantsToOverrideDefaultCatalog() {
 		return Boolean.valueOf(this.getDefaultUserWantsToOverrideDefaultCatalog_());
 	}
-	
+
 	protected boolean getDefaultUserWantsToOverrideDefaultCatalog_() {
-		return this.getJpaProject().getUserOverrideDefaultCatalog() != null;
+		return this.getDefaultCatalogIdentifier() != null;
 	}
-	
+
 	@Override
 	protected String getDefaultCatalogIdentifier() {
-		return getJpaProject().getUserOverrideDefaultCatalog();
+		return JpaPreferences.getUserOverrideDefaultCatalog(this.getProject());
 	}
-	
+
 	@Override
 	protected Boolean getDefaultUserWantsToOverrideDefaultSchema() {
 		return Boolean.valueOf(this.getDefaultUserWantsToOverrideDefaultSchema_());
 	}
-	
+
 	protected boolean getDefaultUserWantsToOverrideDefaultSchema_() {
-		return this.getJpaProject().getUserOverrideDefaultSchema() != null;
+		return this.getDefaultSchemaIdentifier() != null;
 	}
-	
+
 	@Override
 	protected String getDefaultSchemaIdentifier() {
-		return getJpaProject().getDefaultSchema();
+		return JpaPreferences.getUserOverrideDefaultSchema(this.getProject());
 	}
-	
+
 	@Override
 	protected Boolean getDefaultDiscoverAnnotatedClasses() {
 		return Boolean.valueOf(this.getDefaultDiscoverAnnotatedClasses_());
 	}
-	
+
 	protected boolean getDefaultDiscoverAnnotatedClasses_() {
-		return getJpaProject().discoversAnnotatedClasses();
+		return JpaPreferences.getDiscoverAnnotatedClasses(this.getProject());
+	}
+
+	protected IProject getProject() {
+		return ResourcesPlugin.getWorkspace().getRoot().getProject(this.getProjectName());
+	}
+
+	protected String getProjectName() {
+		return this.getStringProperty(FACET_PROJECT_NAME);
 	}
-	
-	
-	// **************** valid property descriptors ****************************
-	
+
+
+	// ********** valid property descriptors **********
+
 	@Override
 	protected Iterable<JpaPlatform.Config> buildValidPlatformConfigs() {
 		// add existing platform to list of choices
 		Iterable<JpaPlatform.Config> validPlatformConfigs = super.buildValidPlatformConfigs();
-		if (! IterableTools.contains(validPlatformConfigs, getDefaultPlatformConfig())) {
-			validPlatformConfigs = IterableTools.insert(getDefaultPlatformConfig(), validPlatformConfigs);
+		if (! IterableTools.contains(validPlatformConfigs, this.getDefaultPlatformConfig())) {
+			validPlatformConfigs = IterableTools.insert(this.getDefaultPlatformConfig(), validPlatformConfigs);
 		}
 		return validPlatformConfigs;
 	}
-	
-	
-	// **************** validation ********************************************
-	
+
+
+	// ********** validation **********
+
 	@Override
 	protected IStatus validatePlatform() {
 		IStatus status = super.validatePlatform();
-		
 		if (status.isOK()) {
-			if (! getPlatformConfig().supportsJpaFacetVersion(getProjectFacetVersion())) {
+			if (! this.getPlatformConfig().supportsJpaFacetVersion(this.getProjectFacetVersion())) {
 				status = JptJpaCorePlugin.instance().buildErrorStatus(JptJpaCoreMessages.VALIDATE_PLATFORM_DOES_NOT_SUPPORT_FACET_VERSION);
 			}
 		}
-		
 		return status;
 	}
 }
diff --git a/jpa/plugins/org.eclipse.jpt.jpa.ui/src/org/eclipse/jpt/jpa/ui/internal/properties/JpaProjectPropertiesPage.java b/jpa/plugins/org.eclipse.jpt.jpa.ui/src/org/eclipse/jpt/jpa/ui/internal/properties/JpaProjectPropertiesPage.java
index cb5485334e..9bd75b69ff 100644
--- a/jpa/plugins/org.eclipse.jpt.jpa.ui/src/org/eclipse/jpt/jpa/ui/internal/properties/JpaProjectPropertiesPage.java
+++ b/jpa/plugins/org.eclipse.jpt.jpa.ui/src/org/eclipse/jpt/jpa/ui/internal/properties/JpaProjectPropertiesPage.java
@@ -529,17 +529,17 @@ public class JpaProjectPropertiesPage
 	}
 
 	@Override
-	protected void engageListeners() {
-		super.engageListeners();
+	protected void engageListeners_() {
+		super.engageListeners_();
 		this.jpaPlatformConfigModel.addPropertyChangeListener(PropertyValueModel.VALUE, this.jpaPlatformConfigListener);
 		this.disconnectedModel.addPropertyChangeListener(PropertyValueModel.VALUE, this.disconnectedModelListener);
 	}
 
 	@Override
-	protected void disengageListeners() {
+	protected void disengageListeners_() {
 		this.jpaPlatformConfigModel.removePropertyChangeListener(PropertyValueModel.VALUE, this.jpaPlatformConfigListener);
 		this.disconnectedModel.removePropertyChangeListener(PropertyValueModel.VALUE, this.disconnectedModelListener);
-		super.disengageListeners();
+		super.disengageListeners_();
 	}