Re: RFR: 8258117: jar tool sets the time stamp of module-info.class entries to the current time [v2]

[Jaikiran Pai]

On Fri, 17 Sep 2021 12:54:07 GMT, Jaikiran Pai  wrote:

>> The commit here is a potential fix for the issue noted in 
>> https://bugs.openjdk.java.net/browse/JDK-8258117.
>> 
>> The change here repurposes an existing internal interface `ModuleInfoEntry` 
>> to keep track of the last modified timestamp of a `module-info.class` 
>> descriptor.
>> 
>> This commit uses the timestamp of the `module-info.class` on the filesystem 
>> to set the time on the `JarEntry`. There are a couple of cases to consider 
>> here:
>> 
>> 1. When creating a jar  (using `--create`), we use the source 
>> `module-info.class` from the filesystem and then add extended info to it 
>> (attributes like packages, module version etc...). In such cases, this patch 
>> will use the lastmodified timestamp from the filesystem of 
>> `module-info.class` even though we might end up updating/extending/modifying 
>> (for example by adding a module version) its content while storing it as a 
>> `JarEntry`. 
>> 
>> 2. When updating a jar (using `--update`), this patch will use the 
>> lastmodified timestamp of `module-info.class` either from the filesystem or 
>> from the source jar's entry (depending on whether a new `module-info.class` 
>> is being passed to the command). Here too, it's possible that we might end 
>> up changing/modifying/extending the `module-info.class` (for example, 
>> changing the module version to a new version) that gets written into the 
>> updated jar file, but this patch _won't_ use `System.currentTimeMillis()` 
>> even in such cases.
>> 
>> If we do have to track actual changes that might happen to 
>> `module-info.class` while extending its info (in `extendedInfoBytes()`) and 
>> then decide whether to use current system time as last modified time, then 
>> this will require a bigger change and also a discussion on what kind of 
>> extending of module-info.class content will require a change to the 
>> lastmodifiedtime of that entry.
>
> Jaikiran Pai has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains two additional 
> commits since the last revision:
> 
>  - Merge latest from master branch
>  - 8258117: jar tool sets the time stamp of module-info.class entries to the 
> current time

Any reviews for this one, please?

-

PR: https://git.openjdk.java.net/jdk/pull/5486

Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server [v3]

[Jaikiran Pai]

On Wed, 22 Sep 2021 15:20:21 GMT, Julia Boes  wrote:

> > Thanks for sharing your experience on this, it's appreciated. 0.0.0.0 is 
> > common default for Apache httpd [1], Ngnix [2], the Python web server [3]. 
> > This being said, I want to make sure we're taking the right decision here 
> > so let me seek some further advice on this.
> > [1] http://httpd.apache.org/docs/2.4/bind.html
> > [2] https://docs.nginx.com/nginx/admin-guide/web-server/web-server/
> > [3] https://github.com/python/cpython/blob/3.4/Lib/http/server.py
> 
> Further review concluded that a default binding to 0.0.0.0 creates too a high 
> level of exposure, particularly for a low-threshold utility like this server. 
> Binding to an unrestricted address is a known way for attackers to launch a 
> Denial-of-Service attack, classified by MITRE as CWE-1327 [1]. We therefore 
> update the default binding to the loopback address and amend the help output 
> with information on how to bind to 0.0.0.0

Thank you Julia for considering this input and coordinating the change.

-

PR: https://git.openjdk.java.net/jdk/pull/5505

Re: RFR: 8273656: Improve java.lang.invoke.MethodType.parameterList() and its usage

[Claes Redestad]

On Mon, 13 Sep 2021 11:06:15 GMT, Сергей Цыпанов 
 wrote:

> Currently the method is implemented like
> 
> public List> parameterList() {
>   return Collections.unmodifiableList(Arrays.asList(ptypes.clone()));
> }
> 
> This seems to be excessive, as three objects are allocated here. Instead we 
> can use `List.of(ptypes)` which doesn't allocate anything for empty array and 
> for one of length 1 and 2 it allocates lightweight objects with 2 fields, 
> still copying longer arrays. This is likely to be fruitful as most of methods 
> have 0-2 parameters.
> 
> Also there is a couple of cases when `MethodType.parameterLis()` is called to 
> get its size, which is excessive either as we can use 
> `MethodType.parameterCount()` instead.

I think it's OK and even expected to file a CSRs retroactively when you realize 
post integration that there's a behavior change. I recall doing so at least 
once in the past.

-

PR: https://git.openjdk.java.net/jdk/pull/5489

Re: RFR: 8236505: Mark jdk/editpad/EditPadTest.java as @headful

[Phil Race]

On Thu, 16 Sep 2021 09:13:15 GMT, Aleksey Shipilev  wrote:

> This is a GUI test, and it should be `@headful`.
> 
> Additional testing:
>  - [x] Test still runs in default, and does not run with `!headful`

Well .. since our test framework doesn't run whatever test group this is in on 
headful systems, what this change has effectively done is say "don't run this 
test anymore. ever".
You'd need to add it to the desktop test group.

-

PR: https://git.openjdk.java.net/jdk/pull/5544

Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server [v6]

[Daniel Fuchs]

On Wed, 22 Sep 2021 15:26:33 GMT, Julia Boes  wrote:

>> This change implements a simple web server that can be run on the 
>> command-line with `java -m jdk.httpserver`.
>> 
>> This is facilitated by adding an entry point for the `jdk.httpserver` 
>> module, an implementation class whose main method is run when the above 
>> command is executed. This is the first such module entry point in the JDK.
>> 
>> The server is a minimal HTTP server that serves the static files of a given 
>> directory, similar to existing alternatives on other platforms and 
>> convenient for testing, development, and debugging.
>> 
>> Additionally, a small API is introduced for programmatic creation and 
>> customization.
>> 
>> Testing: tier1-3.
>
> Julia Boes has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - change default bind address from anylocal to loopback
>  - address PR comments

When the -b option is not explicitly specified on the command line it would be 
good to print a message that says that the server is bound to the loopback by 
default, and print the command line that would be needed to bind to all 
interfaces instead (or instruct the user to call `java -m jdk.httpserver 
--help` to learn how to bind to all interfaces). I don't think your latest 
changes include that.

src/jdk.httpserver/share/classes/module-info.java line 55:

> 53:  *  [-o none|info|verbose] [-h to show 
> options]
> 54:  *Options:
> 55:  *-b, --bind-address- Address to bind to. Default: 127.0.0.1 
> (loopback).

This assumes that the machine on which the server is run has IPv4 configured. 
It might not be the case. It can also depend on whether 
-Djdk.net.preferIPv6Addresses=true is specified on the java command line. Maybe 
this should be: `127.0.0.1 (or ::1), (loopback).`

src/jdk.httpserver/share/classes/sun/net/httpserver/simpleserver/SimpleFileServerImpl.java
 line 52:

> 50:  *
> 51:  *  Unless specified as arguments, the default values are:
> 52:  * bind address: 127.0.0.1 (loopback)

maybe this should say: `127.0.01 (or ::1), (loopback)`

src/jdk.httpserver/share/classes/sun/net/httpserver/simpleserver/resources/simpleserver.properties
 line 32:

> 30: options=\
> 31: Options:\n\
> 32: -b, --bind-address- Address to bind to. Default: 127.0.0.1 
> (loopback).\n\

This assumes that the machine on which the server is run has IPv4 configured. 
It might not be the case. It can also depend on whether 
-Djdk.net.preferIPv6Addresses=true is specified on the java command line. The 
actual address of the loopback (as returned by 
InetAddress.getLoopackeAddress()) should therefore preferably be passed as 
parameter to any message that talks about the loopback. It is not such an issue 
for the wildcard - because AFAIU there's no difference between 0.0.0.0 and ::0 
at the OS level

src/jdk.httpserver/share/classes/sun/net/httpserver/simpleserver/resources/simpleserver.properties
 line 42:

> 40: opt.bindaddress=\
> 41: \-b, --bind-address- Address to bind to. Default: 127.0.0.1 
> (loopback).\n\
> 42: \ For 0.0.0.0 (all interfaces) use -b 0.0.0.0 or 
> -b ::0.

is `opt.bindaddress` used somewhere? I couldn't find it.
Same for the other `opt.*` properties below.

-

PR: https://git.openjdk.java.net/jdk/pull/5505

Re: RFR: 8269850: Most JDK releases report macOS version 12 as 10.16 instead of 12.0 [v2]

[Naoto Sato]

On Wed, 22 Sep 2021 17:41:18 GMT, Roger Riggs  wrote:

>> The Mac OS specific code to determine the os.version property in 
>> java_props_macosx.c is updated
>> to replace the code extracting the version from the SystemVersion.plist by 
>> reading the version using t\
>> he hidden link:
>
> Roger Riggs has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Check for missing version from SystemVersion plist.
>   Refactor avoid repetition of conversion of NSString to char *.

