hg: jdk8/tl: 7132204: Default testset in JPRT should not run all tests

[alan . bateman]

Changeset: 0f653ee93477
Author:alanb
Date:  2012-01-24 09:08 +
URL:   http://hg.openjdk.java.net/jdk8/tl/rev/0f653ee93477

7132204: Default testset in JPRT should not run all tests
Reviewed-by: ohair

! make/jprt.properties

hg: jdk8/tl/jdk: 7132204: Default testset in JPRT should not run all tests

[alan . bateman]

Changeset: 237319a01a9a
Author:alanb
Date:  2012-01-24 09:09 +
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/237319a01a9a

7132204: Default testset in JPRT should not run all tests
Reviewed-by: ohair

! make/jprt.properties
! test/ProblemList.txt

Re: RFR: 7131399: Poll system call appears to be broken on Mac OS [macosx]

[Chris Hegarty]

I'm OK with this as it, but here are a few comments:

The logic around the initial setting of the timeout seems a little 
unnecessary (and the additional pointer), but not wrong.


The comments should also be updated.

Given this problem should the build be setting USE_SELECT?

-Chris.

On 01/23/12 10:32 PM, Michael McMahon wrote:

On 23/01/12 21:30, Alan Bateman wrote:

On 23/01/2012 17:09, Michael McMahon wrote:

Can I get the following change reviewed please?

http://cr.openjdk.java.net/~michaelm/7131399/webrev.1/

The problem is that poll(2) doesn't seem to work in a specific edge
case tested by JCK,
namely, when a zero length UDP message is sent on a DatagramSocket.
The problem is only
detected on timed reads, ie. normal blocking reads work fine.

The fix is to make the NET_Timeout() function use select() instead of
poll().

Thanks,
Michael.


The first argument to select is s+1, shouldn't is be 1?

-alan

No, I don't think so. fd_sets are bit masks and you have to specify the
highest numbered bit in the
mask (+1).

- Michael.

Re: RFR: 7131399: Poll system call appears to be broken on Mac OS [macosx]

[Michael McMahon]

On 24/01/12 10:23, Chris Hegarty wrote:

I'm OK with this as it, but here are a few comments:

The logic around the initial setting of the timeout seems a little 
unnecessary (and the additional pointer), but not wrong.



Chris,

The setting of the timeval could be combined ok, but I wanted to avoid 
calling gettimeofday()

in the case where timeout == 0

The comments should also be updated.

Yes, I'll do that. It still refers to poll() instead of select() now. 
I'll add a comment about

the bug too.

Given this problem should the build be setting USE_SELECT?


I thought about that, and decided against it for two reasons.

1) it's only used by PlainSocketImpl in the networking code, and that 
usage doesn't seem
to be affected by this problem. poll() is potentially a little more 
efficient than select() also


2) USE_SELECT is referenced in other places in other native code (in AWT)
and I don't want to affect those usages. Granted that raises a question 
as to whether they
are affected by this bug. But, I don't believe they are since, as far as 
I can see UDP sockets

aren't used there.

- Michael



-Chris.

On 01/23/12 10:32 PM, Michael McMahon wrote:

On 23/01/12 21:30, Alan Bateman wrote:

On 23/01/2012 17:09, Michael McMahon wrote:

Can I get the following change reviewed please?

http://cr.openjdk.java.net/~michaelm/7131399/webrev.1/

The problem is that poll(2) doesn't seem to work in a specific edge
case tested by JCK,
namely, when a zero length UDP message is sent on a DatagramSocket.
The problem is only
detected on timed reads, ie. normal blocking reads work fine.

The fix is to make the NET_Timeout() function use select() instead of
poll().

Thanks,
Michael.


The first argument to select is s+1, shouldn't is be 1?

-alan

No, I don't think so. fd_sets are bit masks and you have to specify the
highest numbered bit in the
mask (+1).

- Michael.

Re: RFR: 7131399: Poll system call appears to be broken on Mac OS [macosx]

[Chris Hegarty]

Thanks the explanations. As I said, I'm fine with the change as is.

-Chris.

On 01/24/12 10:41 AM, Michael McMahon wrote:

On 24/01/12 10:23, Chris Hegarty wrote:

