date:20190604

On Wed, Jun 5, 2019 at 5:06 AM Martin Buchholz  wrote:

>
>
> On Tue, Jun 4, 2019 at 2:01 PM Roger Riggs  wrote:
>
>> Hmm.  Can_JOHNNY_EXEC naming is a bit quirky, not intuitive...
>>
>
> I'm the original author of that joke^H deeply evocative symbol name, so I
> might be biased.
>
>

I like the name :)

But thinking seriously about this, I think that "fail_pipe" is still a good
clear name: any communication over this channel has to do with handling
errors, included the child-alive-signal.

I thought about alternatives: "oob_pipe" (as in out-of-band) or "comm_pipe"
or "child_pipe" but all feel slightly wrong.

I would vote for keeping the name as it is: I would have to change a number
of places, and when in doubt I'd like to keep the changes minimal to make
backporting future changes easier.

..Thomas

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

On Tue, Jun 4, 2019 at 2:01 PM Roger Riggs  wrote:

> Hmm.  Can_JOHNNY_EXEC naming is a bit quirky, not intuitive...
>

I'm the original author of that joke^H deeply evocative symbol name, so I
might be biased.

RFR(s): 8205131: remove Runtime trace methods

2019-06-04 Thread Stuart Marks


Hi all,

Please review this changeset and CSR request that remove the traceInstructions() 
and traceMethodCalls() methods from java.lang.Runtime. These methods have been 
deprecated for removal since Java 9, and they do nothing. I've also removed a 
couple mentions of these methods from some tests.


After this changeset, the only times these methods are mentioned is in javac's 
symbol tables (for example, make/data/symbols/java.base-9.sym.txt) where they 
are kept because they are present in earlier releases.


They are also mentioned in the file

test/hotspot/jtreg/runtime/appcds/ExtraSymbols.symbols.txt

However, this file has a comment


68 -1: # The values in this file are only used for testing the operation of
63 -1: # adding extra symbols into the CDS archive. None of the values
70 -1: # are interpreted in any way. So even if they contain names of classes
70 -1: # that have been renamed or removed, or string literals that have been
66 -1: # changed or remove from Java source code, it would not affect the
26 -1: # correctness of the test.


so it seems that leaving mention of these methods in this file is harmless. 
Based on this comment I've decided not to change this file. Nonetheless, I'm 
including hotspot-dev in this review to make sure this is ok. (I seem to recall 
a similar issue came up the last time I removed something.)


Bug:

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

Webrev:

http://cr.openjdk.java.net/~smarks/reviews/8205131/webrev.0/

CSR request:

https://bugs.openjdk.java.net/browse/JDK-8225330
(if you review, please edit this issue and add yourself to the
Reviewed By field)

Thanks,

s'marks

Re: RFR (XS): 8225305: ProblemList java/lang/invoke/VarHandles tests

2019-06-04 Thread Igor Ignatev

LGTM!

— Igor

> On Jun 4, 2019, at 7:29 PM, Mikael Vidstedt  
> wrote:
> 
> 
> I, too, agree. :)
> 
> New webrev which adds a new ProblemList-aot.txt to be used when running tests 
> with AOT.
> 
> Webrev: 
> http://cr.openjdk.java.net/~mikael/webrevs/8225305/webrev.01/open/webrev/
> 
> Cheers,
> Mikael
> 
>> On Jun 4, 2019, at 3:18 PM, Mandy Chung  wrote:
>> 
>> I agree with Igor.  The best is to keep running these tests except
>> the AOT, perhaps + fastdebug, run only. 
>> 
>> Mandy
>> 
>>> On 6/4/19 2:06 PM, Igor Ignatyev wrote:
>>> Hi Mikael,
>>> 
>>> as it looks like 8222445 isn't going to be fixed for a long time (as it's 
>>> "targeted" to tbd), and the defect seems to affect only AOT run, I don't 
>>> think it's a good idea to put these tests into a general problem list. I'd 
>>> suggest to either create aot-specific problem list or put them into -graal 
>>> specific problem list.
>>> 
>>> -- Igor 
>>> 
 On Jun 4, 2019, at 1:38 PM, Mikael Vidstedt  
 wrote:
 
 
 The following java/lang/invoke/VarHandles tests frequently fail when run 
 with AOT. Until https://bugs.openjdk.java.net/browse/JDK-8222445 
  has been fixed they 
 should be problem listed. 
 
 java/lang/invoke/VarHandles/VarHandleTestByteArrayAsShort.java 
 java/lang/invoke/VarHandles/VarHandleTestByteArrayAsChar.java 
 java/lang/invoke/VarHandles/VarHandleTestAccessBoolean.java
 
 bug: https://bugs.openjdk.java.net/browse/JDK-8225305 
 
 webrev: 
 http://cr.openjdk.java.net/~mikael/webrevs/8225305/webrev.00/open/webrev/ 
 
 
 Cheers,
 Mikael
 
>> 
>

Re: RFR (XS): 8225305: ProblemList java/lang/invoke/VarHandles tests

2019-06-04 Thread Mikael Vidstedt



I, too, agree. :)

New webrev which adds a new ProblemList-aot.txt to be used when running tests 
with AOT.

Webrev: 
http://cr.openjdk.java.net/~mikael/webrevs/8225305/webrev.01/open/webrev/ 


Cheers,
Mikael

> On Jun 4, 2019, at 3:18 PM, Mandy Chung  wrote:
> 
> I agree with Igor.  The best is to keep running these tests except
> the AOT, perhaps + fastdebug, run only. 
> 
> Mandy
> 
> On 6/4/19 2:06 PM, Igor Ignatyev wrote:
>> Hi Mikael,
>> 
>> as it looks like 8222445 isn't going to be fixed for a long time (as it's 
>> "targeted" to tbd), and the defect seems to affect only AOT run, I don't 
>> think it's a good idea to put these tests into a general problem list. I'd 
>> suggest to either create aot-specific problem list or put them into -graal 
>> specific problem list.
>> 
>> -- Igor 
>> 
>>> On Jun 4, 2019, at 1:38 PM, Mikael Vidstedt  
>>>  wrote:
>>> 
>>> 
>>> The following java/lang/invoke/VarHandles tests frequently fail when run 
>>> with AOT. Until https://bugs.openjdk.java.net/browse/JDK-8222445 
>>>  
>>>  
>>>  has been fixed they 
>>> should be problem listed. 
>>> 
>>> java/lang/invoke/VarHandles/VarHandleTestByteArrayAsShort.java 
>>> java/lang/invoke/VarHandles/VarHandleTestByteArrayAsChar.java 
>>> java/lang/invoke/VarHandles/VarHandleTestAccessBoolean.java
>>> 
>>> bug: https://bugs.openjdk.java.net/browse/JDK-8225305 
>>>  
>>>  
>>> 
>>> webrev: 
>>> http://cr.openjdk.java.net/~mikael/webrevs/8225305/webrev.00/open/webrev/ 
>>>  
>>>  
>>> 
>>> 
>>> Cheers,
>>> Mikael
>>> 
>

Re: RFR(xs): 8225315 test java/util/ArrayDeque/WhiteBox.java isn't part of the jdk_collections test group

2019-06-04 Thread Stuart Marks


On 6/4/19 5:56 PM, Joseph D. Darcy wrote:

Should the test be removed from the catch-all group as part of this change?


Ah, good question. But the answer is that it doesn't need to be. The "catch-all" 
group jdk_util_other is defined thus:


--

# All util components not part of some other util category
jdk_util_other = \
java/util \
sun/util \
-:jdk_collections \
-:jdk_concurrent \
-:jdk_stream

--

This syntax defines jdk_util_other to be the tests in the java/util and sun/util 
directories, MINUS the tests in the jdk_collections and a couple other groups. 
Thus adding the test to jdk_collections automatically removes it from 
jdk_util_other.


s'marks

Re: RFR: JDK-8225213: Backport jsr166 tck tests to jdk8u

On Tue, Jun 4, 2019 at 6:07 PM Andrew John Hughes 
wrote:

>
> I don't think this is the best approach. JDK-8146467 looks like a pretty
> clean snapshot of the tests from not long after 8u GA. It doesn't
> include any *9Test.java files for a start. It would seem better to start
> with backporting this, rather than taking a random snapshot of the tests
> now, and hacking out all the later changes.
>

I understand the virtues of backport hygiene, having done many backports
myself, but here we're just going to disagree.
I have a changeset for you, and it is a much better test corpus for jdk8u
than the JDK-8146467 backport.

> Yes, it may be more tedious to then apply the follow-up test changesets
>

I'm not applying 36 changesets !

> than to just copy files over initially, but using the original
> changesets has the advantage that it also may include fixes to the
> source code that go with the test (e.g. JDK-8185830) If you just import
> the tests in bulk, you then have to hunt down and analyse each failure,
> eventually ending up importing the source code changes from many of
> these changesets anyway.
>

When I'm doing archaeology I look at the much more fine-grained changes in
CVS, not hg!

> We also then have the administrative benefit of being able to query the
> repository as to whether a fix is present or not, by searching for its
> bug ID.
>

When you backport an actual fix later, you will backport the change to src/
as usual and remove one of the DISABLED_JDK8_ prefixes.
(Also, the actual jdk8 fix is already in CVS!)

Re: RFR: 8225179: (regex) Minor Pattern cleanup

2019-06-04 Thread Ivan Gerasimov


Hi Claes!

Looks good to me, thanks!

LookBehindNode may be better named LookBehindEndNode, as it should only 
match at the very end of the look-behind token.


With kind regards,

Ivan


On 6/4/19 1:58 AM, Claes Redestad wrote:

Hi,

please review this j.u.regex.Pattern cleanup.

