date:20160908

RE: RFR 8075484:SocketInputStream.socketRead0 can hang even with soTimeout set

2016-09-08 Thread Langer, Christoph

Hi Vyom,

ok, thanks. I have one addition to my proposal: I don't think there's a need 
for a global NET_GetCurrentTime function. This one should probably be a static 
function inside net_util_md.c (static long getCurrentTime()).

Best
Christoph

> -Original Message-
> From: Vyom Tewari [mailto:vyom.tew...@oracle.com]
> Sent: Mittwoch, 7. September 2016 18:31
> To: Langer, Christoph 
> Cc: net-dev@openjdk.java.net; Chris Hegarty 
> Subject: Re: RFR 8075484:SocketInputStream.socketRead0 can hang even with
> soTimeout set
> 
> Hi Chris,
> 
> Sorry I was mostly focusing on our test  failure first, i will check
> your suggestions.
> 
> Thanks,
> 
> Vyom
> 
> 
> On Wednesday 07 September 2016 08:44 PM, Langer, Christoph wrote:
> > Hi Vyom,
> >
> > did you have a look at my suggestions regarding AIX and refactoring? I can't
> find none of it in the new webrev nor did you comment on it.
> >
> > Best regards
> > Christoph
> >
> >> -Original Message-
> >> From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of
> Vyom
> >> Tewari
> >> Sent: Mittwoch, 7. September 2016 17:10
> >> To: Chris Hegarty 
> >> Cc: net-dev@openjdk.java.net
> >> Subject: Re: RFR 8075484:SocketInputStream.socketRead0 can hang even
> with
> >> soTimeout set
> >>
> >> Hi All,
> >>
> >> Please find the latest
> >> webrev(http://cr.openjdk.java.net/~vtewari/8075484/webrev0.4/index.html
> >> ).
> >> there were some test failures in JPRT and Chris also pointed the same
> >> issue in modified code. Now with latest code JPRT is clean.
> >>
> >> Thanks,
> >>
> >> Vyom
> >>
> >>
> >> On Wednesday 07 September 2016 03:18 PM, Chris Hegarty wrote:
> >>>
> >>> On 07/09/16 07:45, Vyom Tewari wrote:
>  Hi Chris,
> 
>  Please find the latest
> 
> webrev(http://cr.openjdk.java.net/~vtewari/8075484/webrev0.3/index.html
> 
> ). I
>  did some code refactoring, JPRT is in progress.
> >>> In terms of NET_Timeout**, I'm happy with this version, but I
> >>> have an additional comment.
> >>>
> >>> If NET_ReadWithTimeout returns -1 because
> NET_TimeoutWithCurrentTime
> >>> returns <= 0, then a JNI pending exception will be set. So the code
> >>> calling NET_ReadWithTimeout should, a) free bufP, and b) return -1,
> >>> immediately rather than continuing and possibly trying to set another
> >>> JNI pending exception.
> >>>
> >>> One option is to check the return value from NET_ReadWithTimeout and
> >>> also (*env)->ExceptionCheck(env). Another is to possibly consolidate
> >>> the setting of JNI pending exceptions in one place, towards the end
> >>> of Java_java_net_SocketInputStream_socketRead0, for example EBADF is
> >>> handled in two places.
> >>>
> >>> -Chris.
> >>>
>  I need help from some one to build and test latest changes on AIX, may
>  be Christoph can help.
> 
>  Thanks,
> 
>  Vyom
> 
> 
>  On Tuesday 06 September 2016 06:25 PM, Chris Hegarty wrote:
> > Vyom,
> >
> >> On 6 Sep 2016, at 10:20, Vyom Tewari 
> wrote:
> >>
> >> Hi,
> >>
> >> Please find the latest
> >>
> >> webrev(http://cr.openjdk.java.net/~vtewari/8075484/webrev0.2/index.html
> >>
> >> ).
> >> I
> >> have incorporated the review comments.
> > Your changes completely avoid NET_Timeout, which is quite complex on
> > Linux, Mac, and AIX, for various reasons. ( NET_Timeout will use the
> > async close mechanism to signal/interrupt a thread blocked in a poll /
> > select ). Also, select is used on Mac, which will use poll after your
> > changes ( I do remember that there was a bug in poll on Mac around
> > the time of the original Mac port ).
> >
> > Would is be better, rather than replicating the logic in NET_Timeout,
> > to have a version that supports passing a function pointer, to run if
> > the poll/select returns before the timeout? Just an idea.
> >
> > -Chris.
> >
> >> Thanks,
> >>
> >> Vyom
> >>
> >>
> >> On Monday 05 September 2016 08:37 PM, Chris Hegarty wrote:
> >>> On 05/09/16 15:37, Mark Sheppard wrote:
>  if the desire is to avoid making double calls on gettimeofday in the
>  NET_ReadWithTimeout's while loop for its main call flow,
> >>> Yes, the desire is to make no more calls to gettimeofday, than are
> >>> already done.
> >>>
>  then consider a modification to NET_Timeout and create a version
>  int NET_TimeoutWithCurrentTime(int s, long timeout, stuct timeval *
>  current_time)
> 
>  int NET_TimeoutWithCurrentTime(int s, long timeout, stuct timeval *
>  current_time) {
>   int result;
>   struct timeval t;
>   long prevtime, newtime;
>   struct pollfd

Re: RFR 8075484:SocketInputStream.socketRead0 can hang even with soTimeout set

2016-09-08 Thread Vyom Tewari


Hi All,

Please find the latest 
webrev(http://cr.openjdk.java.net/~vtewari/8075484/webrev0.5/index.html 
).


I fixed the AIX flag issue and move some come common code to 
"net_util_md.c" file.


Thanks,
Vyom

On 9/8/2016 12:32 PM, Langer, Christoph wrote:

Hi Vyom,

ok, thanks. I have one addition to my proposal: I don't think there's a need 
for a global NET_GetCurrentTime function. This one should probably be a static 
function inside net_util_md.c (static long getCurrentTime()).

Best
Christoph


-Original Message-
From: Vyom Tewari [mailto:vyom.tew...@oracle.com]
Sent: Mittwoch, 7. September 2016 18:31
To: Langer, Christoph 
Cc: net-dev@openjdk.java.net; Chris Hegarty 
Subject: Re: RFR 8075484:SocketInputStream.socketRead0 can hang even with
soTimeout set

Hi Chris,

Sorry I was mostly focusing on our test  failure first, i will check
your suggestions.

Thanks,

Vyom


On Wednesday 07 September 2016 08:44 PM, Langer, Christoph wrote:

Hi Vyom,

did you have a look at my suggestions regarding AIX and refactoring? I can't

find none of it in the new webrev nor did you comment on it.

Best regards
Christoph


-Original Message-
From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of

Vyom

Tewari
Sent: Mittwoch, 7. September 2016 17:10
To: Chris Hegarty 
Cc: net-dev@openjdk.java.net
Subject: Re: RFR 8075484:SocketInputStream.socketRead0 can hang even

with

soTimeout set

Hi All,

Please find the latest
webrev(http://cr.openjdk.java.net/~vtewari/8075484/webrev0.4/index.html
).
there were some test failures in JPRT and Chris also pointed the same
issue in modified code. Now with latest code JPRT is clean.

Thanks,

Vyom


On Wednesday 07 September 2016 03:18 PM, Chris Hegarty wrote:

On 07/09/16 07:45, Vyom Tewari wrote:

Hi Chris,

Please find the latest


webrev(http://cr.openjdk.java.net/~vtewari/8075484/webrev0.3/index.html
). I

did some code refactoring, JPRT is in progress.

In terms of NET_Timeout**, I'm happy with this version, but I
have an additional comment.

If NET_ReadWithTimeout returns -1 because

NET_TimeoutWithCurrentTime

returns <= 0, then a JNI pending exception will be set. So the code
calling NET_ReadWithTimeout should, a) free bufP, and b) return -1,
immediately rather than continuing and possibly trying to set another
JNI pending exception.