I'm OK with this as it, but here are a few comments:

The logic around the initial setting of the timeout seems a little
unnecessary (and the additional pointer), but not wrong.


Chris,

The setting of the timeval could be combined ok, but I wanted to avoid
calling gettimeofday()
in the case where timeout == 0

The comments should also be updated.


Yes, I'll do that. It still refers to poll() instead of select() now.
I'll add a comment about
the bug too.

Given this problem should the build be setting USE_SELECT?


I thought about that, and decided against it for two reasons.

1) it's only used by PlainSocketImpl in the networking code, and that
usage doesn't seem
to be affected by this problem. poll() is potentially a little more
efficient than select() also

2) USE_SELECT is referenced in other places in other native code (in AWT)
and I don't want to affect those usages. Granted that raises a question
as to whether they
are affected by this bug. But, I don't believe they are since, as far as
I can see UDP sockets
aren't used there.

- Michael



-Chris.

On 01/23/12 10:32 PM, Michael McMahon wrote:

On 23/01/12 21:30, Alan Bateman wrote:

On 23/01/2012 17:09, Michael McMahon wrote:

Can I get the following change reviewed please?

http://cr.openjdk.java.net/~michaelm/7131399/webrev.1/

The problem is that poll(2) doesn't seem to work in a specific edge
case tested by JCK,
namely, when a zero length UDP message is sent on a DatagramSocket.
The problem is only
detected on timed reads, ie. normal blocking reads work fine.

The fix is to make the NET_Timeout() function use select() instead of
poll().

Thanks,
Michael.


The first argument to select is s+1, shouldn't is be 1?

-alan

No, I don't think so. fd_sets are bit masks and you have to specify the
highest numbered bit in the
mask (+1).

- Michael.

Re: RFR: 7131399: Poll system call appears to be broken on Mac OS [macosx]

[Alan Bateman]

On 23/01/2012 22:32, Michael McMahon wrote:
No, I don't think so. fd_sets are bit masks and you have to specify 
the highest numbered bit in the

mask (+1).

Sorry, I wasn't thinking, it's nfds+1.

In that case, I think the change is okay although having t, t1, and t1_p 
looks a bit messy. It might be cleaner to rename t to something like 
"tod" and move it to be a local in the places where gettimeofday is 
used. That you allow you to rename t1 and t1_p and t and tp which I 
think would be more readable.


-Alan

Re: RFR: 7131399: Poll system call appears to be broken on Mac OS [macosx]

[Michael McMahon]

On 24/01/12 10:46, Alan Bateman wrote:

On 23/01/2012 22:32, Michael McMahon wrote:
No, I don't think so. fd_sets are bit masks and you have to specify 
the highest numbered bit in the

mask (+1).

Sorry, I wasn't thinking, it's nfds+1.

In that case, I think the change is okay although having t, t1, and 
t1_p looks a bit messy. It might be cleaner to rename t to something 
like "tod" and move it to be a local in the places where gettimeofday 
is used. That you allow you to rename t1 and t1_p and t and tp which I 
think would be more readable.


-Alan

Ok, I've updated the webrev to do the above and to adjust the comments

http://cr.openjdk.java.net/~michaelm/7131399/webrev.2/

Thanks
Michael.

Re: RFR: 7131399: Poll system call appears to be broken on Mac OS [macosx]

[Alan Bateman]

On 24/01/2012 11:19, Michael McMahon wrote:

Ok, I've updated the webrev to do the above and to adjust the comments

http://cr.openjdk.java.net/~michaelm/7131399/webrev.2/

This looks much cleaner. I'd probably use tp = NULL myself but what you 
have is okay.


-Alan

Re: RFR: 7131399: Poll system call appears to be broken on Mac OS [macosx]

[Damjan Jovanovic]

On Mon, Jan 23, 2012 at 7:09 PM, Michael McMahon <
michael.x.mcma...@oracle.com> wrote:

> Can I get the following change reviewed please?
>
> http://cr.openjdk.java.net/~**michaelm/7131399/webrev.1/
>
> The problem is that poll(2) doesn't seem to work in a specific edge case
> tested by JCK,
> namely, when a zero length UDP message is sent on a DatagramSocket.  The
> problem is only
> detected on timed reads, ie. normal blocking reads work fine.
>
> The fix is to make the NET_Timeout() function use select() instead of
> poll().
>
> Thanks,
> Michael.
>
>
>
>

