Fixed Issue 81: SimpleUniverse.cleanup() doesn't dispose of all threads

Fixed Issue 83: Deadlock when removing Canvas3D and universe


git-svn-id: https://svn.java.net/svn/j3d-core~svn/trunk@76 ba19aa83-45c5-6ac9-afd3-db810772062c

2 files changed, 42 insertions, 13 deletions

diff --git a/src/classes/share/javax/media/j3d/MasterControl.java b/src/classes/share/javax/media/j3d/MasterControl.java
index 798d308..a3c90dd 100644
--- a/src/classes/share/javax/media/j3d/MasterControl.java
+++ b/src/classes/share/javax/media/j3d/MasterControl.java
@@ -896,7 +896,7 @@ class MasterControl {
 	}
 	else {
 	    if (canvasBitCount > 31) {
-		throw new InternalError();
+		throw new RuntimeException("Cannot render to more than 32 Canvas3Ds");
 	    }
 	    return (1 << canvasBitCount++);
 	}
@@ -2948,8 +2948,22 @@ class MasterControl {
 	    } else if (type == PHYSICAL_ENV_CHANGE) {
 		evaluatePhysicalEnv((View) o);
 	    } else if (type == EMPTY_UNIVERSE) {
-		if (views.isEmpty()) {
-		    destroyUniverseThreads((VirtualUniverse) o);
+		// Issue 81: We need to process this message as long
+		// as there are no views associated with this
+		// universe. Previously, this message was ignored if
+		// there were views associated with *any* universe,
+		// which led to a memory / thread leak.
+	        boolean foundView = false;
+		VirtualUniverse univ = (VirtualUniverse) o;
+		View v[] = (View []) views.toArray(false);
+		for (int j = views.size() - 1; j >= 0; j--) {
+		    if (v[j].universe == univ) {
+			foundView = true;
+			break;
+		    }
+		}
+		if (!foundView) {
+		    destroyUniverseThreads(univ);
 		    threadListsChanged = true;
 		}
 	    } else if (type == START_RENDERER) {
diff --git a/src/classes/share/javax/media/j3d/View.java b/src/classes/share/javax/media/j3d/View.java
index 62bed3d..b4684a7 100644
--- a/src/classes/share/javax/media/j3d/View.java
+++ b/src/classes/share/javax/media/j3d/View.java
@@ -16,6 +16,8 @@ import javax.vecmath.*;
 import java.lang.Math;
 import java.util.Vector;
 import java.util.ArrayList;
+import java.util.LinkedList;
+import java.util.Iterator;
 import java.util.Enumeration;
 import java.awt.*;
 import java.awt.event.*;
@@ -2312,6 +2314,8 @@ public class View extends Object {
      * @since Java 3D 1.3
      */
     public void removeAllCanvas3Ds() {
+	LinkedList tmpCanvases = new LinkedList();
+
 	synchronized(canvasList) {
 	    int numCanvases = canvases.size();
 
@@ -2321,23 +2325,34 @@ public class View extends Object {
 
 		cv = (Canvas3D) canvases.elementAt(index);
 
+		// Record list of canvases to be deleted;
+		tmpCanvases.add(cv);
+
 		canvases.removeElementAt(index);
 		removeFromCanvasList(cv);
 		canvasesDirty = true;
+	    }
+	}
 
-		// reset canvas will set view to null also
-		VirtualUniverse.mc.postRequest(MasterControl.RESET_CANVAS, 
-                                      cv);
-		cv.pendingView = null;
+	// ISSUE 83: postRequest must *not* be called while holding
+	// canvasList lock. Holding the lock can cause a deadlock.
 
-		if (cv.added) {
-		    cv.active = false;
-		}
-	    }
+	Iterator iterator = tmpCanvases.iterator();
+	while (iterator.hasNext()) {
+	    Canvas3D cv = (Canvas3D)iterator.next();
 
-	    computeCanvasesCached();
+	    // reset canvas will set view to null also
+	    VirtualUniverse.mc.postRequest(MasterControl.RESET_CANVAS, 
+					   cv);
+	    cv.pendingView = null;
+
+	    if (cv.added) {
+		cv.active = false;
+	    }
 	}
 
+	computeCanvasesCached();
+
 	evaluateActive();
 
 	if (universe != null) {
@@ -2387,7 +2402,7 @@ public class View extends Object {
 	}
     }
 
-    // Locks are already acquired before this is called.
+    // Locks are not acquired before this is called.
     void computeCanvasesCached() {
 
         synchronized (canvasList) {