Marked as reviewed by naoto (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/5633

Re: RFR: 8274075: Fix miscellaneous typos in java.base [v4]

[Iris Clark]

On Wed, 22 Sep 2021 15:38:33 GMT, Pavel Rappo  wrote:

>> 8274075: Fix miscellaneous typos in java.base
>
> Pavel Rappo has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Add missing "the"
>   
>   (Spotted by Brian Burkhalter.)

Marked as reviewed by iris (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/5610

Re: RFR: 8269850: Most JDK releases report macOS version 12 as 10.16 instead of 12.0 [v2]

[Roger Riggs]

> The Mac OS specific code to determine the os.version property in 
> java_props_macosx.c is updated
> to replace the code extracting the version from the SystemVersion.plist by 
> reading the version using t\
> he hidden link:

Roger Riggs has updated the pull request incrementally with one additional 
commit since the last revision:

  Check for missing version from SystemVersion plist.
  Refactor avoid repetition of conversion of NSString to char *.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/5633/files
  - new: https://git.openjdk.java.net/jdk/pull/5633/files/d1e2bd20..f44119d9

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5633&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5633&range=00-01

  Stats: 16 lines in 1 file changed: 6 ins; 7 del; 3 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5633.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5633/head:pull/5633

PR: https://git.openjdk.java.net/jdk/pull/5633

Re: RFR: 8269850: Most JDK releases report macOS version 12 as 10.16 instead of 12.0

[Naoto Sato]

On Wed, 22 Sep 2021 15:00:59 GMT, Roger Riggs  wrote:

> The Mac OS specific code to determine the os.version property in 
> java_props_macosx.c is updated
> to replace the code extracting the version from the SystemVersion.plist by 
> reading the version using t\
> he hidden link:

src/java.base/macosx/native/libjava/java_props_macosx.c line 270:

> 268:  
> @"/System/Library/CoreServices/.SystemVersionPlatform.plist"];
> 269: if (version != NULL) {
> 270: NSString *nsVerStr = [version objectForKey : 
> @"ProductVersion"];

Should we check `nsVerStr != NULL` here?

-

PR: https://git.openjdk.java.net/jdk/pull/5633

Re: RFR: 8274075: Fix miscellaneous typos in java.base [v4]

[Brian Burkhalter]

On Wed, 22 Sep 2021 15:38:33 GMT, Pavel Rappo  wrote:

>> 8274075: Fix miscellaneous typos in java.base
>
> Pavel Rappo has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Add missing "the"
>   
>   (Spotted by Brian Burkhalter.)

Marked as reviewed by bpb (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/5610

Integrated: 8271602: Add Math.ceilDiv() family parallel to Math.floorDiv() family

[Raffaello Giulietti]

On Wed, 1 Sep 2021 20:13:38 GMT, Raffaello Giulietti 
 wrote:

> This PR ideally continues #5285, which has been closed as a consequence of 
> inadvertently removing the branch on my repo. See there for initial 
> discussion.
> 
> Sorry for the mess.

This pull request has now been integrated.

Changeset: da38ced3
Author:Raffaello Giulietti 
Committer: Brian Burkhalter 
URL:   
https://git.openjdk.java.net/jdk/commit/da38ced3299cbd02f210bea4130990a43f6104bf
Stats: 1093 lines in 4 files changed: 1040 ins; 0 del; 53 mod

8271602: Add Math.ceilDiv() family parallel to Math.floorDiv() family

Reviewed-by: bpb

-

PR: https://git.openjdk.java.net/jdk/pull/5341

Re: RFR: 8271820: Implementation of JEP 416: Reimplement Core Reflection with Method Handle [v9]

[Mandy Chung]

> This reimplements core reflection with method handles.
> 
> For `Constructor::newInstance` and `Method::invoke`, the new implementation 
> uses `MethodHandle`.  For `Field` accessor, the new implementation uses 
> `VarHandle`.For the first few invocations of one of these reflective 
> methods on a specific reflective object we invoke the corresponding method 
> handle directly. After that we spin a dynamic bytecode stub defined in a 
> hidden class which loads the target `MethodHandle` or `VarHandle` from its 
> class data as a dynamically computed constant. Loading the method handle from 
> a constant allows JIT to inline the method-handle invocation in order to 
> achieve good performance.
> 
> The VM's native reflection methods are needed during early startup, before 
> the method-handle mechanism is initialized. That happens soon after 
> System::initPhase1 and before System::initPhase2, after which we switch to 
> using method handles exclusively.
> 
> The core reflection and method handle implementation are updated to handle 
> chained caller-sensitive method calls [1] properly. A caller-sensitive method 
> can define with a caller-sensitive adapter method that will take an 
> additional caller class parameter and the adapter method will be annotated 
> with `@CallerSensitiveAdapter` for better auditing.   See the detailed 
> description from [2].
> 
> Ran tier1-tier8 tests.   
> 
> [1] https://bugs.openjdk.java.net/browse/JDK-8013527
> [2] 
> https://bugs.openjdk.java.net/browse/JDK-8271820?focusedCommentId=14439430&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14439430

Mandy Chung has updated the pull request with a new target base due to a merge 
or a rebase. The pull request now contains 27 commits:

 - Make MethodAccessor non-volatile.  Remove MHInvoker/VHInvoker.
 - Merge
 - minor cleanup and more test case.
 - Rename InvokerBuilder to ReflectionInvokerBuilder
 - Fix NativeAccessor.c build issue for the renamed classes
 - Shorten the class name
 - Rename the accessor classes to make it explicit for method handles
 - Add a new test for testing methods whose parameter types and return type not 
visible to the caller
 - Move the handling of native accessor to the factory method for 
method/constructor accessor
 - DirectConstructorAccessorImpl should take the MHInvoker parameter
 - ... and 17 more: https://git.openjdk.java.net/jdk/compare/161fdb4a...1e68d004

-

Changes: https://git.openjdk.java.net/jdk/pull/5027/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5027&range=08
  Stats: 6529 lines in 75 files changed: 5990 ins; 327 del; 212 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5027.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5027/head:pull/5027

PR: https://git.openjdk.java.net/jdk/pull/5027

Re: Unused HashMap's in ThaiBuddhistChronology

[Naoto Sato]

I would think these are some left-overs from the 310 project, as I see 
the same fields in 310's Hijrah chronology. These should be cleaned up.


https://github.com/ThreeTen/threetenbp/blob/master/src/main/java/org/threeten/bp/chrono/HijrahChronology.java

BTW, it would be moot once they are gone, but I found a typo:

   ERA_FULL_NAMES.put(FALLBACK_LANGUAGE, new String[]{"Before 
Buddhist", "Budhhist Era"});


"Budhhist" -> "Buddhist"

Naoto


On 9/21/21 2:30 PM, Roger Riggs wrote:

Indeed, they seem unused...

Stephen, do you recall their original use?

Thanks, Roger


On 9/21/21 5:09 PM, Andrey Turbanov wrote:

Hello.
Today I discovered 3 static HashMap's in ThaiBuddhistChronology class:
ERA_NARROW_NAMES, ERA_SHORT_NAMES, ERA_FULL_NAMES

static initializer put some values into them. But their content seems
unused after that.
Do I miss something and they are used somewhere?


Andrey Turbanov

Integrated: 8273924: ArrayIndexOutOfBoundsException thrown in java.util.JapaneseImperialCalendar.add()

[Naoto Sato]

On Tue, 21 Sep 2021 12:47:00 GMT, Naoto Sato  wrote:

> Fixing an AIOOBE on normalizing the month value.

This pull request has now been integrated.

Changeset: d39aad92
Author:Naoto Sato 
URL:   
https://git.openjdk.java.net/jdk/commit/d39aad92308fbc28bd2de164e331062ebf62da85
Stats: 26 lines in 3 files changed: 24 ins; 0 del; 2 mod

8273924: ArrayIndexOutOfBoundsException thrown in 
java.util.JapaneseImperialCalendar.add()

Reviewed-by: rriggs, iris, joehw

-

PR: https://git.openjdk.java.net/jdk/pull/5611

Re: [External] : Re: What causes java.lang.InternalError: platform encoding not initialized ?

[Scott Palmer]

> On Sep 22, 2021, at 11:22 AM, Andy Herrick  wrote:
> 
> I still don't get the error your report.  Is there something else that needs 
> to be set up for using instrumentation ?

Nothing that I’m aware of.  Maybe there is something to do with Visual C/C++ 
runtime libraries that I have installed globally? -just a wild guess.

Ohhh… here’s something…

I had JDK 17 ‘bin’ folder on my PATH (along with many other things)
If I clear the PATH env var with:
 
 set PATH=

Then I get an different error:

Error occurred during initialization of VM
Could not find agent library instrument on the library path, with error: Can’t 
find dependent libraries
Module java.instrument may be missing from runtime image.

All I have to do to et back to the original error is

set PATH=C:\Tools\jdk-17\bin

So this is definitely related to the DLL search path, and it was just lucky 
that some libraries were found via PATH in my environment.

> 
> I do get a different error when I have both runtime and jre directories after 
> "cp -r jre runtime" (in FetchURL/build/application/FetchURL dir)
> 
> As built (before copy):
> 
>> $ ./FetchURL
>> *** java.lang.instrument ASSERTION FAILED ***: "!errorOutstanding" with 
>> message c
>> an't find transform methodID at JPLISAgent.c line: 552
>> *** java.lang.instrument ASSERTION FAILED ***: "result" at JPLISAgent.c 
>> line: 400
>> 
>> FATAL ERROR in native method: processing of -javaagent failed
>> 
> after copy:
> 
>> $ ./FetchURL
>> Exception in thread "main" java.lang.ClassNotFoundException: 
>> io.m3si.utils.ClassL
>> oaderUtils$Agent
>> at 
>> java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClas
>> sLoader.java:636)
>> at 
>> java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(Cl
>> assLoaders.java:182)
>> at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:519)
>> at 
>> java.instrument/sun.instrument.InstrumentationImpl.loadClassAndStartAg
>> ent(InstrumentationImpl.java:433)
>> at 
>> java.instrument/sun.instrument.InstrumentationImpl.loadClassAndCallPre
>> main(InstrumentationImpl.java:527)
>> *** java.lang.instrument ASSERTION FAILED ***: "result" with message agent 
>> load/p
>> remain call failed at 
>> t:\workspace\open\src\java.instrument\share\native\libinstr
>> ument\JPLISAgent.c line: 422
>> FATAL ERROR in native method: processing of -javaagent failed, 
>> processJavaStart f
>> ailed
> 
> but then with jre removed and line in cfg removed I get the same error:
> 
>> $ ./FetchURL
>> Exception in thread "main" java.lang.ClassNotFoundException: 
>> io.m3si.utils.ClassLoaderUt
>> ils$Agent
>> at 
>> java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader
>> .java:636)
>> at 
>> java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoad
>> ers.java:182)
>> at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:519)
>> at 
>> java.instrument/sun.instrument.InstrumentationImpl.loadClassAndStartAgent(Ins
>> trumentationImpl.java:433)
>> at 
>> java.instrument/sun.instrument.InstrumentationImpl.loadClassAndCallPremain(In
>> strumentationImpl.java:527)
>> *** java.lang.instrument ASSERTION FAILED ***: "result" with message agent 
>> load/premain
>> call failed at 
>> t:\workspace\open\src\java.instrument\share\native\libinstrument\JPLISAge
>> nt.c line: 422
>> FATAL ERROR in native method: processing of -javaagent failed, 
>> processJavaStart failed
> 
> note the "t:\workspace" ?  I don't have a t: drive, where is that coming from 
> ?

