Stop the SWT thread calling clear children in the teiid view

* A deadlock is possible when the refresh thread is trying to display the master password dialog (via the SWT thread) and the SWT thread is trying to clear the current TeiidResourceNode's children (blocked by the synchronized blocks). * Rather than the SWT thread doing any clearing at all, set a simple flag instead, causing the node to have a reload job scheduled instead. That way the node is cleared before reload but on the reload job thread instead.
Teiid-Designer · Dec 7, 2012 · f857330 · f857330
1 parent 96a5bdc
commit f857330
Show file tree

Hide file tree

Showing 6 changed files with 38 additions and 23 deletions.
diff --git a/...d.designer.dqp.ui/src/org/teiid/designer/runtime/ui/views/TeiidServerContentProvider.java b/...d.designer.dqp.ui/src/org/teiid/designer/runtime/ui/views/TeiidServerContentProvider.java
@@ -348,6 +348,7 @@ public Object[] getChildren( Object parentElement ) {
 
         if (parentElement instanceof IServer) {
             ITeiidResourceNode node = TeiidResourceNode.getInstance((IServer) parentElement, this);
+            node.setDirty();
             return new Object[] { node };
 
         } else if (parentElement instanceof ITeiidContainerNode) {

diff --git a/....designer.dqp.ui/src/org/teiid/designer/runtime/ui/views/content/AbstractTeiidFolder.java b/....designer.dqp.ui/src/org/teiid/designer/runtime/ui/views/content/AbstractTeiidFolder.java
@@ -64,6 +64,8 @@ public TeiidServerContainerNode getContainer() {
 
     @Override
     public void load() {
+        clearChildren();
+
         if (getTeiidServer() == null || !getTeiidServer().isConnected()) {
             return;
         }
@@ -85,8 +87,7 @@ public boolean hasChildren() {
         return children != null && ! children.isEmpty();
     }
 
-    @Override
-    public void clearChildren() {
+    private void clearChildren() {
         if (children != null) {
             for (ITeiidContentNode<? extends ITeiidContainerNode<?>> child : children) {
                 child.dispose();

diff --git a/....designer.dqp.ui/src/org/teiid/designer/runtime/ui/views/content/ITeiidContainerNode.java b/....designer.dqp.ui/src/org/teiid/designer/runtime/ui/views/content/ITeiidContainerNode.java
@@ -32,11 +32,4 @@ public interface ITeiidContainerNode<T extends ITeiidContainerNode<?>> extends I
      */
     void load();
 
-    /**
-     * Clears the children of this container. After this method has been
-     * invoked, getChildren() must return null. This method is called when a
-     * refresh is requested.
-     */
-    void clearChildren();
-
 }
diff --git a/...d.designer.dqp.ui/src/org/teiid/designer/runtime/ui/views/content/ITeiidResourceNode.java b/...d.designer.dqp.ui/src/org/teiid/designer/runtime/ui/views/content/ITeiidResourceNode.java
@@ -23,4 +23,10 @@ public interface ITeiidResourceNode extends ITeiidContainerNode {
      */
     TeiidServer getTeiidServer();
 
+    /**
+     * Set the node's children to be dirty, implying a refresh
+     * is necessary
+     */
+    void setDirty();
+
 }
diff --git a/...id.designer.dqp.ui/src/org/teiid/designer/runtime/ui/views/content/TeiidResourceNode.java b/...id.designer.dqp.ui/src/org/teiid/designer/runtime/ui/views/content/TeiidResourceNode.java
@@ -35,6 +35,8 @@ public class TeiidResourceNode extends TeiidContentNode implements ITeiidResourc
     private TeiidServer teiidServer;
 
     private TeiidErrorNode error;
+
+    private boolean dirty = true;
 
     /**
      * @param server
@@ -48,10 +50,6 @@ public static ITeiidResourceNode getInstance(IServer server, TeiidServerContentP
         if (node == null) {
             node = new TeiidResourceNode(server, provider);
             nodeCache.put(key, node); 
-        } else {
-            // Existing node but children may be out of date
-            // Remove children so they are refreshed
-            node.clearChildren();
         }
 
         return node; 
@@ -67,9 +65,23 @@ private TeiidResourceNode(IServer server, TeiidServerContentProvider provider) {
         super(server, DqpUiConstants.UTIL.getString(TeiidResourceNode.class.getSimpleName() + ".label")); //$NON-NLS-1$
         this.provider = provider;
     }
+
+    @Override
+    public void setDirty() {
+        dirty = true;
+    }
 
     @Override
     public final List<? extends ITeiidContentNode<?>> getChildren() {
+        if (dirty) {
+            /* 
+             * node flagged as dirty so the children are out-of-date. Avoid
+             * returning them thereby making the content provider reload 
+             * them.
+             */
+            return null;
+        }
+
         if (error != null) {
             return Collections.singletonList(error);
         }
@@ -79,8 +91,11 @@ public final List<? extends ITeiidContentNode<?>> getChildren() {
 
     @Override
     public final void load() {
+        clearChildren();
+
         if (getServer().getServerState() != IServer.STATE_STARTED) {
             setError(new TeiidErrorNode(this, null, DqpUiConstants.UTIL.getString(getClass().getSimpleName() + ".labelNotConnected"))); //$NON-NLS-1$
+            dirty = false;
             return;
         }
 
@@ -105,12 +120,13 @@ public final void load() {
                 DqpUiConstants.UTIL.log(e);
                 setError(new TeiidErrorNode(this, teiidServer, DqpUiConstants.UTIL.getString(getClass().getSimpleName()
                                                                                              + ".labelRetrievalError"))); //$NON-NLS-1$
+            } finally {
+                dirty = false;
             }
         }
     }
 
-    @Override
-    public final void clearChildren() {
+    private void clearChildren() {
         synchronized (provider) {
             clearError();
             if (children != null) {
@@ -147,17 +163,14 @@ public void dispose() {
     /**
      * @return the teiidServer
      */
+    @Override
     public TeiidServer getTeiidServer() {
         return this.teiidServer;
     }
 
-    /**
-     * Does this node have any children
-     * 
-     * @return true if there are children.
-     */
+    @Override
     public boolean hasChildren() {
-        return children != null && ! children.isEmpty();
+        return dirty || (children != null && ! children.isEmpty());
     }
 
     @Override

diff --git a/...gner.dqp.ui/src/org/teiid/designer/runtime/ui/views/content/TeiidServerContainerNode.java b/...gner.dqp.ui/src/org/teiid/designer/runtime/ui/views/content/TeiidServerContainerNode.java
@@ -64,8 +64,7 @@ public final List<? extends ITeiidContentNode<?>> getChildren() {
         return children;
     }
 
-    @Override
-    public final void clearChildren() {
+    private void clearChildren() {
         clearError();
 
         if (children != null) {
@@ -90,6 +89,8 @@ public void dispose() {
 
     @Override
     public final void load() {
+        clearChildren();
+
         if (getServer().getServerState() != IServer.STATE_STARTED) {
             setError(new TeiidErrorNode(this, teiidServer, DqpUiConstants.UTIL.getString(TeiidServerContainerNode.class.getSimpleName() + "ServerContentLabelNotConnected"))); //$NON-NLS-1$
             return;