- refactor BitClass to be a BmpCharPredicate (which allows removing
two identical(!) lambdas), which improves startup and reduces
allocations when compiling Patterns.
- remove unused GroupRef class
- made anonymous lookbehindEnd Node instance into an explicit class
which will be lazily rather than eagerly loaded
- various cleanups of unused variables, methods and redundant
inititialization

Webrev: http://cr.openjdk.java.net/~redestad/8225179/open.00/
Bug:https://bugs.openjdk.java.net/browse/JDK-8225179

Testing: tier1-3

Thanks!

/Claes



--
With kind regards,
Ivan Gerasimov

Re: RFR: JDK-8225213: Backport jsr166 tck tests to jdk8u

2019-06-04 Thread Andrew John Hughes

On 05/06/2019 01:15, Martin Buchholz wrote:
> (Trying to correct my mistake of not having cc'ed jdk8u-dev)
> 
> On Mon, Jun 3, 2019 at 5:15 PM Martin Buchholz  > wrote:
> 
> 
> 
> -- Forwarded message -
> From: *Martin Buchholz (JBS)*  >
> Date: Mon, Jun 3, 2019 at 5:11 PM
> Subject: [JBS] {Commented} (JDK-8225213) Backport jsr166 tck tests
> to jdk8u
> To: mailto:marti...@google.com>>
> 
> 
> __
>   Martin Buchholz
> 
> *commented* on Enhancement JDK-8225213
> 
> 
>  
> Re: Backport jsr166 tck tests to jdk8u
> 
> 
> https://cr.openjdk.java.net/~martin/webrevs/jdk8/jsr166-tck/
> 
> Backport Strategy:
> Copy the tck directory from jdk head into the corresponding place in
> jdk8u, then delete all jdk9 API files (*9Test.java and
> SubmissionPublisherTest.java), then fix all "unknown symbol" compile
> errors by commenting out the test that uses it, then disable all
> failing tests by prepending the test name with "DISABLE_JDK_".
> 
> Follow-on changes can re-enable some of the tests by backporting
> fixes and removing "DISABLE_JDK_" from relevant tests. There are
> files in jsr166 CVS in src/jdk8 that pass these tests and run on jdk8
> BUT:
> - these fixes were not considered important and/or safe enough at
> the time to be worth backporting
> - the files in src/jdk8 implement a superset of the jdk8u API (they
> include enhancements) that would need to be removed.
> - jsr166 code is subtle, concurrency bugs tend to be latent, and so
> backports are risky, and jdk8u is supposed to be very stable. That
> said, there are known concurrency buglets in jdk8u. With hindsight,
> it would have been good to fix them around the jdk10 release time.
> 
> Add Comment
> Add
> Comment 
> 
>  
> 
> This message was sent by Atlassian Jira (v7.13.3#713003-sha1:0709f78) 
> Atlassian logo
> 

Thanks for forwarding.

I don't think this is the best approach. JDK-8146467 looks like a pretty
clean snapshot of the tests from not long after 8u GA. It doesn't
include any *9Test.java files for a start. It would seem better to start
with backporting this, rather than taking a random snapshot of the tests
now, and hacking out all the later changes.

Yes, it may be more tedious to then apply the follow-up test changesets
than to just copy files over initially, but using the original
changesets has the advantage that it also may include fixes to the
source code that go with the test (e.g. JDK-8185830) If you just import
the tests in bulk, you then have to hunt down and analyse each failure,
eventually ending up importing the source code changes from many of
these changesets anyway.

We also then have the administrative benefit of being able to query the
repository as to whether a fix is present or not, by searching for its
bug ID.

Thanks,
-- 
Andrew :)

Senior Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net)
Fingerprint = 5132 579D D154 0ED2 3E04  C5A0 CFDA 0F9B 3596 4222
https://keybase.io/gnu_andrew

Re: RFR(xs): 8225315 test java/util/ArrayDeque/WhiteBox.java isn't part of the jdk_collections test group

2019-06-04 Thread Joseph D. Darcy


Looks okay.

Should the test be removed from the catch-all group as part of this change?

-Joe

On 6/4/2019 5:53 PM, Stuart Marks wrote:

Hi all,

This test was added some time ago, but it wasn't added to the 
jdk_collections test group. Please review the patch appended below, 
which adds it to the right group. (The test was still run as part of 
the catch-all jdk_util_other group though.)


Bug link: https://bugs.openjdk.java.net/browse/JDK-8225315

s'marks


diff -r ef23ea332077 test/jdk/TEST.groups
--- a/test/jdk/TEST.groupsTue Jun 04 11:55:51 2019 -0700
+++ b/test/jdk/TEST.groupsTue Jun 04 17:52:11 2019 -0700
@@ -137,6 +137,7 @@
 java/util/AbstractList \
 java/util/AbstractMap \
 java/util/AbstractSequentialList \
+java/util/ArrayDeque \
 java/util/ArrayList \
 java/util/Arrays \
 java/util/BitSet \

Re: RFR(xs): 8225315 test java/util/ArrayDeque/WhiteBox.java isn't part of the jdk_collections test group

Looks good to me!

On Tue, Jun 4, 2019 at 5:55 PM Stuart Marks  wrote:

> Hi all,
>
> This test was added some time ago, but it wasn't added to the
> jdk_collections
> test group. Please review the patch appended below, which adds it to the
> right
> group. (The test was still run as part of the catch-all jdk_util_other
> group
> though.)
>
> Bug link: https://bugs.openjdk.java.net/browse/JDK-8225315
>
> s'marks
>
>
> diff -r ef23ea332077 test/jdk/TEST.groups
> --- a/test/jdk/TEST.groups  Tue Jun 04 11:55:51 2019 -0700
> +++ b/test/jdk/TEST.groups  Tue Jun 04 17:52:11 2019 -0700
> @@ -137,6 +137,7 @@
>   java/util/AbstractList \
>   java/util/AbstractMap \
>   java/util/AbstractSequentialList \
> +java/util/ArrayDeque \
>   java/util/ArrayList \
>   java/util/Arrays \
>   java/util/BitSet \
>

RFR(xs): 8225315 test java/util/ArrayDeque/WhiteBox.java isn't part of the jdk_collections test group

2019-06-04 Thread Stuart Marks


Hi all,

This test was added some time ago, but it wasn't added to the jdk_collections 
test group. Please review the patch appended below, which adds it to the right 
group. (The test was still run as part of the catch-all jdk_util_other group 
though.)


Bug link: https://bugs.openjdk.java.net/browse/JDK-8225315

s'marks


diff -r ef23ea332077 test/jdk/TEST.groups
--- a/test/jdk/TEST.groups  Tue Jun 04 11:55:51 2019 -0700
+++ b/test/jdk/TEST.groups  Tue Jun 04 17:52:11 2019 -0700
@@ -137,6 +137,7 @@
 java/util/AbstractList \
 java/util/AbstractMap \
 java/util/AbstractSequentialList \
+java/util/ArrayDeque \
 java/util/ArrayList \
 java/util/Arrays \
 java/util/BitSet \

Re: RFR: JDK-8225213: Backport jsr166 tck tests to jdk8u