Must be from how the JDK was built.  I don’t have a T: drive either.

jar -tvf build\application\FetchURL\app\libs\FetchURL-0.0.01.jar
 0 Wed Sep 22 11:16:40 EDT 2021 META-INF/
   248 Wed Sep 22 11:16:40 EDT 2021 META-INF/MANIFEST.MF
 0 Wed Sep 22 11:10:30 EDT 2021 io/
 0 Wed Sep 22 11:10:30 EDT 2021 io/m3si/
 0 Wed Sep 22 11:10:30 EDT 2021 io/m3si/utils/
 0 Wed Sep 22 11:10:30 EDT 2021 io/m3si/utils/fetchurl/
  3988 Wed Sep 22 11:10:30 EDT 2021 io/m3si/utils/fetchurl/Main.class
  1000 Wed Sep 22 11:10:30 EDT 2021 io/m3si/utils/ClassLoaderUtils$Agent.class
  4613 Wed Sep 22 11:10:30 EDT 2021 io/m3si/utils/ClassLoaderUtils.class

I extracted MANIFEST.MF just to be sure …

Manifest-Version: 1.0
Implementation-Title: Kayak Plugins Bootstrap
Premain-Class: io.m3si.utils.ClassLoaderUtils$Agent
Implementation-Version: 1.0.1
Agent-Class: io.m3si.utils.ClassLoaderUtils$Agent
Main-Class: io.m3si.utils.fetchurl.Main

Looks right to me.

Scott

> 
> /Andy
> 
> 
> On 9/22/2021 10:25 AM, Scott Palmer wrote:
>> Sorry That last build with the Oracle OpenJDK needed the build file changed 
>> to have
>> 
>>vendor = JvmVendorSpec.ORACLE
>> 
>> as well as the command line property pointing to the path to the jdk,
>> 
>> Scott
>> 
>> 
>>> On Sep 22, 2021, at 10:24 AM, Scott Palmer  wrote:
>>> 
>>> I reproduced it with the AZUL's distribution of OpenJDK (with JavaFX 
>>> module

Re: RFR: 8274075: Fix miscellaneous typos in java.base [v4]

[Lance Andersen]

On Wed, 22 Sep 2021 15:35:54 GMT, Pavel Rappo  wrote:

>> 8274075: Fix miscellaneous typos in java.base
>
> Pavel Rappo has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Add missing "the"
>   
>   (Spotted by Brian Burkhalter.)

