Re: RFR: JDK-8256844: Make NMT late-initializable

2021-07-29 Thread Kim Barrett
On Thu, 29 Jul 2021 22:53:42 GMT, Coleen Phillimore  wrote:

>> Short: this patch makes NMT available in custom-launcher scenarios and 
>> during gtests. It simplifies NMT initialization. It adds a lot of 
>> NMT-specific testing, cleans them up and makes them sideeffect-free.
>> 
>> -
>> 
>> NMT continues to be an extremely useful tool for SAP to tackle memory 
>> problems in the JVM.
>> 
>> However, NMT is of limited use due to the following restrictions:
>> 
>> - NMT cannot be used if the hotspot is embedded into a custom launcher 
>> unless the launcher actively cooperates. Just creating and invoking the JVM 
>> is not enough, it needs to do some steps prior to loading the hotspot. This 
>> limitation is not well known (nor, do I believe, documented). Many products 
>> don't do this, e.g., you cannot use NMT with IntelliJ. For us at SAP this 
>> problem limits NMT usefulness greatly since our VMs are often embedded into 
>> custom launchers and modifying every launcher is impossible.
>> - Worse, if that custom launcher links the libjvm *statically* there is just 
>> no way to activate NMT at all. This is the reason NMT cannot be used in the 
>> `gtestlauncher`.
>> - Related to that is that we cannot pass NMT options via `JAVA_TOOL_OPTIONS` 
>> and `-XX:Flags=`.
>> - The fact that NMT cannot be used in gtests is really a pity since it would 
>> allow us to both test NMT itself more rigorously and check for memory leaks 
>> while testing other stuff.
>> 
>> The reason for all this is that NMT initialization happens very early, on 
>> the first call to `os::malloc()`. And those calls happen already during 
>> dynamic C++ initialization - a long time before the VM gets around parsing 
>> arguments. So, regular VM argument parsing is too late to parse NMT 
>> arguments.
>> 
>> The current solution is to pass NMT arguments via a specially prepared 
>> environment variable: `NMT_LEVEL_=`. That environment 
>> variable has to be set by the embedding launcher, before it loads the 
>> libjvm. Since its name contains the PID, we cannot even set that variable in 
>> the shell before starting the launcher.
>> 
>> All that means that every launcher needs to especially parse and process the 
>> NMT arguments given at the command line (or via whatever method) and prepare 
>> the environment variable. `java` itself does this. This only works before 
>> the libjvm.so is loaded, before its dynamic C++ initialization. For that 
>> reason, it does not work if the launcher links statically against the 
>> hotspot, since in that case C++ initialization of the launcher and hotspot 
>> are folded into one phase with no possibility of executing code beforehand.
>> 
>> And since it bypasses argument handling in the VM, it bypasses a number of 
>> argument processing ways, e.g., `JAVA_TOOL_OPTIONS`.
>> 
>> --
>> 
>> This patch fixes these shortcomings by making NMT late-initializable: it can 
>> now be initialized after normal VM argument parsing, like all other parts of 
>> the VM. This greatly simplifies NMT initialization and makes it work 
>> automagically for every third party launcher, as well as within our gtests.
>> 
>> The glaring problem with late-initializing NMT is the NMT malloc headers. If 
>> we rule out just always having them (unacceptable in terms of memory 
>> overhead), there is no safe way to determine, in os::free(), if an 
>> allocation came from before or after NMT initialization ran, and therefore 
>> what to do with its malloc headers. For a more extensive explanation, please 
>> see the comment block `nmtPreInit.hpp` and the discussion with @kimbarrett 
>> and @zhengyu123 in the JBS comment section.
>> 
>> The heart of this patch is a new way to track early, pre-NMT-init 
>> allocations. These are tracked via a lookup table. This was a suggestion by 
>> Kim and it worked out well.
>> 
>> Changes in detail:
>> 
>> - pre-NMT-init handling:
>>  - the new files `nmtPreInit.hpp/cpp` take case of NMT pre-init 
>> handling. They contain a small global lookup table managing C-heap blocks 
>> allocated in the pre-NMT-init phase.
>>  - `os::malloc()/os::realloc()/os::free()` defer to this code before 
>> doing anything else.
>>  - Please see the extensive comment block at the start of 
>> `nmtPreinit.hpp` explaining the details.
>> 
>> - Changes to NMT:
>>  - Before, NMT initialization was spread over two phases, `initialize()` 
>> and `late_initialize()`. Those were merged into one and simplified - there 
>> is only one initialization now which happens after argument parsing.
>>  - Minor changes were needed for the `NMT_TrackingLevel` enum - to 
>> simplify code, I changed NMT_unknown to be numerically 0. A new comment 
>> block in `nmtCommon.hpp` now clearly specifies what's what, including 
>> allowed level state transitions.
>>  - New utility functions to translate tracking level from/to strings 
>> added to `NMTUtil`
>>  - NMT has never been able to handle virtual 

Re: RFR: JDK-8256844: Make NMT late-initializable

2021-07-29 Thread Kim Barrett
On Thu, 29 Jul 2021 22:52:07 GMT, Coleen Phillimore  wrote:

>> Short: this patch makes NMT available in custom-launcher scenarios and 
>> during gtests. It simplifies NMT initialization. It adds a lot of 
>> NMT-specific testing, cleans them up and makes them sideeffect-free.
>> 
>> -
>> 
>> NMT continues to be an extremely useful tool for SAP to tackle memory 
>> problems in the JVM.
>> 
>> However, NMT is of limited use due to the following restrictions:
>> 
>> - NMT cannot be used if the hotspot is embedded into a custom launcher 
>> unless the launcher actively cooperates. Just creating and invoking the JVM 
>> is not enough, it needs to do some steps prior to loading the hotspot. This 
>> limitation is not well known (nor, do I believe, documented). Many products 
>> don't do this, e.g., you cannot use NMT with IntelliJ. For us at SAP this 
>> problem limits NMT usefulness greatly since our VMs are often embedded into 
>> custom launchers and modifying every launcher is impossible.
>> - Worse, if that custom launcher links the libjvm *statically* there is just 
>> no way to activate NMT at all. This is the reason NMT cannot be used in the 
>> `gtestlauncher`.
>> - Related to that is that we cannot pass NMT options via `JAVA_TOOL_OPTIONS` 
>> and `-XX:Flags=`.
>> - The fact that NMT cannot be used in gtests is really a pity since it would 
>> allow us to both test NMT itself more rigorously and check for memory leaks 
>> while testing other stuff.
>> 
>> The reason for all this is that NMT initialization happens very early, on 
>> the first call to `os::malloc()`. And those calls happen already during 
>> dynamic C++ initialization - a long time before the VM gets around parsing 
>> arguments. So, regular VM argument parsing is too late to parse NMT 
>> arguments.
>> 
>> The current solution is to pass NMT arguments via a specially prepared 
>> environment variable: `NMT_LEVEL_=`. That environment 
>> variable has to be set by the embedding launcher, before it loads the 
>> libjvm. Since its name contains the PID, we cannot even set that variable in 
>> the shell before starting the launcher.
>> 
>> All that means that every launcher needs to especially parse and process the 
>> NMT arguments given at the command line (or via whatever method) and prepare 
>> the environment variable. `java` itself does this. This only works before 
>> the libjvm.so is loaded, before its dynamic C++ initialization. For that 
>> reason, it does not work if the launcher links statically against the 
>> hotspot, since in that case C++ initialization of the launcher and hotspot 
>> are folded into one phase with no possibility of executing code beforehand.
>> 
>> And since it bypasses argument handling in the VM, it bypasses a number of 
>> argument processing ways, e.g., `JAVA_TOOL_OPTIONS`.
>> 
>> --
>> 
>> This patch fixes these shortcomings by making NMT late-initializable: it can 
>> now be initialized after normal VM argument parsing, like all other parts of 
>> the VM. This greatly simplifies NMT initialization and makes it work 
>> automagically for every third party launcher, as well as within our gtests.
>> 
>> The glaring problem with late-initializing NMT is the NMT malloc headers. If 
>> we rule out just always having them (unacceptable in terms of memory 
>> overhead), there is no safe way to determine, in os::free(), if an 
>> allocation came from before or after NMT initialization ran, and therefore 
>> what to do with its malloc headers. For a more extensive explanation, please 
>> see the comment block `nmtPreInit.hpp` and the discussion with @kimbarrett 
>> and @zhengyu123 in the JBS comment section.
>> 
>> The heart of this patch is a new way to track early, pre-NMT-init 
>> allocations. These are tracked via a lookup table. This was a suggestion by 
>> Kim and it worked out well.
>> 
>> Changes in detail:
>> 
>> - pre-NMT-init handling:
>>  - the new files `nmtPreInit.hpp/cpp` take case of NMT pre-init 
>> handling. They contain a small global lookup table managing C-heap blocks 
>> allocated in the pre-NMT-init phase.
>>  - `os::malloc()/os::realloc()/os::free()` defer to this code before 
>> doing anything else.
>>  - Please see the extensive comment block at the start of 
>> `nmtPreinit.hpp` explaining the details.
>> 
>> - Changes to NMT:
>>  - Before, NMT initialization was spread over two phases, `initialize()` 
>> and `late_initialize()`. Those were merged into one and simplified - there 
>> is only one initialization now which happens after argument parsing.
>>  - Minor changes were needed for the `NMT_TrackingLevel` enum - to 
>> simplify code, I changed NMT_unknown to be numerically 0. A new comment 
>> block in `nmtCommon.hpp` now clearly specifies what's what, including 
>> allowed level state transitions.
>>  - New utility functions to translate tracking level from/to strings 
>> added to `NMTUtil`
>>  - NMT has never been able to handle virtual 

