Re: Code Review Request: 6953455 CookieStore.add() cannot handle null URI parameter, contrary to the API specification

2012-07-05 Thread Neil Richards 
Hi Chris,
Some QA folk round these parts observed the problem in 7.

I told them of the fix in 8 [1] & they expressed interest in it being
backported to 7u.

I've confirmed that the problem still exists in the jdk7u code.

I've uploaded a webrev of the fix applied back onto jdk7u-dev [2], for
review.

If approved, I could push the change to jdk7u-dev too, unless that's
considered to be stepping on Kurchi's toes or otherwise bad form.

Regards,
Neil

[1] http://hg.openjdk.java.net/jdk8/jdk8/jdk/rev/74f5fef1d961
[2] http://cr.openjdk.java.net/~ngmr/6953455.7u/webrev.00/

On Wed, 2012-07-04 at 21:34 +0100, Chris Hegarty wrote:
> Seems like a reasonable candidate for a backport. Are you encountering it in 
> 7?
> 
> -Chris
> 
> On 4 Jul 2012, at 15:19, Neil Richards  wrote:
> 
> > On Fri, 2011-09-30 at 10:08 -0700, Kurchi Hazra wrote:
> >> 
> >> 
> >> Hi,
> >> 
> >>The CookieStore.add() method throws a Null Pointer Exception when
> >> null is passed as the uri parameter, although this is allowed
> >> according to the method spec. 
> >> 
> >> The exception is thrown because uri.getHost() is called on a null
> >> uri in an effort to add it to the uriIndex, one of the hash maps
> >> constituting the CookieStore. The fix would be to simply bypass adding
> >> the cookie to the uriIndex when uri is null.
> >> 
> >> 
> >> The fix involves updates in: 
> >> src/share/classes/java/net/InMemoryCookieStore.java
> >> 
> >> Webrev : http://cr.openjdk.java.net/~chegar/6953455/webrev.00/webrev/ 
> >> 
> >> Thanks,
> >> Kurchi
> >> 
> > 
> > Hi,
> > This bug fix looks to have been well bedded into the openjdk 8 code
> > stream at this point.
> > 
> > Would this be a good item to be applied to the jdk7u code stream ?
> > 
> > Regards,
> > 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
> > 


-- 
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

Re: Code Review Request: 6953455 CookieStore.add() cannot handle null URI parameter, contrary to the API specification

2012-07-05 Thread Kurchi Hazra 

Hi Neil,

I do not have a problem with you pushing this fix.  Thanks for taking 
this up.


- Kurchi

On 7/5/2012 8:48 AM, Neil Richards wrote:

Hi Chris,
Some QA folk round these parts observed the problem in 7.

I told them of the fix in 8 [1]&  they expressed interest in it being
backported to 7u.

I've confirmed that the problem still exists in the jdk7u code.

I've uploaded a webrev of the fix applied back onto jdk7u-dev [2], for
review.

If approved, I could push the change to jdk7u-dev too, unless that's
considered to be stepping on Kurchi's toes or otherwise bad form.

Regards,
Neil

[1] http://hg.openjdk.java.net/jdk8/jdk8/jdk/rev/74f5fef1d961
[2] http://cr.openjdk.java.net/~ngmr/6953455.7u/webrev.00/

On Wed, 2012-07-04 at 21:34 +0100, Chris Hegarty wrote:

Seems like a reasonable candidate for a backport. Are you encountering it in 7?

-Chris

On 4 Jul 2012, at 15:19, Neil Richards  wrote:


On Fri, 2011-09-30 at 10:08 -0700, Kurchi Hazra wrote:


Hi,

The CookieStore.add() method throws a Null Pointer Exception when
null is passed as the uri parameter, although this is allowed
according to the method spec.

 The exception is thrown because uri.getHost() is called on a null
uri in an effort to add it to the uriIndex, one of the hash maps
constituting the CookieStore. The fix would be to simply bypass adding
the cookie to the uriIndex when uri is null.


The fix involves updates in:
src/share/classes/java/net/InMemoryCookieStore.java

Webrev : http://cr.openjdk.java.net/~chegar/6953455/webrev.00/webrev/

Thanks,
Kurchi


Hi,
This bug fix looks to have been well bedded into the openjdk 8 code
stream at this point.

Would this be a good item to be applied to the jdk7u code stream ?

Regards,
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





--
-Kurchi

hg: jdk8/tl/jdk: 6948101: java/rmi/transport/pinLastArguments/PinLastArguments.java failing intermittently

