Request for review: 6597112: Referential integrity loophole during remote object export
Neil Richards
neil.richards at ngmr.net
Fri Apr 8 17:17:49 UTC 2011
On Fri, 2011-04-08 at 16:40 +0100, Alan Bateman wrote:
>
> Neil - I think I offered to help get this one in and I'm just
> wondering if your patch of 3/24 is the latest? Just checking as it
> would be good to get this one in before jdk7 gets locked down.
>
> -Alan
Hi Alan, thanks for the prod (and apologies for the delay - see my
excuses below).
> Peter Jones wrote:
> > Hi. For background, I filed 6597112. Thanks for looking into it.
> >
> > Another possibility, though, is to just not throw this
> > ExportException (which is effectively an assertion failure) in
> > ObjectTable.putTarget, but rather let the remote object's export
> > appear to succeed even though it is known to have been garbage
> > collected.
Hi Peter,
Thank you for your review on this problem - it really helped in
highlighting some invalid assumptions I was making about the way that
the RMI code behaves.
I had assumed that, once the stub is returned from exportObject, the
'liveness' of the stub would control the the 'liveness' of the exported
object (as the stub is, logically, a kind of remote reference to the
object).
I'd argue this is a naturally reasonable expectation to have (ie. it
would be good to enhance the RMI mechanism so that behaves this way),
but, as you point out, that's not how it currently behaves (there are
current get-out clauses in the RMI spec which warn about the behaviour
being different than one would initially expect in this case).
(I easily found a conversation [1] highlighting the confusion the
current behaviour causes for a Java developer unversed in the deeper
machinations of the RMI server - I suspect the developer's confusion
here is not unique).
If the RMI were enhanced so that reality met with my initial assumptions
(!), I now see that the messing about with pinning & unpinning WeakRef
would probably be unnecessary (as the impl would be held strongly by
another means whilst the stub lives).
Until / unless it is enhanced in this way, I agree that messing about
with pinning & unpinning WeakRef is overly complex, given that the impl
object is going to disappear immediately anyhow.
I've been looking into how one would change the code to make the
behavioural enhancement, but realise I'm running out of runway to
develop / champion such a change at this point in the schedule.
Given all of this, your suggestion of modifying ObjectTable.putTarget()
such that it is tolerant of being given Targets whose impl objects may
have been GC'd (ie. without it throwing an exception) seems excellent to
me.
Please find below a (far simpler) revised suggested changeset, which
addresses the problem in this fashion.
(NB: In the new changeset, the check for the impl having been GC'd is
now within the block synchronized on tableLock. The operation of the
Reaper thread is also performed synchronized on this lock. This ensures
that the keep alive count is incremented / decremented properly, even if
the impl happens to be GC'd after the check).
Please let me know if you have any further comments or suggestions on
this,
Thanks,
Neil
[1] http://www.coderanch.com/t/492590/java/java/RMI-GC
--
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 1302196316 -3600
# Branch j6597112
# Node ID cfa1f7fcaabb03ab79aae8f051e4c084b45c75b9
# Parent aa13e7702cd9d8aca9aa38f1227f966990866944
6597112: referential integrity loophole during remote object export
Summary: Make ObjectTable.putTarget() tolerant of Target's with a GC'd impl
Contributed-by: <neil.richards at ngmr.net>
diff -r aa13e7702cd9 -r cfa1f7fcaabb src/share/classes/sun/rmi/transport/ObjectTable.java
--- a/src/share/classes/sun/rmi/transport/ObjectTable.java Tue Mar 29 20:19:55 2011 -0700
+++ b/src/share/classes/sun/rmi/transport/ObjectTable.java Thu Apr 07 18:11:56 2011 +0100
@@ -175,25 +175,21 @@
DGCImpl.dgcLog.log(Log.VERBOSE, "add object " + oe);
}
- Remote impl = target.getImpl();
- if (impl == null) {
- throw new ExportException(
- "internal error: attempt to export collected object");
- }
+ synchronized (tableLock) {
+ if (null != target.getImpl()) {
+ if (objTable.containsKey(oe)) {
+ throw new ExportException(
+ "internal error: ObjID already in use");
+ } else if (implTable.containsKey(weakImpl)) {
+ throw new ExportException("object already exported");
+ }
- synchronized (tableLock) {
- if (objTable.containsKey(oe)) {
- throw new ExportException(
- "internal error: ObjID already in use");
- } else if (implTable.containsKey(weakImpl)) {
- throw new ExportException("object already exported");
- }
+ objTable.put(oe, target);
+ implTable.put(weakImpl, target);
- objTable.put(oe, target);
- implTable.put(weakImpl, target);
-
- if (!target.isPermanent()) {
- incrementKeepAliveCount();
+ if (!target.isPermanent()) {
+ incrementKeepAliveCount();
+ }
}
}
}
diff -r aa13e7702cd9 -r cfa1f7fcaabb test/java/rmi/server/UnicastRemoteObject/gcDuringExport/GcDuringExport.java
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/java/rmi/server/UnicastRemoteObject/gcDuringExport/GcDuringExport.java Thu Apr 07 18:11:56 2011 +0100
@@ -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 GC'ing objects whilst being exported to RMI should not cause exceptions
+ * @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