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
