river-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From peter_firmst...@apache.org
Subject svn commit: r1452827 - /river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/jeri/internal/runtime/ImplRefManager.java
Date Tue, 05 Mar 2013 14:57:42 GMT
Author: peter_firmstone
Date: Tue Mar  5 14:57:42 2013
New Revision: 1452827

URL: http://svn.apache.org/r1452827
Log:
Found synchronization issue in JERI DGC, may be related to

com/sun/jini/test/spec/javaspace/conformance/snapshot/SnapshotExpirationNotifyTest.td

test failure due to events not being received, caused by no object in table.

No guarantee, since I don't have access to hardware where this fails.

Modified:
    river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/jeri/internal/runtime/ImplRefManager.java

Modified: river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/jeri/internal/runtime/ImplRefManager.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/jeri/internal/runtime/ImplRefManager.java?rev=1452827&r1=1452826&r2=1452827&view=diff
==============================================================================
--- river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/jeri/internal/runtime/ImplRefManager.java
(original)
+++ river/jtsk/skunk/qa_refactor/trunk/src/com/sun/jini/jeri/internal/runtime/ImplRefManager.java
Tue Mar  5 14:57:42 2013
@@ -160,10 +160,10 @@ final class ImplRefManager {
 	private boolean removed = false;
 
 	/** targets for all exports of referenced impl; guarded by "this" */
-	private final Set targets = new HashSet(1);
+	private final Set<Target> targets = new HashSet<Target>(1);
 
 	/** targets that have pinned this reference; guarded by "this"  */
-	private final Set pinningTargets = new HashSet(1);
+	private final Set<Target> pinningTargets = new HashSet<Target>(1);
 
 	/** strong reference to impl, when pinned; guarded by "this" */
 	private Remote strongRef = null;
@@ -265,20 +265,20 @@ final class ImplRefManager {
 	    assert targets.contains(target);
 	    if (strongRef instanceof Unreferenced) {
 		final Unreferenced obj = (Unreferenced) strongRef;
-		final Thread t = (Thread) AccessController.doPrivileged(
-		    new NewThreadAction(new Runnable() {
-			public void run() {
-			    SecurityContext securityContext =
-				target.getSecurityContext();
-			    AccessController.doPrivileged(securityContext.wrap(
-				new PrivilegedAction() {
-				    public Object run() {
-					obj.unreferenced();
-					return null;
-				    }
-				}), securityContext.getAccessControlContext());
-			}
-		    }, "Unreferenced", false, true));
+		final Thread t = AccessController.doPrivileged(
+           new NewThreadAction(new Runnable() {
+               public void run() {
+                   SecurityContext securityContext =
+                       target.getSecurityContext();
+                   AccessController.doPrivileged(securityContext.wrap(
+                       new PrivilegedAction() {
+                           public Object run() {
+                               obj.unreferenced();
+                               return null;
+                           }
+                       }), securityContext.getAccessControlContext());
+               }
+           }, "Unreferenced", false, true));
 		AccessController.doPrivileged(new PrivilegedAction() {
 		    public Object run() {
 			t.setContextClassLoader(
@@ -431,7 +431,7 @@ final class ImplRefManager {
 		    break;	// pass away if interrupted
 		}
 
-		Set collectedTargets;
+		Set<Target> collectedTargets;
 		synchronized (lock) {
 		    /*
 		     * Prevent interrupts and clear interrupted state
@@ -447,10 +447,13 @@ final class ImplRefManager {
 		    if (implRef == null) {
 			continue; // may have been removed via unexport
 		    }
-		    assert !implRef.removed;
+		    
 		    synchronized (implRef) {
+                        assert !implRef.removed; // originally checked outside of sync, changed
6th March 2013
 			assert !implRef.isPinned();
-			collectedTargets = implRef.targets;
+                        // Copy the Targets! Originally access was unsynchronized.
+			collectedTargets = new HashSet<Target>(implRef.targets.size());
+                        collectedTargets.addAll(implRef.targets);
 			implRef.remove();
 
 			if (logger.isLoggable(Level.FINEST)) {
@@ -464,8 +467,11 @@ final class ImplRefManager {
 		}
 
 		// notify targets without holding any locks
-		for (Iterator i = collectedTargets.iterator(); i.hasNext();) {
-		    ((Target) i.next()).collect();
+                // Why?  This is bad, is it to avoid deadlock?
+                // We should at least copy first while synchronized.
+                // Changed 6th March 2013
+		for (Iterator<Target> i = collectedTargets.iterator(); i.hasNext();) {
+		    i.next().collect();
 		}
 
 	    } while (true);



Mime
View raw message