PhantomReferences

2021-07-29 Thread Hans Boehm
Here's another finalization-related issue, this time hopefully appropriate
for this list. This was inspired by looking at the Ugawa, Jones, and Ritson
paper from ISMM 2014, which I belatedly had a chance to look at.

The java.lang.ref spec says:

"An object is phantom reachable if it is neither strongly, softly, nor
weakly reachable, it has been finalized, and some phantom reference refers
to it."

It notably does not say that such an object must not be reachable from
unfinalized objects.

I currently believe that:

1) This spec is not as intended, in that it allows a PhantomReference to X
to be enqueued while X is still actively being used. My understanding is
that PhantomReferences were invented largely to make that impossible.

2) Real production implementations enforce a stronger requirement, which
includes that the PhantomReference must not reachable from unfinalized
objects with a nontrivial finalizer, which prevents this problem.

3) The ISMM 2014 paper may have been confused by this, in that it seems to
mirror the official spec rather than the usual implementation. It
(surprisingly to me) does not appear to address the fact that
implementations generally mark reachable objects in at least two stages:
(1) Reachability from roots, and (2) Reachability from roots U unfinalized
finalizable objects, where the result of the first phase is used to
determine WeakReference clearing, while the result of the second phase
determines PhantomReference clearing, and what to collect.

Am I correct?

A scenario that I believe can fail according to the spec, but cannot and
must not fail in real life, is the following, where F1 and F2 are objects
with nontrivial finalizers, and P is the referent of a PhantomReference:

Consider F1 --> P,  where P has a PhantomReference referring to it, and
 -> F2 -> null.  Then

1) F1's finalizer runs and notionally P's (empty) finalizer runs. F1
modifies F2, so it gets a strong reference to P.

[ P has now been finalized. We have  -> F2 -> P ]

2)  is cleared, making F2 unreachable.

[ P is not strongly, softly or weakly referenced, and has been finalized.
Therefore P is phantom reachable. ]

3) The PhantomReference to P is enqueued, resulting in running a Cleaner
that e.g. deallocates native memory required by P.

4) F2's finalizer runs and accesses P.

5) Bad stuff.

Although this is arguably a weird corner case that is unlikely to occur
frequently, I think it profoundly changes the algorithms used to implement
this. "Has been finalized" is not the correct check; it's reachability from
a not-yet-finalized object that matters. Hence the implementation must do a
reachability analysis not technically required by the current spec.

[ Just saying that in the spec probably doesn't work either. I suspect the
fact that the finalizer is nontrivial also matters to get reasonable
progress guarantees. Currently I think the spec doesn't have that notion,
but it seems annoyingly essential. ]

Clearly, this problem goes away if you get rid of finalizers and merge
{Phantom,Weak}References, which is presumably the intended end state, but
not one that looks imminent to me.

Hans


Re: RFR: 8251329: (zipfs) Files.walkFileTree walks infinitely if zip has dir named "." inside [v3]

2021-07-29 Thread Jaikiran Pai
On Thu, 29 Jul 2021 18:21:07 GMT, Lance Andersen  wrote:

>> Hi,
>> 
>> As discussed in the 
>> https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-July/079621.html 
>> thread, this is the revised patch to address the use of '.' and '..' within 
>> Zip FS
>> 
>> Zip FS needs to use "." and ".." as links to the current and parent 
>> directories and cannot reliably support entries that have "." and ".." as 
>> name elements.  This patch updates Zip Fs  to reject ZIP files that have 
>> entries in the CEN that can't be used for files in a file system.
>> 
>> 
>> Mach5 tiers 1 through 3 have been run without any errors encountered .
>> 
>> Best,
>> Lance
>
> Lance Andersen 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 six additional 
> commits since the last revision:
> 
>  - Add Impl Note to Zip FS module-info
>  - Merge
>  - Add missing Copyright header and address minor comments
>  - Address missing linefeed  after package name
>  - Address overzelous intellij import update
>  - Patch to address JDK-8251329

src/jdk.zipfs/share/classes/module-info.java line 49:

> 47:  *
> 48:  * @implNote The Zip File System will throw a ZipException when opening an
> 49:  * existing Zip file that contains Zip entries with "." or ".." in its 
> name elements.

Hello Lance, reading this sentence adds a bit of confusion since it uses the 
word "contains". Had I not known the implemenation details, this sentence would 
have made me think zip file with name elements of the form `foo.bar` or 
`hello..`  would also be rejected since these name elements "contain" `.` or 
`..`

Do you think we should change the word to something like "The Zip File System 
will throw a ZipException when opening an existing Zip file that has "." or 
".." named entries"?

-

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


Re: RFR: 8251329: (zipfs) Files.walkFileTree walks infinitely if zip has dir named "." inside [v4]

2021-07-29 Thread Lance Andersen
> Hi,
> 
> As discussed in the 
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-July/079621.html 
> thread, this is the revised patch to address the use of '.' and '..' within 
> Zip FS
> 
> Zip FS needs to use "." and ".." as links to the current and parent 
> directories and cannot reliably support entries that have "." and ".." as 
> name elements.  This patch updates Zip Fs  to reject ZIP files that have 
> entries in the CEN that can't be used for files in a file system.
> 
> 
> Mach5 tiers 1 through 3 have been run without any errors encountered .
> 
> Best,
> Lance

Lance Andersen has updated the pull request incrementally with one additional 
commit since the last revision:

  Remove uneeded variable

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4900/files
  - new: https://git.openjdk.java.net/jdk/pull/4900/files/22731ac3..67c726af

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4900=03
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4900=02-03

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

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


Re: RFR: 8265891: (ch) InputStream returned by Channels.newInputStream should override transferTo [v6]

2021-07-29 Thread Brian Burkhalter
On Thu, 29 Jul 2021 19:56:40 GMT, Markus KARG 
 wrote:

>> It's actually a matter of convention but I think it can remain as it is.
>
> Ok for me, otherwise just clearly tell me and I do change it.

I think you can leave it unless someone else thinks otherwise.

-

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


