Thanks David.
Shanliang
David Holmes wrote:
Hi Shanliang,
After our off-list discussion I've had a closer look at this and it
looks okay to me too.
Reviewed.
Thanks,
David
On 26/08/2013 6:35 PM, shanliang wrote:
Hi,
Still need OK from a reviewer.
Thanks Erik, Jaroslav and Daniel for the code review.
Thanks,
Shanliang
Daniel Fuchs wrote:
On 8/21/13 10:27 AM, shanliang wrote:
Jaroslav Bachorik wrote:
On 08/21/2013 12:00 AM, Daniel Fuchs wrote:
On 8/20/13 11:12 PM, shanliang wrote:
Thanks Daniel and Erik for the comments, here is new version:
http://cr.openjdk.java.net/~sjiang/JDK-6566891/01/
This new version looks reasonable. I guess you could dispense of
the new
nullSubjectConnRef by just using 'null' as key in the WeakHashMap
(as this
seems to be the way it used to work before) - but I'm OK with your
proposed changes.
Indeed, the change could be simplified by just using the null key.
If using "null" as a key, then the pair null/weakRef will never be
removed from WeakHashMap, even the value of weakRef is null. Of course
this is a minor memory leak, or almost nothing, but it seems more
clean
to use nullSubjectConnRef.
Ah. Good point. I didn't think of that...
Still need a reviewer.
Thanks,
Shanliang
The rest seems fine. Thumbs up!
-JB-
-- daniel
Erik Gahlin wrote:
Shanliang,
Let's say the delgationSubject is null and there is previous
connection cached (nullSubjectConn != null &&
nullSubjectConn.get()
!= null) so you hit:
2016: conn = nullSubjectConn.get();
but before you get the object a GC occurs and clears reference,
then
getConnectionWithSubject would return null.
Shouldn't you assign to nullSubjectConn.get() to variable so the
reference is kept alive? Or it OK to return null?
Yes, should avoid calling 2 times.
Also, is the
2020: if (wr != null) {
This line is no more there.
Shanliang
necessary?
Thanks
Erik
nullSibwhen you hit the second if-statement
happens
shanliang skrev 2013-08-20 17:35:
webrev:http://cr.openjdk.java.net/~sjiang/JDK-6566891/00/
shanliang wrote:
Hi,
Please review:
webrev:
http://amos.fr.oracle.com/jmgt/user/sjiang/webrevs/jdk8-6566891/00/
bug:https://jbs.oracle.com/bugs/browse/JDK-6566891
I have passed JCK tests and unit tests.
Thanks,
Shanliang
