date:20190905

Re: RFR: 8212117 : Class.forName may return a reference to a loaded but not linked Class

2019-09-05 Thread Mandy Chung





On 9/5/19 5:09 PM, Brent Christian wrote:

Updated webrev:
http://cr.openjdk.java.net/~bchristi/8212117/webrev10/


jvm.h

 349  * Link the 'arg' class (unless ClassForNameDeferLinking is set)

I suggest to drop "(unless ...)" phrase because the flag is temporary.

jni.cpp

The implementation of JNI FindClass has the same behavior as 1-arg
Class.forName(name).  The JNI spec needs fixing but it's a separate
issue.

find_class_from_class_loader
  I like David's suggestion to assert that if init == true, link
  must be true.  Alternatively, pass an enum instead of two booleans
  clearly tell linking or initializing.

Lookup::findClass:

"In particular, the method first attempts to load the requested class"

I think this sentence is no longer needed together with your change.  What 
about:

 /**
- * Looks up a class by name from the lookup context defined by this 
{@code Lookup} object. The static
- * initializer of the class is not run.
+ * Looks up a class by name from the lookup context defined by this 
{@code Lookup} object.
+ * This method attempts to locate, load, and link the class, and then 
determines whether
+ * the class is accessible to this {@code Lookup} object.
+ * The static initializer of the class is not run.
  * 
  * The lookup context here is determined by the {@linkplain 
#lookupClass() lookup class}, its class
- * loader, and the {@linkplain #lookupModes() lookup modes}. In 
particular, the method first attempts to
- * load the requested class, and then determines whether the class is 
accessible to this lookup object.
+ * loader, and the {@linkplain #lookupModes() lookup modes}.


The note you added in this method should also be added to 2-arg
Class::forName for consistency.

Otherwise, the fix looks good.

Mandy

Re: Comments on jpackage (JEP 343)

2019-09-05 Thread Scott Palmer

I use a very similar workflow, but I’m building for all platforms. I want the 
image to produce a simple zipped version of the app, and I want all the 
installer/bundles/packages as well. 

I also agree with all of the “would be nice to haves” - Particularly 
service/daemon support. 
I also agree with the recent comments here about getting back the user options 
support and having a way for args in the configure file to stay and be 
augmented by arguments added on the command line by the user. 

Scott

> On Sep 5, 2019, at 8:51 AM, Rachel Greenham  wrote:
> 
> (Sorry for non-threading, i read the digest)
> 
> As you've been lacking feedback from people using the jpackage EA builds, 
> here's mine FWIW.
> 
> I've been quiet because it's been working well enough for us. That said, our 
> needs and process probably simplify matters in that:
> 
> 1. We're only producing Windows installers
> 2. We've been lucky in having patient clients during this post-webstart, 
> post-javapackager disruption.
> 3. We were happy to modify our versioning to match Windows standards
> 4. Our application is non-modular
> 5. We do it in three steps: jlink to make a JRE, then jpackage to make an app 
> image, then jpackage again to make both an exe and msi installer based on 
> that image. (client slow to reply which one they'd actually prefer!) Not 
> trying to do everything in one step.
> 
> Since the fix that made new versions of our app correctly replace older ones 
> I've mostly just been testing new EA builds to make sure they don't break it! 
> They do sometimes, usually because of changes in the parameter names, and of 
> course we lost our Inno Setup customisations. I haven't yet made any attempt 
> to customise the EXE setup installer since then.
> 
> Would be nice:
> 
> 1. For it to use the supplied app icon for the installer, or be able to 
> supply another specifically for the installer. For it to be shown in the 
> installer in some fashion. Other exe customisations of straightforward 
> branding and/or flags to control what questions they're asked would be very 
> nice.
> 2. For it to be able to sign the installer in the fashion of, or actually 
> using, signtool. (Ideally internalised as installing signtool itself is a 
> pain.) Currently that's an extra step after the installers are built
> 
> But I can wait for them, I want it in a release so I can use it via 
> ToolProvider rather than execing an external JDK. All the while it's the way 
> it is it massively complicates the build.
> 
> Later would-be-nices, not for this desktop app, but ability to use it to 
> package background service-type apps, as a service for windows, using launchd 
> for osx, and systemd for linux.
> 
> -- 
> Rachel

Re: RFR: 8212117 : Class.forName may return a reference to a loaded but not linked Class

2019-09-05 Thread serguei . spitsyn


Hi Brent,

Some quick reply below.


On 9/5/19 5:09 PM, Brent Christian wrote:

Hi, David

On 9/5/19 12:52 AM, David Holmes wrote:


Good to see this all come together :)


:)

So to clarify for others any current caller to 
find_class_from_class_loader that passes true for "init" was already 
implicitly asking for a linked class (as you must be linked before 
you can be initialized). With that in mind I would suggest that in 
find_class_from_class_loader you add an assert:


! jclass find_class_from_class_loader(JNIEnv* env, Symbol* name, 
jboolean init, jboolean link,
   Handle loader, Handle 
protection_domain,

   jboolean throwError, TRAPS) {
+assert((init && link) || !init, "incorrect use of init/link 
arguments");


just to ensure the callers understand this.


I'm good with adding an assert to check for coherent arguments.

(Another interpretation is that if 'init' is set, then the 'link' 
argument is ignored, since linking is implied).


Aside: in looking at this I've spotted another existing bug. JNI 
FindClass is not specified to perform class initialization, but the 
implementation passes init=true!


OK, thanks.  I've noted this in JDK-8226540.


src/java.base/share/classes/java/lang/invoke/MethodHandles.java

I don't see the need for the new note given it already has

* @throws LinkageError if the linkage fails


(Discussed in the CSR)


src/java.base/share/classes/sun/launcher/LauncherHelper.java
... > Is AccessControlException the only exception, that is not a
LinkageError, that might be thrown when linking? I would think a 
number of others are possible - in particular IllegalAccessError 
might occur during verification. Other than the fact a test obviously 
triggered this, it's not clear why ACE should be singled out here. ??


Also discussed in the CSR[1].


test/hotspot/jtreg/serviceability/jvmti/ClassStatus/ClassStatus.java

45 // public static void foo(Foo f) { }

Unclear why this exists. Also the test refers initially to class Foo 
but then uses Foo2 and Foo3. ??


I'm guessing it's just a leftover from an earlier version of the test. 
I've removed the comment, and updated the test @summary.


Yes, it is just a left over from earlier version of the test.


Serguei, anything to add?


I'm happy this test got used and included into the webrev, thanks!


There is also a CSR[2] in need of review.


I've added a couple of comments and will add myself as a reviewer 
once things are near finalized.


Thanks for taking a look.

Updated webrev at:
http://cr.openjdk.java.net/~bchristi/8212117/webrev10/


Will look at the webrev soon.

Thanks,
serguei


-Brent

1. 
https://bugs.openjdk.java.net/browse/JDK-8222071?focusedCommentId=14288303=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14288303




The spec for the 2-arg and 3-arg Class.forName() methods states they 
will "locate, load, and link" the class, however the linking part is 
not ensured to happen.


Although the VM spec allows flexibility WRT when classes are linked, 
this is a corner where the Class.forName() spec is not being upheld. 
While this is not an issue for most usages, 8181144 [3] demonstrates 
how this can be a problem (with the debugging interface, in this case).


This fix ensures that linking happens during the course of 
Class.forName().  Class.forName() already @throws LinkageError, so 
no spec change is needed there. MethodHandles.Lookup.findClass() 
needs a small spec update, due to calling Class.forName with 
init=false,


Of course Errors are not required to be caught.  It is therefore 
possible that the new behavior could introduce previously unseen, 
possibly unhandled LinkageErrors.  A new VM flag 
(-XX:+ClassForNameDeferLinking) is introduced to restore the 
previous behavior (to keep such code running until it can be updated).


This change surfaced a couple new "A JNI error has occurred" 
situations (see 8181033[5]) in the Launcher, by way of

   test/jdk/tools/launcher/MainClassCantBeLoadedTest.java
(using the 3-arg Class::forName, detailed in the bug report[4]),
and
   test/jdk/tools/launcher/modules/basic/LauncherErrors.java
(using the 2-arg Class::forName).

The Launcher is updated to maintain non-confusing error messages :).

The new test included with this fix ensures that 8181144[3] is 
addressed.  Thanks go to Serguei Spitsyn for writing the test.


Automated corelibs and hotspot tests pass cleanly.

Thanks,
-Brent

--
1. https://bugs.openjdk.java.net/browse/JDK-8212117

2. https://bugs.openjdk.java.net/browse/JDK-8222071

3. https://bugs.openjdk.java.net/browse/JDK-8181144

4. 
https://bugs.openjdk.java.net/browse/JDK-8212117?focusedCommentId=14215986=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-14215986 



5. https://bugs.openjdk.java.net/browse/JDK-8181033

Re: RFR: 8212117 : Class.forName may return a reference to a loaded but not linked Class

2019-09-05 Thread Mandy Chung


On 9/4/19 2:12 PM, Brent Christian wrote:



There is also a CSR[2] in need of review.


The javadoc for Lookup::findClass:

"In particular, the method first attempts to load the requested class"

I think this sentence is no longer needed together with your change.  
What about:


 /**
- * Looks up a class by name from the lookup context defined by this 
{@code Lookup} object. The static
- * initializer of the class is not run.
+ * Looks up a class by name from the lookup context defined by this 
{@code Lookup} object.
+ * This method attempts to locate, load, and link the class, and then 
determines whether
+ * the class is accessible to this {@code Lookup} object.
+ * The static initializer of the class is not run.
  * 
  * The lookup context here is determined by the {@linkplain 
#lookupClass() lookup class}, its class
- * loader, and the {@linkplain #lookupModes() lookup modes}. In 
particular, the method first attempts to
- * load the requested class, and then determines whether the class is 
accessible to this lookup object.
+ * loader, and the {@linkplain #lookupModes() lookup modes}.


The note you added in this method should also be added to 2-arg 
Class::forName for consistency.


We should add @jls as Joe suggests.

Mandy

Re: RFR: 8212117 : Class.forName may return a reference to a loaded but not linked Class

2019-09-05 Thread Brent Christian

Hi, Joe

@jls tags added (and as long as we're in Class.java, I added @jls to the
3-arg Class.forName(), which has an equivalent paragraph).

Updated webrev:
http://cr.openjdk.java.net/~bchristi/8212117/webrev10/

Thanks,
-Brent

On 9/5/19 3:30 PM, Joe Darcy wrote:

Hello,

For the doc changes in MethodHandle, please supplement the paragraph

1937 *
1938 * Note that this method throws errors related to loading
and linking as
1939 * specified in Sections 12.2 and 12.3 of The Java
Language

1940 * Specification.

with @jls tags like

@jls 12.2 Loading of Classes and Interfaces
@jsl 12.3 Linking of Classes and Interfaces

as done elsewhere in the base module. I think it would be slightly
better to remove the section numbers from the main text but keep them in
the @jls tags since it is now possible to check that the listed name of
the section matches what is in the JLS.

Thanks,

-Joe

On 9/4/2019 2:12 PM, Brent Christian wrote:

Hi,

Please review my fix for JDK-8212117[1]. The webrev is here:

http://cr.openjdk.java.net/~bchristi/8212117/webrev09/

There is also a CSR[2] in need of review.

The spec for the 2-arg and 3-arg Class.forName() methods states they
will "locate, load, and link" the class, however the linking part is
not ensured to happen.

Although the VM spec allows flexibility WRT when classes are linked,
this is a corner where the Class.forName() spec is not being upheld.
While this is not an issue for most usages, 8181144 [3] demonstrates
how this can be a problem (with the debugging interface, in this case).

This fix ensures that linking happens during the course of
Class.forName(). Class.forName() already @throws LinkageError, so no
spec change is needed there. MethodHandles.Lookup.findClass() needs a
small spec update, due to calling Class.forName with init=false,

Of course Errors are not required to be caught. It is therefore
possible that the new behavior could introduce previously unseen,
possibly unhandled LinkageErrors. A new VM flag
(-XX:+ClassForNameDeferLinking) is introduced to restore the previous
behavior (to keep such code running until it can be updated).

This change surfaced a couple new "A JNI error has occurred"
situations (see 8181033[5]) in the Launcher, by way of

test/jdk/tools/launcher/MainClassCantBeLoadedTest.java
(using the 3-arg Class::forName, detailed in the bug report[4]),
and
test/jdk/tools/launcher/modules/basic/LauncherErrors.java
(using the 2-arg Class::forName).

The Launcher is updated to maintain non-confusing error messages :).

The new test included with this fix ensures that 8181144[3] is
addressed. Thanks go to Serguei Spitsyn for writing the test.

Automated corelibs and hotspot tests pass cleanly.

Thanks,
-Brent

--
1. https://bugs.openjdk.java.net/browse/JDK-8212117

2. https://bugs.openjdk.java.net/browse/JDK-8222071

3. https://bugs.openjdk.java.net/browse/JDK-8181144

4.
https://bugs.openjdk.java.net/browse/JDK-8212117?focusedCommentId=14215986=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-14215986

5. https://bugs.openjdk.java.net/browse/JDK-8181033

Re: RFR: 8212117 : Class.forName may return a reference to a loaded but not linked Class

2019-09-05 Thread Brent Christian


Hi, David

On 9/5/19 12:52 AM, David Holmes wrote:


Good to see this all come together :)


:)

So to clarify for others any current caller to 
find_class_from_class_loader that passes true for "init" was already 
implicitly asking for a linked class (as you must be linked before you 
can be initialized). With that in mind I would suggest that in 
find_class_from_class_loader you add an assert:


! jclass find_class_from_class_loader(JNIEnv* env, Symbol* name, 
jboolean init, jboolean link,
   Handle loader, Handle 
protection_domain,

   jboolean throwError, TRAPS) {
+assert((init && link) || !init, "incorrect use of init/link arguments");

just to ensure the callers understand this.


I'm good with adding an assert to check for coherent arguments.

(Another interpretation is that if 'init' is set, then the 'link' 
argument is ignored, since linking is implied).


Aside: in looking at this I've spotted another existing bug. JNI 
FindClass is not specified to perform class initialization, but the 
implementation passes init=true!


OK, thanks.  I've noted this in JDK-8226540.


src/java.base/share/classes/java/lang/invoke/MethodHandles.java

I don't see the need for the new note given it already has

* @throws LinkageError if the linkage fails


(Discussed in the CSR)


src/java.base/share/classes/sun/launcher/LauncherHelper.java
... > Is AccessControlException the only exception, that is not a
LinkageError, that might be thrown when linking? I would think a number 
of others are possible - in particular IllegalAccessError might occur 
during verification. Other than the fact a test obviously triggered 
this, it's not clear why ACE should be singled out here. ??


Also discussed in the CSR[1].


test/hotspot/jtreg/serviceability/jvmti/ClassStatus/ClassStatus.java

45 // public static void foo(Foo f) { }

Unclear why this exists. Also the test refers initially to class Foo but 
then uses Foo2 and Foo3. ??


I'm guessing it's just a leftover from an earlier version of the test. 
I've removed the comment, and updated the test @summary.


Serguei, anything to add?


There is also a CSR[2] in need of review.


I've added a couple of comments and will add myself as a reviewer once 
things are near finalized.


Thanks for taking a look.

Updated webrev at:
http://cr.openjdk.java.net/~bchristi/8212117/webrev10/

-Brent

1. 
https://bugs.openjdk.java.net/browse/JDK-8222071?focusedCommentId=14288303=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14288303




The spec for the 2-arg and 3-arg Class.forName() methods states they 
will "locate, load, and link" the class, however the linking part is 
not ensured to happen.


Although the VM spec allows flexibility WRT when classes are linked, 
this is a corner where the Class.forName() spec is not being upheld. 
While this is not an issue for most usages,  8181144 [3] demonstrates 
how this can be a problem (with the debugging interface, in this case).


This fix ensures that linking happens during the course of 
Class.forName().  Class.forName() already @throws LinkageError, so no 
spec change is needed there. MethodHandles.Lookup.findClass() needs a 
small spec update, due to calling Class.forName with init=false,


Of course Errors are not required to be caught.  It is therefore 
possible that the new behavior could introduce previously unseen, 
possibly unhandled LinkageErrors.  A new VM flag 
(-XX:+ClassForNameDeferLinking) is introduced to restore the previous 
behavior (to keep such code running until it can be updated).


This change surfaced a couple new "A JNI error has occurred" 
situations (see 8181033[5]) in the Launcher, by way of

   test/jdk/tools/launcher/MainClassCantBeLoadedTest.java
(using the 3-arg Class::forName, detailed in the bug report[4]),
and
   test/jdk/tools/launcher/modules/basic/LauncherErrors.java
(using the 2-arg Class::forName).

The Launcher is updated to maintain non-confusing error messages :).

The new test included with this fix ensures that 8181144[3] is 
addressed.  Thanks go to Serguei Spitsyn for writing the test.


Automated corelibs and hotspot tests pass cleanly.

Thanks,
-Brent

--
1. https://bugs.openjdk.java.net/browse/JDK-8212117

2. https://bugs.openjdk.java.net/browse/JDK-8222071

3. https://bugs.openjdk.java.net/browse/JDK-8181144

4. 
https://bugs.openjdk.java.net/browse/JDK-8212117?focusedCommentId=14215986=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-14215986 



5. https://bugs.openjdk.java.net/browse/JDK-8181033

Re: JDK 14 RFR of JDK-8230681: Add @since tag to java.io.Serial

2019-09-05 Thread Brian Burkhalter

++1

> On Sep 5, 2019, at 4:00 PM, Lance Andersen  wrote:
> 
> +1
>> On Sep 5, 2019, at 6:43 PM, Joe Darcy  wrote:
>> 
>> Hello,
>> 
>> As spotted by Martin, please review the addition of an @since tag to the 
>> new-in-JDK-14 java.io.Serial type; patch below.
>> 
>> Thanks,
>> 
>> -Joe
>> 
>> diff -r 06f3d5092832 src/java.base/share/classes/java/io/Serial.java
>> --- a/src/java.base/share/classes/java/io/Serial.javaThu Sep 05 11:12:12 
>> 2019 -0700
>> +++ b/src/java.base/share/classes/java/io/Serial.javaThu Sep 05 15:24:42 
>> 2019 -0700
>> @@ -95,6 +95,7 @@
>>  *
>>  * @see Serializable
>>  * @see Externalizable
>> + * @since 14
>>  */
>> @Target({ElementType.METHOD, ElementType.FIELD})
>> @Retention(RetentionPolicy.SOURCE)
>> 
> 
> 
>  
> 
> Lance Andersen| 
> Principal Member of Technical Staff | +1.781.442.2037
> Oracle Java Engineering 
> 1 Network Drive 
> Burlington, MA 01803
> lance.ander...@oracle.com 
> 
> 
>