Marked as reviewed by lancea (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/5610

Re: RFR: 8274075: Fix miscellaneous typos in java.base [v4]

[Pavel Rappo]

> 8274075: Fix miscellaneous typos in java.base

Pavel Rappo has updated the pull request incrementally with one additional 
commit since the last revision:

  Add missing "the"
  
  (Spotted by Brian Burkhalter.)

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/5610/files
  - new: https://git.openjdk.java.net/jdk/pull/5610/files/05505d97..fa81cacd

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5610&range=03
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5610&range=02-03

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5610.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5610/head:pull/5610

PR: https://git.openjdk.java.net/jdk/pull/5610

Re: RFR: 8273656: Improve java.lang.invoke.MethodType.parameterList() and its usage

[Jorn Vernee]

On Mon, 13 Sep 2021 11:06:15 GMT, Сергей Цыпанов 
 wrote:

> Currently the method is implemented like
> 
> public List> parameterList() {
>   return Collections.unmodifiableList(Arrays.asList(ptypes.clone()));
> }
> 
> This seems to be excessive, as three objects are allocated here. Instead we 
> can use `List.of(ptypes)` which doesn't allocate anything for empty array and 
> for one of length 1 and 2 it allocates lightweight objects with 2 fields, 
> still copying longer arrays. This is likely to be fruitful as most of methods 
> have 0-2 parameters.
> 
> Also there is a couple of cases when `MethodType.parameterLis()` is called to 
> get its size, which is excessive either as we can use 
> `MethodType.parameterCount()` instead.

The null-hostile-ness of `contains` is not something I thought about. It would 
have been good to mention that in the PR body as well. FWIW, I did a search on 
grep.app for `parameterList()` calls in java [1] and don't really see any calls 
to `contains`.

I think process-wise creating a CSR would be good, and shouldn't be too much 
work. Though, I'm not familiar with the process of filing a CSR after a patch 
has already been merged.

[1] : https://grep.app/search?q=.parameterList%28%29&filter[lang][0]=Java

-

PR: https://git.openjdk.java.net/jdk/pull/5489

RFR: 8269850: Most JDK releases report macOS version 12 as 10.16 instead of 12.0

[Roger Riggs]

The Mac OS specific code to determine the os.version property in 
java_props_macosx.c is updated
to replace the code extracting the version from the SystemVersion.plist by 
reading the version using t\
he hidden link:

-

Commit messages:
 - 8269850: Most JDK releases report macOS version 12 as 10.16 instead of 12.0

Changes: https://git.openjdk.java.net/jdk/pull/5633/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5633&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8269850
  Stats: 11 lines in 1 file changed: 1 ins; 6 del; 4 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5633.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5633/head:pull/5633

PR: https://git.openjdk.java.net/jdk/pull/5633

Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server [v3]

[Julia Boes]

On Fri, 17 Sep 2021 14:11:38 GMT, Julia Boes  wrote:

> Thanks for sharing your experience on this, it's appreciated. 0.0.0.0 is 
> common default for Apache httpd [1], Ngnix [2], the Python web server [3]. 
> This being said, I want to make sure we're taking the right decision here so 
> let me seek some further advice on this.
> 
> [1] http://httpd.apache.org/docs/2.4/bind.html
> [2] https://docs.nginx.com/nginx/admin-guide/web-server/web-server/
> [3] https://github.com/python/cpython/blob/3.4/Lib/http/server.py

Further review concluded that a default binding to 0.0.0.0 creates too a high 
level of exposure, particularly for a low-threshold utility like this server. 
Binding to an unrestricted address is a known way for attackers to launch a 
Denial-of-Service attack,  classified by MITRE as CWE-1327 [1]. We therefore 
update the default binding to the loopback address and amend the help output 
with information on how to bind to 0.0.0.0, e.g.:


$ java -m jdk.httpserver -h
Usage: java -m jdk.httpserver [-b bind address] [-p port] [-d directory]
  [-o none|info|verbose] [-h to show options]
Options:
-b, --bind-address- Address to bind to. Default: 127.0.0.1 (loopback).
For 0.0.0.0 (all interfaces) use -b 0.0.0.0 or -b ::0.
-d, --directory   - Directory to serve. Default: current directory.
-o, --output  - Output format. none|info|verbose. Default: info.
-p, --port- Port to listen on. Default: 8000.
-h, -?, --help- Print this help message.
To stop the server, press Ctrl + C.
```  
Thanks again for flagging this, @jaikiran .

[1] https://cwe.mitre.org/data/definitions/1327.html

-

PR: https://git.openjdk.java.net/jdk/pull/5505

Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server [v6]

[Julia Boes]

> This change implements a simple web server that can be run on the 
> command-line with `java -m jdk.httpserver`.
> 
> This is facilitated by adding an entry point for the `jdk.httpserver` module, 
> an implementation class whose main method is run when the above command is 
> executed. This is the first such module entry point in the JDK.
> 
> The server is a minimal HTTP server that serves the static files of a given 
> directory, similar to existing alternatives on other platforms and convenient 
> for testing, development, and debugging.
> 
> Additionally, a small API is introduced for programmatic creation and 
> customization.
> 
> Testing: tier1-3.

Julia Boes has updated the pull request incrementally with two additional 
commits since the last revision:

 - change default bind address from anylocal to loopback
 - address PR comments

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/5505/files
  - new: https://git.openjdk.java.net/jdk/pull/5505/files/fe059131..639e018a

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5505&range=05
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5505&range=04-05

  Stats: 103 lines in 14 files changed: 22 ins; 38 del; 43 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5505.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5505/head:pull/5505

PR: https://git.openjdk.java.net/jdk/pull/5505

Re: [External] : Re: What causes java.lang.InternalError: platform encoding not initialized ?

[Andy Herrick]

I still don't get the error your report.  Is there something else that 
needs to be set up for using instrumentation ?


I do get a different error when I have both runtime and jre directories 
after "cp -r jre runtime" (in FetchURL/build/application/FetchURL dir)


As built (before copy):


$ ./FetchURL
*** java.lang.instrument ASSERTION FAILED ***: "!errorOutstanding" 
with message c

an't find transform methodID at JPLISAgent.c line: 552
*** java.lang.instrument ASSERTION FAILED ***: "result" at 
JPLISAgent.c line: 400


FATAL ERROR in native method: processing of -javaagent failed


after copy:


$ ./FetchURL
Exception in thread "main" java.lang.ClassNotFoundException: 
io.m3si.utils.ClassL

oaderUtils$Agent
    at 
java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClas

sLoader.java:636)
    at 
java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(Cl

assLoaders.java:182)
    at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:519)
    at 
java.instrument/sun.instrument.InstrumentationImpl.loadClassAndStartAg

ent(InstrumentationImpl.java:433)
    at 
java.instrument/sun.instrument.InstrumentationImpl.loadClassAndCallPre

main(InstrumentationImpl.java:527)
*** java.lang.instrument ASSERTION FAILED ***: "result" with message 
agent load/p
remain call failed at 
t:\workspace\open\src\java.instrument\share\native\libinstr

ument\JPLISAgent.c line: 422
FATAL ERROR in native method: processing of -javaagent failed, 
processJavaStart f

ailed


but then with jre removed and line in cfg removed I get the same error:


$ ./FetchURL
Exception in thread "main" java.lang.ClassNotFoundException: 
io.m3si.utils.ClassLoaderUt

ils$Agent
    at 
java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader

.java:636)
    at 
java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoad

ers.java:182)
    at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:519)
    at 
java.instrument/sun.instrument.InstrumentationImpl.loadClassAndStartAgent(Ins

trumentationImpl.java:433)
    at 
java.instrument/sun.instrument.InstrumentationImpl.loadClassAndCallPremain(In

strumentationImpl.java:527)
*** java.lang.instrument ASSERTION FAILED ***: "result" with message 
agent load/premain
call failed at 
t:\workspace\open\src\java.instrument\share\native\libinstrument\JPLISAge

nt.c line: 422
FATAL ERROR in native method: processing of -javaagent failed, 
processJavaStart failed


note the "t:\workspace" ?  I don't have a t: drive, where is that coming 
from ?


/Andy


On 9/22/2021 10:25 AM, Scott Palmer wrote:

Sorry That last build with the Oracle OpenJDK needed the build file changed to 
have

vendor = JvmVendorSpec.ORACLE

as well as the command line property pointing to the path to the jdk,

Scott



On Sep 22, 2021, at 10:24 AM, Scott Palmer  wrote:

I reproduced it with the AZUL's distribution of OpenJDK (with JavaFX modules 
bundled) and I just changed:

vendor = JvmVendorSpec.ADOPTOPENJDK

And get the same error with that build of OpenJDK as well.  “OpenJDK Runtime 
Environment Temurin-17+35 (build 17+35)”
You can try that, as Gradle should download the JDK for you.

I then downloaded the Oracle build of OpenJDK from 
https://urldefense.com/v3/__https://jdk.java.net/17/__;!!ACWV5N9M2RV99hQ!eG4msXvYMONuZH2PKW1Mddol9UY2VkepPpI36eTZz9c_70fXHLm84-lrbzZBwkTMGA$
 (build 17+35-2724)
unzipped it to C:\Tools
building with:

gradlew -Porg.gradle.java.installations.paths=C:\Tools\jdk-17 clean build

I still reproduce the error.

Though the assertion in native JVM code that you are getting looks like yet 
another bug!  So perhaps it is good that it is discovered as well.


Scott


On Sep 22, 2021, at 9:54 AM, Andy Herrick  wrote:

I can't seem to get your testcase to work with the Oracle JDK configured.

I can build, but then when I run

$ ./build/application/FetchURL/FetchURL.exe

I get:


*** java.lang.instrument ASSERTION FAILED ***: "!errorOutstanding" with message 
c
an't find transform methodID at JPLISAgent.c line: 552
*** java.lang.instrument ASSERTION FAILED ***: "result" at JPLISAgent.c line: 
400

FATAL ERROR in native method: processing of -javaagent failed

same behavior if I restore "runtime" directory and fix FetchURL.cfg to remove 
app.runtime entry.

anyway I used your two source files to build the test app without gradle, and 
the test can download 
https://urldefense.com/v3/__https://linear-89.frequency.stream/dist/localnow/89/hls/master/playlist.m3u8__;!!ACWV5N9M2RV99hQ!eG4msXvYMONuZH2PKW1Mddol9UY2VkepPpI36eTZz9c_70fXHLm84-lrbzbGSGkGog$
  without any problems.

I then moved runtime to jre and added line "app.runtime=$APPDIR/../jre" to cfg 
file and app still ran fine (downloaded the above)

So I still don't have any way to reproduce this problemwith the Oracle jdk.

/Andy


PS:

Although something somewhere else may have changed to counter the problem, it's 
clear from your des

Withdrawn: 8269850: Most JDK releases report macOS version 12 as 10.16 instead of 12.0

[Roger Riggs]

On Tue, 21 Sep 2021 19:33:05 GMT, Roger Riggs  wrote:

> The Mac OS specific code to determine the os.version property in 
> java_props_macosx.c is updated
> to replace the code extracting the version from the SystemVersion.plist by 
> reading the version using the hidden link:
>`/System/Library/CoreServices/.SystemVersionPlatform.plist`
> 
> Its contents are not dependent on the SYSTEM_VERSION_COMPAT environment 
> variable.
> With this change, 11.5 reports the 11.5.1 minor version and os.version 11.6 
> is correctly reported.
> The change can be backported to other versions as needed.

This pull request has been closed without being integrated.

-

PR: https://git.openjdk.java.net/jdk/pull/5619

Re: [External] : Re: What causes java.lang.InternalError: platform encoding not initialized ?

[Scott Palmer]

Sorry That last build with the Oracle OpenJDK needed the build file changed to 
have

   vendor = JvmVendorSpec.ORACLE

as well as the command line property pointing to the path to the jdk,

Scott 


> On Sep 22, 2021, at 10:24 AM, Scott Palmer  wrote:
> 
> I reproduced it with the AZUL's distribution of OpenJDK (with JavaFX modules 
> bundled) and I just changed:
> 
>   vendor = JvmVendorSpec.ADOPTOPENJDK
> 
> And get the same error with that build of OpenJDK as well.  “OpenJDK Runtime 
> Environment Temurin-17+35 (build 17+35)”
> You can try that, as Gradle should download the JDK for you.
> 
> I then downloaded the Oracle build of OpenJDK from https://jdk.java.net/17/   
>  (build 17+35-2724)
> unzipped it to C:\Tools
> building with:
> 
> gradlew -Porg.gradle.java.installations.paths=C:\Tools\jdk-17 clean build
> 
> I still reproduce the error.
> 
> Though the assertion in native JVM code that you are getting looks like yet 
> another bug!  So perhaps it is good that it is discovered as well.
> 
> 
> Scott
> 
>> On Sep 22, 2021, at 9:54 AM, Andy Herrick  wrote:
>> 
>> I can't seem to get your testcase to work with the Oracle JDK configured.
>> 
>> I can build, but then when I run
>> 
>> $ ./build/application/FetchURL/FetchURL.exe
>> 
>> I get:
>> 
>>> *** java.lang.instrument ASSERTION FAILED ***: "!errorOutstanding" with 
>>> message c
>>> an't find transform methodID at JPLISAgent.c line: 552
>>> *** java.lang.instrument ASSERTION FAILED ***: "result" at JPLISAgent.c 
>>> line: 400
>>> 
>>> FATAL ERROR in native method: processing of -javaagent failed
>> same behavior if I restore "runtime" directory and fix FetchURL.cfg to 
>> remove app.runtime entry.
>> 
>> anyway I used your two source files to build the test app without gradle, 
>> and the test can download 
>> https://linear-89.frequency.stream/dist/localnow/89/hls/master/playlist.m3u8 
>> without any problems.
>> 
>> I then moved runtime to jre and added line "app.runtime=$APPDIR/../jre" to 
>> cfg file and app still ran fine (downloaded the above)
>> 
>> So I still don't have any way to reproduce this problemwith the Oracle jdk.
>> 
>> /Andy
>> 
>> 
>> PS:
>> 
>> Although something somewhere else may have changed to counter the problem, 
>> it's clear from your description, that since the only place in the code the 
>> default runtime path is used (instead of the actual runtime path, which may 
>> be different) is the line in WinLauncher.cpp to call SetDllDirectory().
>> 
>> This should be fixed - but would like a way to reproduce the problem it 
>> causes first.
>> 
>> /Andy
>> 
>> On 9/21/2021 12:12 PM, Andy Herrick wrote:
>>> I thought I could create a reproduction scenario by using an app with 
>>> "-splash:" arg then moving the jre as you did, but I have not 
>>> yet been able make it fail.
>>> 
>>> /Andy
>>> 
>>> On 9/21/2021 11:43 AM, Scott Palmer wrote:
> On Sep 21, 2021, at 11:40 AM, Alan Bateman  
> wrote:
> 
> On 21/09/2021 15:54, Andy Herrick wrote:
>> I found the problem in 
>> open/src/jdk.jpackage/windows/native/applauncher/WinLauncher.cpp
>> 
>> we call SetDllDirectory() with the path to the bin dir in the default 
>> runtime directory, not the actual runtime directory.
>> 
>> since on Windows we never use other than the default runtime location - 
>> we had not seen a problem, but is bug I will file and fix.
>> 
> Good to see that you found it quickly. However I'm puzzled as why 
> initializingEncoding wasn't called, I would have expected the VM to blow 
> up during early startup. Would you have cycles to dig into that a bit 
> more in case something has been masked, like for example an exception 
> being swallowed.  Running with -Xlog:exceptions might reveal something.
 
 Do you have a case that reproduces the issue?
 
 Scott
> 

Re: [External] : Re: What causes java.lang.InternalError: platform encoding not initialized ?

[Scott Palmer]

I reproduced it with the AZUL's distribution of OpenJDK (with JavaFX modules 
bundled) and I just changed:

vendor = JvmVendorSpec.ADOPTOPENJDK

And get the same error with that build of OpenJDK as well.  “OpenJDK Runtime 
Environment Temurin-17+35 (build 17+35)”
You can try that, as Gradle should download the JDK for you.

I then downloaded the Oracle build of OpenJDK from https://jdk.java.net/17/
(build 17+35-2724)
unzipped it to C:\Tools
building with:

gradlew -Porg.gradle.java.installations.paths=C:\Tools\jdk-17 clean build

I still reproduce the error.

Though the assertion in native JVM code that you are getting looks like yet 
another bug!  So perhaps it is good that it is discovered as well.


Scott

> On Sep 22, 2021, at 9:54 AM, Andy Herrick  wrote:
> 
> I can't seem to get your testcase to work with the Oracle JDK configured.
> 
> I can build, but then when I run
> 
> $ ./build/application/FetchURL/FetchURL.exe
> 
> I get:
> 
>> *** java.lang.instrument ASSERTION FAILED ***: "!errorOutstanding" with 
>> message c
>> an't find transform methodID at JPLISAgent.c line: 552
>> *** java.lang.instrument ASSERTION FAILED ***: "result" at JPLISAgent.c 
>> line: 400
>> 
>> FATAL ERROR in native method: processing of -javaagent failed
> same behavior if I restore "runtime" directory and fix FetchURL.cfg to remove 
> app.runtime entry.
> 
> anyway I used your two source files to build the test app without gradle, and 
> the test can download 
> https://linear-89.frequency.stream/dist/localnow/89/hls/master/playlist.m3u8 
> without any problems.
> 
> I then moved runtime to jre and added line "app.runtime=$APPDIR/../jre" to 
> cfg file and app still ran fine (downloaded the above)
> 
> So I still don't have any way to reproduce this problemwith the Oracle jdk.
> 
> /Andy
> 
> 
> PS:
> 
> Although something somewhere else may have changed to counter the problem, 
> it's clear from your description, that since the only place in the code the 
> default runtime path is used (instead of the actual runtime path, which may 
> be different) is the line in WinLauncher.cpp to call SetDllDirectory().
> 
> This should be fixed - but would like a way to reproduce the problem it 
> causes first.
> 
> /Andy
> 
> On 9/21/2021 12:12 PM, Andy Herrick wrote:
>> I thought I could create a reproduction scenario by using an app with 
>> "-splash:" arg then moving the jre as you did, but I have not 
>> yet been able make it fail.
>> 
>> /Andy
>> 
>> On 9/21/2021 11:43 AM, Scott Palmer wrote:
 On Sep 21, 2021, at 11:40 AM, Alan Bateman  wrote:
 
 On 21/09/2021 15:54, Andy Herrick wrote:
> I found the problem in 
> open/src/jdk.jpackage/windows/native/applauncher/WinLauncher.cpp
> 
> we call SetDllDirectory() with the path to the bin dir in the default 
> runtime directory, not the actual runtime directory.
> 
> since on Windows we never use other than the default runtime location - 
> we had not seen a problem, but is bug I will file and fix.
> 
 Good to see that you found it quickly. However I'm puzzled as why 
 initializingEncoding wasn't called, I would have expected the VM to blow 
 up during early startup. Would you have cycles to dig into that a bit more 
 in case something has been masked, like for example an exception being 
 swallowed.  Running with -Xlog:exceptions might reveal something.
>>> 
>>> Do you have a case that reproduces the issue?
>>> 
>>> Scott

Integrated: 8274003: ProcessHandleImpl.Info toString has an if check which is always true

[Roger Riggs]

On Tue, 21 Sep 2021 18:14:17 GMT, Roger Riggs  wrote:

> Correct the check if any field has been appended to the StringBuilder in 
> ProcessHandleImpl.Info.toString().

This pull request has now been integrated.

Changeset: 33df388a
Author:Roger Riggs 
URL:   
https://git.openjdk.java.net/jdk/commit/33df388a24267e868574e4604b2e2ab170dc5a09
Stats: 5 lines in 1 file changed: 0 ins; 0 del; 5 mod

8274003: ProcessHandleImpl.Info toString has an if check which is always true

Reviewed-by: bpb, naoto, iris

-

PR: https://git.openjdk.java.net/jdk/pull/5618

Integrated: 8272600: (test) Use native "sleep" in Basic.java

[Roger Riggs]

On Tue, 24 Aug 2021 15:55:28 GMT, Roger Riggs  wrote:

> The intermittent test in java/lang/ProcessBuilder/Basic.java has identified 
> unexpected messages from a child Java VM
> as the cause of the test failure.  Attempts to control the output of the 
> child VM have failed, the VM is unrepentant .
> 
> There is no functionality in the child except to wait long enough for the 
> test to finish and the child is destroyed.
> The fix is to switch from using a Java child to using a native child; a new 
> executable `sleepmillis`.

This pull request has now been integrated.

Changeset: 0a361638
Author:Roger Riggs 
URL:   
https://git.openjdk.java.net/jdk/commit/0a361638c5ea4d3e109d950e34a61b4737c8f670
Stats: 174 lines in 2 files changed: 111 ins; 35 del; 28 mod

8272600: (test) Use native "sleep" in Basic.java

Reviewed-by: iklam, dholmes

-

PR: https://git.openjdk.java.net/jdk/pull/5239

Re: RFR: 8272600: (test) Use native "sleep" in Basic.java [v6]

[Roger Riggs]

On Wed, 22 Sep 2021 00:03:41 GMT, David Holmes  wrote:

>> I thought of that too, but notice the parens "()" around that /bin/sleep; 
>> that creates and extra level of forked processes and its harder to get that 
>> pid. There probably is a way to traverse the hierarchy but I'll keep it as 
>> is for now.
>
> Ah right. Begs the question why we need to use bash to execute sleep? Could 
> it be shell script instead of a binary?

I think that's the author's prerogative; the test was written in 2010
to test conditions related to deeply inherited file descriptors.

-

PR: https://git.openjdk.java.net/jdk/pull/5239

Re: [External] : Re: What causes java.lang.InternalError: platform encoding not initialized ?

[Andy Herrick]

I can't seem to get your testcase to work with the Oracle JDK configured.

I can build, but then when I run

$ ./build/application/FetchURL/FetchURL.exe

I get:

*** java.lang.instrument ASSERTION FAILED ***: "!errorOutstanding" 
with message c

an't find transform methodID at JPLISAgent.c line: 552
*** java.lang.instrument ASSERTION FAILED ***: "result" at 
JPLISAgent.c line: 400


FATAL ERROR in native method: processing of -javaagent failed
same behavior if I restore "runtime" directory and fix FetchURL.cfg to 
remove app.runtime entry.


anyway I used your two source files to build the test app without 
gradle, and the test can download 
https://linear-89.frequency.stream/dist/localnow/89/hls/master/playlist.m3u8 
without any problems.


I then moved runtime to jre and added line "app.runtime=$APPDIR/../jre" 
to cfg file and app still ran fine (downloaded the above)


So I still don't have any way to reproduce this problemwith the Oracle jdk.

/Andy


PS:

Although something somewhere else may have changed to counter the 
problem, it's clear from your description, that since the only place in 
the code the default runtime path is used (instead of the actual runtime 
path, which may be different) is the line in WinLauncher.cpp to call 
SetDllDirectory().


This should be fixed - but would like a way to reproduce the problem it 
causes first.


/Andy

On 9/21/2021 12:12 PM, Andy Herrick wrote:
I thought I could create a reproduction scenario by using an app with 
"-splash:" arg then moving the jre as you did, but I have 
not yet been able make it fail.


/Andy

On 9/21/2021 11:43 AM, Scott Palmer wrote:
On Sep 21, 2021, at 11:40 AM, Alan Bateman  
wrote:


On 21/09/2021 15:54, Andy Herrick wrote:
I found the problem in 
open/src/jdk.jpackage/windows/native/applauncher/WinLauncher.cpp


we call SetDllDirectory() with the path to the bin dir in the 
default runtime directory, not the actual runtime directory.


since on Windows we never use other than the default runtime 
location - we had not seen a problem, but is bug I will file and fix.


Good to see that you found it quickly. However I'm puzzled as why 
initializingEncoding wasn't called, I would have expected the VM to 
blow up during early startup. Would you have cycles to dig into that 
a bit more in case something has been masked, like for example an 
exception being swallowed.  Running with -Xlog:exceptions might 
reveal something.


Do you have a case that reproduces the issue?

Scott

Integrated: 8274071: Clean up java.lang.ref comments and documentation

[Pavel Rappo]

On Tue, 21 Sep 2021 12:05:27 GMT, Pavel Rappo  wrote:

> This PR fixes an inline comment typo and reduces "overlinking" in a doc 
> comment in `java.lang.ref.Reference`. Overlinking happens because the 
> `reachabilityFence` method:
>  * Links `package-summary.html#reachability` twice.
>  * Refers to JLS 12.6 twice: once from a block tag and once from an inline 
> tag.

This pull request has now been integrated.

Changeset: c6df3c95
Author:Pavel Rappo 
URL:   
https://git.openjdk.java.net/jdk/commit/c6df3c9571cfa9607f3deffeaa77701dde9fea15
Stats: 4 lines in 1 file changed: 0 ins; 2 del; 2 mod

8274071: Clean up java.lang.ref comments and documentation

Reviewed-by: rriggs, kbarrett, mchung, iris, lancea

-

PR: https://git.openjdk.java.net/jdk/pull/5609

Re: RFR: 8273656: Improve java.lang.invoke.MethodType.parameterList() and its usage

[Claes Redestad]

On Wed, 22 Sep 2021 00:21:01 GMT, Claes Redestad  wrote:

>> Currently the method is implemented like
>> 
>> public List> parameterList() {
>>   return Collections.unmodifiableList(Arrays.asList(ptypes.clone()));
>> }
>> 
>> This seems to be excessive, as three objects are allocated here. Instead we 
>> can use `List.of(ptypes)` which doesn't allocate anything for empty array 
>> and for one of length 1 and 2 it allocates lightweight objects with 2 
>> fields, still copying longer arrays. This is likely to be fruitful as most 
>> of methods have 0-2 parameters.
>> 
>> Also there is a couple of cases when `MethodType.parameterLis()` is called 
>> to get its size, which is excessive either as we can use 
>> `MethodType.parameterCount()` instead.
>
> This change introduces a subtle behavior change in that `List.of` produce a 
> `null`-hostile list, for example `list.contains(null)` will throw NPE. Does 
> this need to be spelled out? (FTR I think such null-hostile behavior should 
> be reconsidered)

> @cl4es null-issue was the one I was afraid most of all in this case. I think 
> that the initial array `ptypes` never contains null elements due to it's 
> business meaning (we cannot have a param without type). Also we do explicit 
> null-check in `checkPtypes()` method, called from `makeImpl()`.

No, on the producer side there's no issue since a parameter type can't be 
`null`. The compatibility issue I have in mind is only the (very minor) issue 
that the returned list would now throw NPE if you call 
`mt.parameterList().contains(null)` where it before would just return `false`. 
I don't think this is a real issue, but the subtle behavior change might 
warrant a CSR to document the stricter behavior.

-

PR: https://git.openjdk.java.net/jdk/pull/5489

Re: RFR: 8274075: Fix miscellaneous typos in java.base [v3]

[Lance Andersen]

On Wed, 22 Sep 2021 13:01:34 GMT, Pavel Rappo  wrote:

>> 8274075: Fix miscellaneous typos in java.base
>
> Pavel Rappo has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Fix "non-white space"
>
>JDK predominantly uses "non-whitespace". There's only one more use of 
> "non-white space", in com/sun/tools/javac/tree/DocTreeMaker.java:716, which 
> will go away soon.
>  - Undo "wrapped" -> "wrapping"

Marked as reviewed by lancea (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/5610

Re: RFR: 8274075: Fix miscellaneous typos in java.base [v2]

[Pavel Rappo]

On Tue, 21 Sep 2021 13:16:02 GMT, Pavel Rappo  wrote:

>> 8274075: Fix miscellaneous typos in java.base
>
> Pavel Rappo has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Tweak wording for Throwable constructor parameters

Please re-review this change since I've added two commits.

-

PR: https://git.openjdk.java.net/jdk/pull/5610

Re: RFR: 8274075: Fix miscellaneous typos in java.base [v2]

[Pavel Rappo]

On Wed, 22 Sep 2021 10:24:12 GMT, Pavel Rappo  wrote:

>> Note that we don't throw the "wrapped exception" we throw the exception that 
>> wraps it. The "wrapped exception" is the original cause. The wording as 
>> presented now is correct in that regard. You could also say "Throwing a 
>> wrapper exception (i.e. one that has a cause)" - I think both are 
>> grammatically correct.
>> 
>> The later text "where a wrapped exception is thrown from the same method" is 
>> again incorrect because the wrapped exception (the cause) is not what gets 
>> thrown. But I find that whole sentence rather jarring anyway - I'm not 
>> certain exactly what it means.
>
> I'm inclined to revert this change on Throwable.java:68, because of the lack 
> of consensus. It clearly is not a simple typo.

Reverted in 57b71c5.

-

PR: https://git.openjdk.java.net/jdk/pull/5610

Re: RFR: 8274075: Fix miscellaneous typos in java.base [v3]

[Pavel Rappo]

> 8274075: Fix miscellaneous typos in java.base

Pavel Rappo has updated the pull request incrementally with two additional 
commits since the last revision:

 - Fix "non-white space"
   
   JDK predominantly uses "non-whitespace". There's only one more use of 
"non-white space", in com/sun/tools/javac/tree/DocTreeMaker.java:716, which 
will go away soon.
 - Undo "wrapped" -> "wrapping"

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/5610/files
  - new: https://git.openjdk.java.net/jdk/pull/5610/files/19b6f496..05505d97

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5610&range=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5610&range=01-02

  Stats: 3 lines in 2 files changed: 0 ins; 0 del; 3 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5610.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5610/head:pull/5610

PR: https://git.openjdk.java.net/jdk/pull/5610

Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server [v5]

[Julia Boes]

On Tue, 21 Sep 2021 17:16:08 GMT, Michael McMahon  wrote:

>> Julia Boes has updated the pull request with a new target base due to a 
>> merge or a rebase. The pull request now contains 16 commits:
>> 
>>  - Merge branch 'master' into simpleserver
>>  - Merge remote-tracking branch 'origin/simpleserver' into simpleserver
>>  - Merge branch 'master' into simpleserver
>>  - refactor isHidden,isReadable,isSymlink checks and cleanup tests
>>  - Merge branch 'master' into simpleserver
>>  - check isHidden, isSymlink, isReadable for all path segments 
>>  - add checks for all path segments
>>  - Merge branch 'master' into componentcheck
>>  - Merge branch 'master' into simpleserver
>>  - improve output on startup
>>  - ... and 6 more: 
>> https://git.openjdk.java.net/jdk/compare/6d91a3eb...fe059131
>
> src/jdk.httpserver/share/classes/sun/net/httpserver/simpleserver/SimpleFileServerImpl.java
>  line 135:
> 
>> 133: var socketAddr = new InetSocketAddress(addr, port);
>> 134: var server = SimpleFileServer.createFileServer(socketAddr, 
>> root, outputLevel);
>> 135: server.setExecutor(Executors.newSingleThreadExecutor());
> 
> I think this code has the effect of creating one thread for the selector and 
> a second one for the execution of the handlers. If we want to keep thread 
> usage to an absolute minimum then it might be better to not set an executor. 
> Then, the selector thread executes the handlers as well.

I did a quick stress test of both versions (with and without executor 
specified) and don’t see any difference in how many requests can be handled. 
Probably good to keep thread usage to a minimum, I'll update that.

-

PR: https://git.openjdk.java.net/jdk/pull/5505

Re: RFR: 8273656: Improve java.lang.invoke.MethodType.parameterList() and its usage

[Jorn Vernee]

On Mon, 13 Sep 2021 11:06:15 GMT, Сергей Цыпанов 
 wrote:

> Currently the method is implemented like
> 
> public List> parameterList() {
>   return Collections.unmodifiableList(Arrays.asList(ptypes.clone()));
> }
> 
> This seems to be excessive, as three objects are allocated here. Instead we 
> can use `List.of(ptypes)` which doesn't allocate anything for empty array and 
> for one of length 1 and 2 it allocates lightweight objects with 2 fields, 
> still copying longer arrays. This is likely to be fruitful as most of methods 
> have 0-2 parameters.
> 
> Also there is a couple of cases when `MethodType.parameterLis()` is called to 
> get its size, which is excessive either as we can use 
> `MethodType.parameterCount()` instead.

> Mailing list message from John Rose on core-libs-dev:
> 
> On Sep 13, 2021, at 10:24 AM, Vladimir Ivanov  openjdk.java.net> wrote:
> 
> BTW it can be improved even further by caching the immutable List view of 
> parameters.
> 
> I would go further: If I were writing MethodType.java today
> I would probably use List.of as the backing store for the
> parameters, instead of the private array. So ptypes should
> be List> not Class[]. I don?t think the footprint
> or polymorphism effects would be dealbreakers, and the
> code would (I think) be simpler overall. But that?s a messy
> change, since the array representation is dug in.

Another thing that makes this change hard is that `MethodType` is mapped 
directly in HotSpot, so that HotSpot can retrieve for instance the parameter 
types [1]. The VM already knows how to dig into arrays, but adding support for 
`List` as well seems more cumbersome, since the list internals can change, and 
AFAIK most VM code can not just call `List::get`.

[1] : 
https://github.com/openjdk/jdk/blob/master/src/hotspot/share/classfile/javaClasses.cpp#L4135-L4137

-

PR: https://git.openjdk.java.net/jdk/pull/5489

Re: [External] : Re: What causes java.lang.InternalError: platform encoding not initialized ?

[Andy Herrick]

Thanks Scott

This will help a lot because I couldn't reproduce this using just a 
splash screen as I expected I could.


I will look at this example this morning.

/Andy

On 9/21/21 7:05 PM, Scott Palmer wrote:
I’ve uploaded a project that reproduces the problem to JDK-8274087 



Regards,

Scott

On Sep 21, 2021, at 3:49 PM, Scott Palmer > wrote:


I have discovered that I have to have my Java Agent configured  in 
the .cfg file for it to happen:


[JavaOptions]
java-option=-javaagent:$APPDIR\libs\MyAgent.jar


The agent is needed in my real app only to get an instance of the 
Instrumentation interface. In my test code, where I have reproduced 
this, it is never used.  My test app just takes a URL and fetches it 
via a HttpURLConnection with setInstanceFollowRedirects(false) so I 
can capture the new location.  If you are still having trouble 
reproducing I will have the code cleaned up for the bug report soon...



Regards,

Scott



On Sep 21, 2021, at 12:12 PM, Andy Herrick > wrote:


I thought I could create a reproduction scenario by using an app 
with "-splash:" arg then moving the jre as you did, but 
I have not yet been able make it fail.


/Andy

On 9/21/2021 11:43 AM, Scott Palmer wrote:
On Sep 21, 2021, at 11:40 AM, Alan Bateman 
mailto:alan.bate...@oracle.com>> wrote:


On 21/09/2021 15:54, Andy Herrick wrote:
I found the problem in 
open/src/jdk.jpackage/windows/native/applauncher/WinLauncher.cpp


we call SetDllDirectory() with the path to the bin dir in the 
default runtime directory, not the actual runtime directory.


since on Windows we never use other than the default runtime 
location - we had not seen a problem, but is bug I will file and fix.


Good to see that you found it quickly. However I'm puzzled as why 
initializingEncoding wasn't called, I would have expected the VM 
to blow up during early startup. Would you have cycles to dig into 
that a bit more in case something has been masked, like for 
example an exception being swallowed.  Running with 
-Xlog:exceptions might reveal something.


Do you have a case that reproduces the issue?

Scott

Re: RFR: 8231640: (prop) Canonical property storage [v21]

[Jaikiran Pai]

On Wed, 22 Sep 2021 10:05:04 GMT, Alan Bateman  wrote:

>> Jaikiran Pai has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Roger's suggestion to reword the implSpec text
>
> src/java.base/share/classes/java/lang/System.java line 766:
> 
>> 764:  * {@systemProperty java.properties.date}
>> 765:  * Text for the comment that must replace the default date 
>> comment
>> 766:  * written out by {@code Properties.store()} methods 
>> (optional) 
> 
> To date, the table in getProperties has listed the supported system 
> properties that the runtime makes available to applications. It hasn't 
> historically listed the many other standard properties that can be set on the 
> command line. So I'm sure about adding this one to the table as it opens the 
> door to expanding the table to all the other system properties that are 
> documented elsewhere in the API docs. Note that javadoc creates a table of 
> system properties from usages of `@systemProperty` so there is already a more 
> complete table in the javadoc build.

After this change to include this property in System::getProperties() javadoc 
was added, there have been inputs (here and on the CSR) which suggest that we 
probably shouldn't include it here. I've now updated this PR to revert this 
specific change.

> src/java.base/share/classes/java/util/Properties.java line 928:
> 
>> 926: // in the natural order of their key. Else, we consider 
>> that the subclassed implementation may potentially
>> 927: // have returned a differently ordered entries and so we 
>> just use the iteration order of the returned instance.
>> 928: if (entries instanceof Collections.SynchronizedSet ss
> 
> Would you mind re-formatting the comment to keep the line length a bit more 
> consistent with the rest of the code, is's just a bit annoying to have it 
> wrapping.

Done. I've updated the PR to reduce the line length of that code comment.

-

PR: https://git.openjdk.java.net/jdk/pull/5372

Re: RFR: 8231640: (prop) Canonical property storage [v22]

[Jaikiran Pai]

> The commit in this PR implements the proposal for enhancement that was 
> discussed in the core-libs-dev mailing list recently[1], for 
> https://bugs.openjdk.java.net/browse/JDK-8231640
> 
> At a high level - the `store()` APIs in `Properties` have been modified to 
> now look for the `SOURCE_DATE_EPOCH` environment variable[2]. If that env 
> variable is set, then instead of writing out the current date time as a date 
> comment, the `store()` APIs instead will use the value set for this env 
> variable to parse it to a `Date` and write out the string form of such a 
> date. The implementation here uses the `d MMM  HH:mm:ss 'GMT'` date 
> format and `Locale.ROOT` to format and write out such a date. This should 
> provide reproducibility whenever the `SOURCE_DATE_EPOCH` is set. Furthermore, 
> intentionally, no changes in the date format of the "current date" have been 
> done.
> 
> These  modified `store()` APIs work in the presence of the `SecurityManager` 
> too. The caller is expected to have a read permission on the 
> `SOURCE_DATE_EPOCH` environment variable. If the caller doesn't have that 
> permission, then the implementation of these `store()` APIs will write out 
> the "current date" and will ignore any value that has been set for the 
> `SOURCE_DATE_EPOCH` env variable. This should allow for backward 
> compatibility of existing applications, where, when they run under a 
> `SecurityManager` and perhaps with an existing restrictive policy file, the 
> presence of `SOURCE_DATE_EPOCH` shouldn't impact their calls to the `store()` 
> APIs.
> 
> The modified `store()` APIs will also ignore any value for 
> `SOURCE_DATE_EPOCH` that  cannot be parsed to an `long` value. In such cases, 
> the `store()` APIs will write out the "current date" and ignore the value set 
> for this environment variable. No exceptions will be thrown for such invalid 
> values. This is an additional backward compatibility precaution to prevent 
> any rogue value for `SOURCE_DATE_EPOCH` from breaking applications.
> 
> An additional change in the implementation of these `store()` APIs and 
> unrelated to the date comment, is that these APIs will now write out the 
> property keys in a deterministic order. The keys will be written out in the 
> natural ordering as specified by `java.lang.String#compareTo()` API.
> 
> The combination of the ordering of the property keys when written out and the 
> usage of `SOURCE_DATE_EPOCH` environment value to determine the date comment 
> should together allow for reproducibility of the output generated by these 
> `store()` APIs.
> 
> New jtreg test classes have been introduced to verify these changes. The 
> primary focus of `PropertiesStoreTest` is the ordering aspects of the 
> property keys that are written out. On the other hand 
> `StoreReproducibilityTest` focuses on the reproducibility of the output 
> generated by these APIs.  The `StoreReproducibilityTest` runs these tests 
> both in the presence and absence of `SecurityManager`. Plus, in the presence 
> of SecurityManager, it tests both the scenarios where the caller is granted 
> the requisite permission and in other case not granted that permission.
> 
> These new tests and existing tests under `test/jdk/java/util/Properties/` 
> pass with these changes.
> 
> [1] 
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-August/080758.html
> [2] https://reproducible-builds.org/specs/source-date-epoch/

Jaikiran Pai has updated the pull request incrementally with two additional 
commits since the last revision:

 - Revert "Implement Mark's suggestion in CSR to include the 
java.properties.date in the list of system properties listed in 
System::getProperties()"
   
   Additional inputs since this specific commit was introduced have leaned 
towards not listing this property in System::getProperties()
   
   This reverts commit 458c1fd37cf1e8e8bd0a60a171d173ce5da12b7c.
 - reduce the line length of code comment

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/5372/files
  - new: https://git.openjdk.java.net/jdk/pull/5372/files/e350721a..944cbf1e

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5372&range=21
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5372&range=20-21

  Stats: 9 lines in 2 files changed: 2 ins; 3 del; 4 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5372.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5372/head:pull/5372

PR: https://git.openjdk.java.net/jdk/pull/5372

Re: RFR: 8274075: Fix miscellaneous typos in java.base [v2]

[Pavel Rappo]

On Tue, 21 Sep 2021 22:00:29 GMT, David Holmes  wrote:

>> Instead of "common case where a wrapped exception is thrown from same 
>> method" could one write "common case where an enclosing exception is thrown 
>> from the same method"?
>
> Note that we don't throw the "wrapped exception" we throw the exception that 
> wraps it. The "wrapped exception" is the original cause. The wording as 
> presented now is correct in that regard. You could also say "Throwing a 
> wrapper exception (i.e. one that has a cause)" - I think both are 
> grammatically correct.
> 
> The later text "where a wrapped exception is thrown from the same method" is 
> again incorrect because the wrapped exception (the cause) is not what gets 
> thrown. But I find that whole sentence rather jarring anyway - I'm not 
> certain exactly what it means.

I'm inclined to revert this change on Throwable.java:68, because of the lack of 
consensus. It clearly is not a simple typo.

-

PR: https://git.openjdk.java.net/jdk/pull/5610

Re: RFR: 8231640: (prop) Canonical property storage [v21]

[Alan Bateman]

On Sat, 18 Sep 2021 03:52:17 GMT, Jaikiran Pai  wrote:

>> The commit in this PR implements the proposal for enhancement that was 
>> discussed in the core-libs-dev mailing list recently[1], for 
>> https://bugs.openjdk.java.net/browse/JDK-8231640
>> 
>> At a high level - the `store()` APIs in `Properties` have been modified to 
>> now look for the `SOURCE_DATE_EPOCH` environment variable[2]. If that env 
>> variable is set, then instead of writing out the current date time as a date 
>> comment, the `store()` APIs instead will use the value set for this env 
>> variable to parse it to a `Date` and write out the string form of such a 
>> date. The implementation here uses the `d MMM  HH:mm:ss 'GMT'` date 
>> format and `Locale.ROOT` to format and write out such a date. This should 
>> provide reproducibility whenever the `SOURCE_DATE_EPOCH` is set. 
>> Furthermore, intentionally, no changes in the date format of the "current 
>> date" have been done.
>> 
>> These  modified `store()` APIs work in the presence of the `SecurityManager` 
>> too. The caller is expected to have a read permission on the 
>> `SOURCE_DATE_EPOCH` environment variable. If the caller doesn't have that 
>> permission, then the implementation of these `store()` APIs will write out 
>> the "current date" and will ignore any value that has been set for the 
>> `SOURCE_DATE_EPOCH` env variable. This should allow for backward 
>> compatibility of existing applications, where, when they run under a 
>> `SecurityManager` and perhaps with an existing restrictive policy file, the 
>> presence of `SOURCE_DATE_EPOCH` shouldn't impact their calls to the 
>> `store()` APIs.
>> 
>> The modified `store()` APIs will also ignore any value for 
>> `SOURCE_DATE_EPOCH` that  cannot be parsed to an `long` value. In such 
>> cases, the `store()` APIs will write out the "current date" and ignore the 
>> value set for this environment variable. No exceptions will be thrown for 
>> such invalid values. This is an additional backward compatibility precaution 
>> to prevent any rogue value for `SOURCE_DATE_EPOCH` from breaking 
>> applications.
>> 
>> An additional change in the implementation of these `store()` APIs and 
>> unrelated to the date comment, is that these APIs will now write out the 
>> property keys in a deterministic order. The keys will be written out in the 
>> natural ordering as specified by `java.lang.String#compareTo()` API.
>> 
>> The combination of the ordering of the property keys when written out and 
>> the usage of `SOURCE_DATE_EPOCH` environment value to determine the date 
>> comment should together allow for reproducibility of the output generated by 
>> these `store()` APIs.
>> 
>> New jtreg test classes have been introduced to verify these changes. The 
>> primary focus of `PropertiesStoreTest` is the ordering aspects of the 
>> property keys that are written out. On the other hand 
>> `StoreReproducibilityTest` focuses on the reproducibility of the output 
>> generated by these APIs.  The `StoreReproducibilityTest` runs these tests 
>> both in the presence and absence of `SecurityManager`. Plus, in the presence 
>> of SecurityManager, it tests both the scenarios where the caller is granted 
>> the requisite permission and in other case not granted that permission.
>> 
>> These new tests and existing tests under `test/jdk/java/util/Properties/` 
>> pass with these changes.
>> 
>> [1] 
>> https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-August/080758.html
>> [2] https://reproducible-builds.org/specs/source-date-epoch/
>
> Jaikiran Pai has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Roger's suggestion to reword the implSpec text

src/java.base/share/classes/java/lang/System.java line 766:

> 764:  * {@systemProperty java.properties.date}
> 765:  * Text for the comment that must replace the default date 
> comment
> 766:  * written out by {@code Properties.store()} methods 
> (optional) 

To date, the table in getProperties has listed the supported system properties 
that the runtime makes available to applications. It hasn't historically listed 
the many other standard properties that can be set on the command line. So I'm 
sure about adding this one to the table as it opens the door to expanding the 
table to all the other system properties that are documented elsewhere in the 
API docs. Note that javadoc creates a table of system properties from usages of 
`@systemProperty` so there is already a more complete table in the javadoc 
build.

-

PR: https://git.openjdk.java.net/jdk/pull/5372

Re: RFR: 8231640: (prop) Canonical property storage [v21]

[Alan Bateman]

On Sat, 18 Sep 2021 03:52:17 GMT, Jaikiran Pai  wrote:

>> The commit in this PR implements the proposal for enhancement that was 
>> discussed in the core-libs-dev mailing list recently[1], for 
>> https://bugs.openjdk.java.net/browse/JDK-8231640
>> 
>> At a high level - the `store()` APIs in `Properties` have been modified to 
>> now look for the `SOURCE_DATE_EPOCH` environment variable[2]. If that env 
>> variable is set, then instead of writing out the current date time as a date 
>> comment, the `store()` APIs instead will use the value set for this env 
>> variable to parse it to a `Date` and write out the string form of such a 
>> date. The implementation here uses the `d MMM  HH:mm:ss 'GMT'` date 
>> format and `Locale.ROOT` to format and write out such a date. This should 
>> provide reproducibility whenever the `SOURCE_DATE_EPOCH` is set. 
>> Furthermore, intentionally, no changes in the date format of the "current 
>> date" have been done.
>> 
>> These  modified `store()` APIs work in the presence of the `SecurityManager` 
>> too. The caller is expected to have a read permission on the 
>> `SOURCE_DATE_EPOCH` environment variable. If the caller doesn't have that 
>> permission, then the implementation of these `store()` APIs will write out 
>> the "current date" and will ignore any value that has been set for the 
>> `SOURCE_DATE_EPOCH` env variable. This should allow for backward 
>> compatibility of existing applications, where, when they run under a 
>> `SecurityManager` and perhaps with an existing restrictive policy file, the 
>> presence of `SOURCE_DATE_EPOCH` shouldn't impact their calls to the 
>> `store()` APIs.
>> 
>> The modified `store()` APIs will also ignore any value for 
>> `SOURCE_DATE_EPOCH` that  cannot be parsed to an `long` value. In such 
>> cases, the `store()` APIs will write out the "current date" and ignore the 
>> value set for this environment variable. No exceptions will be thrown for 
>> such invalid values. This is an additional backward compatibility precaution 
>> to prevent any rogue value for `SOURCE_DATE_EPOCH` from breaking 
>> applications.
>> 
>> An additional change in the implementation of these `store()` APIs and 
>> unrelated to the date comment, is that these APIs will now write out the 
>> property keys in a deterministic order. The keys will be written out in the 
>> natural ordering as specified by `java.lang.String#compareTo()` API.
>> 
>> The combination of the ordering of the property keys when written out and 
>> the usage of `SOURCE_DATE_EPOCH` environment value to determine the date 
>> comment should together allow for reproducibility of the output generated by 
>> these `store()` APIs.
>> 
>> New jtreg test classes have been introduced to verify these changes. The 
>> primary focus of `PropertiesStoreTest` is the ordering aspects of the 
>> property keys that are written out. On the other hand 
>> `StoreReproducibilityTest` focuses on the reproducibility of the output 
>> generated by these APIs.  The `StoreReproducibilityTest` runs these tests 
>> both in the presence and absence of `SecurityManager`. Plus, in the presence 
>> of SecurityManager, it tests both the scenarios where the caller is granted 
>> the requisite permission and in other case not granted that permission.
>> 
>> These new tests and existing tests under `test/jdk/java/util/Properties/` 
>> pass with these changes.
>> 
>> [1] 
>> https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-August/080758.html
>> [2] https://reproducible-builds.org/specs/source-date-epoch/
>
> Jaikiran Pai has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Roger's suggestion to reword the implSpec text

src/java.base/share/classes/java/util/Properties.java line 928:

> 926: // in the natural order of their key. Else, we consider that 
> the subclassed implementation may potentially
> 927: // have returned a differently ordered entries and so we 
> just use the iteration order of the returned instance.
> 928: if (entries instanceof Collections.SynchronizedSet ss

Would it be possible to re-format the comment to keep the lines a bit 
inconsistent, it's just a bit annoying to have overly long lines just here.

-

PR: https://git.openjdk.java.net/jdk/pull/5372

Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server [v5]

[Julia Boes]

On Tue, 21 Sep 2021 16:04:21 GMT, Daniel Fuchs  wrote:

>> Julia Boes has updated the pull request with a new target base due to a 
>> merge or a rebase. The pull request now contains 16 commits:
>> 
>>  - Merge branch 'master' into simpleserver
>>  - Merge remote-tracking branch 'origin/simpleserver' into simpleserver
>>  - Merge branch 'master' into simpleserver
>>  - refactor isHidden,isReadable,isSymlink checks and cleanup tests
>>  - Merge branch 'master' into simpleserver
>>  - check isHidden, isSymlink, isReadable for all path segments 
>>  - add checks for all path segments
>>  - Merge branch 'master' into componentcheck
>>  - Merge branch 'master' into simpleserver
>>  - improve output on startup
>>  - ... and 6 more: 
>> https://git.openjdk.java.net/jdk/compare/6d91a3eb...fe059131
>
> src/jdk.httpserver/share/classes/sun/net/httpserver/simpleserver/FileServerHandler.java
>  line 314:
> 
>> 312: + "\n");
>> 313: try (var paths = Files.list(path)) {
>> 314: paths.filter(p -> !isHiddenOrSymLink(p))
> 
> Shouldn't we filter paths that are not readable here too? There's no point in 
> printing a link that will result in 404, is there?

Totally, thanks for noting!

-

PR: https://git.openjdk.java.net/jdk/pull/5505

Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server [v5]

[Julia Boes]

On Tue, 21 Sep 2021 15:23:33 GMT, Michael McMahon  wrote:

>> Julia Boes has updated the pull request with a new target base due to a 
>> merge or a rebase. The pull request now contains 16 commits:
>> 
>>  - Merge branch 'master' into simpleserver
>>  - Merge remote-tracking branch 'origin/simpleserver' into simpleserver
>>  - Merge branch 'master' into simpleserver
>>  - refactor isHidden,isReadable,isSymlink checks and cleanup tests
>>  - Merge branch 'master' into simpleserver
>>  - check isHidden, isSymlink, isReadable for all path segments 
>>  - add checks for all path segments
>>  - Merge branch 'master' into componentcheck
>>  - Merge branch 'master' into simpleserver
>>  - improve output on startup
>>  - ... and 6 more: 
>> https://git.openjdk.java.net/jdk/compare/6d91a3eb...fe059131
>
> test/jdk/com/sun/net/httpserver/FilterTest.java line 330:
> 
>> 328: var response = client.send(request, 
>> HttpResponse.BodyHandlers.ofString());
>> 329: assertEquals(response.statusCode(), 200);
>> 330: assertEquals(inspectedURI.get(), URI.create("/"));
> 
> Could you make the request URI something like /foo/bar instead of just / here?

Yes, I'll do that.

-

PR: https://git.openjdk.java.net/jdk/pull/5505

Re: RFR: 8273656: Improve java.lang.invoke.MethodType.parameterList() and its usage

[Сергей Цыпанов]

On Wed, 22 Sep 2021 00:21:01 GMT, Claes Redestad  wrote:

>> Currently the method is implemented like
>> 
>> public List> parameterList() {
>>   return Collections.unmodifiableList(Arrays.asList(ptypes.clone()));
>> }
>> 
>> This seems to be excessive, as three objects are allocated here. Instead we 
>> can use `List.of(ptypes)` which doesn't allocate anything for empty array 
>> and for one of length 1 and 2 it allocates lightweight objects with 2 
>> fields, still copying longer arrays. This is likely to be fruitful as most 
>> of methods have 0-2 parameters.
>> 
>> Also there is a couple of cases when `MethodType.parameterLis()` is called 
>> to get its size, which is excessive either as we can use 
>> `MethodType.parameterCount()` instead.
>
> This change introduces a subtle behavior change in that `List.of` produce a 
> `null`-hostile list, for example `list.contains(null)` will throw NPE. Does 
> this need to be spelled out? (FTR I think such null-hostile behavior should 
> be reconsidered)

@cl4es null-issue was the one I was afraid most of all in this case. I think 
that the initial array `ptypes` never contains null elements due to it's 
business meaning (we cannot have a param without type). Also we do explicit 
null-check in `checkPtypes()` method, called from `makeImpl()`.

-

PR: https://git.openjdk.java.net/jdk/pull/5489

Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server [v5]

[Chris Hegarty]

On Tue, 21 Sep 2021 14:09:54 GMT, Julia Boes  wrote:

>> This change implements a simple web server that can be run on the 
>> command-line with `java -m jdk.httpserver`.
>> 
>> This is facilitated by adding an entry point for the `jdk.httpserver` 
>> module, an implementation class whose main method is run when the above 
>> command is executed. This is the first such module entry point in the JDK.
>> 
>> The server is a minimal HTTP server that serves the static files of a given 
>> directory, similar to existing alternatives on other platforms and 
>> convenient for testing, development, and debugging.
>> 
>> Additionally, a small API is introduced for programmatic creation and 
>> customization.
>> 
>> Testing: tier1-3.
>
> Julia Boes has updated the pull request with a new target base due to a merge 
> or a rebase. The pull request now contains 16 commits:
> 
>  - Merge branch 'master' into simpleserver
>  - Merge remote-tracking branch 'origin/simpleserver' into simpleserver
>  - Merge branch 'master' into simpleserver
>  - refactor isHidden,isReadable,isSymlink checks and cleanup tests
>  - Merge branch 'master' into simpleserver
>  - check isHidden, isSymlink, isReadable for all path segments 
>  - add checks for all path segments
>  - Merge branch 'master' into componentcheck
>  - Merge branch 'master' into simpleserver
>  - improve output on startup
>  - ... and 6 more: 
> https://git.openjdk.java.net/jdk/compare/6d91a3eb...fe059131

Overall, this is a very nice addition. Good work.

-

Marked as reviewed by chegar (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/5505