RFR: 8271225: Add floorDivExact() method to java.lang.[Strict]Math

2021-07-29 Thread Brian Burkhalter
Add methods `floorDivExact(int,int)` and `floorDivExact(long,long)` to 
`java.lang.Math` and `java.lang.StrictMath`.

-

Commit messages:
 - 8271225: Add floorDivExact() method to java.lang.[Strict]Math

Changes: https://git.openjdk.java.net/jdk/pull/4941/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4941=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8271225
  Stats: 245 lines in 3 files changed: 236 ins; 0 del; 9 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4941.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4941/head:pull/4941

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


Re: RFR: 8271225: Add floorDivExact() method to java.lang.[Strict]Math

2021-07-29 Thread Brian Burkhalter
On Thu, 29 Jul 2021 23:27:32 GMT, Brian Burkhalter  wrote:

> Add methods `floorDivExact(int,int)` and `floorDivExact(long,long)` to 
> `java.lang.Math` and `java.lang.StrictMath`.

The `floorDivExact()` methods are identical to their `floorDiv()` counterparts 
aside from that they throw an `ArithmeticException` when the dividend is 
`MIN_VALUE` and the divisor is `-1`. These methods behave with respect to the 
methods `floorDiv()` as the methods `divideExact()` behave with respect to the 
division operator `/`.

-

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


Re: RFR: JDK-8256844: Make NMT late-initializable

2021-07-29 Thread Coleen Phillimore
On Thu, 22 Jul 2021 14:58:47 GMT, Thomas Stuefe  wrote:

> Short: this patch makes NMT available in custom-launcher scenarios and during 
> gtests. It simplifies NMT initialization. It adds a lot of NMT-specific 
> testing, cleans them up and makes them sideeffect-free.
> 
> -
> 
> NMT continues to be an extremely useful tool for SAP to tackle memory 
> problems in the JVM.
> 
> However, NMT is of limited use due to the following restrictions:
> 
> - NMT cannot be used if the hotspot is embedded into a custom launcher unless 
> the launcher actively cooperates. Just creating and invoking the JVM is not 
> enough, it needs to do some steps prior to loading the hotspot. This 
> limitation is not well known (nor, do I believe, documented). Many products 
> don't do this, e.g., you cannot use NMT with IntelliJ. For us at SAP this 
> problem limits NMT usefulness greatly since our VMs are often embedded into 
> custom launchers and modifying every launcher is impossible.
> - Worse, if that custom launcher links the libjvm *statically* there is just 
> no way to activate NMT at all. This is the reason NMT cannot be used in the 
> `gtestlauncher`.
> - Related to that is that we cannot pass NMT options via `JAVA_TOOL_OPTIONS` 
> and `-XX:Flags=`.
> - The fact that NMT cannot be used in gtests is really a pity since it would 
> allow us to both test NMT itself more rigorously and check for memory leaks 
> while testing other stuff.
> 
> The reason for all this is that NMT initialization happens very early, on the 
> first call to `os::malloc()`. And those calls happen already during dynamic 
> C++ initialization - a long time before the VM gets around parsing arguments. 
> So, regular VM argument parsing is too late to parse NMT arguments.
> 
> The current solution is to pass NMT arguments via a specially prepared 
> environment variable: `NMT_LEVEL_=`. That environment 
> variable has to be set by the embedding launcher, before it loads the libjvm. 
> Since its name contains the PID, we cannot even set that variable in the 
> shell before starting the launcher.
> 
> All that means that every launcher needs to especially parse and process the 
> NMT arguments given at the command line (or via whatever method) and prepare 
> the environment variable. `java` itself does this. This only works before the 
> libjvm.so is loaded, before its dynamic C++ initialization. For that reason, 
> it does not work if the launcher links statically against the hotspot, since 
> in that case C++ initialization of the launcher and hotspot are folded into 
> one phase with no possibility of executing code beforehand.
> 
> And since it bypasses argument handling in the VM, it bypasses a number of 
> argument processing ways, e.g., `JAVA_TOOL_OPTIONS`.
> 
> --
> 
> This patch fixes these shortcomings by making NMT late-initializable: it can 
> now be initialized after normal VM argument parsing, like all other parts of 
> the VM. This greatly simplifies NMT initialization and makes it work 
> automagically for every third party launcher, as well as within our gtests.
> 
> The glaring problem with late-initializing NMT is the NMT malloc headers. If 
> we rule out just always having them (unacceptable in terms of memory 
> overhead), there is no safe way to determine, in os::free(), if an allocation 
> came from before or after NMT initialization ran, and therefore what to do 
> with its malloc headers. For a more extensive explanation, please see the 
> comment block `nmtPreInit.hpp` and the discussion with @kimbarrett and 
> @zhengyu123 in the JBS comment section.
> 
> The heart of this patch is a new way to track early, pre-NMT-init 
> allocations. These are tracked via a lookup table. This was a suggestion by 
> Kim and it worked out well.
> 
> Changes in detail:
> 
> - pre-NMT-init handling:
>   - the new files `nmtPreInit.hpp/cpp` take case of NMT pre-init 
> handling. They contain a small global lookup table managing C-heap blocks 
> allocated in the pre-NMT-init phase.
>   - `os::malloc()/os::realloc()/os::free()` defer to this code before 
> doing anything else.
>   - Please see the extensive comment block at the start of 
> `nmtPreinit.hpp` explaining the details.
> 
> - Changes to NMT:
>   - Before, NMT initialization was spread over two phases, `initialize()` 
> and `late_initialize()`. Those were merged into one and simplified - there is 
> only one initialization now which happens after argument parsing.
>   - Minor changes were needed for the `NMT_TrackingLevel` enum - to 
> simplify code, I changed NMT_unknown to be numerically 0. A new comment block 
> in `nmtCommon.hpp` now clearly specifies what's what, including allowed level 
> state transitions.
>   - New utility functions to translate tracking level from/to strings 
> added to `NMTUtil`
>   - NMT has never been able to handle virtual memory allocations before 
> initialization, which is fine since os::reserve_memory() is not 

Re: RFR: 8251329: (zipfs) Files.walkFileTree walks infinitely if zip has dir named "." inside [v3]

2021-07-29 Thread Naoto Sato
On Thu, 29 Jul 2021 18:21:07 GMT, Lance Andersen  wrote:

>> Hi,
>> 
>> As discussed in the 
>> https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-July/079621.html 
>> thread, this is the revised patch to address the use of '.' and '..' within 
>> Zip FS
>> 
>> Zip FS needs to use "." and ".." as links to the current and parent 
>> directories and cannot reliably support entries that have "." and ".." as 
>> name elements.  This patch updates Zip Fs  to reject ZIP files that have 
>> entries in the CEN that can't be used for files in a file system.
>> 
>> 
>> Mach5 tiers 1 through 3 have been run without any errors encountered .
>> 
>> Best,
>> Lance
>
> Lance Andersen 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 six additional 
> commits since the last revision:
> 
>  - Add Impl Note to Zip FS module-info
>  - Merge
>  - Add missing Copyright header and address minor comments
>  - Address missing linefeed  after package name
>  - Address overzelous intellij import update
>  - Patch to address JDK-8251329

src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java line 1622:

> 1620: index++;
> 1621: }
> 1622: return dotOrDotDotFound;

Could simply return `false` here, and eliminate the local variable 
`dotOrDotDotFound`.

-

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


Re: RFR: Merge jdk17 [v2]

2021-07-29 Thread Jesper Wilhelmsson
> Forwardport JDK 17 -> JDK 18