Re: JDK 14 RFR of JDK-8230681: Add @since tag to java.io.Serial

2019-09-05 Thread Lance Andersen

+1
> On Sep 5, 2019, at 6:43 PM, Joe Darcy  wrote:
> 
> Hello,
> 
> As spotted by Martin, please review the addition of an @since tag to the 
> new-in-JDK-14 java.io.Serial type; patch below.
> 
> Thanks,
> 
> -Joe
> 
> diff -r 06f3d5092832 src/java.base/share/classes/java/io/Serial.java
> --- a/src/java.base/share/classes/java/io/Serial.javaThu Sep 05 11:12:12 
> 2019 -0700
> +++ b/src/java.base/share/classes/java/io/Serial.javaThu Sep 05 15:24:42 
> 2019 -0700
> @@ -95,6 +95,7 @@
>   *
>   * @see Serializable
>   * @see Externalizable
> + * @since 14
>   */
>  @Target({ElementType.METHOD, ElementType.FIELD})
>  @Retention(RetentionPolicy.SOURCE)
> 

 
  

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

JDK 14 RFR of JDK-8230681: Add @since tag to java.io.Serial

2019-09-05 Thread Joe Darcy


Hello,

As spotted by Martin, please review the addition of an @since tag to the 
new-in-JDK-14 java.io.Serial type; patch below.


Thanks,

-Joe

diff -r 06f3d5092832 src/java.base/share/classes/java/io/Serial.java
--- a/src/java.base/share/classes/java/io/Serial.java    Thu Sep 05 
11:12:12 2019 -0700
+++ b/src/java.base/share/classes/java/io/Serial.java    Thu Sep 05 
15:24:42 2019 -0700

@@ -95,6 +95,7 @@
  *
  * @see Serializable
  * @see Externalizable
+ * @since 14
  */
 @Target({ElementType.METHOD, ElementType.FIELD})
 @Retention(RetentionPolicy.SOURCE)

Re: RFR: 8212117 : Class.forName may return a reference to a loaded but not linked Class

2019-09-05 Thread Joe Darcy

Hello,

For the doc changes in MethodHandle, please supplement the paragraph

1937 *
1938 * Note that this method throws errors related to loading
and linking as

1939 * specified in Sections 12.2 and 12.3 of The Java Language
1940 * Specification.

with @jls tags like

@jls 12.2 Loading of Classes and Interfaces
@jsl 12.3 Linking of Classes and Interfaces

Thanks,

-Joe

On 9/4/2019 2:12 PM, Brent Christian wrote:

Hi,

Please review my fix for JDK-8212117[1]. The webrev is here:

http://cr.openjdk.java.net/~bchristi/8212117/webrev09/

There is also a CSR[2] in need of review.

The spec for the 2-arg and 3-arg Class.forName() methods states they
will "locate, load, and link" the class, however the linking part is
not ensured to happen.

This change surfaced a couple new "A JNI error has occurred"
situations (see 8181033[5]) in the Launcher, by way of

The Launcher is updated to maintain non-confusing error messages :).

The new test included with this fix ensures that 8181144[3] is
addressed. Thanks go to Serguei Spitsyn for writing the test.

Automated corelibs and hotspot tests pass cleanly.

Thanks,
-Brent

--
1. https://bugs.openjdk.java.net/browse/JDK-8212117

2. https://bugs.openjdk.java.net/browse/JDK-8222071

3. https://bugs.openjdk.java.net/browse/JDK-8181144

4.
https://bugs.openjdk.java.net/browse/JDK-8212117?focusedCommentId=14215986=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-14215986

5. https://bugs.openjdk.java.net/browse/JDK-8181033

Re: RFR 8230365 : Pattern for a control-char matches non-control characters

2019-09-05 Thread Ivan Gerasimov

Hi Stuart!

Thanks for the comments, and please see my answers inline:

On 9/5/19 1:18 PM, Stuart Marks wrote:

Conceptually I think having the restriction is fine. If we were 
designing this as a new feature, I'd recommend putting in the 
restrictions from the very beginning.

However, since the old behavior has been out there for 20 years,  my 
main concern is compatibility. Having system properties to control 
this is ... ok ... I guess. I wish there were a better way, but it 
seems the simplest. However, I'd strongly advise making the behavior 
change and the compatibility mode(s) as simple as possible. Having 
more configuration options complicates the code and complicates the 
testing, and I really don't see the need for it.

So yes, I agree with abandoning ALLOW_LOWERCASE_CONTROL_CHAR_IDS.

Okay.  I've just sent out the updated webrev without this option:
http://cr.openjdk.java.net/~igerasim/8230365/03/webrev/

And filed the CSR for the spec change and the new property:
https://bugs.openjdk.java.net/browse/JDK-8230675

I guess now we get to have a bikeshed on the property name. Does this 
apply only to mapping of control characters, or is there some other 
form of pattern syntax that could be made more strict? Ivan, I seem to 
recall you talking to me about making some changes in the syntax or 
interpretation of the construct that enables/disables various flags. 
This one:

|(?idmsuxU-idmsuxU)|

Would it make sense to have a single property, e.g. 
jdk.util.regex.strictSyntax, that governs this one as well? And are 
there other possibilities for tuning up the parsing behavior that we 
should be taking?

Yes, that was this bug:

https://bugs.openjdk.java.net/browse/JDK-8225021 (Treat ambiguous 
embedded flags as parse syntax errors)

And actually I have yet another bug of similar flavor:

https://bugs.openjdk.java.net/browse/JDK-8214245 (Case insensitive 
matching doesn't work correctly for some character classes)

However, this last one is about changing the matching rules, and not 
about the syntax (And it is still waiting for review!).

I'd rather not have a situation where we fix up one area of syntax and 
add a property for it, fix up another area and add a different 
property, leading to property proliferation.

Personally, I don't have a strong preference here.

The compatibility property are meant to be temporary anyways.

Maybe if we have a single option that will control several different 
aspects of behavior, it will be harder to get rid of it?

Partially, because it will be tempting to reuse it for other similar 
changes, should they be needed.

With kind regards,

Ivan

s'marks

On 9/4/19 9:00 PM, Martin Buchholz wrote:

Thanks, Ivan.  We're mostly in agreement.

+ * If {@code true} then lower-case control-character ids are mapped to the
+ * their upper-case counterparts.
Extra "the".

After all these decades I only now realize that c ^= 0x40 moves '?' 
to the end of the ASCII range and all the other controls to the start!

Should we support lower-case controls? Compatibility with perl regex 
still matters, but a lot less than in 2003.  But the key is that we 
got the WRONG ANSWER previously, so when we restrict the control ids 
let's just make lower case controls syntax errors.  Silently changing 
behavior is bad for users.  ... so let's abandon 
ALLOW_LOWERCASE_CONTROL_CHAR_IDS.

An alternative:
int ch = read() ^ 0x40;
if (!RESTRICTED_CONTROL_CHAR_IDS || ch < 0x20 || ch == 0x7f) return ch;

On Wed, Sep 4, 2019 at 6:49 PM Ivan Gerasimov 
mailto:ivan.gerasi...@oracle.com>> wrote:

Thank you Martin!

On 8/30/19 6:19 PM, Martin Buchholz wrote:
> There's a strong expectation that ctrl-A and ctrl-a both map to
> \u0001, so I support Ivan's initiative.
> I'm surprised java regex gets this wrong.
> Might need a transitional system property.
>
Right.  I think it would be best to introduce two system properties:

The first, to turn on/off the restrictions on the control-char
names.
This will be enabled by default, and will permit names from the
limited
list: capital letters and a few other special characters.

The second one, to enable mapping of lower-case control-char
names to
their upper-case counterpart.  This option should be turned off by
default for the current release of JDK, and then turned on by
default
for some subsequent release (when, presumably, most applications
that
use this kind of regexp are fixed).

This all feels like a little bit too much for such a rarely used
feature, but probably is a proper thing to do anyway :-)

If we have an agreement on these system properties, I can create a
separate test to verify all possible combinations.