One option is to check the return value from NET_ReadWithTimeout and
also (*env)->ExceptionCheck(env). Another is to possibly consolidate
the setting of JNI pending exceptions in one place, towards the end
of Java_java_net_SocketInputStream_socketRead0, for example EBADF is
handled in two places.

-Chris.


I need help from some one to build and test latest changes on AIX, may
be Christoph can help.

Thanks,

Vyom


On Tuesday 06 September 2016 06:25 PM, Chris Hegarty wrote:

Vyom,


On 6 Sep 2016, at 10:20, Vyom Tewari 

wrote:

Hi,

Please find the latest


webrev(http://cr.openjdk.java.net/~vtewari/8075484/webrev0.2/index.html
).

I
have incorporated the review comments.

Your changes completely avoid NET_Timeout, which is quite complex on
Linux, Mac, and AIX, for various reasons. ( NET_Timeout will use the
async close mechanism to signal/interrupt a thread blocked in a poll /
select ). Also, select is used on Mac, which will use poll after your
changes ( I do remember that there was a bug in poll on Mac around
the time of the original Mac port ).

Would is be better, rather than replicating the logic in NET_Timeout,
to have a version that supports passing a function pointer, to run if
the poll/select returns before the timeout? Just an idea.

-Chris.


Thanks,

