Request for review: 6597112: Referential integrity loophole during remote object export
neil.richards at ngmr.net
Wed Mar 9 11:46:43 UTC 2011
Hi Alan, thanks for looking at this.
On 9 March 2011 10:45, Alan Bateman <Alan.Bateman at oracle.com> wrote:
> You've changed WeakRef so that an instance is initially "strong". Does this
> have implications for other uses of WeakRef? I see one in
> sun.rmi.transport.ObjectTable for example.
You'll notice that the WeakRef constructed in ObjectTable is created
solely for the purpose of looking for an entry in the table (ie. as a
parameter to implTable.get()), and then immediately discarded.
So whether it holds its reference strongly or weakly is unimportant in
As WeakRef is only accessible within the sun.rmi.transport package
(having the 'default' access modifier), I can be certain that the only
two places where WeakRefs are constructed are for this lookup in
ObjectTable, and in the constructor of Target.
> I wonder if it would be better to
> change the constructors so that the user of WeakRef decides the reference is
> initially pinned or not. Alternatively maybe factory methods should be
> introduced for all creation of WeakRefs and they be named so that it's clear
> where the WeakRefs are initially strong or not.
I don't believe the extra complexity is necessary, given WeakRef's
current limited usage.
> Same thing with Target where
> I wonder about other uses as they will need to know that the impl is
> initially pinned.
Searching through the OpenJDK source, I found only two places where
new Target objects are constructed:
Here the Target object is called with a 'permanent' parameter setting
(That's the 5th parameter to the constructor).
You'll see that, in the original code for Target's constructor, if
'permanent' is 'true', a call was made from the constructor to its
pinImpl() method, to pin the reference.
With the change, this logic is no longer needed, as the reference
starts life pinned.
This is covered by my previous description of the suggested fix, which
adds an explicit call to 'target.unpinImpl()' after the call to
On reflection, I suppose it might be more clearly robust to put the
unpinImpl() call in a 'finally' block (with the 'try' around the
Therefore, I attach an updated 'hg export' of the suggested fix, which
adds this try/finally protection.
On the basis of these searches, I believe the suggested fix is correct
(for all uses of Target and WeakRef).
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
More information about the core-libs-dev