Request for review: 6597112: Referential integrity loophole during remote object export
Neil Richards
neil.richards at ngmr.net
Thu Mar 24 20:54:31 UTC 2011
Hi all,
Can I encourage further review / comment on my suggested fix (and
testcase) for this problem [1] [2], please ?
For reference, I include the 'hg export' for the latest version of the
change below,
Any feedback gratefully received,
Thanks,
Neil
PS: I see the bug details [2] are not currently externally visible, but
don't know of a good reason for this. Could this be reviewed, please?
[1] http://mail.openjdk.java.net/pipermail/core-libs-dev/2011-March/006144.html
[2] http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6597112
--
Unless stated above:
IBM email: neil_richards at uk.ibm.com
IBM United Kingdom Limited - Registered in England and Wales with number 741598.
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
# HG changeset patch
# User Neil Richards <neil.richards at ngmr.net>, <neil_richards at uk.ibm.com>
# Date 1298369045 0
# Branch j6597112
# Node ID 0c22e02a9016bd340e2182e96a10fe96cd75c8ac
# Parent 6bbc7a4734952ae7604578f270e1566639fa8752
6597112: referential integrity loophole during remote object export
Summary: Eliminate weak ref GC window during RMI export
Contributed-by: <neil.richards at ngmr.net>
diff --git a/src/share/classes/sun/rmi/server/UnicastServerRef.java b/src/share/classes/sun/rmi/server/UnicastServerRef.java
--- a/src/share/classes/sun/rmi/server/UnicastServerRef.java
+++ b/src/share/classes/sun/rmi/server/UnicastServerRef.java
@@ -204,7 +204,11 @@
Target target =
new Target(impl, this, stub, ref.getObjID(), permanent);
- ref.exportObject(target);
+ try {
+ ref.exportObject(target);
+ } finally {
+ target.unpinImpl();
+ }
hashToMethod_Map = hashToMethod_Maps.get(implClass);
return stub;
}
diff --git a/src/share/classes/sun/rmi/transport/Target.java b/src/share/classes/sun/rmi/transport/Target.java
--- a/src/share/classes/sun/rmi/transport/Target.java
+++ b/src/share/classes/sun/rmi/transport/Target.java
@@ -77,11 +77,11 @@
* Construct a Target for a remote object "impl" with
* a specific object id.
*
- * If "permanent" is true, then the impl is pinned permanently
- * (the impl will not be collected via distributed and/or local
- * GC). If "on" is false, than the impl is subject to
- * collection. Permanent objects do not keep a server from
- * exiting.
+ * Upon return, the impl is pinned, thus not initially eligible
+ * for collection via distributed and/or local GC).
+ * If "permanent" is false, the impl subsequently may be made subject to
+ * collection by calling {@link #unpinImpl()}.
+ * Permanent or pinned objects do not keep a server from exiting.
*/
public Target(Remote impl, Dispatcher disp, Remote stub, ObjID id,
boolean permanent)
@@ -114,9 +114,6 @@
}
this.permanent = permanent;
- if (permanent) {
- pinImpl();
- }
}
/**
@@ -212,7 +209,7 @@
* can be garbage collected locally. But only if there the refSet
* is empty. All of the weak/strong handling is in WeakRef
*/
- synchronized void unpinImpl() {
+ public synchronized void unpinImpl() {
/* only unpin if:
* a) impl is not permanent, and
* b) impl is not already unpinned, and
diff --git a/src/share/classes/sun/rmi/transport/WeakRef.java b/src/share/classes/sun/rmi/transport/WeakRef.java
--- a/src/share/classes/sun/rmi/transport/WeakRef.java
+++ b/src/share/classes/sun/rmi/transport/WeakRef.java
@@ -33,7 +33,7 @@
*
* This class extends the functionality of java.lang.ref.WeakReference in
* several ways. The methods pin() and unpin() can be used to set
- * whether the contained reference is strong or weak (it is weak upon
+ * whether the contained reference is strong or weak (it is strong upon
* construction). The hashCode() and equals() methods are overridden so
* that WeakRef objects hash and compare to each other according to the
* object identity of their referents.
@@ -51,18 +51,24 @@
/**
* Create a new WeakRef to the given object.
+ * The given object is initially pinned, ie. referenced strongly.
+ * It may be subsequently unpinned by calling {@link #unpin()}.
*/
public WeakRef(Object obj) {
super(obj);
setHashValue(obj); // cache object's "identity" hash code
+ strongRef = obj;
}
/**
* Create a new WeakRef to the given object, registered with a queue.
+ * The given object is initially pinned, ie. referenced strongly.
+ * It may be subsequently unpinned by calling {@link #unpin()}.
*/
public WeakRef(Object obj, ReferenceQueue q) {
super(obj, q);
setHashValue(obj); // cache object's "identity" hash code
+ strongRef = obj;
}
/**
diff --git a/test/java/rmi/server/UnicastRemoteObject/gcDuringExport/GcDuringExport.java b/test/java/rmi/server/UnicastRemoteObject/gcDuringExport/GcDuringExport.java
new file mode 100644
--- /dev/null
+++ b/test/java/rmi/server/UnicastRemoteObject/gcDuringExport/GcDuringExport.java
@@ -0,0 +1,67 @@
+/*
+ * Copyright (c) 2011 Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * Portions Copyright (c) 2011 IBM Corporation
+ */
+
+/*
+ * @test
+ * @bug 6597112
+ * @summary Objects should not be GC-able as they are being exported to RMI
+ * @author Neil Richards <neil.richards at ngmr.net>, <neil_richards at uk.ibm.com>
+ */
+
+import java.rmi.Remote;
+import java.rmi.server.UnicastRemoteObject;
+
+public class GcDuringExport {
+ private static final long MAX_EXPORT_ITERATIONS = 50000;
+
+ public static void main(String[] args) throws Exception {
+ final Thread gcInducingThread = new Thread() {
+ public void run() { while (true) {
+ System.gc();
+ try { Thread.sleep(1); } catch (InterruptedException e) { }
+ }
+ }
+ };
+
+ gcInducingThread.setDaemon(true);
+ gcInducingThread.start();
+
+ long i = 0;
+ try {
+ while (i < MAX_EXPORT_ITERATIONS) {
+ i++;
+ UnicastRemoteObject.exportObject(new Remote() { }, 0);
+ }
+ } catch (Throwable e) {
+ throw new RuntimeException("Test FAILED on iteration " + i + ".", e);
+ }
+
+ System.out.println("Test successfully exported " + i + " objects.");
+ }
+}
+
+
More information about the core-libs-dev
mailing list