Jesper Wilhelmsson has updated the pull request with a new target base due to a 
merge or a rebase. The pull request now contains 358 commits:

 - Merge
 - 8271396: Spelling errors
   
   Reviewed-by: tschatzl, chegar, iris, psadhukhan, cjplummer
 - 8268019: C2: assert(no_dead_loop) failed: dead loop detected
   
   Reviewed-by: kvn, thartmann
 - 8270886: Crash in PhaseIdealLoop::verify_strip_mined_scheduling
   
   Reviewed-by: kvn, thartmann
 - Merge
 - 8225082: Remove IdenTrust certificate that is expiring in September 2021
   
   Reviewed-by: shade, mullan
 - 8269851: OperatingSystemMXBean getProcessCpuLoad reports incorrect process 
cpu usage in containers
   
   Co-authored-by: Severin Gehwolf 
   Reviewed-by: sgehwolf
 - 8271353: PerfDataManager::destroy crashes in VM_Exit
   
   Reviewed-by: dholmes, stuefe, minqi
 - 8270061: Change parameter order of ResourceHashtable
   
   Reviewed-by: coleenp, stuefe
 - 8270925: replay dump using CICrashAt does not include inlining data
   
   Reviewed-by: kvn, thartmann
 - ... and 348 more: 
https://git.openjdk.java.net/jdk/compare/286d3136...ead1faac

-

Changes: https://git.openjdk.java.net/jdk/pull/4939/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4939=01
  Stats: 61561 lines in 1328 files changed: 29006 ins; 26122 del; 6433 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4939.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4939/head:pull/4939

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


Integrated: Merge jdk17

2021-07-29 Thread Jesper Wilhelmsson
On Thu, 29 Jul 2021 21:05:31 GMT, Jesper Wilhelmsson  
wrote:

> Forwardport JDK 17 -> JDK 18

This pull request has now been integrated.

Changeset: 048fb2cb
Author:Jesper Wilhelmsson 
URL:   
https://git.openjdk.java.net/jdk/commit/048fb2cb179234c403ee01ddc4acbdc4795c08ee
Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod

Merge

-

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


RFR: Merge jdk17

2021-07-29 Thread Jesper Wilhelmsson
Forwardport JDK 17 -> JDK 18

-

Commit messages:
 - Merge
 - 8271489: (doc) Clarify Filter Factory example

The webrevs contain the adjustments done while merging with regards to each 
parent branch:
 - master: https://webrevs.openjdk.java.net/?repo=jdk=4939=00.0
 - jdk17: https://webrevs.openjdk.java.net/?repo=jdk=4939=00.1

Changes: https://git.openjdk.java.net/jdk/pull/4939/files
  Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4939.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4939/head:pull/4939

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


[jdk17] Integrated: 8271489: (doc) Clarify Filter Factory example

2021-07-29 Thread Roger Riggs
On Thu, 29 Jul 2021 19:05:58 GMT, Roger Riggs  wrote:

> Improve the clarity of comments in the ObjectInputFilter FilterInThread 
> example.

This pull request has now been integrated.

Changeset: 286d3136
Author:Roger Riggs 
URL:   
https://git.openjdk.java.net/jdk17/commit/286d31363551b00c4b3f50f5ee388f8e7875d0a1
Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod

8271489: (doc) Clarify Filter Factory example

Reviewed-by: iris, kcr, naoto, bpb

-

PR: https://git.openjdk.java.net/jdk17/pull/293


Re: RFR: 8265891: (ch) InputStream returned by Channels.newInputStream should override transferTo [v6]

2021-07-29 Thread Markus KARG
On Thu, 29 Jul 2021 16:42:35 GMT, Brian Burkhalter  wrote:

>> The JavaDocs in `InputStream::transferTo` *cleary* tell the caller that 
>> there is **no** guarantee of *any* specific behavior in that particular 
>> case: 
>>>The behavior for the case where the input and/or output stream is 
>>>asynchronously closed, or the thread interrupted during the transfer, is 
>>>highly input and output stream specific, and therefore not specified.
>
> Good point.

:-)

>> This is a question of style and personal likes. Code maintenance is much 
>> easier if variables are defined *near* to its first use, not *far away* (as 
>> in the Pascal or C language). If the reviewers want me to change it, I will 
>> do it.
>
> It's actually a matter of convention but I think it can remain as it is.

Ok for me, otherwise just clearly tell me and I do change it.

-

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


Re: [jdk17] RFR: 8271489: (doc) Clarify Filter Factory example

2021-07-29 Thread Brian Burkhalter
On Thu, 29 Jul 2021 19:05:58 GMT, Roger Riggs  wrote:

> Improve the clarity of comments in the ObjectInputFilter FilterInThread 
> example.

Marked as reviewed by bpb (Reviewer).

-

PR: https://git.openjdk.java.net/jdk17/pull/293


Re: [jdk17] RFR: 8271489: (doc) Clarify Filter Factory example

2021-07-29 Thread Naoto Sato
On Thu, 29 Jul 2021 19:05:58 GMT, Roger Riggs  wrote:

> Improve the clarity of comments in the ObjectInputFilter FilterInThread 
> example.

Marked as reviewed by naoto (Reviewer).

-

PR: https://git.openjdk.java.net/jdk17/pull/293


Re: [jdk17] RFR: 8271489: (doc) Clarify Filter Factory example

2021-07-29 Thread Iris Clark
On Thu, 29 Jul 2021 19:05:58 GMT, Roger Riggs  wrote:

> Improve the clarity of comments in the ObjectInputFilter FilterInThread 
> example.

Marked as reviewed by iris (Reviewer).

-

PR: https://git.openjdk.java.net/jdk17/pull/293


Re: [jdk17] RFR: 8271489: (doc) Clarify Filter Factory example

2021-07-29 Thread Kevin Rushforth
On Thu, 29 Jul 2021 19:05:58 GMT, Roger Riggs  wrote:

> Improve the clarity of comments in the ObjectInputFilter FilterInThread 
> example.

Marked as reviewed by kcr (Author).

-

PR: https://git.openjdk.java.net/jdk17/pull/293


[jdk17] RFR: 8271489: (doc) Clarify Filter Factory example

2021-07-29 Thread Roger Riggs
Improve the clarity of comments in the ObjectInputFilter FilterInThread example.

-

Commit messages:
 - 8271489: (doc) Clarify Filter Factory example

Changes: https://git.openjdk.java.net/jdk17/pull/293/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk17=293=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8271489
  Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod
  Patch: https://git.openjdk.java.net/jdk17/pull/293.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk17 pull/293/head:pull/293

PR: https://git.openjdk.java.net/jdk17/pull/293


Re: RFR: 8251329: (zipfs) Files.walkFileTree walks infinitely if zip has dir named "." inside [v3]

2021-07-29 Thread Lance Andersen
> Hi,
> 
> As discussed in the 
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-July/079621.html 
> thread, this is the revised patch to address the use of '.' and '..' within 
> Zip FS
> 
> Zip FS needs to use "." and ".." as links to the current and parent 
> directories and cannot reliably support entries that have "." and ".." as 
> name elements.  This patch updates Zip Fs  to reject ZIP files that have 
> entries in the CEN that can't be used for files in a file system.
> 
> 
> Mach5 tiers 1 through 3 have been run without any errors encountered .
> 
> Best,
> Lance

Lance Andersen 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 six additional 
commits since the last revision:

 - Add Impl Note to Zip FS module-info
 - Merge
 - Add missing Copyright header and address minor comments
 - Address missing linefeed  after package name
 - Address overzelous intellij import update
 - Patch to address JDK-8251329

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4900/files
  - new: https://git.openjdk.java.net/jdk/pull/4900/files/2265ffe1..22731ac3

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4900=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4900=01-02

  Stats: 3049 lines in 76 files changed: 1357 ins; 359 del; 1333 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4900.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4900/head:pull/4900

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


[jdk17] Withdrawn: 8271489: (doc) Clarify Filter Factory example

2021-07-29 Thread Roger Riggs
On Thu, 29 Jul 2021 16:36:21 GMT, Roger Riggs  wrote:

> Improve the clarity of comments in the ObjectInputFilter FilterInThread 
> example.

This pull request has been closed without being integrated.