2012-07-05 Thread stuart . marks 
Changeset: 97eb7a4b1fdd
Author:smarks
Date:  2012-07-05 15:12 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/97eb7a4b1fdd

6948101: java/rmi/transport/pinLastArguments/PinLastArguments.java failing 
intermittently
Reviewed-by: dholmes, smarks
Contributed-by: Eric Wang 

! test/ProblemList.txt
! test/java/rmi/transport/pinLastArguments/PinLastArguments.java

hg: jdk8/tl/jdk: 7123972: test/java/lang/annotation/loaderLeak/Main.java fails intermittently

2012-07-05 Thread stuart . marks 
Changeset: 4ad204cc7433
Author:smarks
Date:  2012-07-05 15:13 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/4ad204cc7433

7123972: test/java/lang/annotation/loaderLeak/Main.java fails intermittently
Reviewed-by: dholmes, smarks
Contributed-by: Eric Wang 

! test/ProblemList.txt
! test/java/lang/annotation/loaderLeak/Main.java

hg: jdk8/tl/jdk: 7181353: Update error message to distinguish native OOM and java OOM in net

2012-07-05 Thread luchsh 
Changeset: 15a6b0bceb1e
Author:zhouyx
Date:  2012-07-06 10:36 +0800
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/15a6b0bceb1e

7181353: Update error message to distinguish native OOM and java OOM in net
Reviewed-by: chegar

! src/solaris/native/java/net/Inet4AddressImpl.c
! src/solaris/native/java/net/Inet6AddressImpl.c
! src/solaris/native/java/net/NetworkInterface.c
! src/solaris/native/java/net/PlainDatagramSocketImpl.c
! src/windows/native/java/net/DualStackPlainDatagramSocketImpl.c
! src/windows/native/java/net/Inet6AddressImpl.c
! src/windows/native/java/net/NetworkInterface.c
! src/windows/native/java/net/TwoStacksPlainDatagramSocketImpl.c

Re: A little modification to error message

2012-07-05 Thread Jonathan Lu 

Hi Sean,

Patch committed

Changeset: 15a6b0bceb1e
Author:zhouyx
Date:  2012-07-06 10:36 +0800
URL:http://hg.openjdk.java.net/jdk8/tl/jdk/rev/15a6b0bceb1e

7181353: Update error message to distinguish native OOM and java OOM in net
Reviewed-by: chegar

! src/solaris/native/java/net/Inet4AddressImpl.c
! src/solaris/native/java/net/Inet6AddressImpl.c
! src/solaris/native/java/net/NetworkInterface.c
! src/solaris/native/java/net/PlainDatagramSocketImpl.c
! src/windows/native/java/net/DualStackPlainDatagramSocketImpl.c
! src/windows/native/java/net/Inet6AddressImpl.c
! src/windows/native/java/net/NetworkInterface.c
! src/windows/native/java/net/TwoStacksPlainDatagramSocketImpl.c


Please verify

Regards
Jonathan

On 07/03/2012 02:10 PM, Sean Chou wrote:


A bug is reported. 
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7181353 .


On Mon, Jul 2, 2012 at 3:57 PM, Chris Hegarty 
mailto:chris.hega...@oracle.com>> wrote:


Thanks Sean, looks fine to me.

-Chris

Sean Chou mailto:zho...@linux.vnet.ibm.com>> wrote:

>Hello,
>
>Any one would like to take a look again ?
>
>-- Forwarded message --
>From: Sean Chou mailto:zho...@linux.vnet.ibm.com>>
>Date: Tue, Jun 26, 2012 at 1:03 PM
>Subject: Re: A little modification to error message
>To: Chris Hegarty mailto:chris.hega...@oracle.com>>
>Cc: net-dev@openjdk.java.net 
>
>
>Hi Chris,
>
>They are not all native OOMs, I made a new one which includes
all of
>them. Please take a look.
>
>webrev: http://cr.openjdk.java.net/~zhouyx/OJDK-528/webrev.01/

>
>
>On Thu, Jun 21, 2012 at 5:23 PM, Chris Hegarty
mailto:chris.hega...@oracle.com>>
> wrote:
>
>> Sean,
>>
>> The updated error messages look fine to me.
>>
>> Are these all the possibly native OOM throw sites in the
networking area,
>> or just some you came across?
>>
>> -Chris.
>>
>>
>> On 21/06/2012 06:41, Sean Chou wrote:
>>
>>> Hi all,
>>>
>>> We did some modification in these error messages because
of some
>>> user feedback “It is confusing to realize if the OOM is from
java heap
>>> or native heap”. So these error messages are modified from "heap
>>> allocation failed" to "Native heap allocation failed". And in some
>>> places a little more to help locate to functions like "send
buffer heap
>>> allocation failed".
>>> Would any one like to take a look?
>>>
>>> webrev:
http://cr.openjdk.java.net/~**zhouyx/OJDK-528/webrev.00/