Hi

I don't work at Oracle or anything, but IMHO this is a bad idea.

The finite length bitset used by select() means that there is a limit on
the maximum integer that can fit in the bitset. With 1024 bits (a common
value), you only have to create >= 1021 file descriptors (and of course
stdin/stdout/stderr) to exceed this limit, and end up with a file
descriptor for which FD_SET breaks. This will be the case even if that file
descriptor is the only file descriptor you are trying to add to the bitset.

Please reconsider.

Regards
Damjan Jovanovic

Re: RFR: 7131399: Poll system call appears to be broken on Mac OS [macosx]

[Michael McMahon]

On 24/01/12 11:27, Damjan Jovanovic wrote:



On Mon, Jan 23, 2012 at 7:09 PM, Michael McMahon 
mailto:michael.x.mcma...@oracle.com>> 
wrote:


Can I get the following change reviewed please?

http://cr.openjdk.java.net/~michaelm/7131399/webrev.1/


The problem is that poll(2) doesn't seem to work in a specific
edge case tested by JCK,
namely, when a zero length UDP message is sent on a
DatagramSocket.  The problem is only
detected on timed reads, ie. normal blocking reads work fine.

The fix is to make the NET_Timeout() function use select() instead
of poll().

Thanks,
Michael.





Hi

I don't work at Oracle or anything, but IMHO this is a bad idea.

The finite length bitset used by select() means that there is a limit 
on the maximum integer that can fit in the bitset. With 1024 bits (a 
common value), you only have to create >= 1021 file descriptors (and 
of course stdin/stdout/stderr) to exceed this limit, and end up with a 
file descriptor for which FD_SET breaks. This will be the case even if 
that file descriptor is the only file descriptor you are trying to add 
to the bitset.


Please reconsider.

Regards
Damjan Jovanovic



Damjan,

We can only deal with a finite number of file descriptors already in 
this file, although the actual
value can be set as high as required through setrlimit(). getFdEntry() 
checks that the fd number
is within the particular limits and all I/O operations will return EBADF 
if they happen to be outside.

This was the case even when poll() was used.

- Michael

Re: RFR: 7131399: Poll system call appears to be broken on Mac OS [macosx]

[Alan Bateman]

On 24/01/2012 11:47, Michael McMahon wrote:


Damjan,

We can only deal with a finite number of file descriptors already in 
this file, although the actual
value can be set as high as required through setrlimit(). getFdEntry() 
checks that the fd number
is within the particular limits and all I/O operations will return 
EBADF if they happen to be outside.

This was the case even when poll() was used.

- Michael
Another point is that this switching from poll to select is really just 
a temporary band aid in order to get this going. Either poll gets fixed 
or this code is changed to use kqueue. Without some initial band aid we 
have test failures and a compatibility issue. This goes for other areas 
of the code too and it will take a bit of time to clean up all areas.


-Alan

hg: jdk8/tl/jdk: 7132386: makefile support for tracing/Java Flight Recorder framework phase I

[rickard . backman]

Changeset: 718bca4e685f
Author:rbackman
Date:  2012-01-17 16:20 +0100
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/718bca4e685f

7132386: makefile support for tracing/Java Flight Recorder framework phase I
Reviewed-by: ohair, dholmes, rottenha
Contributed-by: Markus Gronlund , Erik Gahlin 
, Nils Loodin , Rickard Backman 
, Staffan Larsen 

! make/com/oracle/Makefile
+ make/com/oracle/jfr/Makefile
! make/common/Defs.gmk
! make/common/Release.gmk

hg: jdk8/tl/langtools: 7129801: Merge the two method applicability routines

[maurizio . cimadamore]

Changeset: 51fb17abfc32
Author:mcimadamore
Date:  2012-01-24 17:52 +
URL:   http://hg.openjdk.java.net/jdk8/tl/langtools/rev/51fb17abfc32

7129801: Merge the two method applicability routines
Summary: Resolve.java and Infer.java should reuse the same method applicability 
check routine
Reviewed-by: dlsmith, jjg

