March 2020 - core-libs-dev

Method references in annotations
by Julian Ruppel 04 Apr '24

04 Apr '24

Hi, being aware that there has been a discussion quite some time ago ( http://mail.openjdk.java.net/pipermail/lambda-dev/2011-August/003768.html) I wanted to come up again with this topic as this feature would be beneficial especially for framework developers who wants to appoint behaviour to annotations. The following snippet shows what I want to vote for. @MyAnnotation(method = MyClass::myMethod)private String myVariable; This would be far more type-safe compared to using string literals to find the method via reflection. - Julian

2 2

Re: [jpackage] Issue with upgrading from javapackager to jpackage on Windows
by James Elliott 29 Oct '20

29 Oct '20

Sorry, tried sending this with the incorrect email personality first. On Mon, 17 Feb 2020 15:32:14 +0100 Daniel Peintner <daniel.peintner(a)gmail.com <mailto:daniel.peintner@gmail.com>> wrote: > The only *working* solution we found was to *manually* uninstall the old > application before starting msi installer. > Any future update with msi installers work fine. But the first update fails! I had been previously manually creating WiX installers (with the help of a script built by one of my Windows-based users), and he discovered the same problem when I migrated to jpackage. It turns out that the jpackage-built WiX installers are missing an upgrade ID. I mentioned that on this list a few weeks ago, but there was not any interest in investigating the issue. So now I use jpackage to just create the application image, and then separately use WiX directly to create an installer that can properly upgrade older installations. You can find the GitHub Actions workflow that drives the process here: https://github.com/Deep-Symmetry/beat-link-trigger/blob/master/.github/work… <https://github.com/Deep-Symmetry/beat-link-trigger/blob/master/.github/work…> The PowerShell script that runs jpackage and WiX is here: https://github.com/Deep-Symmetry/beat-link-trigger/blob/master/.github/scri… <https://github.com/Deep-Symmetry/beat-link-trigger/blob/master/.github/scri…> And the WIX template it uses is here: https://github.com/Deep-Symmetry/beat-link-trigger/blob/master/.github/reso… <https://github.com/Deep-Symmetry/beat-link-trigger/blob/master/.github/reso…> Hopefully this can help you, and perhaps jpackage can be updated to do this on its own as well. Cheers, -James

4 8

RFR 8214761 : Bug in parallel Kahan summation implementation
by Ivan Gerasimov 03 Sep '20

03 Sep '20

Hello! DoubleSummaryStatistics takes advantage of Kahan summation algorithm to reduce the error of the total sum. Internally it maintains a field double sumCompensation, which keeps lower bits (which were rounded off) of the last addition. Note, that the compensation has to be subtracted from the result to add those bits back: 166 private void sumWithCompensation(double value) { 167 double tmp = value - sumCompensation; 168 double velvel = sum + tmp; // Little wolf of rounding error 169 sumCompensation = (velvel - sum) - tmp; 170 sum = velvel; 171 } At the line 169, tmp normally has more lower bits than (velvel - sum). However, when two DoubleSummaryStatistics objects are combined, this compensation part is *added* to the total, which may result in a less accurate result. The same bug is replicated in DoubleStreams. Would you please help review the fix? BUGURL: https://bugs.openjdk.java.net/browse/JDK-8214761 WEBREV: http://cr.openjdk.java.net/~igerasim/8214761/00/webrev/ -- With kind regards, Ivan Gerasimov

3 4

RFR: JDK-8236138: Add tests for jmod applications
by Alexey Semenyuk 10 Aug '20

10 Aug '20

Please review fix [2] for jpackage bug [1]. The fix updates jtreg tests by adding test coverage for scenarios when jpackage is used for packaging applications bundled in .jmod files. - Alexey [1] https://bugs.openjdk.java.net/browse/JDK-8236138 [2] http://cr.openjdk.java.net/~asemenyuk/8236138/webrev.00

8 104

[15] 8235792: LineNumberReader.getLineNumber() behavior is inconsistent with respect to EOF
by Brian Burkhalter 20 Jul '20

20 Jul '20

This issue [1] attempts to reinstate the change for [2] which was backed out to fix [3] thereby maintaining longstanding behavior. The proposed change [4] modifies the specification to increment the line number also when EOF is encountered and is not immediately preceded by a line terminator. If this all seems reasonable then a CSR will be filed after the code review. One thing I am uncertain about is the change to skip() at lines 293-295 of LineNumberReader. I am not sure that those lines should be added. Thanks, Brian [1] https://bugs.openjdk.java.net/browse/JDK-8235792 [2] https://bugs.openjdk.java.net/browse/JDK-8230342 [3] https://bugs.openjdk.java.net/browse/JDK-8235668 [4] http://cr.openjdk.java.net/~bpb/8235792/webrev.00/

3 12

Re: 8202469 / 8202473: Correct type annotation resolution for class type variables
by Joel Borggrén-Franck 07 Jun '20

07 Jun '20

Hi Rafael, Thanks for reaching out and reminding me of this! I managed to find some time to look into 8202469 during the weekend. By swapping place of the TVars in the test I managed to isolate this without depending on 8202473, take a look at my hacky copy-paste update to your test at http://cr.openjdk.java.net/~jfranck/8202469/ . The comment on lines 269-276 needs to be updated. Perhaps include a table with the start index depending of the type? Also referencing JVMLS 4.4 for the structure of the bound might be instructive for future readers. My current understanding is that there are two problems with the code on (the original) lines 277-287: 1) Type Variables should have index 0 while getting index 1 due to lines 279-280. 2) Bounds that are instances of ParameterizedType always gets index 1 but if the first bound represents a Class type the index should be 0. Does this make sense? Can you make the if-switches positive? I find my original code with the negative tests hard to read and the update doesn't help. Also can you expand the test to cover the different kinds of bounds mentioned in 4.4 and also test Type Variables on methods, I suspect javac treats method (and constructor) tvars similarly but there have been bugs ... TL;DR please update the webrev with: - Split out test and fix for 8202469 - More test coverage of different kinds of bounds - Test for method tvars - See if you can rework the logic to have (mostly) positive tests in if switch Thanks again for looking into this, I'll start looking into 8202473, I think the fix for that one opens up a bigger rework of the code which is why I want to separate the two. cheers /Joel On Sun, Aug 4, 2019 at 10:12 PM Rafael Winterhalter <rafael.wth(a)gmail.com> wrote: > Hi, > > appologies for the delay, I only managed to get my patched up to webrev > today (http://cr.openjdk.java.net/~winterhalter/) It's three patches in > total now, for the third one I could not add a test case, it would require > to compile an annotation property against an enumeration type and load it > against another class where the enumeration was turned into an annotation. > I struggled a bit with jtreg to make it work but I cannot see myself > complete this in a meaningful amount of time. However, I hope that the > patch and the error it solves are straightforward to see. > > I only submitted a patch for 8202469. 8202473 is solved by it. It's two > bugs but they are a symptom of the same oversight. > > I hope this helps you to review it but let me know if you have any > questions or if I should further adjust my patch. > > Best regards, Rafael > > Am Fr., 2. Aug. 2019 um 12:18 Uhr schrieb Rafael Winterhalter < > rafael.wth(a)gmail.com>: > >> Thanks Daniel, >> I did some work on Glassfish a bunch of years ago, I had no idea. >> I will try to split up the changes (I fixed another bug in annotation >> processing yesterday) and upload everything there. >> Best regards, Rafael >> >> Am Fr., 2. Aug. 2019 um 11:59 Uhr schrieb Daniel Fuchs < >> daniel.fuchs(a)oracle.com>: >> >>> Hi Rafael, >>> >>> On 02/08/2019 09:00, Joel Borggrén-Franck wrote: >>> > I can host webrevs for you on cr to make it easier for other >>> reviewers, if >>> > you also send me the patches or webrefs off-list to get around the >>> > attachment stripping I can upload them to cr. >>> >>> I see that you are a JDK project author, so you already own a page >>> on cr (http://cr.openjdk.java.net/~winterhalter/) where you can >>> upload webrevs (e.g. using `scp` as in: >>> $ scp -r webrev-NNNNN.01 winterhalter(a)cr.openjdk.java.net: ) >>> >>> best regards and welcome! >>> >>> -- daniel >>> >>

2 6

6202130: Need to handle UTF-8 values and break up lines longer than 72 bytes
by Philipp Kunz 18 May '20

18 May '20

Hi, The specification says, a line break in a manifest can occur beforeor after a Unicode character encoded in UTF-8. > ...> value: SPACE *otherchar newline *continuation> continuation: SPACE *otherchar newline> ...> otherchar: any UTF-8 character except NUL, CR and LF The current implementation breaks manifest lines at 72 bytes regardless ofhow the bytes around the break are part of a sequence of bytes encoding acharacter. Code points may use up to four bytes when encoded in UTF-8.Manifests with line breaks inside of sequences of bytes encoding Unicodecharacters in UTF-8 with more than one bytes not only are invalid UTF-8but also look ugly in text editors.For example, a manifest could look like this: import java.util.jar.Manifest;import java.util.jar.Attributes;import static java.util.jar.Attributes.Name; public class CharacterBrokenDemo1 { public static void main(String[] args) throws Exception{ Manifest mf = new Manifest(); Attributes attrs = mf.getMainAttributes(); attrs.put(Name.MANIFEST_VERSION, "1.0"); attrs.put(new Name("Some-Key"), "Some languages have decorated characters, " + "for example: español"); // or "espa\u00D1ol" mf.write(System.out); }} Above code produces a result as follows with some unexpected question markswhere the encoding is invalid: > Manifest-Version: 1.0> Some-Key: Some languages have decorated characters, for example: espa?> ?ol This is of course an example written with actual question marks to get a validtext for this message. The trick here is that "Some-Key" to "example :espa"amounts to exactly one byte less encoded in UTF-8 than would fit on one linewith the 72 byte limit so that the subsequent character encoded with two bytesgets broken inside of the sequence of two bytes for this character across acontinuation line break. When decoding the resulting bytes from UTF-8 as one whole string, the twoquestion marks will not fit together again even if the line break with thecontinuation space is removed. However, Manifest::read removes the continuationline breaks ("\r\n ") before decoding the manifest header value from UTF-8 andhence can reproduce the original value. Characters encoded in UTF-8 can not only span up to four bytes for one codepoint each, there are also combining characters or classes thereof or combiningdiacritical marks or whatever the appropriate term could be, that combine morethan one code point into what is usually experienced and referred to as acharacter. The term character really gets ambiguous at this point. I wouldn't know whatthe specification actually refers to with that term "character". So rather thandiving in too much specification or any sorts of theory, let's look at anotherexample: import java.util.jar.Manifest;import java.util.jar.Attributes;import static java.util.jar.Attributes.Name; public class DemoCharacterBroken2 { public static void main(String[] args) throws Exception{ Manifest mf = new Manifest(); Attributes attrs = mf.getMainAttributes(); attrs.put(Name.MANIFEST_VERSION, "1.0"); attrs.put(new Name("Some-Key"), " ".repeat(53) + "Angstro\u0308m"); mf.write(System.out); }} which produces console output as follows: > Manifest-Version: 1.0> Some- Key: Angstro> ̈m (In case this does not display well, the diaeresis is on the m on the last line) When the whole Manifest is decoded from UTF-8 as one big single string andcontinuation line breaks are not removed until after UTF-8 decoding the wholemanifest, the diaeresis (umlaut, two points above, u0308) apparently kind ofjumps onto the following letter m because somehow it cannot be combined withthe preceding space. The UTF-8 decoder (all of my editors I tried, not onlyEclipse and its console view, also less, gedit, cat and terminal) somehowtries to fix that but the diaeresis may not necessarily jump back on the "o"where it originally belonged by removing the continuation line break ("\r\n ")after UTF-8 decoding has taken place, at least that did not work for me. Hence, ideally combining diacritical marks should better not be separated fromwhatever they combine with when breaking manifest lines onto a continuationline. Such combinations, however, seem to be unlimited in terms of number ofcode points combining into the same "experienced" character. I was able tofind combinations that not only exceed the limit of 72 bytes per line but alsoexceed the line buffer size of 512 bytes in Manifest::read. These may be ratheruncommon but still possible to my own surprise. Next consideration would then be to remove that limit of 512 bytes per manifestline but exceeding it would make such manifests incompatible with previousManifest::read implementations and is not really an immediately availableoption at the moment. As a compromise, those characters including combining diacritical marks whichcombine only so many code points as that their binarily encoded form in UTF-8remains within a limit of 71 bytes can be written without an interruptingcontinuation line break, which applies to most cases, but not all. I guess thisshould suit practically and realistically to be expected values well. Another possibility would be to allow for characters that are combinations ofmultiple Unicode code points to be kept together in their encoded form in UTF-8up to 512 bytes line length limit when reading minus a space and a line breakamounting to 509 bytes, but that would still not make manifests be representedas valid Unicode in all corner cases and I guess would not probably make a realimprovement in practice over a limit of 71 bytes. Attached is a patch that tries to implement what was described above using aBreakIterator. While it works from a functional point of view, this might beless desirable performance-wise. Alternatively could be considered to do withoutthe BreakIterator and only keep Unicode code points together by not placingline breaks before a continuation byte, which however would not addresscombining diacritical marks as in the second example above. The jar file specification does not explicitly state that manifest should bevalid UTF-8, and they were not always, but it also does not state otherwise,leaving an impression that manifests could be (mis)taken for UTF-8 encodedstrings, which they also are in many or most cases and which has been confusedmany times. At the moment, the only case where a valid manifest is not also avalid UTF-8 encoded string is when a sequence of bytes encoding the samecharacter happens to be interrupted with a continuation line break. To the bestof my knowledge, all other valid manifests are also valid UTF-8 encoded strings. It would be nice, if manifests could be viewed and manipulated with all Unicodecapable editors and not only be parsed correctly with Manifest::read. Any opinions? Would someone sponsor this patch? Regards,Philipp https://docs.oracle.com/en/java/javase/13/docs/specs/jar/jar.html#specifica…

4 16

Test Unicode support for "SplashScreen-Image" manifest attribute and potential clean up of some c code
by Philipp Kunz 17 May '20

17 May '20

Hi, When I tried to improve Unicode support in JAR manifests in [1], independent of what will happen with that, I found that there are not only Manifest and Attributes classes parsing manifests but also some c code which parses "SplashScreen-Image" attribute and also used to parse some other attributes such as "Main-Class" and others. There already are tests for splash screen images but those existing ones work with the "-splash" command line option and not with the "SplashScreen-Image" manifest attribute. I found "SplashScreen-Image" manifest attribute not yet covered with a test and extended the existing UnicodeTest accordingly, see attached patch which confirmed that the "SplashScreen-Image" manifest attribute already fully supports Unicode. Support for "JRE-Version" manifest attribute and "-jre-restrict-search" and "-jre-no-restrict-search" command line attributes has already been removed earlier already and the relevant lines of code determining the main class from the manifest when launching have already been moved to or near LauncherHelper::getMainClassFromJar in earlier versions, apparently leaving them with no use any longer in java.c, java.h, manifest_info.h, and parse_manifest.c, I figure. Hence, I propose to remove those parts as in the attached patch. This leaves manifest_info.h and parse_manifest.c with "SplashScreen- Image" as the only attribute parsed there. Certainly it would be a different change but anyway it might be worth a consideration to move the code opening the splash screen image to LauncherHelper or a similar appropriate place in Java which would allow to remove quite a number of some lines of c code, provided it could be acceptable to show the splash screen image slightly later. There is no existing related bug and I didn't find a new one. It would be nice to have "SplashScreen-Image" manifest attribute covered with a test and there is some potential for cleaning up unused code which certainly is not urgent at all and I would not know how desirable this really is. Also I'm not sure whether it's better or not to add SPLASHSCREEN_IMAGE to Attributes.Name.KNOWN_NAMES. Any opinion about to how to proceed with this, if at all or would someone sponsor this patch? Regards, Philipp [1] https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-December/064149.…

2 2

Sponsor Request: 8241100: Make Boolean, Character, Byte, and Short implement Constable
by Jorn Vernee 01 May '20

01 May '20

Hi, Can someone please sponsor this patch that makes Boolean, Character, Byte, and Short implement Constable? Bug: https://bugs.openjdk.java.net/browse/JDK-8241100 Webrev: http://cr.openjdk.java.net/~jvernee/8241100/webrev.00/ Having the other box types implement Constable makes them easier to use with APIs that accept any Constable. Though I'm mostly interesting in boolean, for which I'm currently falling back to "true" & "false" strings, but the downside is that this also requires parsing the string again and having to deal with random other strings. This patch also adds the ConstantBootstraps::convert method that is used to facilitate the conversion from int to (short|char|byte). This currently takes a source type explicitly. In practice, it seems that Object can always be used as a source type for the same behavior, but explicitly specifying source and destination types seems more robust to me in case this behavior ever changes, or we want to expand on the supported kinds of conversion. (for instance it is currently not possible to convert from an int to a Long directly, or from Short to Integer, but maybe those cases could be supported in the future as well). Testing: tier1-3 & downstream testing for my particular use case Thanks, Jorn

5 11

RFR(S): 8241638: launcher time metrics alway report 1 on Linux when _JAVA_LAUNCHER_DEBUG set
by linzang(臧琳) 26 Apr '20

26 Apr '20

Dear All, May I ask your help to review this tiny fix? Bug: https://bugs.openjdk.java.net/browse/JDK-8241638 Webrev: http://cr.openjdk.java.net/~lzang/8241638/webrev01/ Thanks! BRs, Lin

5 36