-

PR: https://git.openjdk.java.net/jdk17/pull/292


Re: RFR: 8265891: (ch) InputStream returned by Channels.newInputStream should override transferTo [v6]

2021-07-29 Thread Brian Burkhalter
On Thu, 29 Jul 2021 08:12:23 GMT, Markus KARG 
 wrote:

>> src/java.base/share/classes/sun/nio/ch/ChannelInputStream.java line 179:
>> 
>>> 177: for (long n = srcSize - srcPos; bytesWritten < n;)
>>> 178: bytesWritten += src.transferTo(srcPos + bytesWritten, 
>>> Long.MAX_VALUE, dest);
>>> 179: return bytesWritten;
>> 
>> If `src` is extended at an inconvenient point in time, for example between 
>> the return from a call to `src.transferTo()` that makes `bytesWritten < n` 
>> false and the evaluation of that terminating condition, then it appears that 
>> not all the content of `src` will be transferred to `dest`. I am not however 
>> sure that this violates any contract but it could be a behavioral change 
>> from the existing implementation.
>
> The JavaDocs in `InputStream::transferTo` *cleary* tell the caller that there 
> is **no** guarantee of *any* specific behavior in that particular case: 
>>The behavior for the case where the input and/or output stream is 
>>asynchronously closed, or the thread interrupted during the transfer, is 
>>highly input and output stream specific, and therefore not specified.

Good point.

>> src/java.base/share/classes/sun/nio/ch/ChannelOutputStream.java line 85:
>> 
>>> 83: private byte[] bs;   // Invoker's previous array
>>> 84: private byte[] b1;
>>> 85: 
>> 
>> It might be better to put the field declarations at the beginning of the 
>> class before the static methods.
>
> This is a question of style and personal likes. Code maintenance is much 
> easier if variables are defined *near* to its first use, not *far away* (as 
> in the Pascal or C language). If the reviewers want me to change it, I will 
> do it.

It's actually a matter of convention but I think it can remain as it is.

>> test/jdk/sun/nio/ch/ChannelInputStream/TransferTo.java line 67:
>> 
>>> 65: test(readableByteChannelInput(), defaultOutput());
>>> 66: }
>>> 67: 
>> 
>> This test looks like it's doing what it's supposed to do. I'm not asking to 
>> change it, but I think using TestNG might have given a simpler result with 
>> less work. For example, there could be one `DataProvider` supplying inputs 
>> which feed a `@Test` which expects an NPE, and another `DataProvider` 
>> supplying input-output pairs which feeds a `@Test` which validates the 
>> functionality.
>
> No doubt, using modern tools would have spared me work, and indeed I would 
> have chosen JUnit or TestNG if there would be a clear commitment to that tool 
> *upfront*. For now, I see now use in reworking the code afterwards.

No need to rework it now.

-

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


[jdk17] RFR: 8271489: (doc) Clarify Filter Factory example

2021-07-29 Thread Roger Riggs
Improve the clarity of comments in the ObjectInputFilter FilterInThread example.

-

Commit messages:
 - 8271489: (doc) Clarify Filter Factory example
 - 8270398: Enhance canonicalization
 - 8270404: Better canonicalization
 - Merge
 - Merge
 - 8263531: Remove unused buffer int
 - 8262731: [macOS] Exception from "Printable.print" is swallowed during 
"PrinterJob.print"
 - 8269763: The JEditorPane is blank after JDK-8265167
 - 8265580: Enhanced style for RTF kit
 - 8265574: Improve handling of sheets
 - ... and 15 more: 
https://git.openjdk.java.net/jdk17/compare/c1304519...650e1561

Changes: https://git.openjdk.java.net/jdk17/pull/292/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk17=292=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8271489
  Stats: 1001 lines in 42 files changed: 625 ins; 181 del; 195 mod
  Patch: https://git.openjdk.java.net/jdk17/pull/292.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk17 pull/292/head:pull/292

PR: https://git.openjdk.java.net/jdk17/pull/292


Integrated: 8271396: Spelling errors

2021-07-29 Thread Emmanuel Bourg
On Wed, 3 Feb 2021 19:12:25 GMT, Emmanuel Bourg 
 wrote:

> This PR fixes the following spelling errors:
> 
>  choosen  -> chosen
>  commad   -> command
>  hiearchy -> hierarchy
>  leagacy  -> legacy
>  minium   -> minimum
>  subsytem -> subsystem
>  unamed   -> unnamed

This pull request has now been integrated.

Changeset: d09b0284
Author:Emmanuel Bourg 
Committer: Julia Boes 
URL:   
https://git.openjdk.java.net/jdk/commit/d09b028407ff9d0e8c2dfd9cc5d0dca19c4497e3
Stats: 103 lines in 34 files changed: 0 ins; 0 del; 103 mod

8271396: Spelling errors

Reviewed-by: tschatzl, chegar, iris, psadhukhan, cjplummer

-

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


Re: RFR: 8271396: Spelling errors [v2]

2021-07-29 Thread Emmanuel Bourg
On Wed, 28 Jul 2021 17:12:04 GMT, Emmanuel Bourg 
 wrote:

>> This PR fixes the following spelling errors:
>> 
>>  choosen  -> chosen
>>  commad   -> command
>>  hiearchy -> hierarchy
>>  leagacy  -> legacy
>>  minium   -> minimum
>>  subsytem -> subsystem
>>  unamed   -> unnamed
>
> Emmanuel Bourg 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 one additional 
> commit since the last revision:
> 
>   8271396: Fix spelling errors
>   
>choosen  -> chosen
>commad   -> command
>hiearchy -> hierarchy
>leagacy  -> legacy
>minium   -> minimum
>subsytem -> subsystem
>unamed   -> unnamed

Thank you, glad to land my first commit to OpenJDK

-

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


Re: RFR: 8271396: Spelling errors

2021-07-29 Thread Julia Boes
On Wed, 28 Jul 2021 17:23:51 GMT, Emmanuel Bourg 
 wrote:

>> @ebourg for future PRs please do not force push after the PR is out for 
>> review. Just push incremental commits normally. The Skara tooling will 
>> squash them all into a single commit.
>
> @kevinrushforth I'll do that, thank you for the hint

@ebourg Thanks for updating the copyright. If you integrate again, I can 
sponsor.

-

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


Re: [External] : Re: jpackage MacOS Notarization

2021-07-29 Thread Andy Herrick
sorry - code looks for certificate key starting with: "Developer ID 
Application: " +  in order to not 
have to put full user name in.  I missed that that with null user name 
that causes it to look for anything starting with "Developer ID 
Application: " (same thing with "Developer ID Installer: " for .pkg 
signing).  And macos  looks at the non-default keychains as well as the 
default ones when no keychain is specified.


/Andy

On 7/29/2021 10:00 AM, Daniel Peintner wrote:

Hi Andy,

Since I don't know your setup I did not put anything there.

'--mac-sign' is enough to use the defaults in my setup.

It looks for the signing keys installed on my machine that start with 
"Developer ID Application " similar to

'--mac-signing-key-user-name', 'Developer ID Application: '
etc.

If you want to test it you need to add your credentials which I do not 
know.


Hope this clarifies things,

-- Daniel



On Thu, Jul 29, 2021 at 3:29 PM Andy Herrick > wrote:


The 'build.gradle' in this branch has --mac-signing-key-user-name
commented out.


    installerOptions += [
    '--mac-sign',
    //
'--mac-s'SIGNING_KEY_USER_NAME'igning-key-user-name',
System.getenv('SIGNING_KEY_USER_NAME'),
    // '--mac-signing-keychain',
System.getenv('SIGNING_KEYCHAIN_PATH')
    ]


clearly this cannot work, I assume you were just trying things ...

What is the full name of the certificate you intended to use, what
keychain is it shown in "Keychain Access", and what are you normal
values for your environment variables: 'SIGNING_KEY_USER_NAME' and
'SIGNING_KEYCHAIN_PATH' ?