>
>>>   .
>>>
>>> --
>>> Best Regards,
>>> Sean Chou
>>>
>>>
>
>
>--
>Best Regards,
>Sean Chou
>
>
>
>
>--
>Best Regards,
>Sean Chou




--
Best Regards,
Sean Chou

Re: A little modification to error message

2012-07-05 Thread Sean Chou 
Thanks Jonathan and Chris. Verified.

On Fri, Jul 6, 2012 at 10:44 AM, Jonathan Lu wrote:

>  Hi Sean,
>
> Patch committed
>
> Changeset: 15a6b0bceb1e
> Author:zhouyx
> Date:  2012-07-06 10:36 +0800
> URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/15a6b0bceb1e
>
> 7181353: Update error message to distinguish native OOM and java OOM in net
> Reviewed-by: chegar
>
> ! src/solaris/native/java/net/Inet4AddressImpl.c
> ! src/solaris/native/java/net/Inet6AddressImpl.c
> ! src/solaris/native/java/net/NetworkInterface.c
> ! src/solaris/native/java/net/PlainDatagramSocketImpl.c
> ! src/windows/native/java/net/DualStackPlainDatagramSocketImpl.c
> ! src/windows/native/java/net/Inet6AddressImpl.c
> ! src/windows/native/java/net/NetworkInterface.c
> ! src/windows/native/java/net/TwoStacksPlainDatagramSocketImpl.c
>
>
> Please verify
>
> Regards
> Jonathan
>
>
> On 07/03/2012 02:10 PM, Sean Chou wrote:
>
>
>  A bug is reported.
> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7181353  .
>
>  On Mon, Jul 2, 2012 at 3:57 PM, Chris Hegarty 
> wrote:
>
>> Thanks Sean, looks fine to me.
>>
>> -Chris
>>
>> Sean Chou  wrote:
>>
>> >Hello,
>> >
>> >Any one would like to take a look again ?
>> >
>> >-- Forwarded message --
>> >From: Sean Chou 
>> >Date: Tue, Jun 26, 2012 at 1:03 PM
>> >Subject: Re: A little modification to error message
>> >To: Chris Hegarty 
>> >Cc: net-dev@openjdk.java.net
>> >
>> >
>> >Hi Chris,
>> >
>> >They are not all native OOMs, I made a new one which includes all of
>> >them. Please take a look.
>> >
>> >webrev: http://cr.openjdk.java.net/~zhouyx/OJDK-528/webrev.01/
>> >
>> >
>> >On Thu, Jun 21, 2012 at 5:23 PM, Chris Hegarty > >
>> > wrote:
>> >
>> >> Sean,
>> >>
>> >> The updated error messages look fine to me.
>> >>
>> >> Are these all the possibly native OOM throw sites in the networking
>> area,
>> >> or just some you came across?
>> >>
>> >> -Chris.
>> >>
>> >>
>> >> On 21/06/2012 06:41, Sean Chou wrote:
>> >>
>> >>> Hi all,
>> >>>
>> >>> We did some modification in these error messages because of some
>> >>> user feedback “It is confusing to realize if the OOM is from java heap
>> >>> or native heap”. So these error messages are modified from "heap
>> >>> allocation failed" to "Native heap allocation failed". And in some
>> >>> places a little more to help locate to functions like "send buffer
>> heap
>> >>> allocation failed".
>> >>> Would any one like to take a look?
>> >>>
>>  >>> webrev: http://cr.openjdk.java.net/~**zhouyx/OJDK-528/webrev.00/<
>> http://cr.openjdk.java.net/~zhouyx/OJDK-528/webrev.00/>
>>  >>>   .
>> >>>
>> >>> --
>> >>> Best Regards,
>> >>> Sean Chou
>> >>>
>> >>>
>> >
>> >
>> >--
>> >Best Regards,
>> >Sean Chou
>> >
>> >
>> >
>> >
>> >--
>> >Best Regards,
>> >Sean Chou
>>
>
>
>
>  --
> Best Regards,
> Sean Chou
>
>
>
>


-- 
Best Regards,
Sean Chou