Request for review: 6597112: Referential integrity loophole during remote object export

[Peter Jones]

On Apr 13, 2011, at 7:08 AM, Alan Bateman wrote:
> Peter Jones wrote:
>> :
>> This fix looks good to me.  The synchronization subtlety may be worth a brief comment, to explain that you don't want the Reaper processing to occur in between the null guard and the put/increment actions.
>> 
>> Cheers,
>> 
>> -- Peter
>> 
>>  
> Thanks Peter, thanks Neil, looks like we are almost done with this one.
> 
> Neil - I did a hg import --no-commit of the last change-set that you attached, ran the RMI tests and all seems okay. I noticed a couple of minor things:
> 
> 1. You had "if (null != target.getImpl())" so changed it to "if (target.getImpl() != null)" to make is consistent
> 2. The run method in the test had inconsistent formatting
> 3. I "moved" the test to test/java/rmi/server/UnicastRemoteObject/exportObject so that additional tests could be added in the future
> 
> The updated webrev is here:
> http://cr.openjdk.java.net/~alanb/6597112/webrev/
> 
> If you are okay with this then I can push this later today.
> 
> -Alan.

Alan,

Your webrev looks good to me-- my only suggestion would be to add a comment explaining the null check.  Here are some suggested words:

Do nothing if impl has already been collected (see 6597112).  Check while holding tableLock to ensure that Reaper cannot process weakImpl in between null check and put/increment effects.

-- Peter