Vyom


On Monday 05 September 2016 08:37 PM, Chris Hegarty wrote:

On 05/09/16 15:37, Mark Sheppard wrote:

if the desire is to avoid making double calls on gettimeofday in the
NET_ReadWithTimeout's while loop for its main call flow,

Yes, the desire is to make no more calls to gettimeofday, than are
already done.


then consider a modification to NET_Timeout and create a version
int NET_TimeoutWithCurrentTime(int s, long timeout, stuct timeval *
current_time)

int NET_TimeoutWithCurrentTime(int s, long timeout, stuct timeval *
current_time) {
  int result;
  struct timeval t;
  long prevtime, newtime;
  struct pollfd pfd;
  pfd.fd = s;
  pfd.events = POLLIN;

  if (timeout > 0) {
  prevtime = (current_time->tv_sec * 1000)  +
current_time->tv_usec / 1000;
  }

  for(;;) {
  result = poll(&pfd, 1, timeout);
  if (result < 0 && errno == EINTR) {
  if (timeout > 0) {
  gettimeofday(&t, NULL);
  newtime = (t.tv_sec * 1000)  + t.tv_usec /1000;
  timeout -= newtime - prevti

Re: RFR 8075484:SocketInputStream.socketRead0 can hang even with soTimeout set

2016-09-08 Thread Vyom Tewari


sending this mail again as my laptop clock got screwed.

Vyom


On Thursday 08 September 2016 08:49 AM, Vyom Tewari wrote:

Hi All,

Please find the latest 
webrev(http://cr.openjdk.java.net/~vtewari/8075484/webrev0.5/index.html 
).


I fixed the AIX flag issue and move some come common code to 
"net_util_md.c" file.


Thanks,
Vyom

On 9/8/2016 12:32 PM, Langer, Christoph wrote:

Hi Vyom,

ok, thanks. I have one addition to my proposal: I don't think there's 
a need for a global NET_GetCurrentTime function. This one should 
probably be a static function inside net_util_md.c (static long 
getCurrentTime()).


Best
Christoph


-Original Message-
From: Vyom Tewari [mailto:vyom.tew...@oracle.com]
Sent: Mittwoch, 7. September 2016 18:31
To: Langer, Christoph 
Cc: net-dev@openjdk.java.net; Chris Hegarty 
Subject: Re: RFR 8075484:SocketInputStream.socketRead0 can hang even 
with

soTimeout set

Hi Chris,

Sorry I was mostly focusing on our test  failure first, i will check
your suggestions.

Thanks,

Vyom


On Wednesday 07 September 2016 08:44 PM, Langer, Christoph wrote:

Hi Vyom,

did you have a look at my suggestions regarding AIX and 
refactoring? I can't

find none of it in the new webrev nor did you comment on it.

Best regards
Christoph


-Original Message-
From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of

Vyom

Tewari
Sent: Mittwoch, 7. September 2016 17:10
To: Chris Hegarty 
Cc: net-dev@openjdk.java.net
Subject: Re: RFR 8075484:SocketInputStream.socketRead0 can hang even

with

soTimeout set

Hi All,

Please find the latest
webrev(http://cr.openjdk.java.net/~vtewari/8075484/webrev0.4/index.html 

). 


there were some test failures in JPRT and Chris also pointed the same
issue in modified code. Now with latest code JPRT is clean.

Thanks,

Vyom


On Wednesday 07 September 2016 03:18 PM, Chris Hegarty wrote:

On 07/09/16 07:45, Vyom Tewari wrote:

Hi Chris,

Please find the latest


webrev(http://cr.openjdk.java.net/~vtewari/8075484/webrev0.3/index.html
). 
I

did some code refactoring, JPRT is in progress.

In terms of NET_Timeout**, I'm happy with this version, but I
have an additional comment.

If NET_ReadWithTimeout returns -1 because

NET_TimeoutWithCurrentTime

returns <= 0, then a JNI pending exception will be set. So the code
calling NET_ReadWithTimeout should, a) free bufP, and b) return -1,
immediately rather than continuing and possibly trying to set 
another

JNI pending exception.

One option is to check the return value from NET_ReadWithTimeout and
also (*env)->ExceptionCheck(env). Another is to possibly consolidate
the setting of JNI pending exceptions in one place, towards the end
of Java_java_net_SocketInputStream_socketRead0, for example EBADF is
handled in two places.

-Chris.

I need help from some one to build and test latest changes on 
AIX, may

be Christoph can help.

Thanks,

Vyom


On Tuesday 06 September 2016 06:25 PM, Chris Hegarty wrote:

Vyom,


On 6 Sep 2016, at 10:20, Vyom Tewari 

wrote:

Hi,

Please find the latest

webrev(http://cr.openjdk.java.net/~vtewari/8075484/webrev0.2/index.html 

). 


I
have incorporated the review comments.
Your changes completely avoid NET_Timeout, which is quite 
complex on
Linux, Mac, and AIX, for various reasons. ( NET_Timeout will 
use the
async close mechanism to signal/interrupt a thread blocked in a 
poll /
select ). Also, select is used on Mac, which will use poll 
after your

changes ( I do remember that there was a bug in poll on Mac around
the time of the original Mac port ).

Would is be better, rather than replicating the logic in 
NET_Timeout,
to have a version that supports passing a function pointer, to 
run if

the poll/select returns before the timeout? Just an idea.

-Chris.


Thanks,

Vyom


On Monday 05 September 2016 08:37 PM, Chris Hegarty wrote:

On 05/09/16 15:37, Mark Sheppard wrote:
if the desire is to avoid making double calls on 
gettimeofday in the

NET_ReadWithTimeout's while loop for its main call flow,
Yes, the desire is to make no more calls to gettimeofday, 
than are

already done.

then consider a modification to NET_Timeout and create a 
version
int NET_TimeoutWithCurrentTime(int s, long timeout, stuct 
timeval *

current_time)

int NET_TimeoutWithCurrentTime(int s, long timeout, stuct 
timeval *

current_time) {
  int result;
  struct timeval t;
  long prevtime, newtime;
  struct pollfd pfd;
  pfd.fd = s;
  pfd.events = POLLIN;

  if (timeout > 0) {
  prevtime = (current_time->tv_sec * 1000)  +
current_time->tv_usec / 1000;
  }

  for(;;) {
  result = poll(&pfd, 1, timeout);
  if (result < 0 && errno == EINTR) {
  if (timeout > 0) {

RE: RFR 8075484:SocketInputStream.socketRead0 can hang even with soTimeout set

2016-09-08 Thread Langer, Christoph

Hi Vyom,

this looks good.

Is there any particular reason why NET_ReadWithTimeout should remain in 
SocketInputStream.c and not also move to net_util_md.c? That way you could also 
have a "static long getCurrentTime()" inside net_util_md.c, instead of an 
exported NET_GetCurrentTime().

And no "#include " would be needed in SocketInputStream.c - maybe 
not even now as it is.

Best regards
Christoph

> -Original Message-
> From: Vyom Tewari [mailto:vyom.tew...@oracle.com]
> Sent: Donnerstag, 8. September 2016 05:20
> To: Langer, Christoph 
> Cc: net-dev@openjdk.java.net
> Subject: Re: RFR 8075484:SocketInputStream.socketRead0 can hang even with
> soTimeout set
> 
> Hi All,
> 
> Please find the latest
> webrev(http://cr.openjdk.java.net/~vtewari/8075484/webrev0.5/index.html
> ).
> 
> I fixed the AIX flag issue and move some come common code to
> "net_util_md.c" file.
> 
> Thanks,
> Vyom
> 
> On 9/8/2016 12:32 PM, Langer, Christoph wrote:
> > Hi Vyom,
> >
> > ok, thanks. I have one addition to my proposal: I don't think there's a 
> > need for
> a global NET_GetCurrentTime function. This one should probably be a static
> function inside net_util_md.c (static long getCurrentTime()).
> >
> > Best
> > Christoph
> >
> >> -Original Message-
> >> From: Vyom Tewari [mailto:vyom.tew...@oracle.com]
> >> Sent: Mittwoch, 7. September 2016 18:31
> >> To: Langer, Christoph 
> >> Cc: net-dev@openjdk.java.net; Chris Hegarty 
> >> Subject: Re: RFR 8075484:SocketInputStream.socketRead0 can hang even
> with
> >> soTimeout set
> >>
> >> Hi Chris,
> >>
> >> Sorry I was mostly focusing on our test  failure first, i will check
> >> your suggestions.
> >>
> >> Thanks,
> >>
> >> Vyom
> >>
> >>
> >> On Wednesday 07 September 2016 08:44 PM, Langer, Christoph wrote:
> >>> Hi Vyom,
> >>>
> >>> did you have a look at my suggestions regarding AIX and refactoring? I
> can't
> >> find none of it in the new webrev nor did you comment on it.
> >>> Best regards
> >>> Christoph
> >>>
>  -Original Message-
>  From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of
> >> Vyom
>  Tewari
>  Sent: Mittwoch, 7. September 2016 17:10
>  To: Chris Hegarty 
>  Cc: net-dev@openjdk.java.net
>  Subject: Re: RFR 8075484:SocketInputStream.socketRead0 can hang even
> >> with
>  soTimeout set
> 
>  Hi All,
> 
>  Please find the latest
> 
> webrev(http://cr.openjdk.java.net/~vtewari/8075484/webrev0.4/index.html
> 
> ).
>  there were some test failures in JPRT and Chris also pointed the same
>  issue in modified code. Now with latest code JPRT is clean.
> 
>  Thanks,
> 
>  Vyom
> 
> 
>  On Wednesday 07 September 2016 03:18 PM, Chris Hegarty wrote:
> > On 07/09/16 07:45, Vyom Tewari wrote:
> >> Hi Chris,
> >>
> >> Please find the latest
> >>
> >> webrev(http://cr.openjdk.java.net/~vtewari/8075484/webrev0.3/index.html
> >> ). I
> >> did some code refactoring, JPRT is in progress.
> > In terms of NET_Timeout**, I'm happy with this version, but I
> > have an additional comment.
> >
> > If NET_ReadWithTimeout returns -1 because
> >> NET_TimeoutWithCurrentTime
> > returns <= 0, then a JNI pending exception will be set. So the code
> > calling NET_ReadWithTimeout should, a) free bufP, and b) return -1,
> > immediately rather than continuing and possibly trying to set another
> > JNI pending exception.
> >
> > One option is to check the return value from NET_ReadWithTimeout and
> > also (*env)->ExceptionCheck(env). Another is to possibly consolidate
> > the setting of JNI pending exceptions in one place, towards the end
> > of Java_java_net_SocketInputStream_socketRead0, for example EBADF
> is
> > handled in two places.
> >
> > -Chris.
> >
> >> I need help from some one to build and test latest changes on AIX, may
> >> be Christoph can help.
> >>
> >> Thanks,
> >>
> >> Vyom
> >>
> >>
> >> On Tuesday 06 September 2016 06:25 PM, Chris Hegarty wrote:
> >>> Vyom,
> >>>
>  On 6 Sep 2016, at 10:20, Vyom Tewari 
> >> wrote:
>  Hi,
> 
>  Please find the latest
> 
> 
> webrev(http://cr.openjdk.java.net/~vtewari/8075484/webrev0.2/index.html
> 
> ).
>  I
>  have incorporated the review comments.
> >>> Your changes completely avoid NET_Timeout, which is quite complex
> on
> >>> Linux, Mac, and AIX, for various reasons. ( NET_Timeout will use the
> >>> async close mechanism to signal/interrupt a thread blocked in a poll /
> >>> select ). Also, select is used on Mac, which will u

Re: RFR 8075484:SocketInputStream.socketRead0 can hang even with soTimeout set

2016-09-08 Thread Vyom Tewari



On Thursday 08 September 2016 02:50 PM, Langer, Christoph wrote:

Hi Vyom,

this looks good.

Is there any particular reason why NET_ReadWithTimeout should remain in 
SocketInputStream.c and not also move to net_util_md.c? That way you could also have a 
"static long getCurrentTime()" inside net_util_md.c, instead of an exported 
NET_GetCurrentTime().
I thought this "NET_ReadWithTimeou" is specific to SocketInputStream so 
i will be good if we put this method to socketinputstream.c.


In future we will in using the monotonic increasing clock so 
"NET_GetCurrentTime()" will help , Just change the implementation you 
are done.


Vyom

And no "#include " would be needed in SocketInputStream.c - maybe 
not even now as it is.

Best regards
Christoph


-Original Message-
From: Vyom Tewari [mailto:vyom.tew...@oracle.com]
Sent: Donnerstag, 8. September 2016 05:20
To: Langer, Christoph 
Cc: net-dev@openjdk.java.net
Subject: Re: RFR 8075484:SocketInputStream.socketRead0 can hang even with
soTimeout set

Hi All,

Please find the latest
webrev(http://cr.openjdk.java.net/~vtewari/8075484/webrev0.5/index.html
).

I fixed the AIX flag issue and move some come common code to
"net_util_md.c" file.

Thanks,
Vyom

On 9/8/2016 12:32 PM, Langer, Christoph wrote:

Hi Vyom,

ok, thanks. I have one addition to my proposal: I don't think there's a need for

a global NET_GetCurrentTime function. This one should probably be a static
function inside net_util_md.c (static long getCurrentTime()).

Best
Christoph


-Original Message-
From: Vyom Tewari [mailto:vyom.tew...@oracle.com]
Sent: Mittwoch, 7. September 2016 18:31
To: Langer, Christoph 
Cc: net-dev@openjdk.java.net; Chris Hegarty 
Subject: Re: RFR 8075484:SocketInputStream.socketRead0 can hang even

with

soTimeout set

Hi Chris,

Sorry I was mostly focusing on our test  failure first, i will check
your suggestions.

Thanks,

Vyom


On Wednesday 07 September 2016 08:44 PM, Langer, Christoph wrote:

Hi Vyom,

did you have a look at my suggestions regarding AIX and refactoring? I

can't

find none of it in the new webrev nor did you comment on it.

Best regards
Christoph


-Original Message-
From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of

Vyom

Tewari
Sent: Mittwoch, 7. September 2016 17:10
To: Chris Hegarty 
Cc: net-dev@openjdk.java.net
Subject: Re: RFR 8075484:SocketInputStream.socketRead0 can hang even

with

soTimeout set

Hi All,

Please find the latest


webrev(http://cr.openjdk.java.net/~vtewari/8075484/webrev0.4/index.html
).

there were some test failures in JPRT and Chris also pointed the same
issue in modified code. Now with latest code JPRT is clean.

Thanks,

Vyom


On Wednesday 07 September 2016 03:18 PM, Chris Hegarty wrote:

On 07/09/16 07:45, Vyom Tewari wrote:

Hi Chris,

Please find the latest


webrev(http://cr.openjdk.java.net/~vtewari/8075484/webrev0.3/index.html
). I

did some code refactoring, JPRT is in progress.

In terms of NET_Timeout**, I'm happy with this version, but I
have an additional comment.

If NET_ReadWithTimeout returns -1 because

NET_TimeoutWithCurrentTime

returns <= 0, then a JNI pending exception will be set. So the code
calling NET_ReadWithTimeout should, a) free bufP, and b) return -1,
immediately rather than continuing and possibly trying to set another
JNI pending exception.

One option is to check the return value from NET_ReadWithTimeout and
also (*env)->ExceptionCheck(env). Another is to possibly consolidate
the setting of JNI pending exceptions in one place, towards the end
of Java_java_net_SocketInputStream_socketRead0, for example EBADF

is

handled in two places.

-Chris.


I need help from some one to build and test latest changes on AIX, may
be Christoph can help.

Thanks,

Vyom


On Tuesday 06 September 2016 06:25 PM, Chris Hegarty wrote:

Vyom,


On 6 Sep 2016, at 10:20, Vyom Tewari 

wrote:

Hi,

Please find the latest


webrev(http://cr.openjdk.java.net/~vtewari/8075484/webrev0.2/index.html
).

I
have incorporated the review comments.

Your changes completely avoid NET_Timeout, which is quite complex

on

Linux, Mac, and AIX, for various reasons. ( NET_Timeout will use the
async close mechanism to signal/interrupt a thread blocked in a poll /
select ). Also, select is used on Mac, which will use poll after your
changes ( I do remember that there was a bug in poll on Mac around
the time of the original Mac port ).

Would is be better, rather than replicating the logic in NET_Timeout,
to have a version that supports passing a function pointer, to run if
the poll/select returns before the timeout? Just an idea.

-Chris.


Thanks,

Vyom


On Monday 05 September 2016 08:37 PM, Chris Hegarty wrote:

On 05/09/16 1

RFR (XS): JDK-8165711:java/net/SetFactoryPermission/SetFactoryPermission.java needs to run in ovm mode

2016-09-08 Thread Sean Coffey


Looking for review for this simple edit. Testcase should run in ovm mode.

https://bugs.openjdk.java.net/browse/JDK-8165711

patch :

diff --git 
a/test/java/net/SetFactoryPermission/SetFactoryPermission.java 
b/test/java/net/SetFactoryPermission/SetFactoryPermission.java

--- a/test/java/net/SetFactoryPermission/SetFactoryPermission.java
+++ b/test/java/net/SetFactoryPermission/SetFactoryPermission.java
@@ -27,7 +27,7 @@
  * @bug 8048052
  * @summary Test a series of methods which requires "setFactory" 
runtime permission

  * @modules java.rmi
- * @run main SetFactoryPermission success
+ * @run main/othervm SetFactoryPermission success
  * @run main/othervm/policy=policy.fail SetFactoryPermission fail
  * @run main/othervm/policy=policy.success SetFactoryPermission success
  */

regards,
Sean.

Re: RFR (XS): JDK-8165711:java/net/SetFactoryPermission/SetFactoryPermission.java needs to run in ovm mode

2016-09-08 Thread Chris Hegarty

Reviewed.

-Chris.

> On 8 Sep 2016, at 16:08, Sean Coffey  wrote:
> 
> Looking for review for this simple edit. Testcase should run in ovm mode.
> 
> https://bugs.openjdk.java.net/browse/JDK-8165711
> 
> patch :
> 
> diff --git a/test/java/net/SetFactoryPermission/SetFactoryPermission.java 
> b/test/java/net/SetFactoryPermission/SetFactoryPermission.java
> --- a/test/java/net/SetFactoryPermission/SetFactoryPermission.java
> +++ b/test/java/net/SetFactoryPermission/SetFactoryPermission.java
> @@ -27,7 +27,7 @@
>  * @bug 8048052
>  * @summary Test a series of methods which requires "setFactory" runtime 
> permission
>  * @modules java.rmi
> - * @run main SetFactoryPermission success
> + * @run main/othervm SetFactoryPermission success
>  * @run main/othervm/policy=policy.fail SetFactoryPermission fail
>  * @run main/othervm/policy=policy.success SetFactoryPermission success
>  */
> 
> regards,
> Sean.
>

Re: HTTP client API

2016-09-08 Thread Anthony Vanelverdinghe


Hi Michael

What's the rationale for not turning all public classes into interfaces, 
since none of them contain any actual implementation code?


On another note, I fail to see the point of 
HttpClient.Builder::priority: as far as I understand, HTTP/2 priority 
only comes into play when multiple resources depend on the same 
resource. But since there's no way to express dependencies between 
resources, how would this be useful?


Lastly, some minor remarks: I believe the following factory methods 
should be moved for consistency

- HttpRequest::noBody --> HttpRequest.BodyProcessor::noBody
- HttpResponse::multiFile --> HttpResponse.MultiProcessor::asFiles

and that "? extends T" should be used instead of "T", in the return type 
of the following methods

- HttpResponse.BodyHandler::apply, i.e. BodyProcessor
- HttpResponse.BodyProcessor::getBody, i.e. CompletionStage
- HttpResponse.MultiProcessor::onRequest, i.e. Optionalextends T>>


Kind regards,
Anthony


On 7/09/2016 19:15, Michael McMahon wrote:

Hi Wenbo,

First, sorry for the delay in replying. We took your comments and 
prototyped how the major ones

might be accommodated. In particular, we did the following:

- moved "business logic" out of HttpRequest. The methods for sending 
requests now
   exist in HttpClient. Given that requests are no longer associated 
with a client and can
   be sent through any client, this means that some properties of a 
request that used
   to be inherited from the client can no longer be visible in the 
HttpRequest object,
   but that is not a problem as far as I can see. This changes the 
look of the sample code

   in HttpRequest, but doesn't make it any harder to read.

- changed some method names as suggested eg newBuilder()

- added protected constructors to the public classes. This allows 
alternative implementations

   for use in mocking/test frameworks etc.

- changed the PUT, POST, GET methods in the request builder to only 
set the method type.
   PUT, POST takes the request body processor as parameter. A new 
build() method returns

   the HttpRequest.

- added a method to HttpResponse which returns the "final" request 
that was sent on the wire
   for the particular response, which might be different from the user 
initiated request.


I put an updated apidoc [1] and specdiff [2] up to show the changes. 
In particular, the sample

code described in the HttpClient docs is updated to reflect the changes.

Thanks,
Michael

[1] http://cr.openjdk.java.net/~michaelm/httpclient/api.1/

[2] 
http://cr.openjdk.java.net/~michaelm/httpclient/specdiffout.1/package-summary.html


On 26/08/2016, 07:59, Wenbo Zhu wrote:

Hi Michael,

Thanks for the update! The adoption of the Flow API is indeed a big 
improvement (async flow-control is very hard to get right).


Attached is a feedback doc on this new version. One specific idea to 
discuss is whether it's possible to release the new HTTP client API 
as a standalone library (that works on JDK 9).


Thanks,
Wenbo



On Mon, Aug 22, 2016 at 12:56 AM, Michael McMahon 
mailto:michael.x.mcma...@oracle.com>> 
wrote:


There is an updated version of the HTTP client API doc [1] and a
specdiff [2] showing the changes
proposed from the current version in JDK9 dev.

The main changes are:

1) The request and response processors are now based on
Flow.Publisher and Flow.Subscriber

2) Response bodies are retrieved synchronously with the response
headers as part of the
HttpRequest.response() and responseAsync() methods

