Hi Alan, thanks for looking at this. On 9 March 2011 10:45, Alan Bateman <alan.bate...@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 this instance. 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: 1) sun.rmi.transport.DGCImpl.<clinit> Here the Target object is called with a 'permanent' parameter setting of 'true'. (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. 2) sun.rmi.server.UnicastServerRef.exportObject() This is covered by my previous description of the suggested fix, which adds an explicit call to 'target.unpinImpl()' after the call to 'ref.exportObject(target)'. On reflection, I suppose it might be more clearly robust to put the unpinImpl() call in a 'finally' block (with the 'try' around the 'ref.exportObject(target)' call). 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). - Neil -- 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