> What's the best bit-twiddle?  Untested:
> if ((c ^= 0x40) < 0x20) return c;
> if ((c ^=0x20)  <= 26 && c > 0) return c;
>
> 0x40 is more readable than 64.
>
`((ch-0x3f)|(0x5f-ch))

Re: RFR 8230365 : Pattern for a control-char matches non-control characters

2019-09-05 Thread Ivan Gerasimov


Thank you Martin again!

Here's the updated webrev without the lower-case control char ids:

http://cr.openjdk.java.net/~igerasim/8230365/03/webrev/

I've also filed a CSR to record the changes in bahavior:

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

Could you please help review it?


On 9/4/19 9:00 PM, Martin Buchholz wrote:

Thanks, Ivan.  We're mostly in agreement.

+ * If {@code true} then lower-case control-character ids are mapped to the
+ * their upper-case counterparts.
Extra "the".

After all these decades I only now realize that c ^= 0x40 moves '?' to 
the end of the ASCII range and all the other controls to the start!


Should we support lower-case controls?  Compatibility with perl regex 
still matters, but a lot less than in 2003.  But the key is that we 
got the WRONG ANSWER previously, so when we restrict the control ids 
let's just make lower case controls syntax errors.  Silently changing 
behavior is bad for users. ... so let's abandon 
ALLOW_LOWERCASE_CONTROL_CHAR_IDS.

An alternative:
int ch = read() ^ 0x40;
if (!RESTRICTED_CONTROL_CHAR_IDS || ch < 0x20 || ch == 0x7f) return ch;




This code will probably be most efficient for the common case.

However, I'd prefer to use the auxiliary method isCntrlId() in this 
case, as it is self-documenting and still efficient enough.


With kind regards,

Ivan

Re: RFR 8230365 : Pattern for a control-char matches non-control characters

2019-09-05 Thread Stuart Marks

Conceptually I think having the restriction is fine. If we were designing this 
as a new feature, I'd recommend putting in the restrictions from the very beginning.

However, since the old behavior has been out there for 20 years, my main concern 
is compatibility. Having system properties to control this is ... ok ... I 
guess. I wish there were a better way, but it seems the simplest. However, I'd 
strongly advise making the behavior change and the compatibility mode(s) as 
simple as possible. Having more configuration options complicates the code and 
complicates the testing, and I really don't see the need for it.

So yes, I agree with abandoning ALLOW_LOWERCASE_CONTROL_CHAR_IDS.

I guess now we get to have a bikeshed on the property name. Does this apply only 
to mapping of control characters, or is there some other form of pattern syntax 
that could be made more strict? Ivan, I seem to recall you talking to me about 
making some changes in the syntax or interpretation of the construct that 
enables/disables various flags. This one:

|(?idmsuxU-idmsuxU)|

Would it make sense to have a single property, e.g. jdk.util.regex.strictSyntax, 
that governs this one as well? And are there other possibilities for tuning up 
the parsing behavior that we should be taking? I'd rather not have a situation 
where we fix up one area of syntax and add a property for it, fix up another 
area and add a different property, leading to property proliferation.

s'marks

On 9/4/19 9:00 PM, Martin Buchholz wrote:

Thanks, Ivan.  We're mostly in agreement.

+ * If {@code true} then lower-case control-character ids are mapped to the
+ * their upper-case counterparts.
Extra "the".

After all these decades I only now realize that c ^= 0x40 moves '?' to the end 
of the ASCII range and all the other controls to the start!

Should we support lower-case controls?  Compatibility with perl regex still 
matters, but a lot less than in 2003.  But the key is that we got the WRONG 
ANSWER previously, so when we restrict the control ids let's just make lower 
case controls syntax errors.  Silently changing behavior is bad for users. ... 
so let's abandon ALLOW_LOWERCASE_CONTROL_CHAR_IDS.

An alternative:
int ch = read() ^ 0x40;
if (!RESTRICTED_CONTROL_CHAR_IDS || ch < 0x20 || ch == 0x7f) return ch;

On Wed, Sep 4, 2019 at 6:49 PM Ivan Gerasimov > wrote:

Thank you Martin!

On 8/30/19 6:19 PM, Martin Buchholz wrote:
> There's a strong expectation that ctrl-A and ctrl-a both map to
> \u0001, so I support Ivan's initiative.
> I'm surprised java regex gets this wrong.
> Might need a transitional system property.
>
Right.  I think it would be best to introduce two system properties:

The first, to turn on/off the restrictions on the control-char names.
This will be enabled by default, and will permit names from the limited
list: capital letters and a few other special characters.

The second one, to enable mapping of lower-case control-char names to
their upper-case counterpart.  This option should be turned off by
default for the current release of JDK, and then turned on by default
for some subsequent release (when, presumably, most applications that
use this kind of regexp are fixed).

This all feels like a little bit too much for such a rarely used
feature, but probably is a proper thing to do anyway :-)

If we have an agreement on these system properties, I can create a
separate test to verify all possible combinations.

> What's the best bit-twiddle?  Untested:
> if ((c ^= 0x40) < 0x20) return c;
> if ((c ^=0x20)  <= 26 && c > 0) return c;
>
> 0x40 is more readable than 64.
>
`((ch-0x3f)|(0x5f-ch)) >= 0` does the trick for regular (non-lower-case)
ids.

> Does ctrol-? get mapped to 0x7f ?
>
Yes. I've got it in the test at the end of the line 4997.

Would you please help review the updated webrev:

http://cr.openjdk.java.net/~igerasim/8230365/02/webrev/

With kind regards,

Ivan

Re: RFR 8230365 : Pattern for a control-char matches non-control characters

2019-09-05 Thread Ivan Gerasimov


Hello Bernd!

Thank you for your comments!

I'm going to proceed with only the restriction part of the change for 
now, so no blind conversion of lower-case control chars will happen.


A system property will allow the users to return to the previous less 
restrictive behavior, should they decide to keep malformed patterns 
unchanged.


I'll post the updated webrev and CSR request shortly.

With kind regards,
Ivan


On 9/4/19 10:54 PM, Bernd Eckenfels wrote:

Hallo,

Since not all combinations make sense (Exception+convert) a multi value might 
be better:

jdk.regex.control=WARN|EXCEPTION|STANDARD|LEGACY

With Exception generating an error, Standard beeing the planned new default 
(treating upper/lower same and error on all undefined chars) and legacy beeing 
the manual fallback to current behavior and WARN the same fallback but with 
logging.

I guess some form of early feedback like EXCPETION or WARN is needed, even when 
it is between a rock and a hard place. Maybe have at least one iteration where 
it defaults to LEGACY (+Release Notes announcement), then WARN and then finally 
STANDARD?

Gruss
Bernd


--
http://bernd.eckenfels.net


Von: core-libs-dev  im Auftrag von Ivan 
Gerasimov 
Gesendet: Donnerstag, September 5, 2019 4:00 AM
An: Martin Buchholz; Stuart Marks
Cc: core-libs-dev
Betreff: Re: RFR 8230365 : Pattern for a control-char matches non-control 
characters

Thank you Martin!

On 8/30/19 6:19 PM, Martin Buchholz wrote:

There's a strong expectation that ctrl-A and ctrl-a both map to
\u0001, so I support Ivan's initiative.
I'm surprised java regex gets this wrong.
Might need a transitional system property.


Right.  I think it would be best to introduce two system properties:

The first, to turn on/off the restrictions on the control-char names.
This will be enabled by default, and will permit names from the limited
list: capital letters and a few other special characters.

The second one, to enable mapping of lower-case control-char names to
their upper-case counterpart.  This option should be turned off by
default for the current release of JDK, and then turned on by default
for some subsequent release (when, presumably, most applications that
use this kind of regexp are fixed).

This all feels like a little bit too much for such a rarely used
feature, but probably is a proper thing to do anyway :-)

If we have an agreement on these system properties, I can create a
separate test to verify all possible combinations.



What's the best bit-twiddle?  Untested:
if ((c ^= 0x40) < 0x20) return c;
if ((c ^=0x20)  <= 26 && c > 0) return c;

0x40 is more readable than 64.


`((ch-0x3f)|(0x5f-ch)) >= 0` does the trick for regular (non-lower-case)
ids.


Does ctrol-? get mapped to 0x7f ?


Yes. I've got it in the test at the end of the line 4997.

Would you please help review the updated webrev:

http://cr.openjdk.java.net/~igerasim/8230365/02/webrev/

With kind regards,

Ivan




--
With kind regards,
Ivan Gerasimov

Re: jdk-14-jpackage+1-33 on jdk.java.net

2019-09-05 Thread Sverre Moe

Problem with running WiX through a Continuous Integration Build System.

It works perfectly fine logging into the Windows desktop with remote
desktop, open my Cygwin console and execute Gradle. Our Jenkins CI runs
into a problem at the end when WiX is running validation on the built
installer.

> WiX v3, Light automatically runs validation-- Windows Installer Internal
Consistency Evaluators (ICEs) --after every successful build.

https://stackoverflow.com/questions/1064580/wix-3-0-throws-error-217-while-being-executed-by-continuous-integration

Some has suggested to disable the validation, but that is not a good
solution in my opinion.
A workaround I attempted which works without meddling with the WiX
configuration, was to change the build user Jenkins uses to log onto the
Windows build agent. I changed its user type from Standard to
Administrator. Not sure why it would work, as I had run the jpackage+wix
using the same standard user through remote desktop. Perhaps some
permissions it didn't get while running in a headless environment.

Generating MSI:
C:\Users\build\jdk.jpackage4130052721241789652\images\win-exe.image\application-1.1.0.msi.
Running [C:\Program Files (x86)\WiX Toolset v3.11\bin\light.exe, -nologo,
-spdb,
C:\Users\build\jdk.jpackage4130052721241789652\tmp\application.wixobj,
-ext, WixUtilExtension, -loc,
C:\Users\build\jdk.jpackage4130052721241789652\config\MsiInstallerStrings_en.wxl,
-out,
C:\Users\build\jdk.jpackage4130052721241789652\images\win-exe.image\application-1.1.0.msi]
in
C:\Users\build\jdk.jpackage4130052721241789652\images\win-msi.image\application
light.exe : error LGHT0217 : Error executing ICE action 'ICE01'. The most
common cause of this kind of ICE failure is an incorrectly registered
scripting engine. See http://wixtoolset.org/documentation/error217/ for
details and how to solve this problem. The following string format was not
expected by the external UI message logger: "The Windows Installer Service
could not be accessed. This can occur if the Windows Installer is not
correctly installed. Contact your support personnel for assistance.".

Windows Installer Service could not be accessed. This can occur if the
Windows Installer is not correctly installed. Contact your support
personnel for assistance.".light.exe : error LGHT0216 : An unexpected Win32
exception with error code 0x643 occurred: Action - 'ICE09' Fatal error
during installation
at jdk.jpackage/jdk.jpackage.internal.IOUtils.exec(IOUtils.java:211)
at jdk.jpackage/jdk.jpackage.internal.IOUtils.exec(IOUtils.java:181)
at
jdk.jpackage/jdk.jpackage.internal.WinMsiBundler.buildMSI(WinMsiBundler.java:1093)
at
jdk.jpackage/jdk.jpackage.internal.WinMsiBundler.bundle(WinMsiBundler.java:518)
at
jdk.jpackage/jdk.jpackage.internal.WinExeBundler.bundle(WinExeBundler.java:105)
at
jdk.jpackage/jdk.jpackage.internal.WinExeBundler.execute(WinExeBundler.java:83)
at
jdk.jpackage/jdk.jpackage.internal.Arguments.generateBundle(Arguments.java:620)
at
jdk.jpackage/jdk.jpackage.internal.Arguments.processArguments(Arguments.java:513)
at jdk.jpackage/jdk.jpackage.main.Main.execute(Main.java:97)
at jdk.jpackage/jdk.jpackage.main.Main.main(Main.java:51)

tor. 5. sep. 2019 kl. 17:12 skrev Sverre Moe :

> I have done some investigation myself.
>
> Found this blog post regarding ampersand in WiX:
>
> http://robmensching.com/blog/posts/2008/4/21/how-to-escape-the-ampersand-in-wix-and-msi-ui/
>
> I tried the solution suggested there. The build works, but the vendor is
> not display correctly in Windows.
> The Publisher name shows the text "". Same goes for using "".
>
> Both of these seems to work on Linux for RPM and DEB package. It does not
> pass the escaped ampersand to jpackage, but Gradle processResourcess task
> seems to replace the escape ampersand with the ampersand in my DEB control
> resource file and RPM spec resource file.
>
> Well, the Publisher does not display correctly on Windows, but the build
> works, the application works. So at least we can move forward, but I hope
> to find the proper solution to get this working and displaying correctly.
>
> /Sverre
>
>
> ons. 4. sep. 2019 kl. 21:01 skrev Andy Herrick :
>
>> This is easily reproducible by putting ampersand in --vendor value on
>> windows.
>>
>> will investigate.
>>
>> /Andy
>> On 9/4/2019 8:05 AM, Sverre Moe wrote:
>>
>> Running WiX failed.
>> The problem it seems is the -dJpAppVendor. It cannot handle special
>> characters in the vendor name. Our company name uses the ampersand (&)
>> instead of "and".
>>
>> Caused by: java.io.IOException: Exec failed with code 104 command
>> [[C:\Program Files (x86)\WiX Toolset v3.11\bin\candle.exe, -nologo,
>> C:\cygwin64\tmp\jdk.jpackage1086156882119031648\config\application.wxs,
>> -ext, WixUtilExtension, -out,
>> C:\cygwin64\tmp\jdk.jpackage1086156882119031648\tmp\application.wixobj,
>> -dJpAppDescription=application, -dJpAppVersion=1.1.0,
>> -dJpWixVersion36OrNewer=yes,
>> -dJpProductCode=2fa37b54-8365-437d-ad34-ceed92844d22,
>>

Re: RFR: 8230648: Replace @exception tag with @throws in java.base

2019-09-05 Thread Julia Boes


Hi,

Thanks for your comments, Lance and Pavel.

The copyright will be updated before pushing, as Daniel suggested.

To address the tag alignment, I adjusted the replacement from 
'@exception' -> '@throws' to '@exception' -> 'throws   ', where the 
added whitespace preserves the original alignment. This doesn't improve 
the alignment (which is not consistent in many places) but at least 
doesn't make it worse.


Updated webrev: http://cr.openjdk.java.net/~dfuchs/jboes/8230648/webrev.02/

Regarding Pavel's comment:

8157682: @inheritDoc doesn't work with @exception

I ran specdiff on the whole JDK and it didn't flag any differences but 
I'll look into additional comparison options.


Cheers,

Julia

Re: Reply： what to do next to fix JDK-8230557. thanks

2019-09-05 Thread Ivan Gerasimov


Hi Peter!

On 9/5/19 7:24 AM, Peter Levart wrote:

Hi Ivan,

On 9/5/19 11:22 AM, Ivan Gerasimov wrote:

Hello!

BitSet is known to be flawed in many ways:?0?2 its size(), length(), 
cardinality() and nextClearBit?6?7() can return meaningless negative 
values.


I was thinking about disallowing setting any index greater than 
(Integer.MAX_VALUE - 63), for example by throwing OutOfMemoryError.


An index of Integer.MAX_VALUE - 64 would be the greatest index then, 
an index of Integer.MAX_VALUE - 63 already needs an array of longs of 
length 2^25 which results in BitSet size() of 2^31 ...




We could do it without changing the specification.


The calls to: new BitSet(Integer.MAX_VALUE - 63) ... new 
BitSet(Integer.MAX_VALUE) would also have to throw then.
BitSet.valueOf(...) methods don't even check the passed in arguments 
lengths, so size() can really return a meaningless negative or 
positive number. They all would have to throw if the passed-in length 
of array/buffer is too large.


So would you not specify when those methods throw?

Yes.?0?2 My point was that any method that requires memory allocation 
should be expected to possibly throw OOM, so we don't have to specify it 
explicitly.


A similar thing happened with compact Strings, for example.?0?2 When the 
internal storage was changed to byte[] array, all of a sudden, it became 
impossible to create UTF16 Strings (or StringBuilders) of length > 
(Integer.MAX_VALUE / 2).


W.r.t. BitSet, personally, I would rather not fix its current behavior 
unless there is a very strong demand for the change.


Just wanted to mention one approach, which would limit usage of this 
class to the boundaries it was originally designed for.


With kind regards,

Ivan



Regards, Peter



With kind regards,

Ivan


On 9/5/19 1:16 AM,  wrote:

Hi, Peter.


I understand your point, but I think it's unreasonable for the 
reason that source code compatibility problem, it's really a bug.



User can't understand why the size method return a negative number.






Best, lamber-ken




----
??:"Peter Levart"??:""<2217232...@qq.com;"core-libs-dev"

:"David Holmes"indexes is: 0 ... 2^31 - 1 (0 ... Integer.MAX_VALUE). The maximum 
"size"

of BitSet is therefore 2^31. Unfortunately, this value can't be
"corectly" represented with signed 32 bit integer (int). Only in this
corner case, - 2^31 (Integer.MIN_VALUE) is the interpreted value
returned from size(). If one would interpret it as unsigned 32 bit
integer value, it is entirely correct. For example, this holds:

Integer.toUnsignedLong(new BitSet(Integer.MAX_VALUE).size()) == 1L 
<< 31


It is also always true what javadoc says about size(): "The maximum
element in the set is the size - 1st element"

The following holds also for this corner case:

new BitSet(Integer.MAX_VALUE).size() - 1 == Integer.MAX_VALUE;

So perhaps, the fix could be just to describe this corner case in the
spec. And perhaps, to support the following use case in the corner 
case:


BitSet set1 = ...
...

BitSet set2 = new BitSet(set1.size());

... by modifying the BitSet constructor to accept the Integer.MIN_VALUE
in addition to all the non-negative values as the 'nbits' parameter.

What do you think?

Regards, Peter

On 9/5/19 8:31 AM, David Holmes wrote:
 Hi,

 On 5/09/2019 4:11 pm,  wrote:

 Thanks very much.

 *BUG-LINK:* https://bugs.openjdk.java.net/browse/JDK-8230557

 *Describe: *
 the return type of the method BitSet#size is int, so the 
method may

 return a negative value in some case, for example, will return
 -2147483648.
 ```
 BitSet bitSet = new BitSet(Integer.MAX_VALUE);
 for (int i = 0; i < Integer.MAX_VALUE - 1000; i++) {
 bitSet.set(i);
 }
 System.out.println(bitSet.size());
 ```
 EXPECTED: 2147483648, but ACTUAL: -2147483648.

 *FIX*
 I have put the patch in the attachment of the mail.

 In case the attachment got stripped form the mailing list the 
proposed

 fix is:

 - public int size() {
 - return words.length 
* BITS_PER_WORD;

 + public long size() {
 + return (long) 
words.length * BITS_PER_WORD;


 Unfortunately this simple fix it not possible. You can't just 
change
 the return type of the method to long as that is a 
source-incompatible
 change and would not be approved [1]. Instead the return value 
should
 be capped at Integer.MAX_VALUE - but I'll leave that for 
someone from

 core-libs team to pick up. Also look at the evaluation in:

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

 Cheers,
 David

 [1] https://wiki.openjdk.java.net/display/csr/CSR+FAQs




 ----
 *??:*"David Holmes" 
*??:*""<2217232...@qq.com;"core-libs-dev"
 d...@openjdk.java.net;
 *:*Re: ?? what to do next to fix JDK-8230557. 
thanks


 On 5/09/2019 3:46 pm,  wrote:
 
  hi, developers.
 
  Can someone help me? thanks very much !!

 Help you how exactly. As I stated your are up to step 2 of 
the how to

Re: RFR(s): 8228580: DnsClient TCP socket timeout

2019-09-05 Thread Florian Weimer

* Milan Mimica:

> On Wed, 4 Sep 2019 at 20:32, Florian Weimer  wrote:
>>
>> If you use the initial UDP timeout (one second, I think), the kernel
>> will not complete the TCP handshake if the initial SYN segment is lost
>> because the retransmit delay during the handshake is longer than that.
>
> We could set a higher timeout value, but I would not prefer that.
> After all, 1 second is just default value, and can be changed. If the
> user wants us to give up on DNS query after the specified timeout then
> lets do just that.

But I think in the UDP case, the client will retry.  I think the total
timeout in the TCP case should equal the total timeout in the UDP case.
That's what I'm going to implement for glibc.  The difference is that in
the TCP case, the TCP stack will take care of the retries, not the
application code.

Thanks,
Florian

Re: Comments on jpackage (JEP 343)

2019-09-05 Thread Alexey Semenyuk


Hi Rachel,

Thank you for your feedback. I created 
https://bugs.openjdk.java.net/browse/JDK-8230668 record to track work on 
your suggestions.


- Alexey

On 9/5/2019 8:50 AM, Rachel Greenham wrote:

(Sorry for non-threading, i read the digest)

As you've been lacking feedback from people using the jpackage EA 
builds, here's mine FWIW.


I've been quiet because it's been working well enough for us. That 
said, our needs and process probably simplify matters in that:


1. We're only producing Windows installers
2. We've been lucky in having patient clients during this 
post-webstart, post-javapackager disruption.

3. We were happy to modify our versioning to match Windows standards
4. Our application is non-modular
5. We do it in three steps: jlink to make a JRE, then jpackage to make 
an app image, then jpackage again to make both an exe and msi 
installer based on that image. (client slow to reply which one they'd 
actually prefer!) Not trying to do everything in one step.


Since the fix that made new versions of our app correctly replace 
older ones I've mostly just been testing new EA builds to make sure 
they don't break it! They do sometimes, usually because of changes in 
the parameter names, and of course we lost our Inno Setup 
customisations. I haven't yet made any attempt to customise the EXE 
setup installer since then.


Would be nice:

1. For it to use the supplied app icon for the installer, or be able 
to supply another specifically for the installer. For it to be shown 
in the installer in some fashion. Other exe customisations of 
straightforward branding and/or flags to control what questions 
they're asked would be very nice.
2. For it to be able to sign the installer in the fashion of, or 
actually using, signtool. (Ideally internalised as installing signtool 
itself is a pain.) Currently that's an extra step after the installers 
are built


But I can wait for them, I want it in a release so I can use it via 
ToolProvider rather than execing an external JDK. All the while it's 
the way it is it massively complicates the build.


Later would-be-nices, not for this desktop app, but ability to use it 
to package background service-type apps, as a service for windows, 
using launchd for osx, and systemd for linux.

Reply： what to do next to fix JDK-8230557. thanks

2019-09-05 Thread 未来阳光

Hi, can we make `size()` and `length()` as deprecated, then write new methods ?


Best.


--原始邮件--
发件人:"Peter Levart"https://bugs.openjdk.java.net/browse/JDK-8230557
 gt;gt;
 gt;gt; *Describe: *
 gt;gt; the return type of the method BitSet#size is int, so 
the 
 method may
 gt;gt; return a negative value in some case, for example, 
will return
 gt;gt; -2147483648.
 gt;gt; ```
 gt;gt; BitSet bitSet = new BitSet(Integer.MAX_VALUE);
 gt;gt; for (int i = 0; i < Integer.MAX_VALUE - 1000; i++) {
 gt;gt; 
nbsp;nbsp;nbsp;nbsp;nbsp;bitSet.set(i);
 gt;gt; }
 gt;gt; System.out.println(bitSet.size());
 gt;gt; ```
 gt;gt; EXPECTED: 2147483648, but ACTUAL: -2147483648.
 gt;gt;
 gt;gt; *FIX*
 gt;gt; I have put the patch in the attachment of the mail.
 gt;
 gt; In case the attachment got stripped form the mailing list the 
 proposed
 gt; fix is:
 gt;
 gt; -nbsp;nbsp;nbsp; public int size() {
 gt; 
-nbsp;nbsp;nbsp;nbsp;nbsp;nbsp;nbsp; return 
words.length 
 * BITS_PER_WORD;
 gt; +nbsp;nbsp;nbsp; public long size() {
 gt; 
+nbsp;nbsp;nbsp;nbsp;nbsp;nbsp;nbsp; return 
(long) 
 words.length * BITS_PER_WORD;
 gt;
 gt; Unfortunately this simple fix it not possible. You can't just 
 change
 gt; the return type of the method to long as that is a 
 source-incompatible
 gt; change and would not be approved [1]. Instead the return 
value 
 should
 gt; be capped at Integer.MAX_VALUE - but I'll leave that for 
someone 
 from
 gt; core-libs team to pick up. Also look at the evaluation in:
 gt;
 gt; https://bugs.openjdk.java.net/browse/JDK-4213570
 gt;
 gt; Cheers,
 gt; David
 gt;
 gt; [1] https://wiki.openjdk.java.net/display/csr/CSR+FAQs
 gt;
 gt;
 gt;
 gt;gt;
 gt;gt; 
--nbsp;原始邮件nbsp;--
 gt;gt; *发件人:*nbsp;"David 
Holmes"http://openjdk.java.net/contribute/
 gt;gt; nbsp;gt;
 gt;gt; nbsp;gt; and you would seem to be up to 
step 2. :)
 gt;gt; nbsp;gt;
 gt;gt; nbsp;gt; Cheers,
 gt;gt; nbsp;gt; David
 gt;gt; nbsp;gt;
 gt;gt; nbsp;gt;nbsp; gt;
 gt;gt; nbsp;gt;nbsp; gt;
 gt;gt; nbsp;gt;nbsp; gt;
 gt;gt; nbsp;gt;nbsp; gt;
 gt;gt; nbsp;gt;nbsp; gt; 
 [1]https://bugs.openjdk.java.net/browse/JDK-8230557
 gt;gt; nbsp;gt;nbsp; gt;
 gt;gt; nbsp;gt;nbsp; gt; 
[2]http://openjdk.java.net/contribute/
 gt;gt; nbsp;gt;nbsp; gt; 
 [3]https://www.oracle.com/technetwork/community/oca-486395.html
 gt;gt; nbsp;gt;nbsp; gt;
 gt;gt; nbsp;gt;nbsp; gt;
 gt;gt; nbsp;gt;nbsp; gt;
 gt;gt; nbsp;gt;nbsp; gt;
 gt;gt; nbsp;gt;nbsp; gt;
 gt;gt; nbsp;gt;nbsp; gt; 
 --amp;nbsp;原始邮件amp;nbsp;--
 gt;gt; nbsp;gt;nbsp; gt; 
发件人:amp;nbsp;"Bug Report
 gt;gt; nbsp;gt; 
 Notification"

Re: RFR(s): 8228580: DnsClient TCP socket timeout

2019-09-05 Thread Milan Mimica

On Wed, 4 Sep 2019 at 20:32, Florian Weimer  wrote:
>
> If you use the initial UDP timeout (one second, I think), the kernel
> will not complete the TCP handshake if the initial SYN segment is lost
> because the retransmit delay during the handshake is longer than that.

We could set a higher timeout value, but I would not prefer that.
After all, 1 second is just default value, and can be changed. If the
user wants us to give up on DNS query after the specified timeout then
lets do just that. True, some queries that previously worked might
start failing, but that is true even for slow socket.read()
operations.

New webrev: http://cr.openjdk.java.net/~mmimica/8228580/webrev.01/

* simplified test with no thread (nice catch Pavel)
* set connect timeout and account for it

-- 
Milan Mimica

Re: jdk-14-jpackage+1-33 on jdk.java.net

2019-09-05 Thread Sverre Moe

I have done some investigation myself.

Found this blog post regarding ampersand in WiX:
http://robmensching.com/blog/posts/2008/4/21/how-to-escape-the-ampersand-in-wix-and-msi-ui/

I tried the solution suggested there. The build works, but the vendor is
not display correctly in Windows.
The Publisher name shows the text "". Same goes for using "".

Both of these seems to work on Linux for RPM and DEB package. It does not
pass the escaped ampersand to jpackage, but Gradle processResourcess task
seems to replace the escape ampersand with the ampersand in my DEB control
resource file and RPM spec resource file.

Well, the Publisher does not display correctly on Windows, but the build
works, the application works. So at least we can move forward, but I hope
to find the proper solution to get this working and displaying correctly.

/Sverre


ons. 4. sep. 2019 kl. 21:01 skrev Andy Herrick :

> This is easily reproducible by putting ampersand in --vendor value on
> windows.
>
> will investigate.
>
> /Andy
> On 9/4/2019 8:05 AM, Sverre Moe wrote:
>
> Running WiX failed.
> The problem it seems is the -dJpAppVendor. It cannot handle special
> characters in the vendor name. Our company name uses the ampersand (&)
> instead of "and".
>
> Caused by: java.io.IOException: Exec failed with code 104 command
> [[C:\Program Files (x86)\WiX Toolset v3.11\bin\candle.exe, -nologo,
> C:\cygwin64\tmp\jdk.jpackage1086156882119031648\config\application.wxs,
> -ext, WixUtilExtension, -out,
> C:\cygwin64\tmp\jdk.jpackage1086156882119031648\tmp\application.wixobj,
> -dJpAppDescription=application, -dJpAppVersion=1.1.0,
> -dJpWixVersion36OrNewer=yes,
> -dJpProductCode=2fa37b54-8365-437d-ad34-ceed92844d22,
> -dJpAppName=application,
> -dJpProductUpgradeCode=53c0f7f6-75c1-419a-86c5-bef18dda408a,
> -dJpIsSystemWide=yes, -dJpAppVendor=Kongsberg Defence & Aerospace,
> -dJpConfigDir=C:\cygwin64\tmp\jdk.jpackage1086156882119031648\config] in
> C:\cygwin64\tmp\jdk.jpackage1086156882119031648\images\win-msi.image\application
> output:
> application.wxsC:\cygwin64\tmp\jdk.jpackage1086156882119031648\config\application.wxs(56)
> : error CNDL0104 : Not a valid source file; detail: An error occurred while
> parsing EntityName. Line 9, position 68.
>
> Is there anyway to allow special characters in the vendor name?
>
>
> It would be very useful to be able to define the release, in addition to
> the version. This is currently only possible on Linux with
> "--linux-app-release".
>
> I could have hacked this by setting "--app-version" to VERSION-RELEASE. It
> would increase the special logic in the build script specific for Windows,
> but it does not seem to be allowed with release in the version
> string:  Version string is not compatible with MSI rules
> [1.1.0-SNAPSHOT20190904133731]
> https://docs.microsoft.com/en-us/windows/win32/msi/productversion
>
> Could this potentially cause problems when installing SNAPSHOTs which have
> the same version?
> Anyway it does not seem WiX XML schema has any release or build attributes.
>
> /Sverre
>
>
> tor. 29. aug. 2019 kl. 17:38 skrev Sverre Moe :
>
>> No, have not installed WIX. Had InnoSetup from when we use javapackager.
>> I will look into the WiX: https://wixtoolset.org
>>
>> /Sverre
>>
>> tor. 29. aug. 2019 kl. 17:34 skrev Kevin Rushforth <
>> kevin.rushfo...@oracle.com>:
>>
>>> Hi Sverre,
>>>
>>> Do you have a WiX installed on your machine? That is a prerequisite.
>>>
>>> Andy: Do we have a bug filed to produce a better error message in this
>>> case? If not, we need to file one.
>>>
>>> -- Kevin
>>>
>>>
>>> On 8/29/2019 7:30 AM, Sverre Moe wrote:
>>>
>>> It is not working creating native installer on Windows.
>>>
>>> It will not take neither exe nor msi as --package-type on Windows.
>>> jdk.jpackage.internal.PackagerException: Error: Invalid or unsupported
>>> package type: [exe].
>>> at
>>> jdk.jpackage/jdk.jpackage.internal.Arguments.generateBundle(Arguments.java:614)
>>> at
>>> jdk.jpackage/jdk.jpackage.internal.Arguments.processArguments(Arguments.java:513)
>>> at jdk.jpackage/jdk.jpackage.main.Main.execute(Main.java:97)
>>> at jdk.jpackage/jdk.jpackage.main.Main.main(Main.java:51)
>>>
>>> The jpackage help output on Windows lists both exe and msi as valid
>>> package types.
>>>
>>> The JDK-8228660 is marked as resolved. I reckon it will make it into the
>>> next build.
>>>
>>> /Sverre
>>>
>>>
>>> tor. 22. aug. 2019 kl. 02:03 skrev Kevin Rushforth <
>>> kevin.rushfo...@oracle.com>:
>>>
 We believe that we have addressed most of the issues, especially those
 affecting the generated Linux packages, both .deb and .rpm. There is
 one
 open issue around the naming of the Debian packages that we will
 address
 in the next EA release. See JDK-8228660 [1] for more information.

 We would love to get some feedback from Linux developers to make sure
 that we didn't miss anything else.

 Thanks.

 -- Kevin

 [1]

8230342: LineNumberReader.getLineNumber() returns inconsistent results after EOF

2019-09-05 Thread Brian Burkhalter

https://bugs.openjdk.java.net/browse/JDK-8230342
http://cr.openjdk.java.net/~bpb/8230342/webrev.00/

When the last line is read by readLine() the line number is incremented due to 
the EOF but not when read() or read(char[]) is used. The specification states 
it is incremented on line terminators only. The alternative solution would be 
to change the specification to state that the line number is also incremented 
on EOF even if there is no line terminator at the end of the stream. That would 
entail a different implementation change.

Thanks,

Brian

Re: RFR(s): 8228580: DnsClient TCP socket timeout

2019-09-05 Thread Pavel Rappo

I think we are almost there. What do you think of the following incremental 
(i.e. on top of your latest webrev) change?

http://cr.openjdk.java.net/~prappo/8228580/webrev.01/

I fixed a couple of trivial typos and addressed the socket relinquishing issue. 
Initializing a socket is not an atomic "all-or-nothing" operation now. Someone 
needs to take care of the socket in case things go not as planned.

-Pavel

> On 5 Sep 2019, at 11:20, Milan Mimica  wrote:
> 
> On Wed, 4 Sep 2019 at 20:32, Florian Weimer  wrote:
>> 
>> If you use the initial UDP timeout (one second, I think), the kernel
>> will not complete the TCP handshake if the initial SYN segment is lost
>> because the retransmit delay during the handshake is longer than that.
> 
> We could set a higher timeout value, but I would not prefer that.
> After all, 1 second is just default value, and can be changed. If the
> user wants us to give up on DNS query after the specified timeout then
> lets do just that. True, some queries that previously worked might
> start failing, but that is true even for slow socket.read()
> operations.
> 
> New webrev: http://cr.openjdk.java.net/~mmimica/8228580/webrev.01/
> 
> * simplified test with no thread (nice catch Pavel)
> * set connect timeout and account for it
> 
> -- 
> Milan Mimica

RFR: 8230662: Remove dead code from MethodTypeForm

2019-09-05 Thread Claes Redestad


Hi,

I noticed some unused methods in java.lang.invoke.MethodTypeForm and
ended up with a rather substantial cleanup after pulling that particular
thread for a bit:

http://cr.openjdk.java.net/~redestad/8230662/jdk.00/
https://bugs.openjdk.java.net/browse/JDK-8230662

Testing: tier1-3

Thanks!

/Claes

Re: Reply： what to do next to fix JDK-8230557. thanks

2019-09-05 Thread Peter Levart


Hi Ivan,

On 9/5/19 11:22 AM, Ivan Gerasimov wrote:

Hello!

BitSet is known to be flawed in many ways:?0?2 its size(), length(), 
cardinality() and nextClearBit?6?7() can return meaningless negative values.


I was thinking about disallowing setting any index greater than 
(Integer.MAX_VALUE - 63), for example by throwing OutOfMemoryError.


An index of Integer.MAX_VALUE - 64 would be the greatest index then, an 
index of Integer.MAX_VALUE - 63 already needs an array of longs of 
length 2^25 which results in BitSet size() of 2^31 ...




We could do it without changing the specification.


The calls to: new BitSet(Integer.MAX_VALUE - 63) ... new 
BitSet(Integer.MAX_VALUE) would also have to throw then.
BitSet.valueOf(...) methods don't even check the passed in arguments 
lengths, so size() can really return a meaningless negative or positive 
number. They all would have to throw if the passed-in length of 
array/buffer is too large.


So would you not specify when those methods throw?

Regards, Peter



With kind regards,

Ivan


On 9/5/19 1:16 AM,  wrote:

Hi, Peter.


I understand your point, but I think it's unreasonable for the reason 
that source code compatibility problem, it's really a bug.



User can't understand why the size method return a negative number.






Best, lamber-ken




----
??:"Peter Levart"??:""<2217232...@qq.com;"core-libs-dev"

:"David Holmes"https://bugs.openjdk.java.net/browse/JDK-8230557

 *Describe: *
 the return type of the method BitSet#size is int, so the 
method may

 return a negative value in some case, for example, will return
 -2147483648.
 ```
 BitSet bitSet = new BitSet(Integer.MAX_VALUE);
 for (int i = 0; i < Integer.MAX_VALUE - 1000; i++) {
 bitSet.set(i);
 }
 System.out.println(bitSet.size());
 ```
 EXPECTED: 2147483648, but ACTUAL: -2147483648.

 *FIX*
 I have put the patch in the attachment of the mail.

 In case the attachment got stripped form the mailing list the 
proposed

 fix is:

 - public int size() {
 - return words.length 
* BITS_PER_WORD;

 + public long size() {
 + return (long) 
words.length * BITS_PER_WORD;


 Unfortunately this simple fix it not possible. You can't just 
change
 the return type of the method to long as that is a 
source-incompatible
 change and would not be approved [1]. Instead the return value 
should
 be capped at Integer.MAX_VALUE - but I'll leave that for someone 
from

 core-libs team to pick up. Also look at the evaluation in:

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

 Cheers,
 David

 [1] https://wiki.openjdk.java.net/display/csr/CSR+FAQs




 ----
 *??:*"David Holmes" 
*??:*""<2217232...@qq.com;"core-libs-dev"
 d...@openjdk.java.net;
 *:*Re: ?? what to do next to fix JDK-8230557. 
thanks


 On 5/09/2019 3:46 pm,  wrote:
 
  hi, developers.
 
  Can someone help me? thanks very much !!

 Help you how exactly. As I stated your are up to step 2 of 
the how to
 contribute process. If you have a suggested fix for the bug 
then put

 that in an email as described.

 Thanks,
 David

 
  
----
  *??:*"David 
Holmes"
  *:*2019??9??5??(??) 1:44
  
*??:*""<2217232...@qq.com;"core-libs-dev"
  d...@openjdk.java.net;
  *:*Re: what to do next to fix 
JDK-8230557. thanks

 
  On 5/09/2019 3:35 pm,  wrote:
   Hi, leaders.
 
  Hi,
 
  No "leaders" here only developers :)
 
  
   A few days ago, I report a bug in core 
lib[1]. According to the
  contribute document[2], I had send oca to oracle 
andnbsp;my name has

  been listed onnbsp;oca[3].
 
  Welcome aboard!
 
   But I still can't push my changes to 
jdk, can someone tell me

 how to
  do next? thanks very match!!
 
  You can't push anything until you become a 
Committer and before

 that you
  have to become an Author. The steps for 
contributing are outlined

 here:
 
  http://openjdk.java.net/contribute/
 
  and you would seem to be up to step 2. :)
 
  Cheers,
  David
 
  
  
  
  
   
[1]https://bugs.openjdk.java.net/browse/JDK-8230557

  
   [2]http://openjdk.java.net/contribute/
   
[3]https://www.oracle.com/technetwork/community/oca-486395.html

  
  
  
  
  
   
--nbsp;nbsp;--

   ??:nbsp;"Bug Report
  
Notification"   :nbsp;2019??9??5??(??) 
3:33
   
??:nbsp;""<2217232...@qq.comgt;;

  
   :nbsp;Update Notification: Bug 
Report - JDK-8230557

  
  
  
  
  
 
[This is an automated response. Please

 do not
  reply.]
   Dear Java Developer,
   We have finished evaluating your 
report and have assigned it a Bug
  ID: JDK-8230557. The issue is visible on 
bugs.java.com at the

 following
  url JDK-8230557.
  
 
To provide more information about this issue,

  click here.
  
 
We work to resolve the issues that are

 submitted to
  us according to their impact to the

Re: Reply： what to do next to fix JDK-8230557. thanks

2019-09-05 Thread Peter Levart

Hi ,

On 9/5/19 10:16 AM,  wrote:

Hi, Peter.

I understand your point, but I think it's unreasonable for the reason 
that source code compatibility problem, it's really a bug.

Unfortunately, when such bug is out in the wild for so long, it becomes 
a feature. It would be unreasonable to fix it the way you propose now, 
because all programs that use BitSet.size() method would break by such 
fix. It is not only source incompatible fix, but binary incompatible too.

User can't understand why the size method return a negative number.

Perhaps, if the javadoc of the method described this corner case, he/she 
would understand?

Regards, Peter

Best, lamber-ken

--?0?2?0?2--
*??:*?0?2"Peter Levart";
*:*?0?22019??9??5??(??) 3:51
*??:*?0?2""<2217232...@qq.com>;"core-libs-dev";
*:*?0?2"David Holmes";
*:*?0?2Re: ?? ?? what to do next to fix JDK-8230557. thanks

Hi ,

As David has pointed out, your proposed fix would break binary and
source compatibility of BitSet.size() method, so it is not acceptable.

BitSet API allows addressing individual bits using non-negative 'int'
typed indexes (analogous to indexes of Java arrays). The range of
indexes is: 0 ... 2^31 - 1 (0 ... Integer.MAX_VALUE). The maximum "size"
of BitSet is therefore 2^31. Unfortunately, this value can't be
"corectly" represented with signed 32 bit integer (int). Only in this
corner case, - 2^31 (Integer.MIN_VALUE) is the interpreted value
returned from size(). If one would interpret it as unsigned 32 bit
integer value, it is entirely correct. For example, this holds:

Integer.toUnsignedLong(new BitSet(Integer.MAX_VALUE).size()) == 1L << 31

It is also always true what javadoc says about size(): "The maximum
element in the set is the size - 1st element"

The following holds also for this corner case:

new BitSet(Integer.MAX_VALUE).size() - 1 == Integer.MAX_VALUE;

So perhaps, the fix could be just to describe this corner case in the
spec. And perhaps, to support the following use case in the corner case:

BitSet set1 = ...
...

BitSet set2 = new BitSet(set1.size());

... by modifying the BitSet constructor to accept the Integer.MIN_VALUE
in addition to all the non-negative values as the 'nbits' parameter.

What do you think?

Regards, Peter

On 9/5/19 8:31 AM, David Holmes wrote:
> Hi,
>
> On 5/09/2019 4:11 pm,  wrote:
>>
>> Thanks very much.
>>
>> *BUG-LINK:* https://bugs.openjdk.java.net/browse/JDK-8230557
>>
>> *Describe: *
>> the return type of the method BitSet#size is int, so the method may
>> return a negative value in some case, for example, will return
>> -2147483648.
>> ```
>> BitSet bitSet = new BitSet(Integer.MAX_VALUE);
>> for (int i = 0; i < Integer.MAX_VALUE - 1000; i++) {
>> ?0?2?0?2?0?2?0?2?0?2bitSet.set(i);
>> }
>> System.out.println(bitSet.size());
>> ```
>> EXPECTED: 2147483648, but ACTUAL: -2147483648.
>>
>> *FIX*
>> I have put the patch in the attachment of the mail.
>
> In case the attachment got stripped form the mailing list the proposed
> fix is:
>
> -?0?2?0?2?0?2 public int size() {
> -?0?2?0?2?0?2?0?2?0?2?0?2?0?2 return words.length * BITS_PER_WORD;
> +?0?2?0?2?0?2 public long size() {
> +?0?2?0?2?0?2?0?2?0?2?0?2?0?2 return (long) words.length * BITS_PER_WORD;
>
> Unfortunately this simple fix it not possible. You can't just change
> the return type of the method to long as that is a source-incompatible
> change and would not be approved [1]. Instead the return value should
> be capped at Integer.MAX_VALUE - but I'll leave that for someone from
> core-libs team to pick up. Also look at the evaluation in:
>
> https://bugs.openjdk.java.net/browse/JDK-4213570
>
> Cheers,
> David
>
> [1] https://wiki.openjdk.java.net/display/csr/CSR+FAQs
>
>
>
>>
>> --?0?2?0?2--
>> *??:*?0?2"David Holmes";
>> *:*?0?22019??9??5??(??) 2:00
>> *??:*?0?2""<2217232...@qq.com>;"core-libs-dev"> d...@openjdk.java.net>;
>> *:*?0?2Re: ?? what to do next to fix JDK-8230557. thanks
>>
>> On 5/09/2019 3:46 pm,  wrote:
>> ?0?2>
>> ?0?2> hi, developers.
>> ?0?2>
>> ?0?2> Can someone help me? thanks very much !!
>>
>> Help you how exactly. As I stated your are up to step 2 of the how to
>> contribute process. If you have a suggested fix for the bug then put
>> that in an email as described.
>>
>> Thanks,
>> David
>>
>> ?0?2>
>> ?0?2> --?0?2?0?2--
>> ?0?2> *??:*?0?2"David Holmes";
>> ?0?2> *:*?0?22019??9??5??(??) 1:44
>> ?0?2> *??:*?0?2""<2217232...@qq.com>;"core-libs-dev"> ?0?2> d...@openjdk.java.net>;
>> ?0?2> *:*?0?2Re: what to do next to fix JDK-8230557. thanks
>> ?0?2>
>> ?0?2> On 5/09/2019 3:35 pm,  wrote:
>> ?0?2>?0?2 > Hi, leaders.
>> ?0?2>
>> ?0?2> Hi,
>> ?0?2>
>> ?0?2> No "leaders" here only developers :)
>> ?0?2>
>> ?0?2>?0?2 >
>> ?0?2>?0?2 > A few

Re: RFR: 8212117 : Class.forName may return a reference to a loaded but not linked Class

2019-09-05 Thread Daniel D. Daugherty

On 9/5/19 1:36 AM, David Holmes wrote:

Hi Dan,

With my CSR Group member hat on

On 5/09/2019 8:06 am, Daniel D. Daugherty wrote:

Brent,

You currently have '-XX:+ClassForNameDeferLinking' as a 'product'
option, but
product options are harder to remove down the road. Would it be
better as a
diagnostic option? A diagnostic option requires

Whether a flag is product or diagnostic (or experimental) should be a
basic property of the flag based on its purpose. I would not want to
see a trend of making new flags diagnostic just because it is easier
to get rid of them later. The expectation with this fix and flag
(which I've been heavily involved in) is that production code may be
impacted by the change and need to use the flag to restore previous
behaviour. So it really is a product flag that end users should be
comfortable in using if they need
it.

This is the key phrase:

> production code may be impacted by the change and need to
> use the flag to restore previous behaviour.

Thanks for making that clear. Maybe I missed it in what I read,
but it is now clear that this should be a product flag.

Dan

The removal process for a product flag is phased (deprecate, obsolete,
expire) but not particularly onerous. There is an expectation that
this flag may be deprecated in 15 as it is intended as a transitional
flag.

Thanks,
David
-

'-XX:+UnlockDiagnosticVMOptions'
to be specified before it can be used, e.g.:

java -XX:+UnlockDiagnosticVMOptions
-XX:+ClassForNameDeferLinking Foo

so it is a bit harder to use, but maybe that's a Good Thing (TM).

Dan

On 9/4/19 5:12 PM, Brent Christian wrote:

Hi,

Please review my fix for JDK-8212117[1]. The webrev is here:

http://cr.openjdk.java.net/~bchristi/8212117/webrev09/

There is also a CSR[2] in need of review.

The spec for the 2-arg and 3-arg Class.forName() methods states they
will "locate, load, and link" the class, however the linking part is
not ensured to happen.

This fix ensures that linking happens during the course of
Class.forName(). Class.forName() already @throws LinkageError, so
no spec change is needed there. MethodHandles.Lookup.findClass()
needs a small spec update, due to calling Class.forName with
init=false,

Of course Errors are not required to be caught. It is therefore
possible that the new behavior could introduce previously unseen,
possibly unhandled LinkageErrors. A new VM flag
(-XX:+ClassForNameDeferLinking) is introduced to restore the
previous behavior (to keep such code running until it can be updated).

This change surfaced a couple new "A JNI error has occurred"
situations (see 8181033[5]) in the Launcher, by way of

The Launcher is updated to maintain non-confusing error messages :).

The new test included with this fix ensures that 8181144[3] is
addressed. Thanks go to Serguei Spitsyn for writing the test.

Automated corelibs and hotspot tests pass cleanly.

Thanks,
-Brent

--
1. https://bugs.openjdk.java.net/browse/JDK-8212117

2. https://bugs.openjdk.java.net/browse/JDK-8222071

3. https://bugs.openjdk.java.net/browse/JDK-8181144

4.
https://bugs.openjdk.java.net/browse/JDK-8212117?focusedCommentId=14215986=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-14215986

5. https://bugs.openjdk.java.net/browse/JDK-8181033

Re: Comments on jpackage (JEP 343)

2019-09-05 Thread Rachel Greenham


(Sorry for non-threading, i read the digest)

As you've been lacking feedback from people using the jpackage EA 
builds, here's mine FWIW.


I've been quiet because it's been working well enough for us. That said, 
our needs and process probably simplify matters in that:


1. We're only producing Windows installers
2. We've been lucky in having patient clients during this post-webstart, 
post-javapackager disruption.

3. We were happy to modify our versioning to match Windows standards
4. Our application is non-modular
5. We do it in three steps: jlink to make a JRE, then jpackage to make 
an app image, then jpackage again to make both an exe and msi installer 
based on that image. (client slow to reply which one they'd actually 
prefer!) Not trying to do everything in one step.


Since the fix that made new versions of our app correctly replace older 
ones I've mostly just been testing new EA builds to make sure they don't 
break it! They do sometimes, usually because of changes in the parameter 
names, and of course we lost our Inno Setup customisations. I haven't 
yet made any attempt to customise the EXE setup installer since then.


Would be nice:

1. For it to use the supplied app icon for the installer, or be able to 
supply another specifically for the installer. For it to be shown in the 
installer in some fashion. Other exe customisations of straightforward 
branding and/or flags to control what questions they're asked would be 
very nice.
2. For it to be able to sign the installer in the fashion of, or 
actually using, signtool. (Ideally internalised as installing signtool 
itself is a pain.) Currently that's an extra step after the installers 
are built


But I can wait for them, I want it in a release so I can use it via 
ToolProvider rather than execing an external JDK. All the while it's the 
way it is it massively complicates the build.


Later would-be-nices, not for this desktop app, but ability to use it to 
package background service-type apps, as a service for windows, using 
launchd for osx, and systemd for linux.


--
Rachel

Re: Comments on jpackage (JEP 343)

2019-09-05 Thread Andy Herrick




Thank you very much for the detailed feedback.

see inline for Change Requests created or associated with each point .

/Andy

On 9/3/2019 2:58 PM, mark.reinh...@oracle.com wrote:

I spent some time last week playing with the jpackage tool, using build
14-jpackage+1-35 from https://jdk.java.net/jpackage.

Overall, I can see that you’ve made good progress, but there’s still some
work to be done.  I’ll start with high-level comments and questions, and
then comment on my experience using the tool on Debian and then macOS.

High-level comments/questions

   - It’s good that you’re publishing EA builds, but I haven’t seen very
 much feedback from those builds.  It may be better to propose this as
 an experimental feature when it’s ready to target.  That would give
 you the freedom to improve it incompatibly over one or two release
 cycles before you commit to a final version.


We like the suggestion to release as an experimental feature.

see: JDK-8230649 



   - The tool still generates an image by default, rather than a package.
 This will surprise many users, especially those who just want to do
 something simple and straightforward.  The least-surprising default
 behavior would be to generate a package of the type most suitable for
 the current platform.  An option to generate just an image would be
 fine, for those who want to tweak it before the actual packaging
 step, but that shouldn’t be the default.

   - Related to the previous point, I should only have to specify the
 `--package-type` option if I want something other than the default
 for the current platform.


We will add "app-image" as an explicit type and change the 
--package-type default to be type most suitable for the current platform.


see: JDK-8230519 



   - The tool assumes that the application being packaged will have a GUI.
 This isn’t surprising, considering its heritage, but as a consequence
 it always produces packages that perform GUI-specific actions, such
 as installing icons and desktop-menu entries.  If I’m just packaging
 a command-line tool then these are unnecessary, and supporting them
 can pull in lots of additional dependencies (e.g., a ton of Perl
 scripts on Debian).  Consider adding an option (`--gui`?) so that
 the user can indicate when an application is to be installed for
 graphical use.


For giving the developer the control of shortcuts we have created : 
JDK-8229779 


If an application does not request shortcut(s) there should be no need 
for gui code.


   - The `--output`/`-o` option is confusing.  It doesn’t name the output
 itself, but rather a directory into which the single item of output
 will be placed.  Typing `-o .` all the time is just annoying.  It’d
 be more logical to rename this option to `--dest`/`-d` and to make it
 optional, with a default value of `.`.


We will change name of option and default as suggested

see: JDK-8230521 



   - If my app is modular, and my main module has a version string, then
 it’d be nice for that to be used as the package version unless a
 specific version is specified on the command line.

see: JDK-8230651 

   - Terminology: For Linux we generally speak of “packages” rather than
 “bundles.”  Rename `--linux-bundle-name` to `--linux-package-name`.
this will be fixed as part of: JDK-8230522 



   - The `--temp-root` option could be shortened to `--temp`.

see: JDK-8230522 


   - Periods at the ends of error messages are unsightly and unnecessary.
 We don’t use them in other JDK tools.  Please remove them.
This is also included in: JDK-8230522 



   - It’d be nice to have an option that displays the programs being run
 by the tool, in a form that can easily be cut-and-pasted into a
 script for those who need to do a lot of customization.  The current
 verbose output shows (at least some of) this information, but in a
 difficult-to-use format.

see:JDK-8230652 


   - What’s the rationale for copying the entire “input” directory as-is?
 Why is its structure important?  Couldn’t you just as well limit this
 to a single directory full of JAR files?
After discussion , I will look for use case where this existing behavior 
is desirable.


Comments on Debian packaging


All of the below comments on Debian packaging  are reflected in 
JDK-8230507 


They may or may not be split up into individual CR's as they are addressed.

Re: RFR: 8230648: Replace @exception tag with @throws in java.base

2019-09-05 Thread Lance Andersen

Hi Julia,

I think the fix is fine overall

I noticed that the copyright was not updated in all of the files modified, 
which may be OK in this case, but I typically change it when I made minor 
changes as well.  Not a big deal.

The only other question is I am wondering if we want to align the text better 
with the rest of the javadocs (maybe a separate bug) as for example you look at 
ZipOutputStream.java, the change on line 144 is slightly off.  Its not a big 
deal but a minor hiccup from probably using a script which is understandable 

HTH

lance
> On Sep 5, 2019, at 6:59 AM, Julia Boes  > wrote:
> 
> ..this time with the right link!
> 
> 
> On 05/09/2019 11:40, Julia Boes wrote:
>> Hi,
>> 
>> This change replaces all occurrences of the @exception tag in java.base with 
>> the @throws tag. The tags are synonyms but @throws is the newer preferred 
>> option [1].
>> 
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8230648 
>> 
> Webrev: http://cr.openjdk.java.net/~dfuchs/jboes/8230648/webrev.00/ 
> 
>> 
>> 
>> 
>> Regards,
>> 
>> Julia
>> 
>> [1] 
>> https://docs.oracle.com/javase/7/docs/technotes/tools/windows/javadoc.html#throws
>>  
>> 
>> 

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

Re: RFR: 8230648: Replace @exception tag with @throws in java.base

2019-09-05 Thread Pavel Rappo

Julia,

Trivially update the copyright years and pay attention to not-so-trivial things 
like:

8157682: @inheritDoc doesn't work with @exception

I'd suggest comparing "before" and "after" javadoc visually to make sure no 
funny business is going on.

-Pavel

> On 5 Sep 2019, at 11:40, Julia Boes  wrote:
> 
> Hi,
> 
> This change replaces all occurrences of the @exception tag in java.base with 
> the @throws tag. The tags are synonyms but @throws is the newer preferred 
> option [1].
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8230648
> 
> Webrev: 
> http://corelibs-linux-1.ie.oracle.com/~jboes/webrevs/8230648/webrev.00/
> 
> 
> Regards,
> 
> Julia
> 
> [1] 
> https://docs.oracle.com/javase/7/docs/technotes/tools/windows/javadoc.html#throws
>

Re: RFR: 8230648: Replace @exception tag with @throws in java.base

2019-09-05 Thread Julia Boes


..this time with the right link!


On 05/09/2019 11:40, Julia Boes wrote:

Hi,

This change replaces all occurrences of the @exception tag in 
java.base with the @throws tag. The tags are synonyms but @throws is 
the newer preferred option [1].


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

Webrev: http://cr.openjdk.java.net/~dfuchs/jboes/8230648/webrev.00/




Regards,

Julia

[1] 
https://docs.oracle.com/javase/7/docs/technotes/tools/windows/javadoc.html#throws

RFR: 8230648: Replace @exception tag with @throws in java.base

2019-09-05 Thread Julia Boes


Hi,

This change replaces all occurrences of the @exception tag in java.base 
with the @throws tag. The tags are synonyms but @throws is the newer 
preferred option [1].


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

Webrev: 
http://corelibs-linux-1.ie.oracle.com/~jboes/webrevs/8230648/webrev.00/



Regards,

Julia

[1] 
https://docs.oracle.com/javase/7/docs/technotes/tools/windows/javadoc.html#throws

Re: Comments on jpackage (JEP 343)

2019-09-05 Thread Lennart Börjeson

Could you please also revive the UserJvmOptionsService, that (for very unclear 
reasons) was removed together with JavaFX?

Since the functionality provided by the UserJvmOptionsService requires some 
cooperation with the launcher created by jpackage, I can't see how I could 
implement some work-around myself.

(I raised this question already in 
https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-March/059419.html, 
but without response.)

Best regards,

Lennart Börjeson

Re: Reply： what to do next to fix JDK-8230557. thanks

2019-09-05 Thread Ivan Gerasimov


Hello!

BitSet is known to be flawed in many ways:?0?2 its size(), length(), 
cardinality() and nextClearBit?6?7() can return meaningless negative values.


I was thinking about disallowing setting any index greater than 
(Integer.MAX_VALUE - 63), for example by throwing OutOfMemoryError.


We could do it without changing the specification.

With kind regards,

Ivan


On 9/5/19 1:16 AM,  wrote:

Hi, Peter.


I understand your point, but I think it's unreasonable for the reason that 
source code compatibility problem, it's really a bug.


User can't understand why the size method return a negative number.






Best, lamber-ken




----
??:"Peter Levart"https://bugs.openjdk.java.net/browse/JDK-8230557

 *Describe: *
 the return type of the method BitSet#size is int, so the method may
 return a negative value in some case, for example, will return
 -2147483648.
 ```
 BitSet bitSet = new BitSet(Integer.MAX_VALUE);
 for (int i = 0; i < Integer.MAX_VALUE - 1000; i++) {
 bitSet.set(i);
 }
 System.out.println(bitSet.size());
 ```
 EXPECTED: 2147483648, but ACTUAL: -2147483648.

 *FIX*
 I have put the patch in the attachment of the mail.

 In case the attachment got stripped form the mailing list the proposed
 fix is:

 - public int size() {
 - return words.length * 
BITS_PER_WORD;
 + public long size() {
 + return (long) words.length * 
BITS_PER_WORD;

 Unfortunately this simple fix it not possible. You can't just change
 the return type of the method to long as that is a source-incompatible
 change and would not be approved [1]. Instead the return value should
 be capped at Integer.MAX_VALUE - but I'll leave that for someone from
 core-libs team to pick up. Also look at the evaluation in:

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

 Cheers,
 David

 [1] https://wiki.openjdk.java.net/display/csr/CSR+FAQs




 ----
 *??:*"David Holmes"http://openjdk.java.net/contribute/
 
  and you would seem to be up to step 2. :)
 
  Cheers,
  David
 
  
  
  
  
   
[1]https://bugs.openjdk.java.net/browse/JDK-8230557
  
   [2]http://openjdk.java.net/contribute/
   
[3]https://www.oracle.com/technetwork/community/oca-486395.html
  
  
  
  
  
   
--nbsp;nbsp;--
   ??:nbsp;"Bug Report
  Notification"

--
With kind regards,
Ivan Gerasimov

Reply?? what to do next to fix JDK-8230557. thanks

2019-09-05 Thread ????????

Hi, Peter.


the length method return a negative number too






Best, lamber-ken




--  --
??:"Peter Levart"https://bugs.openjdk.java.net/browse/JDK-8230557

 *Describe: *
 the return type of the method BitSet#size is int, so the method may 
 return a negative value in some case, for example, will return 
 -2147483648.
 ```
 BitSet bitSet = new BitSet(Integer.MAX_VALUE);
 for (int i = 0; i < Integer.MAX_VALUE - 1000; i++) {
  bitSet.set(i);
 }
 System.out.println(bitSet.size());
 ```
 EXPECTED: 2147483648, but ACTUAL: -2147483648.

 *FIX*
 I have put the patch in the attachment of the mail.

 In case the attachment got stripped form the mailing list the proposed 
 fix is:

 -  public int size() {
 -  return words.length * BITS_PER_WORD;
 +  public long size() {
 +  return (long) words.length * 
BITS_PER_WORD;

 Unfortunately this simple fix it not possible. You can't just change 
 the return type of the method to long as that is a source-incompatible 
 change and would not be approved [1]. Instead the return value should 
 be capped at Integer.MAX_VALUE - but I'll leave that for someone from 
 core-libs team to pick up. Also look at the evaluation in:

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

 Cheers,
 David

 [1] https://wiki.openjdk.java.net/display/csr/CSR+FAQs




 --  --
 *??:* "David Holmes"http://openjdk.java.net/contribute/
 
  and you would seem to be up to step 2. :)
 
  Cheers,
  David
 
  
  
  
  
   
[1]https://bugs.openjdk.java.net/browse/JDK-8230557
  
   [2]http://openjdk.java.net/contribute/
   
[3]https://www.oracle.com/technetwork/community/oca-486395.html
  
  
  
  
  
   
--nbsp;nbsp;--
   ??:nbsp;"Bug Report
  Notification"

Reply?? what to do next to fix JDK-8230557. thanks

2019-09-05 Thread ????????

Hi, Peter.


I understand your point, but I think it's unreasonable for the reason that 
source code compatibility problem, it's really a bug.


User can't understand why the size method return a negative number.






Best, lamber-ken




----
??:"Peter Levart"https://bugs.openjdk.java.net/browse/JDK-8230557

 *Describe: *
 the return type of the method BitSet#size is int, so the method may 
 return a negative value in some case, for example, will return 
 -2147483648.
 ```
 BitSet bitSet = new BitSet(Integer.MAX_VALUE);
 for (int i = 0; i < Integer.MAX_VALUE - 1000; i++) {
 bitSet.set(i);
 }
 System.out.println(bitSet.size());
 ```
 EXPECTED: 2147483648, but ACTUAL: -2147483648.

 *FIX*
 I have put the patch in the attachment of the mail.

 In case the attachment got stripped form the mailing list the proposed 
 fix is:

 - public int size() {
 - return words.length * 
BITS_PER_WORD;
 + public long size() {
 + return (long) words.length * 
BITS_PER_WORD;

 Unfortunately this simple fix it not possible. You can't just change 
 the return type of the method to long as that is a source-incompatible 
 change and would not be approved [1]. Instead the return value should 
 be capped at Integer.MAX_VALUE - but I'll leave that for someone from 
 core-libs team to pick up. Also look at the evaluation in:

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

 Cheers,
 David

 [1] https://wiki.openjdk.java.net/display/csr/CSR+FAQs




 ----
 *??:*"David Holmes"http://openjdk.java.net/contribute/
 
  and you would seem to be up to step 2. :)
 
  Cheers,
  David
 
  
  
  
  
   
[1]https://bugs.openjdk.java.net/browse/JDK-8230557
  
   [2]http://openjdk.java.net/contribute/
   
[3]https://www.oracle.com/technetwork/community/oca-486395.html
  
  
  
  
  
   
--nbsp;nbsp;--
   ??:nbsp;"Bug Report
  Notification"

Re: RFR: 8212117 : Class.forName may return a reference to a loaded but not linked Class

2019-09-05 Thread David Holmes


Hi Brent,

Good to see this all come together :)

A couple of comments below.

On 5/09/2019 7:12 am, Brent Christian wrote:

Hi,

Please review my fix for JDK-8212117[1].  The webrev is here:

http://cr.openjdk.java.net/~bchristi/8212117/webrev09/


So to clarify for others any current caller to 
find_class_from_class_loader that passes true for "init" was already 
implicitly asking for a linked class (as you must be linked before you 
can be initialized). With that in mind I would suggest that in 
find_class_from_class_loader you add an assert:


! jclass find_class_from_class_loader(JNIEnv* env, Symbol* name, 
jboolean init, jboolean link,
  Handle loader, Handle 
protection_domain,

  jboolean throwError, TRAPS) {
+assert((init && link) || !init, "incorrect use of init/link arguments");

just to ensure the callers understand this.

Aside: in looking at this I've spotted another existing bug. JNI 
FindClass is not specified to perform class initialization, but the 
implementation passes init=true!


---

src/java.base/share/classes/java/lang/invoke/MethodHandles.java

I don't see the need for the new note given it already has

* @throws LinkageError if the linkage fails

---

src/java.base/share/classes/sun/launcher/LauncherHelper.java

So to clarify, the launcher would previously load the main class without 
linking via:


   mainClass = LoadMainClass(env, mode, what);
   CHECK_EXCEPTION_NULL_LEAVE(mainClass);

and then it would invoke the main method which implicitly initialized 
which implicitly linked and so any exceptions related to linking would 
be handled when the main thread detaches, and it all gets reported 
nicely. But now that we link earlier the exception could be pending 
after LoadMainClass and so we would "abort". So you catch that 
unexpected exception in the LauncherHelper.


Is AccessControlException the only exception, that is not a 
LinkageError, that might be thrown when linking? I would think a number 
of others are possible - in particular IllegalAccessError might occur 
during verification. Other than the fact a test obviously triggered 
this, it's not clear why ACE should be singled out here. ??


---

test/hotspot/jtreg/serviceability/jvmti/ClassStatus/ClassStatus.java

45 // public static void foo(Foo f) { }

Unclear why this exists. Also the test refers initially to class Foo but 
then uses Foo2 and Foo3. ??


Otherwise all seems good.


There is also a CSR[2] in need of review.


I've added a couple of comments and will add myself as a reviewer once 
things are near finalized.


Thanks,
David
-



The spec for the 2-arg and 3-arg Class.forName() methods states they 
will "locate, load, and link" the class, however the linking part is not 
ensured to happen.


Although the VM spec allows flexibility WRT when classes are linked, 
this is a corner where the Class.forName() spec is not being upheld. 
While this is not an issue for most usages,  8181144 [3] demonstrates 
how this can be a problem (with the debugging interface, in this case).


This fix ensures that linking happens during the course of 
Class.forName().  Class.forName() already @throws LinkageError, so no 
spec change is needed there. MethodHandles.Lookup.findClass() needs a 
small spec update, due to calling Class.forName with init=false,


Of course Errors are not required to be caught.  It is therefore 
possible that the new behavior could introduce previously unseen, 
possibly unhandled LinkageErrors.  A new VM flag 
(-XX:+ClassForNameDeferLinking) is introduced to restore the previous 
behavior (to keep such code running until it can be updated).


This change surfaced a couple new "A JNI error has occurred" situations 
(see 8181033[5]) in the Launcher, by way of

   test/jdk/tools/launcher/MainClassCantBeLoadedTest.java
(using the 3-arg Class::forName, detailed in the bug report[4]),
and
   test/jdk/tools/launcher/modules/basic/LauncherErrors.java
(using the 2-arg Class::forName).

The Launcher is updated to maintain non-confusing error messages :).

The new test included with this fix ensures that 8181144[3] is 
addressed.  Thanks go to Serguei Spitsyn for writing the test.


Automated corelibs and hotspot tests pass cleanly.

Thanks,
-Brent

--
1. https://bugs.openjdk.java.net/browse/JDK-8212117

2. https://bugs.openjdk.java.net/browse/JDK-8222071

3. https://bugs.openjdk.java.net/browse/JDK-8181144

4. 
https://bugs.openjdk.java.net/browse/JDK-8212117?focusedCommentId=14215986=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-14215986 



5. https://bugs.openjdk.java.net/browse/JDK-8181033

Re: 回复：回复： what to do next to fix JDK-8230557. thanks

2019-09-05 Thread Peter Levart

Hi 未来阳光,

As David has pointed out, your proposed fix would break binary and 
source compatibility of BitSet.size() method, so it is not acceptable.

BitSet API allows addressing individual bits using non-negative 'int' 
typed indexes (analogous to indexes of Java arrays). The range of 
indexes is: 0 ... 2^31 - 1 (0 ... Integer.MAX_VALUE). The maximum "size" 
of BitSet is therefore 2^31. Unfortunately, this value can't be 
"corectly" represented with signed 32 bit integer (int). Only in this 
corner case, - 2^31 (Integer.MIN_VALUE) is the interpreted value 
returned from size(). If one would interpret it as unsigned 32 bit 
integer value, it is entirely correct. For example, this holds:

Integer.toUnsignedLong(new BitSet(Integer.MAX_VALUE).size()) == 1L << 31

It is also always true what javadoc says about size(): "The maximum 
element in the set is the size - 1st element"

The following holds also for this corner case:

new BitSet(Integer.MAX_VALUE).size() - 1 == Integer.MAX_VALUE;

So perhaps, the fix could be just to describe this corner case in the 
spec. And perhaps, to support the following use case in the corner case:

BitSet set1 = ...
...

BitSet set2 = new BitSet(set1.size());

... by modifying the BitSet constructor to accept the Integer.MIN_VALUE 
in addition to all the non-negative values as the 'nbits' parameter.

What do you think?

Regards, Peter

On 9/5/19 8:31 AM, David Holmes wrote:

Hi,

On 5/09/2019 4:11 pm, 未来阳光 wrote:

Thanks very much.

*BUG-LINK:* https://bugs.openjdk.java.net/browse/JDK-8230557

*Describe: *
the return type of the method BitSet#size is int, so the method may 
return a negative value in some case, for example, will return 
-2147483648.

```
BitSet bitSet = new BitSet(Integer.MAX_VALUE);
for (int i = 0; i < Integer.MAX_VALUE - 1000; i++) {
 bitSet.set(i);
}
System.out.println(bitSet.size());
```
EXPECTED: 2147483648, but ACTUAL: -2147483648.

*FIX*
I have put the patch in the attachment of the mail.

In case the attachment got stripped form the mailing list the proposed 
fix is:

-    public int size() {
-    return words.length * BITS_PER_WORD;
+    public long size() {
+    return (long) words.length * BITS_PER_WORD;

Unfortunately this simple fix it not possible. You can't just change 
the return type of the method to long as that is a source-incompatible 
change and would not be approved [1]. Instead the return value should 
be capped at Integer.MAX_VALUE - but I'll leave that for someone from 
core-libs team to pick up. Also look at the evaluation in:

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

Cheers,
David

[1] https://wiki.openjdk.java.net/display/csr/CSR+FAQs

-- 原始邮件 --
*发件人:* "David Holmes";
*发送时间:* 2019年9月5日(星期四) 下午2:00
*收件人:* "未来阳光"<2217232...@qq.com>;"core-libs-dev"d...@openjdk.java.net>;

*主题:* Re: 回复： what to do next to fix JDK-8230557. thanks

On 5/09/2019 3:46 pm, 未来阳光 wrote:
 >
 > hi, developers.
 >
 > Can someone help me? thanks very much !!

Help you how exactly. As I stated your are up to step 2 of the how to
contribute process. If you have a suggested fix for the bug then put
that in an email as described.

Thanks,
David

 >
 > -- 原始邮件 --
 > *发件人:* "David Holmes";
 > *发送时间:* 2019年9月5日(星期四) 中午1:44
 > *收件人:* "未来阳光"<2217232...@qq.com>;"core-libs-dev" d...@openjdk.java.net>;
 > *主题:* Re: what to do next to fix JDK-8230557. thanks
 >
 > On 5/09/2019 3:35 pm, 未来阳光 wrote:
 >  > Hi, leaders.
 >
 > Hi,
 >
 > No "leaders" here only developers :)
 >
 >  >
 >  > A few days ago, I report a bug in core lib[1]. According to the
 > contribute document[2], I had send oca to oracle andmy name has
 > been listed onoca[3].
 >
 > Welcome aboard!
 >
 >  > But I still can't push my changes to jdk, can someone tell me 
how to

 > do next? thanks very match!!
 >
 > You can't push anything until you become a Committer and before 
that you
 > have to become an Author. The steps for contributing are outlined 
here:

 >
 > http://openjdk.java.net/contribute/
 >
 > and you would seem to be up to step 2. :)
 >
 > Cheers,
 > David
 >
 >  >
 >  >
 >  >
 >  >
 >  > [1]https://bugs.openjdk.java.net/browse/JDK-8230557
 >  >
 >  > [2]http://openjdk.java.net/contribute/
 >  > [3]https://www.oracle.com/technetwork/community/oca-486395.html
 >  >
 >  >
 >  >
 >  >
 >  >
 >  > --原始邮件--
 >  > 发件人:"Bug Report
 > Notification"  > 发送时间:2019年9月5日(星期四) 凌晨3:33
 >  > 收件人:"未来阳光"<2217232...@qq.com;
 >  >
 >  > 主题:Update Notification: Bug Report  - JDK-8230557
 >  >
 >  >
 >  >
 >  >
 >    [This is an automated response. Please 
do not

 > reply.]
 >  > Dear Java Developer,
 >  > We have finished evaluating your report and have assigned it a Bug
 > ID: JDK-8230557. The issue is visible on bugs.java.com at the 
following

 > url JDK-8230557.
 >  >   To provide more information about this issue,
 > click  here.
 >  >

Re: 回复：回复： what to do next to fix JDK-8230557. thanks

2019-09-05 Thread David Holmes


Hi,

On 5/09/2019 4:11 pm, 未来阳光 wrote:


Thanks very much.

*BUG-LINK:* https://bugs.openjdk.java.net/browse/JDK-8230557

*Describe: *
the return type of the method BitSet#size is int, so the method may 
return a negative value in some case, for example, will return -2147483648.

```
BitSet bitSet = new BitSet(Integer.MAX_VALUE);
for (int i = 0; i < Integer.MAX_VALUE - 1000; i++) {
 bitSet.set(i);
}
System.out.println(bitSet.size());
```
EXPECTED: 2147483648, but ACTUAL: -2147483648.

*FIX*
I have put the patch in the attachment of the mail.


In case the attachment got stripped form the mailing list the proposed 
fix is:


-public int size() {
-return words.length * BITS_PER_WORD;
+public long size() {
+return (long) words.length * BITS_PER_WORD;

Unfortunately this simple fix it not possible. You can't just change the 
return type of the method to long as that is a source-incompatible 
change and would not be approved [1]. Instead the return value should be 
capped at Integer.MAX_VALUE - but I'll leave that for someone from 
core-libs team to pick up. Also look at the evaluation in:


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

Cheers,
David

[1] https://wiki.openjdk.java.net/display/csr/CSR+FAQs





-- 原始邮件 --
*发件人:* "David Holmes";
*发送时间:* 2019年9月5日(星期四) 下午2:00
*收件人:* "未来阳光"<2217232...@qq.com>;"core-libs-dev"d...@openjdk.java.net>;

*主题:* Re: 回复： what to do next to fix JDK-8230557. thanks

On 5/09/2019 3:46 pm, 未来阳光 wrote:
 >
 > hi, developers.
 >
 > Can someone help me? thanks very much !!

Help you how exactly. As I stated your are up to step 2 of the how to
contribute process. If you have a suggested fix for the bug then put
that in an email as described.

Thanks,
David

 >
 > -- 原始邮件 --
 > *发件人:* "David Holmes";
 > *发送时间:* 2019年9月5日(星期四) 中午1:44
 > *收件人:* "未来阳光"<2217232...@qq.com>;"core-libs-dev" d...@openjdk.java.net>;
 > *主题:* Re: what to do next to fix JDK-8230557. thanks
 >
 > On 5/09/2019 3:35 pm, 未来阳光 wrote:
 >  > Hi, leaders.
 >
 > Hi,
 >
 > No "leaders" here only developers :)
 >
 >  >
 >  > A few days ago, I report a bug in core lib[1]. According to the
 > contribute document[2], I had send oca to oracle andmy name has
 > been listed onoca[3].
 >
 > Welcome aboard!
 >
 >  > But I still can't push my changes to jdk, can someone tell me how to
 > do next? thanks very match!!
 >
 > You can't push anything until you become a Committer and before that you
 > have to become an Author. The steps for contributing are outlined here:
 >
 > http://openjdk.java.net/contribute/
 >
 > and you would seem to be up to step 2. :)
 >
 > Cheers,
 > David
 >
 >  >
 >  >
 >  >
 >  >
 >  > [1]https://bugs.openjdk.java.net/browse/JDK-8230557
 >  >
 >  > [2]http://openjdk.java.net/contribute/
 >  > [3]https://www.oracle.com/technetwork/community/oca-486395.html
 >  >
 >  >
 >  >
 >  >
 >  >
 >  > --原始邮件--
 >  > 发件人:"Bug Report
 > Notification"  > 发送时间:2019年9月5日(星期四) 凌晨3:33
 >  > 收件人:"未来阳光"<2217232...@qq.com;
 >  >
 >  > 主题:Update Notification: Bug Report  - JDK-8230557
 >  >
 >  >
 >  >
 >  >
 >    [This is an automated response. Please do not
 > reply.]
 >  > Dear Java Developer,
 >  > We have finished evaluating your report and have assigned it a Bug
 > ID: JDK-8230557. The issue is visible on bugs.java.com at the following
 > url JDK-8230557.
 >  >   To provide more information about this issue,
 > click  here.
 >  >  We work to resolve the issues that are submitted to
 > us according to their impact to the community as a whole, and make no
 > promises as to the time or release in which a bug might be fixed. If
 > this issue has a significant impact on your project you may want to
 > consider using one of the technical support offerings available at
 > Oracle Support.
 >  >   Regards,
 >  >   Java Developer Support
 >  >
 >  >
 >  >
 >  >
 >  >
 >  > Java SE
 >  >   Java SE Documentation
 >  >   Java SE Downloads
 >  >   Java Developer Forums
 >  >   Oracle Java SE Advanced
 >  >   Bug Database
 >  >
 >      Copyright © Oracle and/or
 > its affiliates. All rights reserved.
 >  >
 >    Terms of Use | Privacy
 >  >

回复：回复： what to do next to fix JDK-8230557. thanks

2019-09-05 Thread 未来阳光

Thanks very much.


BUG-LINK:https://bugs.openjdk.java.net/browse/JDK-8230557


Describe:
the return type of the method BitSet#size is int, so the method may return a 
negative value in some case, for example, will return -2147483648.
```
BitSet bitSet = new BitSet(Integer.MAX_VALUE);
for (int i = 0; i < Integer.MAX_VALUE - 1000; i++) {
bitSet.set(i);
}
System.out.println(bitSet.size());
```
EXPECTED:2147483648, butACTUAL:-2147483648.


FIX
I have put the patch in the attachment of the mail.





--原始邮件--
发件人:"David Holmes"http://openjdk.java.net/contribute/
 
 and you would seem to be up to step 2. :)
 
 Cheers,
 David
 
 
 
 
 
  [1]https://bugs.openjdk.java.net/browse/JDK-8230557
 
  [2]http://openjdk.java.net/contribute/
  [3]https://www.oracle.com/technetwork/community/oca-486395.html
 
 
 
 
 
  --nbsp;原始邮件nbsp;--
  发件人:nbsp;"Bug Report 
 Notification"

Re: 回复： what to do next to fix JDK-8230557. thanks

2019-09-05 Thread David Holmes


On 5/09/2019 3:46 pm, 未来阳光 wrote:


hi, developers.

Can someone help me? thanks very much !!


Help you how exactly. As I stated your are up to step 2 of the how to 
contribute process. If you have a suggested fix for the bug then put 
that in an email as described.


Thanks,
David



-- 原始邮件 --
*发件人:* "David Holmes";
*发送时间:* 2019年9月5日(星期四) 中午1:44
*收件人:* "未来阳光"<2217232...@qq.com>;"core-libs-dev"d...@openjdk.java.net>;

*主题:* Re: what to do next to fix JDK-8230557. thanks

On 5/09/2019 3:35 pm, 未来阳光 wrote:
 > Hi, leaders.

Hi,

No "leaders" here only developers :)

 >
 > A few days ago, I report a bug in core lib[1]. According to the 
contribute document[2], I had send oca to oracle andmy name has 
been listed onoca[3].


Welcome aboard!

 > But I still can't push my changes to jdk, can someone tell me how to 
do next? thanks very match!!


You can't push anything until you become a Committer and before that you
have to become an Author. The steps for contributing are outlined here:

http://openjdk.java.net/contribute/

and you would seem to be up to step 2. :)

Cheers,
David

 >
 >
 >
 >
 > [1]https://bugs.openjdk.java.net/browse/JDK-8230557
 >
 > [2]http://openjdk.java.net/contribute/
 > [3]https://www.oracle.com/technetwork/community/oca-486395.html
 >
 >
 >
 >
 >
 > --原始邮件--
 > 发件人:"Bug Report 
Notification"
 > 发送时间:2019年9月5日(星期四) 凌晨3:33
 > 收件人:"未来阳光"<2217232...@qq.com;
 >
 > 主题:Update Notification: Bug Report  - JDK-8230557
 >
 >
 >
 > 
   [This is an automated response. Please do not 
reply.]

 > Dear Java Developer,
 > We have finished evaluating your report and have assigned it a Bug 
ID: JDK-8230557. The issue is visible on bugs.java.com at the following 
url JDK-8230557.
 >   To provide more information about this issue, 
click  here.
 >  We work to resolve the issues that are submitted to 
us according to their impact to the community as a whole, and make no 
promises as to the time or release in which a bug might be fixed. If 
this issue has a significant impact on your project you may want to 
consider using one of the technical support offerings available at   
Oracle Support.

 >   Regards,
 >   Java Developer Support
 >
 >
 >
 >
 >
 > Java SE
 >   Java SE Documentation
 >   Java SE Downloads
 >   Java Developer Forums
 >   Oracle Java SE Advanced
 >   Bug Database
 > 
     Copyright © Oracle and/or 
its affiliates. All rights reserved.
 >  
   Terms of Use | Privacy

 >

43 matches

Mail list logo