3) Because of the change above, to allow code to examine the
headers and decide what to do
with the body before retrieving it, there is a new entity
called a HttpResponse.BodyHandler
which is given the status code and headers, and which returns
a HttpResponse.BodyProcessor.
Static implementations of both body handlers and body
processors are provided, to make the
simple cases easy to code.

We are currently working in the sandbox repository again and will
have these changes
in the main JDK9 dev forest soon.

Thanks,

Michael

[1] http://cr.openjdk.java.net/~michaelm/httpclient/api/


[2]

http://cr.openjdk.java.net/~michaelm/httpclient/specdiffout/package-summary.html

Re: RFR: 6947916: JarURLConnection does not handle useCaches correctly

2016-09-08 Thread Chris Hegarty


> On 7 Sep 2016, at 14:17, Rob McKenna  wrote:
> 
> Hi folks,
> 
> Looking for a review of this simple enough fix:
> 
> http://cr.openjdk.java.net/~robm/6947916/webrev.01/
> https://bugs.openjdk.java.net/browse/JDK-6947916

I think that the source changes are good, but the tests has a
reference to a shell script that is absent. Also, could the test just
create a simple jar file, rather than checking in a binary artifact.

-Chris.

> In a nutshell, if multiple URLConnections are made to several files in a 
> single jar, each will use the same backing JarFile object. Unfortunately 
> JarURLConnections connect() method recreates the jarFileURLConnection for a 
> given JarURLConnection using the default value for getUseCaches instead of 
> the *current* value.
> 
> In effect this means that jarURLConnection.getUseCaches() may return true 
> before calling connect() and false after. This in turn means that when a 
> JarURLConnection's inputstream is closed, it will believe that caching has 
> been turned off and the underlying jarFile will be closed out from under all 
> other JarURLConnection inputstreams.
> 
>   -Rob

RE: RFR 8075484:SocketInputStream.socketRead0 can hang even with soTimeout set

Re: RFR 8075484:SocketInputStream.socketRead0 can hang even with soTimeout set

Re: RFR 8075484:SocketInputStream.socketRead0 can hang even with soTimeout set

RE: RFR 8075484:SocketInputStream.socketRead0 can hang even with soTimeout set

Re: RFR 8075484:SocketInputStream.socketRead0 can hang even with soTimeout set

RFR (XS): JDK-8165711:java/net/SetFactoryPermission/SetFactoryPermission.java needs to run in ovm mode

Re: RFR (XS): JDK-8165711:java/net/SetFactoryPermission/SetFactoryPermission.java needs to run in ovm mode

Re: HTTP client API

Re: RFR: 6947916: JarURLConnection does not handle useCaches correctly

9 matches

Site Navigation

Mail list logo

Footer information