(Trying to correct my mistake of not having cc'ed jdk8u-dev)

On Mon, Jun 3, 2019 at 5:15 PM Martin Buchholz  wrote:

>
>
> -- Forwarded message -
> From: Martin Buchholz (JBS) 
> Date: Mon, Jun 3, 2019 at 5:11 PM
> Subject: [JBS] {Commented} (JDK-8225213) Backport jsr166 tck tests to jdk8u
> To: 
>
>
> Martin Buchholz
> 
> *commented* on [image: Enhancement] JDK-8225213
> 
>
> Re: Backport jsr166 tck tests to jdk8u
> 
> https://cr.openjdk.java.net/~martin/webrevs/jdk8/jsr166-tck/
>
> Backport Strategy:
> Copy the tck directory from jdk head into the corresponding place in
> jdk8u, then delete all jdk9 API files (*9Test.java and
> SubmissionPublisherTest.java), then fix all "unknown symbol" compile errors
> by commenting out the test that uses it, then disable all failing tests by
> prepending the test name with "DISABLE_JDK_".
>
> Follow-on changes can re-enable some of the tests by backporting fixes and
> removing "DISABLE_JDK_" from relevant tests. There are files in jsr166 CVS
> in src/jdk8 that pass these tests and run on jdk8
> BUT:
> - these fixes were not considered important and/or safe enough at the time
> to be worth backporting
> - the files in src/jdk8 implement a superset of the jdk8u API (they
> include enhancements) that would need to be removed.
> - jsr166 code is subtle, concurrency bugs tend to be latent, and so
> backports are risky, and jdk8u is supposed to be very stable. That said,
> there are known concurrency buglets in jdk8u. With hindsight, it would have
> been good to fix them around the jdk10 release time.
> [image: Add Comment]
>  Add Comment
> 
>
> This message was sent by Atlassian Jira (v7.13.3#713003-sha1:0709f78)
> [image: Atlassian logo]
>

Re: RFR 8225198 : Optimize regex tree for greedy quantifiers of type {N, }

2019-06-04 Thread Ivan Gerasimov


Thank you Claes and Brent for reviewing!

I'll edit the comment as suggested before pushing.

And yes, we do have a few testcases to verify this optimization in 
RegExTest.java / BMPTestCases.txt.


With kind regards,

Ivan


On 6/4/19 2:30 PM, Brent Christian wrote:

Hi, Ivan

The change looks fine.

I would call this "noreg-cleanup".  Tests are needed to confirm the 
correctness of the new code, which I imagine we already have.



One small suggestion:

src/java.base/share/classes/java/util/regex/Pattern.java


4271  * and unlimited maximum, for *, + and {N,} quantifiers.


If the maximum is still MAX_REPS, I'd prefer to continue to say that 
(rather than, "unlimited.")


Thanks.
-Brent

On 6/3/19 12:54 PM, Ivan Gerasimov wrote:

Hello!

When building a match-tree, the regex engine optimizes '*' and '+' 
greedy quantifiers by using special node of type CharPropertyGreedy.


This later class can be used unmodified for other greedy quantifiers 
without the upper limit, i.e. of type '{N,}'.


Would you please help review this simple optimization?

BUGURL: https://bugs.openjdk.java.net/browse/JDK-8225198
WEBREV: http://cr.openjdk.java.net/~igerasim/8225198/00/webrev/

(With the fix, one unused class was also removed.  This class was to 
implement conditionals, which were never supported by Java regexs.)






--
With kind regards,
Ivan Gerasimov

Re: RFR XS,docs JDK-8225309 HTML issues in jdk.jdi module

2019-06-04 Thread serguei . spitsyn


Hi Jon,

Looks good to me.

Thanks,
Serguei


On 6/4/19 2:42 PM, Jonathan Gibbons wrote:
Please review another small patch for the jdk.jdi module, to fix a 
missing heading in one file and to adjust a heading in another.


No webrev; patch below.

That being said, the file 
src/jdk.jdi/share/classes/com/sun/jdi/doc-files/signature.html barely 
warrants being its own file, but even what content there is deserves 
to be fixed. The table caption is oversized, the table has no borders 
and not not use any CSS to "stripe" the rows. However, that cleanup is 
out of scope for this accessibility fix for the headings.


-- Jon

JBS: https://bugs.openjdk.java.net/browse/JDK-8225309


$ hg diff -R open
diff -r 4158e6a864d4 
src/jdk.jdi/share/classes/com/sun/jdi/doc-files/signature.html
--- a/src/jdk.jdi/share/classes/com/sun/jdi/doc-files/signature.html 
Tue Jun 04 13:47:59 2019 -0700
+++ b/src/jdk.jdi/share/classes/com/sun/jdi/doc-files/signature.html 
Tue Jun 04 14:35:34 2019 -0700

@@ -41,6 +41,7 @@
 
 
 
+JDI Type Signatures
 
 JDI Type Signatures
 
diff -r 4158e6a864d4 src/jdk.jdi/share/classes/module-info.java
--- a/src/jdk.jdi/share/classes/module-info.java Tue Jun 04 13:47:59 
2019 -0700
+++ b/src/jdk.jdi/share/classes/module-info.java Tue Jun 04 14:35:34 
2019 -0700

@@ -46,7 +46,7 @@
  * This module includes a simple command-line debugger,
  * {@index jdb jdb tool}.
  *
- * Global Exceptions
+ * Global Exceptions
  * 
  * This section documents exceptions which apply to the entire API 
and are thus

  * not documented on individual methods.

Re: RFR: XXS,docs JDK-8225324 Bad HTML in jdk.jfr module-info.java

2019-06-04 Thread Lance Andersen

+1
> On Jun 4, 2019, at 7:47 PM, Jonathan Gibbons  
> wrote:
> 
> Please review a fix to remove an unnecessary trailing  from the 
> module-info file for jdk.jfr
> 
> Patch below.
> 
> -- Jon
> 
> JBS: https://bugs.openjdk.java.net/browse/JDK-8225324
> 
> 
> $ hg diff -R open
> diff -r e079a4cfad75 src/jdk.jfr/share/classes/module-info.java
> --- a/src/jdk.jfr/share/classes/module-info.java Tue Jun 04 15:42:16 2019 
> -0700
> +++ b/src/jdk.jfr/share/classes/module-info.java Tue Jun 04 16:40:53 2019 
> -0700
> @@ -25,7 +25,6 @@
> 
>  /**
>   * Defines the API for JDK Flight Recorder.
> - * 
>   *
>   * @moduleGraph
>   * @since 9
> 
> 

 
  

 Lance Andersen| 
Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com

RFR: XXS,docs JDK-8225324 Bad HTML in jdk.jfr module-info.java

Please review a fix to remove an unnecessary trailing  from the 
module-info file for jdk.jfr


Patch below.

-- Jon

JBS: https://bugs.openjdk.java.net/browse/JDK-8225324


$ hg diff -R open
diff -r e079a4cfad75 src/jdk.jfr/share/classes/module-info.java
--- a/src/jdk.jfr/share/classes/module-info.java Tue Jun 04 15:42:16 
2019 -0700
+++ b/src/jdk.jfr/share/classes/module-info.java Tue Jun 04 16:40:53 
2019 -0700

@@ -25,7 +25,6 @@

 /**
  * Defines the API for JDK Flight Recorder.
- * 
  *
  * @moduleGraph
  * @since 9

Re: RFR: XS,docs JDK-8225314 broken links in java.base

2019-06-04 Thread Joseph D. Darcy


Looks good; thanks Jon,

-Joe

On 6/4/2019 3:28 PM, Jonathan Gibbons wrote:

Please review a small fix for a couple of broken links in java.base.

Both look like the same copy/paste error, trying to refer to a target 
id in another package.


No webrev: patch below.

-- Jon

JBS: https://bugs.openjdk.java.net/browse/JDK-8225314


$ hg diff -R open
diff -r 64fe51ee940e src/java.base/share/classes/java/lang/Enum.java
--- a/src/java.base/share/classes/java/lang/Enum.java   Tue Jun 04 
14:47:25 2019 -0700
+++ b/src/java.base/share/classes/java/lang/Enum.java   Tue Jun 04 
15:17:48 2019 -0700

@@ -283,7 +283,7 @@
 }

 /**
- * A nominal 
descriptor for an
+ * A href="{@docRoot}/java.base/java/lang/constant/package-summary.html#nominal">nominal 
descriptor for an

  * {@code enum} constant.
  *
  * @param  the type of the enum constant
diff -r 64fe51ee940e 
src/java.base/share/classes/java/lang/invoke/VarHandle.java
--- a/src/java.base/share/classes/java/lang/invoke/VarHandle.java Tue 
Jun 04 14:47:25 2019 -0700
+++ b/src/java.base/share/classes/java/lang/invoke/VarHandle.java Tue 
Jun 04 15:17:48 2019 -0700

@@ -2118,7 +2118,7 @@
 }

 /**
- * A nominal 
descriptor for a
+ * A href="{@docRoot}/java.base/java/lang/constant/package-summary.html#nominal">nominal 
descriptor for a

  * {@link VarHandle} constant.
  *
  * @since 12

RFR: XS,docs JDK-8225314 broken links in java.base


Please review a small fix for a couple of broken links in java.base.

Both look like the same copy/paste error, trying to refer to a target id 
in another package.


No webrev: patch below.

-- Jon

JBS: https://bugs.openjdk.java.net/browse/JDK-8225314


$ hg diff -R open
diff -r 64fe51ee940e src/java.base/share/classes/java/lang/Enum.java
--- a/src/java.base/share/classes/java/lang/Enum.java   Tue Jun 04 
14:47:25 2019 -0700
+++ b/src/java.base/share/classes/java/lang/Enum.java   Tue Jun 04 
15:17:48 2019 -0700

@@ -283,7 +283,7 @@
 }

 /**
- * A nominal descriptor 
for an
+ * A href="{@docRoot}/java.base/java/lang/constant/package-summary.html#nominal">nominal 
descriptor for an

  * {@code enum} constant.
  *
  * @param  the type of the enum constant
diff -r 64fe51ee940e 
src/java.base/share/classes/java/lang/invoke/VarHandle.java
--- a/src/java.base/share/classes/java/lang/invoke/VarHandle.java Tue 
Jun 04 14:47:25 2019 -0700
+++ b/src/java.base/share/classes/java/lang/invoke/VarHandle.java Tue 
Jun 04 15:17:48 2019 -0700

@@ -2118,7 +2118,7 @@
 }

 /**
- * A nominal descriptor 
for a
+ * A href="{@docRoot}/java.base/java/lang/constant/package-summary.html#nominal">nominal 
descriptor for a

  * {@link VarHandle} constant.
  *
  * @since 12

Re: RFR (XS): 8225305: ProblemList java/lang/invoke/VarHandles tests

2019-06-04 Thread Mandy Chung


I agree with Igor.  The best is to keep running these tests except
the AOT, perhaps + fastdebug, run only.

Mandy

On 6/4/19 2:06 PM, Igor Ignatyev wrote:

Hi Mikael,

as it looks like 8222445 isn't going to be fixed for a long time (as it's 
"targeted" to tbd), and the defect seems to affect only AOT run, I don't think 
it's a good idea to put these tests into a general problem list. I'd suggest to either 
create aot-specific problem list or put them into -graal specific problem list.

-- Igor


On Jun 4, 2019, at 1:38 PM, Mikael Vidstedt  wrote:


The following java/lang/invoke/VarHandles tests frequently fail when run with AOT. 
Until https://bugs.openjdk.java.net/browse/JDK-8222445 
 has been fixed they should 
be problem listed.

java/lang/invoke/VarHandles/VarHandleTestByteArrayAsShort.java
java/lang/invoke/VarHandles/VarHandleTestByteArrayAsChar.java
java/lang/invoke/VarHandles/VarHandleTestAccessBoolean.java

bug: https://bugs.openjdk.java.net/browse/JDK-8225305 

webrev: http://cr.openjdk.java.net/~mikael/webrevs/8225305/webrev.00/open/webrev/ 


Cheers,
Mikael

Re: RFR XS,docs JDK-8225309 HTML issues in jdk.jdi module

2019-06-04 Thread Lance Andersen

+1
> On Jun 4, 2019, at 5:42 PM, Jonathan Gibbons  
> wrote:
> 
> Please review another small patch for the jdk.jdi module, to fix a missing 
> heading in one file and to adjust a heading in another.
> 
> No webrev; patch below.
> 
> That being said, the file 
> src/jdk.jdi/share/classes/com/sun/jdi/doc-files/signature.html barely 
> warrants being its own file, but even what content there is deserves to be 
> fixed. The table caption is oversized, the table has no borders and not not 
> use any CSS to "stripe" the rows. However, that cleanup is out of scope for 
> this accessibility fix for the headings.
> 
> -- Jon
> 
> JBS: https://bugs.openjdk.java.net/browse/JDK-8225309
> 
> 
> $ hg diff -R open
> diff -r 4158e6a864d4 
> src/jdk.jdi/share/classes/com/sun/jdi/doc-files/signature.html
> --- a/src/jdk.jdi/share/classes/com/sun/jdi/doc-files/signature.html Tue Jun 
> 04 13:47:59 2019 -0700
> +++ b/src/jdk.jdi/share/classes/com/sun/jdi/doc-files/signature.html Tue Jun 
> 04 14:35:34 2019 -0700
> @@ -41,6 +41,7 @@
>  
>  
>  
> +JDI Type Signatures
>  
>  JDI Type Signatures
>  
> diff -r 4158e6a864d4 src/jdk.jdi/share/classes/module-info.java
> --- a/src/jdk.jdi/share/classes/module-info.java Tue Jun 04 13:47:59 2019 
> -0700
> +++ b/src/jdk.jdi/share/classes/module-info.java Tue Jun 04 14:35:34 2019 
> -0700
> @@ -46,7 +46,7 @@
>   * This module includes a simple command-line debugger,
>   * {@index jdb jdb tool}.
>   *
> - * Global Exceptions
> + * Global Exceptions
>   * 
>   * This section documents exceptions which apply to the entire API and are 
> thus
>   * not documented on individual methods.
> 

 
  

 Lance Andersen| 
Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com

RFR XS,docs JDK-8225309 HTML issues in jdk.jdi module

Please review another small patch for the jdk.jdi module, to fix a 
missing heading in one file and to adjust a heading in another.


No webrev; patch below.

That being said, the file 
src/jdk.jdi/share/classes/com/sun/jdi/doc-files/signature.html barely 
warrants being its own file, but even what content there is deserves to 
be fixed. The table caption is oversized, the table has no borders and 
not not use any CSS to "stripe" the rows. However, that cleanup is out 
of scope for this accessibility fix for the headings.


-- Jon

JBS: https://bugs.openjdk.java.net/browse/JDK-8225309


$ hg diff -R open
diff -r 4158e6a864d4 
src/jdk.jdi/share/classes/com/sun/jdi/doc-files/signature.html
--- a/src/jdk.jdi/share/classes/com/sun/jdi/doc-files/signature.html Tue 
Jun 04 13:47:59 2019 -0700
+++ b/src/jdk.jdi/share/classes/com/sun/jdi/doc-files/signature.html Tue 
Jun 04 14:35:34 2019 -0700

@@ -41,6 +41,7 @@
 
 
 
+JDI Type Signatures
 
 JDI Type Signatures
 
diff -r 4158e6a864d4 src/jdk.jdi/share/classes/module-info.java
--- a/src/jdk.jdi/share/classes/module-info.java Tue Jun 04 13:47:59 
2019 -0700
+++ b/src/jdk.jdi/share/classes/module-info.java Tue Jun 04 14:35:34 
2019 -0700

@@ -46,7 +46,7 @@
  * This module includes a simple command-line debugger,
  * {@index jdb jdb tool}.
  *
- * Global Exceptions
+ * Global Exceptions
  * 
  * This section documents exceptions which apply to the entire API and 
are thus

  * not documented on individual methods.

Re: RFR 8225198 : Optimize regex tree for greedy quantifiers of type {N, }

2019-06-04 Thread Brent Christian


Hi, Ivan

The change looks fine.

I would call this "noreg-cleanup".  Tests are needed to confirm the 
correctness of the new code, which I imagine we already have.



One small suggestion:

src/java.base/share/classes/java/util/regex/Pattern.java


4271  * and unlimited maximum, for *, + and {N,} quantifiers.


If the maximum is still MAX_REPS, I'd prefer to continue to say that 
(rather than, "unlimited.")


Thanks.
-Brent

On 6/3/19 12:54 PM, Ivan Gerasimov wrote:

Hello!

When building a match-tree, the regex engine optimizes '*' and '+' 
greedy quantifiers by using special node of type CharPropertyGreedy.


This later class can be used unmodified for other greedy quantifiers 
without the upper limit, i.e. of type '{N,}'.


Would you please help review this simple optimization?

BUGURL: https://bugs.openjdk.java.net/browse/JDK-8225198
WEBREV: http://cr.openjdk.java.net/~igerasim/8225198/00/webrev/

(With the fix, one unused class was also removed.  This class was to 
implement conditionals, which were never supported by Java regexs.)

Re: RFR (XS): 8225305: ProblemList java/lang/invoke/VarHandles tests

2019-06-04 Thread Igor Ignatyev

Hi Mikael,

as it looks like 8222445 isn't going to be fixed for a long time (as it's 
"targeted" to tbd), and the defect seems to affect only AOT run, I don't think 
it's a good idea to put these tests into a general problem list. I'd suggest to 
either create aot-specific problem list or put them into -graal specific 
problem list.

-- Igor 

> On Jun 4, 2019, at 1:38 PM, Mikael Vidstedt  
> wrote:
> 
> 
> The following java/lang/invoke/VarHandles tests frequently fail when run with 
> AOT. Until https://bugs.openjdk.java.net/browse/JDK-8222445 
>  has been fixed they should 
> be problem listed. 
> 
> java/lang/invoke/VarHandles/VarHandleTestByteArrayAsShort.java 
> java/lang/invoke/VarHandles/VarHandleTestByteArrayAsChar.java 
> java/lang/invoke/VarHandles/VarHandleTestAccessBoolean.java
> 
> bug: https://bugs.openjdk.java.net/browse/JDK-8225305 
> 
> webrev: 
> http://cr.openjdk.java.net/~mikael/webrevs/8225305/webrev.00/open/webrev/ 
> 
> 
> Cheers,
> Mikael
>

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error


Hmm.  Can_JOHNNY_EXEC naming is a bit quirky, not intuitive...


On 06/04/2019 04:52 PM, Martin Buchholz wrote:



On Tue, Jun 4, 2019 at 10:40 AM Thomas Stüfe > wrote:




On Tue, Jun 4, 2019 at 7:21 PM Martin Buchholz
mailto:marti...@google.com>> wrote:


Unfortunately, it does make the subprocess implementation even
more complicated, since now "fail" is used to communicate
success as well as failure.  Which probably results in some
comments becoming stale.  Can we come up with a better name
for "fail" that reflects the new implementation?  I suggest
"can_johnny_exec".


As in the can_johnny_exec_pipe and its CAN_JOHNNY_EXEC_PIPE_FD ?
Sure, why not :).


Let's do it.  Not sure whether this will make our readability expert 
Rudolf Flesch roll over in his grave.

Re: RFR (XS): 8225305: ProblemList java/lang/invoke/VarHandles tests

2019-06-04 Thread Vladimir Kozlov

Looks good.

Thanks
Vladimir

> On Jun 4, 2019, at 1:38 PM, Mikael Vidstedt  
> wrote:
> 
> 
> The following java/lang/invoke/VarHandles tests frequently fail when run with 
> AOT. Until https://bugs.openjdk.java.net/browse/JDK-8222445 has been fixed 
> they should be problem listed. 
> 
> java/lang/invoke/VarHandles/VarHandleTestByteArrayAsShort.java 
> java/lang/invoke/VarHandles/VarHandleTestByteArrayAsChar.java 
> java/lang/invoke/VarHandles/VarHandleTestAccessBoolean.java
> 
> bug: https://bugs.openjdk.java.net/browse/JDK-8225305
> webrev: 
> http://cr.openjdk.java.net/~mikael/webrevs/8225305/webrev.00/open/webrev/
> 
> Cheers,
> Mikael
>

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

On Tue, Jun 4, 2019 at 10:40 AM Thomas Stüfe 
wrote:

>
>
> On Tue, Jun 4, 2019 at 7:21 PM Martin Buchholz 
> wrote:
>
>>
>> Unfortunately, it does make the subprocess implementation even
>> more complicated, since now "fail" is used to communicate success as well
>> as failure.  Which probably results in some comments becoming stale.  Can
>> we come up with a better name for "fail" that reflects the new
>> implementation?  I suggest "can_johnny_exec".
>>
>>
> As in the can_johnny_exec_pipe and its CAN_JOHNNY_EXEC_PIPE_FD ? Sure, why
> not :).
>

Let's do it.  Not sure whether this will make our readability expert Rudolf
Flesch roll over in his grave.

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

Hi Florian,

Interesting, but in this case it is not posix_spawn, but plain fork(). The
VM does this, the exec calls come from us, not the libc. See childproc.c .

Cheers, Thomas’

On Tue 4. Jun 2019 at 22:42, Florian Weimer  wrote:

> * Thomas Stüfe:
>
> > Then I ran an strace over it and saw this:
> >
> > 5332 [pid  3911] execve("./sleep2", ["./sleep2"], [/* 78 vars */]
> 
> >
>  ..
> > 5342 [pid  3911] <... execve resumed> )  = -1 ENOEXEC (Exec format
> error)
> > 5343 [pid  3911] execve("/bin/sh", ["/bin/sh", "./sleep2"], [/* 78 vars
> */] 
> > 5347 [pid  3911] <... execve resumed> )  = 0
> >
> > So, if the first exec fails for whatever reason, we try again, passing
> the executable file name
> > as argument to the system shell. This does not feel right? Do you know
> why we do this?
>
> What's your glibc version?  Unless it's ancient, this is probably this
> bug:
>
>   
>
> I don't know why this behavior was part of the initial implementation of
> posix_spawn.  It was subsequently removed for new binaries, but came
> back as a regression (the bug I referenced).
>
> Thanks,
> Florian
>

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-06-04 Thread Florian Weimer