! src/share/classes/com/sun/tools/javac/comp/Infer.java
! src/share/classes/com/sun/tools/javac/comp/Resolve.java
! src/share/classes/com/sun/tools/javac/resources/compiler.properties
+ test/tools/javac/diags/examples/InferVarargsArgumentMismatch.java

hg: jdk8/tl/jdk: 7132270: tools/launcher/DefaultLocaleTestRun.java failing (win)

[kumar . x . srinivasan]

Changeset: f64ea40293db
Author:ksrini
Date:  2012-01-24 09:58 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/f64ea40293db

7132270: tools/launcher/DefaultLocaleTestRun.java failing (win)
Reviewed-by: alanb, chegar

! test/tools/launcher/DefaultLocaleTestRun.java
! test/tools/launcher/TestHelper.java

hg: jdk8/tl/jdk: 7132879: address Findbugs issue in WebRowSetXmlWriter

[lance . andersen]

Changeset: 303b67074666
Author:lancea
Date:  2012-01-24 15:13 -0500
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/303b67074666

7132879: address Findbugs issue in WebRowSetXmlWriter
Reviewed-by: forax

! src/share/classes/com/sun/rowset/internal/WebRowSetXmlWriter.java

Re: Code review request [JDK 8]: 7132248, sun/security/ssl/sun/net/www/protocol/https/HttpsURLConnection/CookieHttpsClientTest.java failing

[Brad Wetmore]

Looks good also...

Brad



On 1/23/2012 4:27 AM, Xuelei Fan wrote:

Remove the serviceabilty-dev.

Thanks for the quick code review.

Xuelei

On 1/23/2012 8:25 PM, Alan Bateman wrote:

On 23/01/2012 12:21, Xuelei Fan wrote:

Webrev: http://cr.openjdk.java.net/~xuelei/7132248/webrev.00/

In JDK 8, the regression tests of JSSE (HTTP/TLS) run in agentvm
mode. In agentvm mode, multiple threads may share the thread pool.
SunJSSE implementation initialize the SSL/TLS context at the first
time the context get loaded, and will not dynamically change the
context anymore after the initialization. If a test case has
initialized the context, another test case share the same thread will
use the same context. New settings in the later will have no impact
on the context.

The cause of the bug is that some other test case updated the
context, and CookieHttpsClientTest cannot setup the context as
expected. Need to make the test case run in othervm mode.

Thanks,
Xuelei

This looks fine to me, thanks for jumping on this annoying test failure.

-Alan.

PS: cc'ing security-dev as I'm guessing you cc'ed serviceabilty-dev in
error.

hg: jdk8/tl/langtools: 7126832: com.sun.tools.javac.api.ClientCodeWrapper$WrappedJavaFileManager cannot be cast

[jim . holmlund]

Changeset: ac36176b7de0
Author:jjh
Date:  2012-01-24 15:51 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/langtools/rev/ac36176b7de0

7126832: com.sun.tools.javac.api.ClientCodeWrapper$WrappedJavaFileManager 
cannot be cast
Reviewed-by: jjg

! src/share/classes/com/sun/tools/javac/api/JavacTaskImpl.java
! src/share/classes/com/sun/tools/javac/main/Main.java
+ test/tools/javah/T7126832/T7126832.java
+ test/tools/javah/T7126832/java.java

hg: jdk8/tl/langtools: 7129225: javac fails to run annotation processors when star import of package of gensrc

[jim . holmlund]

Changeset: d16b464e742c
Author:jjh
Date:  2012-01-24 16:31 -0800
URL:   http://hg.openjdk.java.net/jdk8/tl/langtools/rev/d16b464e742c

7129225: javac fails to run annotation processors when star import of package 
of gensrc
Reviewed-by: jjg

! src/share/classes/com/sun/tools/javac/comp/MemberEnter.java
+ test/tools/javac/7129225/Anno.java
+ test/tools/javac/7129225/AnnoProcessor.java
+ test/tools/javac/7129225/NegTest.ref
+ test/tools/javac/7129225/TestImportStar.java
+ test/tools/javac/7129225/TestImportStar.ref