Re: RFR: 8301627: System.exit and Runtime.exit debug logging [v6]

2023-02-20 Thread Chris Hegarty
On Fri, 17 Feb 2023 17:27:50 GMT, Roger Riggs  wrote:

>> It can be difficult to find the cause of calls to 
>> `java.lang.System.exit(status)` and `Runtime.exit(status)` because the Java 
>> runtime exits.
>> The status value and stack trace are logged using the System Logger named 
>> `java.lang.Runtime` with message level `System.Logger.Level.DEBUG`.
>
> Roger Riggs 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 eight additional 
> commits since the last revision:
> 
>  - Move test to the java/lang/RuntimeTests directory.
>Added implNote to System.exit.
>A few javadoc updates for review comments.
>  - Merge branch 'master' into 8301627-log-system-exit
>  - Improve implNote for Runtime.exit() with review suggestions.
>  - Correct System.getLogger link
>  - Add an @implNote to Runtime.exit to describe the java.lang.Runtime logging.
>  - Added try/catch around lookup of logger so exceptions do not prevent 
> System.exit.
>Added test case with console logger (when java.util.logging) not present.
>Removed @implNote tag its not appropriate in implementation javadoc.
>Still looking into when and where the log configuration should be 
> described.
>  - Locate the System logger before taking the shutdown lock
>  - Add logging of calls to Runtime.exit to the system logger 
> "java.lang.Runtime".

I left one trivial comment relating to a typo, but otherwise LGTM.

src/java.base/share/classes/java/lang/Runtime.java line 160:

> 158:  *
> 159:  * @implNote
> 160:  * If the {@linkplain System#getLogger(String) the system logger} 
> for {@code java.lang.Runtime}

Trivially, remove the double "the the" here.

-

Marked as reviewed by chegar (Reviewer).

PR: https://git.openjdk.org/jdk/pull/12517


Re: RFR: 8301627: System.exit and Runtime.exit debug logging [v6]

2023-02-17 Thread Alan Bateman
On Fri, 17 Feb 2023 17:27:50 GMT, Roger Riggs  wrote:

>> It can be difficult to find the cause of calls to 
>> `java.lang.System.exit(status)` and `Runtime.exit(status)` because the Java 
>> runtime exits.
>> The status value and stack trace are logged using the System Logger named 
>> `java.lang.Runtime` with message level `System.Logger.Level.DEBUG`.
>
> Roger Riggs 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 eight additional 
> commits since the last revision:
> 
>  - Move test to the java/lang/RuntimeTests directory.
>Added implNote to System.exit.
>A few javadoc updates for review comments.
>  - Merge branch 'master' into 8301627-log-system-exit
>  - Improve implNote for Runtime.exit() with review suggestions.
>  - Correct System.getLogger link
>  - Add an @implNote to Runtime.exit to describe the java.lang.Runtime logging.
>  - Added try/catch around lookup of logger so exceptions do not prevent 
> System.exit.
>Added test case with console logger (when java.util.logging) not present.
>Removed @implNote tag its not appropriate in implementation javadoc.
>Still looking into when and where the log configuration should be 
> described.
>  - Locate the System logger before taking the shutdown lock
>  - Add logging of calls to Runtime.exit to the system logger 
> "java.lang.Runtime".

Marked as reviewed by alanb (Reviewer).

-

PR: https://git.openjdk.org/jdk/pull/12517


Re: RFR: 8301627: System.exit and Runtime.exit debug logging [v6]

2023-02-17 Thread Roger Riggs
> It can be difficult to find the cause of calls to 
> `java.lang.System.exit(status)` and `Runtime.exit(status)` because the Java 
> runtime exits.
> The status value and stack trace are logged using the System Logger named 
> `java.lang.Runtime` with message level `System.Logger.Level.DEBUG`.

Roger Riggs 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 eight additional commits since 
the last revision:

 - Move test to the java/lang/RuntimeTests directory.
   Added implNote to System.exit.
   A few javadoc updates for review comments.
 - Merge branch 'master' into 8301627-log-system-exit
 - Improve implNote for Runtime.exit() with review suggestions.
 - Correct System.getLogger link
 - Add an @implNote to Runtime.exit to describe the java.lang.Runtime logging.
 - Added try/catch around lookup of logger so exceptions do not prevent 
System.exit.
   Added test case with console logger (when java.util.logging) not present.
   Removed @implNote tag its not appropriate in implementation javadoc.
   Still looking into when and where the log configuration should be described.
 - Locate the System logger before taking the shutdown lock
 - Add logging of calls to Runtime.exit to the system logger 
"java.lang.Runtime".

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/12517/files
  - new: https://git.openjdk.org/jdk/pull/12517/files/a31645c1..3dc13135

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=12517=05
 - incr: https://webrevs.openjdk.org/?repo=jdk=12517=04-05

  Stats: 48694 lines in 1053 files changed: 18511 ins; 14817 del; 15366 mod
  Patch: https://git.openjdk.org/jdk/pull/12517.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/12517/head:pull/12517

PR: https://git.openjdk.org/jdk/pull/12517