* Thomas Stüfe:

> Then I ran an strace over it and saw this:
>
> 5332 [pid  3911] execve("./sleep2", ["./sleep2"], [/* 78 vars */]  ...>   
>   
> ..  
> 5342 [pid  3911] <... execve resumed> )  = -1 ENOEXEC (Exec format error)
> 5343 [pid  3911] execve("/bin/sh", ["/bin/sh", "./sleep2"], [/* 78 vars */] 
> 
> 5347 [pid  3911] <... execve resumed> )  = 0
>
> So, if the first exec fails for whatever reason, we try again, passing the 
> executable file name
> as argument to the system shell. This does not feel right? Do you know why we 
> do this? 

What's your glibc version?  Unless it's ancient, this is probably this
bug:

  

I don't know why this behavior was part of the initial implementation of
posix_spawn.  It was subsequently removed for new binaries, but came
back as a regression (the bug I referenced).

Thanks,
Florian

Re: RFR: XS,doc JDK-8225306 bad headings in java.sql.rowset SyncProvider.java

2019-06-04 Thread Lance Andersen

 a Tiny +1 ;-)

> On Jun 4, 2019, at 4:33 PM, Jonathan Gibbons  
> wrote:
> 
> Please review a tiny doc fix to make the headings in this file consistent.
> 
> The headings are currently:
> 
> $ grep 'h[1-6]' 
> open/src/java.sql.rowset/share/classes/javax/sql/rowset/spi/SyncProvider.java
>  * 1.0 Naming Convention for Implementations
>  * 2.0 How a RowSet Object Gets Its Provider
>  * 3.0 Violations and Synchronization Issues
>  * 4.0 Updatable SQL VIEWs
>  * 5.0 SyncProvider Constants
> 
> 
> The first one should be changed to .
> 
> No webrev; patch below.
> 
> -- Jon
> 
> 
> JBS: https://bugs.openjdk.java.net/browse/JDK-8225306
> 
> $ hg diff -R open
> diff -r ef23ea332077 
> src/java.sql.rowset/share/classes/javax/sql/rowset/spi/SyncProvider.java
> --- 
> a/src/java.sql.rowset/share/classes/javax/sql/rowset/spi/SyncProvider.java 
> Tue Jun 04 11:55:51 2019 -0700
> +++ 
> b/src/java.sql.rowset/share/classes/javax/sql/rowset/spi/SyncProvider.java 
> Tue Jun 04 13:31:42 2019 -0700
> @@ -55,7 +55,7 @@
>   * data.  It uses the XmlWriter object to write itself to a 
> stream or
>   * java.io.Writer object in XML format.
>   *
> - * 1.0 Naming Convention for Implementations
> + * 1.0 Naming Convention for Implementations
>   * As a guide  to naming SyncProvider
>   * implementations, the following should be noted:
>   * 
> 
> 
> 

 
  

 Lance Andersen| 
Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com

