RFR: 8262157: LingeredApp.startAppExactJvmOpts does not print app output when launching fails

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: 8262157: LingeredApp.startAppExactJvmOpts does not print app output when launching fails

Yumin Qi-3
Hi, Please review

 When debugging for other test case which uses jcmd to attach LingeredApp process, found there is no error information logged when the app started with function 'startAppExactJvmOpts' exits due to some reason. This is not convenient for trace what is the app failure.
 This simple fix for adding finishApp to print out error information when LingeredApp could not start with startAppExactJvmOpts, this is similar to startApp.

Tests: This is a simple fix and done tests with test/jdk/sun/tools/jinfo which uses the function startAppExactJvmOpts to create LingeredApp, also the test case in debugging, which indeed captured the error message upon start error.

Thanks
Yumin

-------------

Commit messages:
 - 8262157: LingeredApp.startAppExactJvmOpts does not print app output when launching fails

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

PR: https://git.openjdk.java.net/jdk/pull/2679
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8262157: LingeredApp.startAppExactJvmOpts does not print app output when launching fails

David Holmes
Hi Yumin,

Isn't this a serviceability test utility?

Cheers,
David

On 23/02/2021 11:39 am, Yumin Qi wrote:

> Hi, Please review
>
>   When debugging for other test case which uses jcmd to attach LingeredApp process, found there is no error information logged when the app started with function 'startAppExactJvmOpts' exits due to some reason. This is not convenient for trace what is the app failure.
>   This simple fix for adding finishApp to print out error information when LingeredApp could not start with startAppExactJvmOpts, this is similar to startApp.
>
> Tests: This is a simple fix and done tests with test/jdk/sun/tools/jinfo which uses the function startAppExactJvmOpts to create LingeredApp, also the test case in debugging, which indeed captured the error message upon start error.
>
> Thanks
> Yumin
>
> -------------
>
> Commit messages:
>   - 8262157: LingeredApp.startAppExactJvmOpts does not print app output when launching fails
>
> Changes: https://git.openjdk.java.net/jdk/pull/2679/files
>   Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2679&range=00
>    Issue: https://bugs.openjdk.java.net/browse/JDK-8262157
>    Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod
>    Patch: https://git.openjdk.java.net/jdk/pull/2679.diff
>    Fetch: git fetch https://git.openjdk.java.net/jdk pull/2679/head:pull/2679
>
> PR: https://git.openjdk.java.net/jdk/pull/2679
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8262157: LingeredApp.startAppExactJvmOpts does not print app output when launching fails

Yumin Qi-3
In reply to this post by Yumin Qi-3
On Tue, 23 Feb 2021 03:43:41 GMT, Chris Plummer <[hidden email]> wrote:

>> It seems error prone to have to call finishApp() manually in order to see the error log. Since LingeredApp.startApp calls finishApp() on exceptions, shouldn't startAppExactJvmOpts do the same thing?
>
> Although you have a point, you've also pointed out another problem with this fix.  I think users of `startApp()` are already going to see the output twice because of the `finishApp()` call present there. By adding yet another `finishApp()` call to `startAppExactJvmOpts()`, now they will see it 3 times.
>
> If you want to "move" the `finishApp()` call from `startApp()` to `startAppExactJvmOpts()`, then at least that will maintain the status quo with existing `startApp()` users, but we still have an issue with the output appearing twice, even before this change, and with this change it is now more common as `startAppExactJvmOpts()` will also start seeing it.
>
> Maybe `finishApp()` should maintain an `alreadyCalled` flag so it does nothing on subsequent calls.

@plummercj @iklam Thanks for review. I will add a flag for finishApp so we will not call it more than once.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2679
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8262157: LingeredApp.startAppExactJvmOpts does not print app output when launching fails [v2]

Yumin Qi-3
In reply to this post by Yumin Qi-3
> Hi, Please review
>
>  When debugging for other test case which uses jcmd to attach LingeredApp process, found there is no error information logged when the app started with function 'startAppExactJvmOpts' exits due to some reason. This is not convenient for trace what is the app failure.
>  This simple fix for adding finishApp to print out error information when LingeredApp could not start with startAppExactJvmOpts, this is similar to startApp.
>
> Tests: This is a simple fix and done tests with test/jdk/sun/tools/jinfo which uses the function startAppExactJvmOpts to create LingeredApp, also the test case in debugging, which indeed captured the error message upon start error.
>
> Thanks
> Yumin

Yumin Qi has updated the pull request incrementally with one additional commit since the last revision:

  Add a flag to remember finishApp already called

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2679/files
  - new: https://git.openjdk.java.net/jdk/pull/2679/files/68a564f5..f8cbc91b

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

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

PR: https://git.openjdk.java.net/jdk/pull/2679
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8262157: LingeredApp.startAppExactJvmOpts does not print app output when launching fails [v3]

Chris Plummer-2
In reply to this post by Yumin Qi-3
On Tue, 23 Feb 2021 21:19:56 GMT, Yumin Qi <[hidden email]> wrote:

>> Hi, Please review
>>
>>  When debugging for other test case which uses jcmd to attach LingeredApp process, found there is no error information logged when the app started with function 'startAppExactJvmOpts' exits due to some reason. This is not convenient for trace what is the app failure.
>>  This simple fix for adding finishApp to print out error information when LingeredApp could not start with startAppExactJvmOpts, this is similar to startApp.
>>
>> Tests: This is a simple fix and done tests with test/jdk/sun/tools/jinfo which uses the function startAppExactJvmOpts to create LingeredApp, also the test case in debugging, which indeed captured the error message upon start error.
>>
>> Thanks
>> Yumin
>
> Yumin Qi has updated the pull request incrementally with one additional commit since the last revision:
>
>   Remove finishApp from startApp, move printout to startAppExactJvmOpts.

Marked as reviewed by cjplummer (Reviewer).

-------------

PR: https://git.openjdk.java.net/jdk/pull/2679
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8262157: LingeredApp.startAppExactJvmOpts does not print app output when launching fails [v2]

Chris Plummer-2
In reply to this post by Yumin Qi-3
On Tue, 23 Feb 2021 19:37:47 GMT, Ioi Lam <[hidden email]> wrote:

>> Yumin Qi has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   Add a flag to remember finishApp already called
>
> LGTM

Looks good. Thanks!

-------------

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