/Andy

On 7/29/2021 4:36 AM, Daniel Peintner wrote:

Kevin, Andy,

Thanks for your quick response.

Full support for notarization in jpackage was added in JDK
17. Can you
try an early access build of JDK 17 [1] and see if that works
for you?


I did try JDK17-ea-32 also with the same result.

Since I do understand it is difficult reproduce the problem I put
together a *very* simple test application which you can find in
the "non-modular" branch here:
https://github.com/danielpeintner/Java11Test/tree/non-modular



It is a gradle project. It uses Java 11 to run but in
build.gradle on line#83[1] one can set the jpackage location
(JDK17-ea-32 in this case).

The process is as follows
* ./gradlew build
* ./gradlew jpackage    // creates the dmg/pkg in folder
build/jpackage
* afterwards Apple notarization process can be started

Note: notarization of dmg or pkg lead to the same failure.
See [2] for the full log w.r.t. pkg.

I hope this helps you to be able to reproduce the issue.

Thanks for your investigations!

-- Daniel

[1]

https://github.com/danielpeintner/Java11Test/blob/6e5f34b1a0ba9c1e8ba1f6b15d6915237d8f5b7e/build.gradle#L83


[2]

https://osxapps-ssl.itunes.apple.com/itunes-assets/Enigma115/v4/90/4a/11/904a111c-01c7-ecc1-466c-40e7e8a09c56/developer_log.json?accessKey=1627741411_2564804966498057981_aHPs%2Fq9bzxGsY5Kd46U1QyWR8hmHJjLJLbUPpbvBqinIjiylTLsQy1APCJPkNN2w%2BZknT9OCl6zkzAyUm99EIBrm6tnOkZoWiwNG7TyukwCtAnIh%2FGpNAkLYfBpyDYjMaf7jQq8JekzxjYewhFuPDcJufWNrfuEX%2FN6zZoyz73I%3D




-- Kevin

[1] https://jdk.java.net/17



On 7/28/2021 8:27 AM, Daniel Peintner wrote:
> All,
>
> I am trying to notarize an app (built with jpackage) for MacOS.
>
> jpackage at first *seems* to properly sign all resources
with the available
> --mac-sign options et cetera.
>
> Having said that, there are still remaining issues
> 1. The app cannot be properly installed
>     (without hacks like xattr -d com.apple.quarantine
/Applications/myAPP.app
> ).
> 

Re: [External] : Re: jpackage MacOS Notarization

2021-07-29 Thread Daniel Peintner
Hi Andy,

Since I don't know your setup I did not put anything there.

'--mac-sign' is enough to use the defaults in my setup.

It looks for the signing keys installed on my machine that start with
"Developer
ID Application " similar to
'--mac-signing-key-user-name', 'Developer ID Application: '
etc.

If you want to test it you need to add your credentials which I do not know.

Hope this clarifies things,

-- Daniel



On Thu, Jul 29, 2021 at 3:29 PM Andy Herrick 
wrote:

> The 'build.gradle' in this branch has --mac-signing-key-user-name
> commented out.
>
> installerOptions += [
> '--mac-sign',
> // '--mac-s'SIGNING_KEY_USER_NAME'igning-key-user-name',
> System.getenv('SIGNING_KEY_USER_NAME'),
> // '--mac-signing-keychain',
> System.getenv('SIGNING_KEYCHAIN_PATH')
> ]
>
> clearly this cannot work, I assume you were just trying things ...
>
> What is the full name of the certificate you intended to use, what
> keychain is it shown in "Keychain Access", and what are you normal values
> for your environment variables: 'SIGNING_KEY_USER_NAME' and
> 'SIGNING_KEYCHAIN_PATH' ?
>
> /Andy
> On 7/29/2021 4:36 AM, Daniel Peintner wrote:
>
> Kevin, Andy,
>
> Thanks for your quick response.
>
> Full support for notarization in jpackage was added in JDK 17. Can you
>> try an early access build of JDK 17 [1] and see if that works for you?
>>
>
> I did try JDK17-ea-32 also with the same result.
>
> Since I do understand it is difficult reproduce the problem I put together
> a *very* simple test application which you can find in the "non-modular"
> branch here:
> https://github.com/danielpeintner/Java11Test/tree/non-modular
> 
>
> It is a gradle project. It uses Java 11 to run but in build.gradle on
> line#83 [1] one can set the jpackage location (JDK17-ea-32 in this case).
>
> The process is as follows
> * ./gradlew build
> * ./gradlew jpackage// creates the dmg/pkg in folder build/jpackage
> * afterwards Apple notarization process can be started
>
> Note: notarization of dmg or pkg lead to the same failure.
> See [2] for the full log w.r.t. pkg.
>
> I hope this helps you to be able to reproduce the issue.
>
> Thanks for your investigations!
>
> -- Daniel
>
> [1]
> https://github.com/danielpeintner/Java11Test/blob/6e5f34b1a0ba9c1e8ba1f6b15d6915237d8f5b7e/build.gradle#L83
> 
> [2]
> https://osxapps-ssl.itunes.apple.com/itunes-assets/Enigma115/v4/90/4a/11/904a111c-01c7-ecc1-466c-40e7e8a09c56/developer_log.json?accessKey=1627741411_2564804966498057981_aHPs%2Fq9bzxGsY5Kd46U1QyWR8hmHJjLJLbUPpbvBqinIjiylTLsQy1APCJPkNN2w%2BZknT9OCl6zkzAyUm99EIBrm6tnOkZoWiwNG7TyukwCtAnIh%2FGpNAkLYfBpyDYjMaf7jQq8JekzxjYewhFuPDcJufWNrfuEX%2FN6zZoyz73I%3D
> 
>
>
>>
>>
>> -- Kevin
>>
>> [1] https://jdk.java.net/17
>> 
>>
>> On 7/28/2021 8:27 AM, Daniel Peintner wrote:
>> > All,
>> >
>> > I am trying to notarize an app (built with jpackage) for MacOS.
>> >
>> > jpackage at first *seems* to properly sign all resources with the
>> available
>> > --mac-sign options et cetera.
>> >
>> > Having said that, there are still remaining issues
>> > 1. The app cannot be properly installed
>> > (without hacks like xattr -d com.apple.quarantine
>> /Applications/myAPP.app
>> > ).
>> > 2. I am also not able to properly notarize the app.
>> >
>> > According to online resources there seem to exist issues in *past* about
>> > notarization but according to [1, 2] the issues are fixed.
>> >
>> > As mentioned, I still have issues :-(
>> > Am I really the only one still having problems?
>> >
>> > Java Version: AdoptOpenJDK-16.0.1+9 (tried Oracle JDK also without
>> success)
>> >
>> > The issue seems to boil down to 2 errors (attached the error log from
>> Apple
>> > notarization process).
>> > * app Folder
>> > * libjli.dylib
>> >
>> > I thought I better ask first on the mailing list before creating an
>> actual
>> > bug report.
>> >
>> > Note1: I used to use the *old* javapackager that 

Re: [External] : Re: jpackage MacOS Notarization

2021-07-29 Thread Andy Herrick
The 'build.gradle' in this branch has --mac-signing-key-user-name 
commented out.



    installerOptions += [
    '--mac-sign',
    // 
'--mac-s'SIGNING_KEY_USER_NAME'igning-key-user-name', 
System.getenv('SIGNING_KEY_USER_NAME'),
    // '--mac-signing-keychain', 
System.getenv('SIGNING_KEYCHAIN_PATH')

    ]


clearly this cannot work, I assume you were just trying things ...

What is the full name of the certificate you intended to use, what 
keychain is it shown in "Keychain Access", and what are you normal 
values for your environment variables: 'SIGNING_KEY_USER_NAME' and 
'SIGNING_KEYCHAIN_PATH' ?


/Andy

On 7/29/2021 4:36 AM, Daniel Peintner wrote:

Kevin, Andy,

Thanks for your quick response.

Full support for notarization in jpackage was added in JDK 17. Can
you
try an early access build of JDK 17 [1] and see if that works for you?


I did try JDK17-ea-32 also with the same result.

Since I do understand it is difficult reproduce the problem I put 
together a *very* simple test application which you can find in the 
"non-modular" branch here:
https://github.com/danielpeintner/Java11Test/tree/non-modular 



It is a gradle project. It uses Java 11 to run but in build.gradle on 
line#83[1] one can set the jpackage location (JDK17-ea-32 in this case).


The process is as follows
* ./gradlew build
* ./gradlew jpackage    // creates the dmg/pkg in folder build/jpackage
* afterwards Apple notarization process can be started

Note: notarization of dmg or pkg lead to the same failure.
See [2] for the full log w.r.t. pkg.

I hope this helps you to be able to reproduce the issue.

Thanks for your investigations!

-- Daniel

[1] 
https://github.com/danielpeintner/Java11Test/blob/6e5f34b1a0ba9c1e8ba1f6b15d6915237d8f5b7e/build.gradle#L83 

[2] 
https://osxapps-ssl.itunes.apple.com/itunes-assets/Enigma115/v4/90/4a/11/904a111c-01c7-ecc1-466c-40e7e8a09c56/developer_log.json?accessKey=1627741411_2564804966498057981_aHPs%2Fq9bzxGsY5Kd46U1QyWR8hmHJjLJLbUPpbvBqinIjiylTLsQy1APCJPkNN2w%2BZknT9OCl6zkzAyUm99EIBrm6tnOkZoWiwNG7TyukwCtAnIh%2FGpNAkLYfBpyDYjMaf7jQq8JekzxjYewhFuPDcJufWNrfuEX%2FN6zZoyz73I%3D 




-- Kevin

[1] https://jdk.java.net/17



On 7/28/2021 8:27 AM, Daniel Peintner wrote:
> All,
>
> I am trying to notarize an app (built with jpackage) for MacOS.
>
> jpackage at first *seems* to properly sign all resources with
the available
> --mac-sign options et cetera.
>
> Having said that, there are still remaining issues
> 1. The app cannot be properly installed
>     (without hacks like xattr -d com.apple.quarantine
/Applications/myAPP.app
> ).
> 2. I am also not able to properly notarize the app.
>
> According to online resources there seem to exist issues in
*past* about
> notarization but according to [1, 2] the issues are fixed.
>
> As mentioned, I still have issues :-(
> Am I really the only one still having problems?
>
> Java Version: AdoptOpenJDK-16.0.1+9 (tried Oracle JDK also
without success)
>
> The issue seems to boil down to 2 errors (attached the error log
from Apple
> notarization process).
> * app Folder
> * libjli.dylib
>
> I thought I better ask first on the mailing list before creating
an actual
> bug report.
>
> Note1: I used to use the *old* javapackager that worked with the
same
> signature/credentials.
> Note2: running jpackage without --mac-sign options causes many
more errors
> in notarization (Hence, jpackage signs most resources but fails
with some)
>
> Any help / hint appreciated.
>
> Thanks,
>
> -- Daniel
>
> [1] https://bugs.openjdk.java.net/browse/JDK-8257488

> [2] https://bugs.openjdk.java.net/browse/JDK-8251892

Re: jpackage MacOS Notarization

2021-07-29 Thread Daniel Peintner
Kevin, Andy,

Thanks for your quick response.

Full support for notarization in jpackage was added in JDK 17. Can you
> try an early access build of JDK 17 [1] and see if that works for you?
>

I did try JDK17-ea-32 also with the same result.

Since I do understand it is difficult reproduce the problem I put together
a *very* simple test application which you can find in the "non-modular"
branch here:
https://github.com/danielpeintner/Java11Test/tree/non-modular

It is a gradle project. It uses Java 11 to run but in build.gradle on
line#83 [1] one can set the jpackage location (JDK17-ea-32 in this case).

The process is as follows
* ./gradlew build
* ./gradlew jpackage// creates the dmg/pkg in folder build/jpackage
* afterwards Apple notarization process can be started

Note: notarization of dmg or pkg lead to the same failure.
See [2] for the full log w.r.t. pkg.

I hope this helps you to be able to reproduce the issue.

Thanks for your investigations!

-- Daniel

[1]
https://github.com/danielpeintner/Java11Test/blob/6e5f34b1a0ba9c1e8ba1f6b15d6915237d8f5b7e/build.gradle#L83
[2]
https://osxapps-ssl.itunes.apple.com/itunes-assets/Enigma115/v4/90/4a/11/904a111c-01c7-ecc1-466c-40e7e8a09c56/developer_log.json?accessKey=1627741411_2564804966498057981_aHPs%2Fq9bzxGsY5Kd46U1QyWR8hmHJjLJLbUPpbvBqinIjiylTLsQy1APCJPkNN2w%2BZknT9OCl6zkzAyUm99EIBrm6tnOkZoWiwNG7TyukwCtAnIh%2FGpNAkLYfBpyDYjMaf7jQq8JekzxjYewhFuPDcJufWNrfuEX%2FN6zZoyz73I%3D


>
>
> -- Kevin
>
> [1] https://jdk.java.net/17
>
> On 7/28/2021 8:27 AM, Daniel Peintner wrote:
> > All,
> >
> > I am trying to notarize an app (built with jpackage) for MacOS.
> >
> > jpackage at first *seems* to properly sign all resources with the
> available
> > --mac-sign options et cetera.
> >
> > Having said that, there are still remaining issues
> > 1. The app cannot be properly installed
> > (without hacks like xattr -d com.apple.quarantine
> /Applications/myAPP.app
> > ).
> > 2. I am also not able to properly notarize the app.
> >
> > According to online resources there seem to exist issues in *past* about
> > notarization but according to [1, 2] the issues are fixed.
> >
> > As mentioned, I still have issues :-(
> > Am I really the only one still having problems?
> >
> > Java Version: AdoptOpenJDK-16.0.1+9 (tried Oracle JDK also without
> success)
> >
> > The issue seems to boil down to 2 errors (attached the error log from
> Apple
> > notarization process).
> > * app Folder
> > * libjli.dylib
> >
> > I thought I better ask first on the mailing list before creating an
> actual
> > bug report.
> >
> > Note1: I used to use the *old* javapackager that worked with the same
> > signature/credentials.
> > Note2: running jpackage without --mac-sign options causes many more
> errors
> > in notarization (Hence, jpackage signs most resources but fails with
> some)
> >
> > Any help / hint appreciated.
> >
> > Thanks,
> >
> > -- Daniel
> >
> > [1] https://bugs.openjdk.java.net/browse/JDK-8257488
> > [2] https://bugs.openjdk.java.net/browse/JDK-8251892
> >
> >
> >
> > ## APPLE LOG ##
> >
> > {
> >"logFormatVersion": 1,
> >"jobId": "...",
> >"status": "Invalid",
> >"statusSummary": "Archive contains critical validation errors",
> >"statusCode": 4000,
> >"archiveFilename": "myAPP-21.07.28.dmg",
> >"uploadDate": "2021-07-28T14:31:24Z",
> >"sha256": "...",
> >"ticketContents": null,
> >"issues": [
> >  {
> >"severity": "error",
> >"code": null,
> >"path": "myAPP-21.07.28.dmg/myAPP.app/Contents/MacOS/myAPP",
> >"message": "The signature of the binary is invalid.",
> >"docUrl": null,
> >"architecture": "x86_64"
> >  },
> >  {
> >"severity": "error",
> >"code": null,
> >"path":
> "myAPP-21.07.28.dmg/myAPP.app/Contents/runtime/Contents/MacOS/libjli.dylib",
> >"message": "The signature of the binary is invalid.",
> >"docUrl": null,
> >"architecture": "x86_64"
> >  }
> >]
> > }
>
>


Re: RFR: 8265891: (ch) InputStream returned by Channels.newInputStream should override transferTo [v6]

2021-07-29 Thread Markus KARG
On Mon, 26 Jul 2021 23:59:05 GMT, Brian Burkhalter  wrote:

>> Markus KARG has refreshed the contents of this pull request, and previous 
>> commits have been removed. The incremental views will show differences 
>> compared to the previous content of the PR.
>
> src/java.base/share/classes/sun/nio/ch/ChannelInputStream.java line 179:
> 
>> 177: for (long n = srcSize - srcPos; bytesWritten < n;)
>> 178: bytesWritten += src.transferTo(srcPos + bytesWritten, 
>> Long.MAX_VALUE, dest);
>> 179: return bytesWritten;
> 
> If `src` is extended at an inconvenient point in time, for example between 
> the return from a call to `src.transferTo()` that makes `bytesWritten < n` 
> false and the evaluation of that terminating condition, then it appears that 
> not all the content of `src` will be transferred to `dest`. I am not however 
> sure that this violates any contract but it could be a behavioral change from 
> the existing implementation.

The JavaDocs in `InputStream::transferTo` *cleary* tell the caller that there 
is **no** guarantee of *any* specific behavior in that particular case: 
>The behavior for the case where the input and/or output stream is 
>asynchronously closed, or the thread interrupted during the transfer, is 
>highly input and output stream specific, and therefore not specified.

-

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


Re: RFR: 8265891: (ch) InputStream returned by Channels.newInputStream should override transferTo [v6]

2021-07-29 Thread Markus KARG
On Tue, 27 Jul 2021 00:57:02 GMT, Brian Burkhalter  wrote:

>> Markus KARG has refreshed the contents of this pull request, and previous 
>> commits have been removed. The incremental views will show differences 
>> compared to the previous content of the PR.
>
> src/java.base/share/classes/sun/nio/ch/ChannelOutputStream.java line 85:
> 
>> 83: private byte[] bs;   // Invoker's previous array
>> 84: private byte[] b1;
>> 85: 
> 
> It might be better to put the field declarations at the beginning of the 
> class before the static methods.

This is a question of style and personal likes. Code maintenance is much easier 
if variables are defined *near* to its first use, not *far away* (as in the 
Pascal or C language). If the reviewers want me to change it, I will do it.

> test/jdk/sun/nio/ch/ChannelInputStream/TransferTo.java line 67:
> 
>> 65: test(readableByteChannelInput(), defaultOutput());
>> 66: }
>> 67: 
> 
> This test looks like it's doing what it's supposed to do. I'm not asking to 
> change it, but I think using TestNG might have given a simpler result with 
> less work. For example, there could be one `DataProvider` supplying inputs 
> which feed a `@Test` which expects an NPE, and another `DataProvider` 
> supplying input-output pairs which feeds a `@Test` which validates the 
> functionality.

No doubt, using modern tools would have spared me work, and indeed I would have 
chosen JUnit or TestNG if there would be a clear commitment to that tool 
*upfront*. For now, I see now use in reworking the code afterwards.

-

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


Re: RFR: JDK-8256844: Make NMT late-initializable

2021-07-29 Thread David Holmes
On Thu, 22 Jul 2021 14:58:47 GMT, Thomas Stuefe  wrote:

> Short: this patch makes NMT available in custom-launcher scenarios and during 
> gtests. It simplifies NMT initialization. It adds a lot of NMT-specific 
> testing, cleans them up and makes them sideeffect-free.
> 
> -
> 
> NMT continues to be an extremely useful tool for SAP to tackle memory 
> problems in the JVM.
> 
> However, NMT is of limited use due to the following restrictions:
> 
> - NMT cannot be used if the hotspot is embedded into a custom launcher unless 
> the launcher actively cooperates. Just creating and invoking the JVM is not 
> enough, it needs to do some steps prior to loading the hotspot. This 
> limitation is not well known (nor, do I believe, documented). Many products 
> don't do this, e.g., you cannot use NMT with IntelliJ. For us at SAP this 
> problem limits NMT usefulness greatly since our VMs are often embedded into 
> custom launchers and modifying every launcher is impossible.
> - Worse, if that custom launcher links the libjvm *statically* there is just 
> no way to activate NMT at all. This is the reason NMT cannot be used in the 
> `gtestlauncher`.
> - Related to that is that we cannot pass NMT options via `JAVA_TOOL_OPTIONS` 
> and `-XX:Flags=`.
> - The fact that NMT cannot be used in gtests is really a pity since it would 
> allow us to both test NMT itself more rigorously and check for memory leaks 
> while testing other stuff.
> 
> The reason for all this is that NMT initialization happens very early, on the 
> first call to `os::malloc()`. And those calls happen already during dynamic 
> C++ initialization - a long time before the VM gets around parsing arguments. 
> So, regular VM argument parsing is too late to parse NMT arguments.
> 
> The current solution is to pass NMT arguments via a specially prepared 
> environment variable: `NMT_LEVEL_=`. That environment 
> variable has to be set by the embedding launcher, before it loads the libjvm. 
> Since its name contains the PID, we cannot even set that variable in the 
> shell before starting the launcher.
> 
> All that means that every launcher needs to especially parse and process the 
> NMT arguments given at the command line (or via whatever method) and prepare 
> the environment variable. `java` itself does this. This only works before the 
> libjvm.so is loaded, before its dynamic C++ initialization. For that reason, 
> it does not work if the launcher links statically against the hotspot, since 
> in that case C++ initialization of the launcher and hotspot are folded into 
> one phase with no possibility of executing code beforehand.
> 
> And since it bypasses argument handling in the VM, it bypasses a number of 
> argument processing ways, e.g., `JAVA_TOOL_OPTIONS`.
> 
> --
> 
> This patch fixes these shortcomings by making NMT late-initializable: it can 
> now be initialized after normal VM argument parsing, like all other parts of 
> the VM. This greatly simplifies NMT initialization and makes it work 
> automagically for every third party launcher, as well as within our gtests.
> 
> The glaring problem with late-initializing NMT is the NMT malloc headers. If 
> we rule out just always having them (unacceptable in terms of memory 
> overhead), there is no safe way to determine, in os::free(), if an allocation 
> came from before or after NMT initialization ran, and therefore what to do 
> with its malloc headers. For a more extensive explanation, please see the 
> comment block `nmtPreInit.hpp` and the discussion with @kimbarrett and 
> @zhengyu123 in the JBS comment section.
> 
> The heart of this patch is a new way to track early, pre-NMT-init 
> allocations. These are tracked via a lookup table. This was a suggestion by 
> Kim and it worked out well.
> 
> Changes in detail:
> 
> - pre-NMT-init handling:
>   - the new files `nmtPreInit.hpp/cpp` take case of NMT pre-init 
> handling. They contain a small global lookup table managing C-heap blocks 
> allocated in the pre-NMT-init phase.
>   - `os::malloc()/os::realloc()/os::free()` defer to this code before 
> doing anything else.
>   - Please see the extensive comment block at the start of 
> `nmtPreinit.hpp` explaining the details.
> 
> - Changes to NMT:
>   - Before, NMT initialization was spread over two phases, `initialize()` 
> and `late_initialize()`. Those were merged into one and simplified - there is 
> only one initialization now which happens after argument parsing.
>   - Minor changes were needed for the `NMT_TrackingLevel` enum - to 
> simplify code, I changed NMT_unknown to be numerically 0. A new comment block 
> in `nmtCommon.hpp` now clearly specifies what's what, including allowed level 
> state transitions.
>   - New utility functions to translate tracking level from/to strings 
> added to `NMTUtil`
>   - NMT has never been able to handle virtual memory allocations before 
> initialization, which is fine since os::reserve_memory() is not