RFR (XS): 8225305: ProblemList java/lang/invoke/VarHandles tests

2019-06-04 Thread Mikael Vidstedt



The following java/lang/invoke/VarHandles tests frequently fail when run with 
AOT. Until https://bugs.openjdk.java.net/browse/JDK-8222445 
 has been fixed they should 
be problem listed. 

java/lang/invoke/VarHandles/VarHandleTestByteArrayAsShort.java 
java/lang/invoke/VarHandles/VarHandleTestByteArrayAsChar.java 
java/lang/invoke/VarHandles/VarHandleTestAccessBoolean.java

bug: https://bugs.openjdk.java.net/browse/JDK-8225305 

webrev: 
http://cr.openjdk.java.net/~mikael/webrevs/8225305/webrev.00/open/webrev/ 


Cheers,
Mikael

RFR: XS,doc JDK-8225306 bad headings in java.sql.rowset SyncProvider.java


Please review a tiny doc fix to make the headings in this file consistent.

The headings are currently:

$ grep 'h[1-6]' 
open/src/java.sql.rowset/share/classes/javax/sql/rowset/spi/SyncProvider.java

 * 1.0 Naming Convention for Implementations
 * 2.0 How a RowSet Object Gets Its Provider
 * 3.0 Violations and Synchronization Issues
 * 4.0 Updatable SQL VIEWs
 * 5.0 SyncProvider Constants


The first one should be changed to .

No webrev; patch below.

-- Jon


JBS: https://bugs.openjdk.java.net/browse/JDK-8225306

$ hg diff -R open
diff -r ef23ea332077 
src/java.sql.rowset/share/classes/javax/sql/rowset/spi/SyncProvider.java
--- 
a/src/java.sql.rowset/share/classes/javax/sql/rowset/spi/SyncProvider.java 
Tue Jun 04 11:55:51 2019 -0700
+++ 
b/src/java.sql.rowset/share/classes/javax/sql/rowset/spi/SyncProvider.java 
Tue Jun 04 13:31:42 2019 -0700

@@ -55,7 +55,7 @@
  * data.  It uses the XmlWriter object to write itself to 
a stream or

  * java.io.Writer object in XML format.
  *
- * 1.0 Naming Convention for Implementations
+ * 1.0 Naming Convention for Implementations
  * As a guide  to naming SyncProvider
  * implementations, the following should be noted:
  *

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

2019-06-04 Thread David Lloyd

In this case, going from what the execve(2) Linux man page says (and Mac
seems to agree), if the script were executable it would have a '#!'
interpreter string and the kernel would execute it anyway, would it not?
What case would be solved by explicitly starting a shell?

On Tue, Jun 4, 2019 at 2:15 PM Roger Riggs  wrote:

> Hi Thomas,
>
> If the argument is not an .exe, then it may be a command shell script
> (.sh, etc.)
> Only the system knows it is not an exe (errno == ENOEXEC), so it then
> passes it as
> the first argument to /bin/sh that will handle the shell files.
>
> And yes, count me as a Reviewer and reviewed.
>
> Roger
>
> On 06/04/2019 12:14 PM, Thomas Stüfe wrote:
>
> On Tue, Jun 4, 2019 at 5:09 PM Roger Riggs  wrote:
>
> ...
>
>
> Then I ran an strace over it and saw this:
>
> 5332 [pid  3911] execve("./sleep2", ["./sleep2"], [/* 78 vars */]
> 
>   ..
> 5342 [pid  3911] <... execve resumed> )  = -1 ENOEXEC (Exec format
> error)
> 5343 [pid  3911] execve("/bin/sh", ["/bin/sh", "./sleep2"], [/* 78 vars
> */] 
> 5347 [pid  3911] <... execve resumed> )  = 0
>
> So, if the first exec fails for whatever reason, we try again, passing the
> executable file name as argument to the system shell. This does not feel
> right? Do you know why we do this?
>
>
>>
>> Thanks, Roger
>>
>>
> Thank you Roger. Can I consider the patch to be reviewed by you?
>
> ..Thomas
>
>
>

-- 
- DML

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

Thank you for clarifying, and for the review.

Cheers, Thomas

On Tue, Jun 4, 2019 at 9:14 PM Roger Riggs  wrote:

> Hi Thomas,
>
> If the argument is not an .exe, then it may be a command shell script
> (.sh, etc.)
> Only the system knows it is not an exe (errno == ENOEXEC), so it then
> passes it as
> the first argument to /bin/sh that will handle the shell files.
>
> And yes, count me as a Reviewer and reviewed.
>
> Roger
>
> On 06/04/2019 12:14 PM, Thomas Stüfe wrote:
>
> On Tue, Jun 4, 2019 at 5:09 PM Roger Riggs  wrote:
>
> ...
>
>
> Then I ran an strace over it and saw this:
>
> 5332 [pid  3911] execve("./sleep2", ["./sleep2"], [/* 78 vars */]
> 
>   ..
> 5342 [pid  3911] <... execve resumed> )  = -1 ENOEXEC (Exec format
> error)
> 5343 [pid  3911] execve("/bin/sh", ["/bin/sh", "./sleep2"], [/* 78 vars
> */] 
> 5347 [pid  3911] <... execve resumed> )  = 0
>
> So, if the first exec fails for whatever reason, we try again, passing the
> executable file name as argument to the system shell. This does not feel
> right? Do you know why we do this?
>
>
>>
>> Thanks, Roger
>>
>>
> Thank you Roger. Can I consider the patch to be reviewed by you?
>
> ..Thomas
>
>
>

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error


Hi Thomas,

If the argument is not an .exe, then it may be a command shell script  
(.sh, etc.)
Only the system knows it is not an exe (errno == ENOEXEC), so it then 
passes it as

the first argument to /bin/sh that will handle the shell files.

And yes, count me as a Reviewer and reviewed.

Roger

On 06/04/2019 12:14 PM, Thomas Stüfe wrote:

On Tue, Jun 4, 2019 at 5:09 PM Roger Riggs > wrote:



...


Then I ran an strace over it and saw this:

5332 [pid  3911] execve("./sleep2", ["./sleep2"], [/* 78 vars */] 
                 ..
5342 [pid  3911] <... execve resumed> )      = -1 ENOEXEC (Exec format 
error)
5343 [pid  3911] execve("/bin/sh", ["/bin/sh", "./sleep2"], [/* 78 
vars */] 

5347 [pid  3911] <... execve resumed> )      = 0

So, if the first exec fails for whatever reason, we try again, passing 
the executable file name as argument to the system shell. This does 
not feel right? Do you know why we do this?



Thanks, Roger


Thank you Roger. Can I consider the patch to be reviewed by you?
..Thomas

Re: Review Request JDK-8221368: Error message when module main class cannot be loaded is missing exception details

2019-06-04 Thread Mandy Chung


Alan, Sundar,

Thanks for the review.

I further clean up the test and rename jdk.test.Main2 class to a new 
test2 module.  No change to the fix.


http://cr.openjdk.java.net/~mchung/jdk13/webrevs/8221368/webrev.01/

Mandy

On 6/4/19 12:10 AM, Alan Bateman wrote:

On 04/06/2019 05:00, Mandy Chung wrote:

The launcher prints out the error message when the main class fails to
initialize but java.launcher.module.error5 resource that intends to
print the Caused-by exception message when running in a named module
but prints an incorrect parameter.

Very simple fix - fix the resource param indices.  Also fix a
typo: "module module " in the output.

http://cr.openjdk.java.net/~mchung/jdk13/webrevs/8221368/webrev.00/

The error message when the main class is in an unnamed module
is printed properly and covered by an existing test
(test/jdk/tools/launcher/MainClassCantBeLoadedTest.java).


This looks good.

-Alan

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

On Tue, Jun 4, 2019 at 7:25 PM Martin Buchholz  wrote:

>
>
> On Tue, Jun 4, 2019 at 8:09 AM Roger Riggs  wrote:
>
>> Hi Thomas,
>>
>> A minor concern is the impact of the extra write and read that can cause
>> rescheduling
>> of the parent and child processes.  But that's probably in the noise
>> compared to the
>> real work of exec.  It would raise the complexity quite a bit/too much to
>> code a single read
>> in the parent that could expect 0/4/8 bytes.
>>
>> At ProcessImpl_md.c: 708: the "Read failed" is less than informative.
>> (Though it is the same as the pre-existing one at 720).
>> But I suppose it has never happened.  The 'Exec failed' is more specific
>> than 'read'.
>> And it has probably never been seen.
>>
>
> The call looks like
> throwIOException(env, errno, "Read failed");
> and that at least includes an errno, so the resulting exception should be
> useful for debugging.  But yeah, we can probably do better than "Read
> failed".
>

How about "Failed to communicate with child process?"

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

On Tue, Jun 4, 2019 at 7:21 PM Martin Buchholz  wrote:

> Hi Thomas,
>
> Thanks - I agree with the approach.
>
> Unfortunately, it does make the subprocess implementation even
> more complicated, since now "fail" is used to communicate success as well
> as failure.  Which probably results in some comments becoming stale.  Can
> we come up with a better name for "fail" that reflects the new
> implementation?  I suggest "can_johnny_exec".
>
>
As in the can_johnny_exec_pipe and its CAN_JOHNNY_EXEC_PIPE_FD ? Sure, why
not :).

I agree on the complexity remark. I do not really like it either. Maybe we
can remove this fix again if in the far future all posix_spawn() variants
in the wild report exec errors back like they should. I believe Florian
added already a patch to the glibc?

Thanks, Thomas



> On Mon, Jun 3, 2019 at 11:07 PM Thomas Stüfe 
> wrote:
>
>> Hi all,
>>
>> may I please have reviews/opinions on this fix?
>>
>> Fix has been live in our test landscape since some weeks.
>>
>> If we do not want this fix to be in JDK13, we may want to revert the
>> posix_spawn-by-default-on-Linux change.
>>
>> Thanks, Thomas
>>
>>
>> On Mon, May 20, 2019 at 4:15 PM Thomas Stüfe 
>> wrote:
>>
>>> Hi all,
>>>
>>> (old mail thread:
>>> https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-May/060139.html
>>> )
>>>
>>> May I please have your reviews and opinions for the following bug fix:
>>>
>>> issue: https://bugs.openjdk.java.net/browse/JDK-8223777
>>> cr:
>>> http://cr.openjdk.java.net/~stuefe/webrevs/8223777-posix_spawn-no-exec-error-alternate-impl/webrev.00/webrev/
>>>
>>> ---
>>>
>>> The fix implements Florians proposal: the jspawnhelper will signal its
>>> aliveness to the parent process the moment it gains control. If the parent
>>> process does not get the signal, it assumes exec'ing the jspawnhelper never
>>> worked.
>>>
>>> I only do this for posix_spawn mode, out of cautiousness.
>>>
>>> (Note that I kept the fix as minimal as possible. I found a minor bug
>>> and some improvement possibilities and opened follow up issues to track
>>> them: JDK-8224180 and JDK-8224181).
>>>
>>> Thanks, Thomas
>>>
>>>
>>>

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

On Tue, Jun 4, 2019 at 8:09 AM Roger Riggs  wrote:

> Hi Thomas,
>
> A minor concern is the impact of the extra write and read that can cause
> rescheduling
> of the parent and child processes.  But that's probably in the noise
> compared to the
> real work of exec.  It would raise the complexity quite a bit/too much to
> code a single read
> in the parent that could expect 0/4/8 bytes.
>
> At ProcessImpl_md.c: 708: the "Read failed" is less than informative.
> (Though it is the same as the pre-existing one at 720).
> But I suppose it has never happened.  The 'Exec failed' is more specific
> than 'read'.
> And it has probably never been seen.
>

The call looks like
throwIOException(env, errno, "Read failed");
and that at least includes an errno, so the resulting exception should be
useful for debugging.  But yeah, we can probably do better than "Read
failed".

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

Hi Thomas,

Thanks - I agree with the approach.

Unfortunately, it does make the subprocess implementation even
more complicated, since now "fail" is used to communicate success as well
as failure.  Which probably results in some comments becoming stale.  Can
we come up with a better name for "fail" that reflects the new
implementation?  I suggest "can_johnny_exec".

On Mon, Jun 3, 2019 at 11:07 PM Thomas Stüfe 
wrote:

> Hi all,
>
> may I please have reviews/opinions on this fix?
>
> Fix has been live in our test landscape since some weeks.
>
> If we do not want this fix to be in JDK13, we may want to revert the
> posix_spawn-by-default-on-Linux change.
>
> Thanks, Thomas
>
>
> On Mon, May 20, 2019 at 4:15 PM Thomas Stüfe 
> wrote:
>
>> Hi all,
>>
>> (old mail thread:
>> https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-May/060139.html
>> )
>>
>> May I please have your reviews and opinions for the following bug fix:
>>
>> issue: https://bugs.openjdk.java.net/browse/JDK-8223777
>> cr:
>> http://cr.openjdk.java.net/~stuefe/webrevs/8223777-posix_spawn-no-exec-error-alternate-impl/webrev.00/webrev/
>>
>> ---
>>
>> The fix implements Florians proposal: the jspawnhelper will signal its
>> aliveness to the parent process the moment it gains control. If the parent
>> process does not get the signal, it assumes exec'ing the jspawnhelper never
>> worked.
>>
>> I only do this for posix_spawn mode, out of cautiousness.
>>
>> (Note that I kept the fix as minimal as possible. I found a minor bug and
>> some improvement possibilities and opened follow up issues to track them:
>> JDK-8224180 and JDK-8224181).
>>
>> Thanks, Thomas
>>
>>
>>

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

Hi Roger,

thanks for the feedback. Please find my answers inline.

On Tue, Jun 4, 2019 at 5:09 PM Roger Riggs  wrote:

> Hi Thomas,
>
> A minor concern is the impact of the extra write and read that can cause
> rescheduling
> of the parent and child processes.  But that's probably in the noise
> compared to the
> real work of exec.
>

I would think so too. exec() needs to read the binary from the file system,
so there is some IO at least to come.


>   It would raise the complexity quite a bit/too much to code a single read
> in the parent that could expect 0/4/8 bytes.
>
>
I guess I could do that.. but I am not sure it is worthwhile. In our
propietary port we have a more involved solution with some more read-write
cycles between child and parent. I am not saying this is better - there are
pros and cons - but I never saw any performance issues with our solution
compared to OpenJDK. Based on that I believe one more read would be fine.


> At ProcessImpl_md.c: 708: the "Read failed" is less than informative.
> (Though it is the same as the pre-existing one at 720).
>

Yes. I have a follow up issue open (linked in the JBS issue) to improve
reporting on errors. That may improve matters a bit.


> But I suppose it has never happened.  The 'Exec failed' is more specific
> than 'read'.
> And it has probably never been seen.
>

You know this is interesting. I thought exec errors should be not that
uncommon or difficult to provoke. I did a test. I maimed an executable such
that I cannot execute it without getting an exec format error (ENOEXEC).
Then I tried to spawn it with Runtime.exec. But that appeared to be
successful, but the child process died immediately. Happened regardless of
launchMechanism.

Then I ran an strace over it and saw this:

5332 [pid  3911] execve("./sleep2", ["./sleep2"], [/* 78 vars */]

  ..
5342 [pid  3911] <... execve resumed> )  = -1 ENOEXEC (Exec format
error)
5343 [pid  3911] execve("/bin/sh", ["/bin/sh", "./sleep2"], [/* 78 vars */]

5347 [pid  3911] <... execve resumed> )  = 0

So, if the first exec fails for whatever reason, we try again, passing the
executable file name as argument to the system shell. This does not feel
right? Do you know why we do this?


>
> Thanks, Roger
>
>
Thank you Roger. Can I consider the patch to be reviewed by you?

..Thomas


> On 06/04/2019 02:06 AM, Thomas Stüfe wrote:
>
> Hi all,
>
> may I please have reviews/opinions on this fix?
>
> Fix has been live in our test landscape since some weeks.
>
> If we do not want this fix to be in JDK13, we may want to revert the
> posix_spawn-by-default-on-Linux change.
>
> Thanks, Thomas
>
>
> On Mon, May 20, 2019 at 4:15 PM Thomas Stüfe 
> wrote:
>
>> Hi all,
>>
>> (old mail thread:
>> https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-May/060139.html
>> )
>>
>> May I please have your reviews and opinions for the following bug fix:
>>
>> issue: https://bugs.openjdk.java.net/browse/JDK-8223777
>> cr:
>> http://cr.openjdk.java.net/~stuefe/webrevs/8223777-posix_spawn-no-exec-error-alternate-impl/webrev.00/webrev/
>>
>> ---
>>
>> The fix implements Florians proposal: the jspawnhelper will signal its
>> aliveness to the parent process the moment it gains control. If the parent
>> process does not get the signal, it assumes exec'ing the jspawnhelper never
>> worked.
>>
>> I only do this for posix_spawn mode, out of cautiousness.
>>
>> (Note that I kept the fix as minimal as possible. I found a minor bug and
>> some improvement possibilities and opened follow up issues to track them:
>> JDK-8224180 and JDK-8224181).
>>
>> Thanks, Thomas
>>
>>
>>
>

Re: RFR(xs): 8224181: On child process spawn, child may write to random file descriptor instead of the fail pipe

Thank you Roger!

On Tue, Jun 4, 2019 at 5:13 PM Roger Riggs  wrote:

> H Thomas,
>
> Looks ok.
>
> Roger
>
>
> On 06/04/2019 09:23 AM, Thomas Stüfe wrote:
> > Hi all,
> >
> > may I please have reviews for this small fix:
> >
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8224181
> > cr:
> >
> http://cr.openjdk.java.net/~stuefe/webrevs/8224181-on-child-process-spawn--child-may-write-to-wrong-file-descriptor-instead-of-the-fail-pipe/webrev.00/webrev/
> >
> > In sub process error handling code (WhyCantJonnyExec) child signals error
> > to parent by writing an error code to the fail pipe. It does that using
> the
> > hard wired fail pipe write-end fd (FAIL_FILENO, usually 4). But that only
> > works as intended after the fail pipe write end has been successfully
> > dup2'ed to FAIL_FILENO.
> >
> > If an error happens before that, error code will still be written to
> > FAIL_FILENO, which may be an invalid file handle - which is almost
> benign -
> > or refer to an unrelated file descriptor the child inherited and did not
> > close yet - which is not good.
> >
> > Cheers, Thomas
>
>

Re: RFR(xs): 8224181: On child process spawn, child may write to random file descriptor instead of the fail pipe


H Thomas,

Looks ok.

Roger


On 06/04/2019 09:23 AM, Thomas Stüfe wrote:

Hi all,

may I please have reviews for this small fix:

Bug: https://bugs.openjdk.java.net/browse/JDK-8224181
cr:
http://cr.openjdk.java.net/~stuefe/webrevs/8224181-on-child-process-spawn--child-may-write-to-wrong-file-descriptor-instead-of-the-fail-pipe/webrev.00/webrev/

In sub process error handling code (WhyCantJonnyExec) child signals error
to parent by writing an error code to the fail pipe. It does that using the
hard wired fail pipe write-end fd (FAIL_FILENO, usually 4). But that only
works as intended after the fail pipe write end has been successfully
dup2'ed to FAIL_FILENO.

If an error happens before that, error code will still be written to
FAIL_FILENO, which may be an invalid file handle - which is almost benign -
or refer to an unrelated file descriptor the child inherited and did not
close yet - which is not good.

Cheers, Thomas

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

Hi Thomas,

A minor concern is the impact of the extra write and read that can cause
rescheduling
of the parent and child processes. But that's probably in the noise
compared to the
real work of exec. It would raise the complexity quite a bit/too much
to code a single read

in the parent that could expect 0/4/8 bytes.

At ProcessImpl_md.c: 708: the "Read failed" is less than informative.
(Though it is the same as the pre-existing one at 720).
But I suppose it has never happened. The 'Exec failed' is more specific
than 'read'.

And it has probably never been seen.

Thanks, Roger

On 06/04/2019 02:06 AM, Thomas Stüfe wrote:

Hi all,

may I please have reviews/opinions on this fix?

Fix has been live in our test landscape since some weeks.

If we do not want this fix to be in JDK13, we may want to revert the
posix_spawn-by-default-on-Linux change.

Thanks, Thomas

On Mon, May 20, 2019 at 4:15 PM Thomas Stüfe > wrote:

Hi all,

(old mail thread:
https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-May/060139.html)

May I please have your reviews and opinions for the following bug fix:

issue: https://bugs.openjdk.java.net/browse/JDK-8223777
cr:

http://cr.openjdk.java.net/~stuefe/webrevs/8223777-posix_spawn-no-exec-error-alternate-impl/webrev.00/webrev/

---

The fix implements Florians proposal: the jspawnhelper will signal
its aliveness to the parent process the moment it gains control.
If the parent process does not get the signal, it assumes exec'ing
the jspawnhelper never worked.

I only do this for posix_spawn mode, out of cautiousness.

(Note that I kept the fix as minimal as possible. I found a minor
bug and some improvement possibilities and opened follow up issues
to track them: JDK-8224180 and JDK-8224181).

Thanks, Thomas

RE: RFR 8213031: (zipfs) Add support for POSIX file permissions

2019-06-04 Thread Langer, Christoph

Hi Alan,

I made some updates to the CSR. The main update was that I transferred the new 
documentation part from module-info.java to the CSR's Specification section.

Can you please review the CSR?

Thanks
Christoph

From: Alan Bateman 
Sent: Montag, 3. Juni 2019 20:42
To: Langer, Christoph ; Lance Andersen 

Cc: nio-dev ; Java Core Libs 

Subject: Re: RFR 8213031: (zipfs) Add support for POSIX file permissions

On 02/06/2019 22:35, Langer, Christoph wrote:

Hi Alan, Lance,

thanks for the updated wording in module-info, that really looks good. I 
incorporated it into my change, no we'd be here:
http://cr.openjdk.java.net/~clanger/webrevs/8213031.13/

To be honest, I was hoping it would still make it into JDK13.

I guess now I shall update the CSR to get it reviewed, correct?

The spec looks good. The CSR is the next step, it can happen while the changes 
are being reviewed here.

-Alan

RFR(xs): 8224181: On child process spawn, child may write to random file descriptor instead of the fail pipe

Hi all,

may I please have reviews for this small fix:

Bug: https://bugs.openjdk.java.net/browse/JDK-8224181
cr:
http://cr.openjdk.java.net/~stuefe/webrevs/8224181-on-child-process-spawn--child-may-write-to-wrong-file-descriptor-instead-of-the-fail-pipe/webrev.00/webrev/

In sub process error handling code (WhyCantJonnyExec) child signals error
to parent by writing an error code to the fail pipe. It does that using the
hard wired fail pipe write-end fd (FAIL_FILENO, usually 4). But that only
works as intended after the fail pipe write end has been successfully
dup2'ed to FAIL_FILENO.

If an error happens before that, error code will still be written to
FAIL_FILENO, which may be an invalid file handle - which is almost benign -
or refer to an unrelated file descriptor the child inherited and did not
close yet - which is not good.

Cheers, Thomas

Re: RFR: 8224974: Implement JEP 352


On 03/06/2019 15:37, Andrew Dinn wrote:

:

The CSR and JEP have been updated accordingly

CSR:  https://bugs.openjdk.java.net/browse/JDK-8224975
JEP:  https://bugs.openjdk.java.net/browse/JDK-8207851

The specification section in the CSR was missing the module definition 
so I added that. The rest looks okay so I've added myself as Reviewer so 
you can finalize it.


-Alan.

RFR: 8225179: (regex) Minor Pattern cleanup

2019-06-04 Thread Claes Redestad


Hi,

please review this j.u.regex.Pattern cleanup.

- refactor BitClass to be a BmpCharPredicate (which allows removing
two identical(!) lambdas), which improves startup and reduces
allocations when compiling Patterns.
- remove unused GroupRef class
- made anonymous lookbehindEnd Node instance into an explicit class
which will be lazily rather than eagerly loaded
- various cleanups of unused variables, methods and redundant
inititialization

Webrev: http://cr.openjdk.java.net/~redestad/8225179/open.00/
Bug:https://bugs.openjdk.java.net/browse/JDK-8225179

Testing: tier1-3

Thanks!

/Claes

Re: RFR 8225198 : Optimize regex tree for greedy quantifiers of type {N, }

2019-06-04 Thread Claes Redestad


Hi Ivan,

looks good to me.

/Claes

On 2019-06-03 21:54, Ivan Gerasimov wrote:

Hello!

When building a match-tree, the regex engine optimizes '*' and '+' 
greedy quantifiers by using special node of type CharPropertyGreedy.


This later class can be used unmodified for other greedy quantifiers 
without the upper limit, i.e. of type '{N,}'.


Would you please help review this simple optimization?

BUGURL: https://bugs.openjdk.java.net/browse/JDK-8225198
WEBREV: http://cr.openjdk.java.net/~igerasim/8225198/00/webrev/

(With the fix, one unused class was also removed.  This class was to 
implement conditionals, which were never supported by Java regexs.)

Re: RFR: 6394787: Typos in javadoc of OIS.readObjectOverride and OOS.writeObjectOverride

2019-06-04 Thread Chris Hegarty

+1 

I can sponsor this for you Andrey.

-Chris.

> On 4 Jun 2019, at 07:56, Alan Bateman  wrote:
> 
> This looks right, I assume a cut issue when the methods were originally 
> added.
> 
> -Alan
> 
> On 02/06/2019 12:23, Andrey Turbanov wrote:
>> Hello.
>> I would like to contribute a small patch for bug:
>> https://bugs.openjdk.java.net/browse/JDK-6394787
>> 
>> Index: src/java.base/share/classes/java/io/ObjectOutputStream.java
>> IDEA additional info:
>> Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
>> <+>UTF-8
>> ===
>> --- src/java.base/share/classes/java/io/ObjectOutputStream.java
>> (revision 640ffa0674929c4416a0f562be2bf404c580474b)
>> +++ src/java.base/share/classes/java/io/ObjectOutputStream.java
>> (date 1559474301051)
>> @@ -1,5 +1,5 @@
>>  /*
>> - * Copyright (c) 1996, 2017, Oracle and/or its affiliates. All rights 
>> reserved.
>> + * Copyright (c) 1996, 2019, Oracle and/or its affiliates. All rights 
>> reserved.
>>   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>>   *
>>   * This code is free software; you can redistribute it and/or modify it
>> @@ -357,8 +357,8 @@
>> 
>>  /**
>>   * Method used by subclasses to override the default writeObject method.
>> - * This method is called by trusted subclasses of ObjectInputStream that
>> - * constructed ObjectInputStream using the protected no-arg constructor.
>> + * This method is called by trusted subclasses of ObjectOutputStream 
>> that
>> + * constructed ObjectOutputStream using the protected no-arg 
>> constructor.
>>   * The subclass is expected to provide an override method with the 
>> modifier
>>   * "final".
>>   *
>> Index: src/java.base/share/classes/java/io/ObjectInputStream.java
>> IDEA additional info:
>> Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
>> <+>UTF-8
>> ===
>> --- src/java.base/share/classes/java/io/ObjectInputStream.java
>> (revision 640ffa0674929c4416a0f562be2bf404c580474b)
>> +++ src/java.base/share/classes/java/io/ObjectInputStream.java
>> (date 1559472438350)
>> @@ -447,8 +447,8 @@
>>  }
>> 
>>  /**
>> - * This method is called by trusted subclasses of ObjectOutputStream 
>> that
>> - * constructed ObjectOutputStream using the protected no-arg 
>> constructor.
>> + * This method is called by trusted subclasses of ObjectInputStream that
>> + * constructed ObjectInputStream using the protected no-arg constructor.
>>   * The subclass is expected to provide an override method with the 
>> modifier
>>   * "final".
>>   *
>> Index: test/jdk/java/io/Serializable/subclass/AbstractObjectOutputStream.java
>> IDEA additional info:
>> Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
>> <+>UTF-8
>> ===
>> --- test/jdk/java/io/Serializable/subclass/AbstractObjectOutputStream.java
>>(revision 640ffa0674929c4416a0f562be2bf404c580474b)
>> +++ test/jdk/java/io/Serializable/subclass/AbstractObjectOutputStream.java
>>(date 1559474030191)
>> @@ -1,5 +1,5 @@
>>  /*
>> - * Copyright (c) 1998, 2004, Oracle and/or its affiliates. All rights 
>> reserved.
>> + * Copyright (c) 1998, 2019, Oracle and/or its affiliates. All rights 
>> reserved.
>>   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>>   *
>>   * This code is free software; you can redistribute it and/or modify it
>> @@ -44,7 +44,7 @@
>>   *
>>   * Since serialization must override java access rules in order to
>>   * access private, protected and package accessible Serializable fields,
>> - * only trusted classes are allowed to subclass AbstractObjectInputStream.
>> + * only trusted classes are allowed to subclass AbstractObjectOutputStream.
>>   * Subclasses of AbstractObjectOututStream must have SerializablePermission
>>   * "enableAbstractSubclass" or this constructor will throw a
>>   * SecurityException.Implementations of this class should protect themselves
>> 
>> 
>> Andrey Turbanov
>

Re: Review Request JDK-8221368: Error message when module main class cannot be loaded is missing exception details


On 04/06/2019 05:00, Mandy Chung wrote:

The launcher prints out the error message when the main class fails to
initialize but java.launcher.module.error5 resource that intends to
print the Caused-by exception message when running in a named module
but prints an incorrect parameter.

Very simple fix - fix the resource param indices.  Also fix a
typo: "module module " in the output.

http://cr.openjdk.java.net/~mchung/jdk13/webrevs/8221368/webrev.00/

The error message when the main class is in an unnamed module
is printed properly and covered by an existing test
(test/jdk/tools/launcher/MainClassCantBeLoadedTest.java).


This looks good.

-Alan

Re: Review Request JDK-8222448: java/lang/reflect/PublicMethods/PublicMethodsTest.java times out


On 04/06/2019 06:06, David Holmes wrote:

Hi Mandy,

Functional fix looks good, but layout and indentation appears off in 
the diff.
I think the formatting got lost when it was pasted into a JIRA comment. 
Anyway, I think this is a good start. It would not surprise me if we 
eventually have to add an @requires so that this test is not run with 
debug builds (as this is a core reflection test, it wasn't developed as 
a HotSpot stress test).


-Alan

Re: RFR: 6394787: Typos in javadoc of OIS.readObjectOverride and OOS.writeObjectOverride

This looks right, I assume a cut issue when the methods were 
originally added.


-Alan

On 02/06/2019 12:23, Andrey Turbanov wrote:

Hello.
I would like to contribute a small patch for bug:
https://bugs.openjdk.java.net/browse/JDK-6394787

Index: src/java.base/share/classes/java/io/ObjectOutputStream.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===
--- src/java.base/share/classes/java/io/ObjectOutputStream.java
(revision 640ffa0674929c4416a0f562be2bf404c580474b)
+++ src/java.base/share/classes/java/io/ObjectOutputStream.java
(date 1559474301051)
@@ -1,5 +1,5 @@
  /*
- * Copyright (c) 1996, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1996, 2019, Oracle and/or its affiliates. All rights reserved.
   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
   *
   * This code is free software; you can redistribute it and/or modify it
@@ -357,8 +357,8 @@

  /**
   * Method used by subclasses to override the default writeObject method.
- * This method is called by trusted subclasses of ObjectInputStream that
- * constructed ObjectInputStream using the protected no-arg constructor.
+ * This method is called by trusted subclasses of ObjectOutputStream that
+ * constructed ObjectOutputStream using the protected no-arg constructor.
   * The subclass is expected to provide an override method with the 
modifier
   * "final".
   *
Index: src/java.base/share/classes/java/io/ObjectInputStream.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===
--- src/java.base/share/classes/java/io/ObjectInputStream.java
(revision 640ffa0674929c4416a0f562be2bf404c580474b)
+++ src/java.base/share/classes/java/io/ObjectInputStream.java
(date 1559472438350)
@@ -447,8 +447,8 @@
  }

  /**
- * This method is called by trusted subclasses of ObjectOutputStream that
- * constructed ObjectOutputStream using the protected no-arg constructor.
+ * This method is called by trusted subclasses of ObjectInputStream that
+ * constructed ObjectInputStream using the protected no-arg constructor.
   * The subclass is expected to provide an override method with the 
modifier
   * "final".
   *
Index: test/jdk/java/io/Serializable/subclass/AbstractObjectOutputStream.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===
--- test/jdk/java/io/Serializable/subclass/AbstractObjectOutputStream.java
(revision 640ffa0674929c4416a0f562be2bf404c580474b)
+++ test/jdk/java/io/Serializable/subclass/AbstractObjectOutputStream.java
(date 1559474030191)
@@ -1,5 +1,5 @@
  /*
- * Copyright (c) 1998, 2004, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1998, 2019, Oracle and/or its affiliates. All rights reserved.
   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
   *
   * This code is free software; you can redistribute it and/or modify it
@@ -44,7 +44,7 @@
   *
   * Since serialization must override java access rules in order to
   * access private, protected and package accessible Serializable fields,
- * only trusted classes are allowed to subclass AbstractObjectInputStream.
+ * only trusted classes are allowed to subclass AbstractObjectOutputStream.
   * Subclasses of AbstractObjectOututStream must have SerializablePermission
   * "enableAbstractSubclass" or this constructor will throw a
   * SecurityException.Implementations of this class should protect themselves


Andrey Turbanov

RE: [11u] RFR (S): 8139965: Hang seen when using com.sun.jndi.ldap.search.replyQueueSize

2019-06-04 Thread Langer, Christoph

Thanks Paul.

> -Original Message-
> From: Hohensee, Paul 
> Sent: Montag, 3. Juni 2019 21:33
> To: Langer, Christoph ; jdk-updates-
> d...@openjdk.java.net
> Cc: Java Core Libs 
> Subject: Re: [11u] RFR (S): 8139965: Hang seen when using
> com.sun.jndi.ldap.search.replyQueueSize
> 
> I agree, looks fine.
> 
> Paul
> 
> On 6/2/19, 11:46 PM, "jdk-updates-dev on behalf of Langer, Christoph"  updates-dev-boun...@openjdk.java.net on behalf of
> christoph.lan...@sap.com> wrote:
> 
> Hi,
> 
> please help reviewing a backport to OpenJDK 11u.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8139965
> 11u-webrev: http://cr.openjdk.java.net/~clanger/webrevs/8139965.11u/
> 
> The patch did apply, however, it contained changes to testcase
> test/jdk/com/sun/jndi/ldap/LdapDnsProviderTest.java. The testcase is not
> part of JDK11u and its backport is also impossible as it belongs to an
> enhancement with associated CSR for JDK12 (JDK-8160768 [0]). The test
> update hasn't been part of the original review thread [1], so I think it is 
> ok to
> go with the change as suggested in the webrev.
> 
> Thanks
> Christoph
> 
> [0] https://bugs.openjdk.java.net/browse/JDK-8160768
> [1] https://mail.openjdk.java.net/pipermail/core-libs-dev/2018-
> September/055115.html
>

Re: Review Request JDK-8221368: Error message when module main class cannot be loaded is missing exception details

2019-06-04 Thread Sundararajan Athijegannathan


Looks good.

-Sundar

On 04/06/19, 9:30 AM, Mandy Chung wrote:

The launcher prints out the error message when the main class fails to
initialize but java.launcher.module.error5 resource that intends to
print the Caused-by exception message when running in a named module
but prints an incorrect parameter.

Very simple fix - fix the resource param indices.  Also fix a
typo: "module module " in the output.

http://cr.openjdk.java.net/~mchung/jdk13/webrevs/8221368/webrev.00/

The error message when the main class is in an unnamed module
is printed properly and covered by an existing test
(test/jdk/tools/launcher/MainClassCantBeLoadedTest.java).

thanks
Mandy

PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error