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

[Chris Plummer]

On Tue, 23 Feb 2021 03:08:16 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> test/lib/jdk/test/lib/apps/LingeredApp.java line 419:
>> 
>>> 417:             theApp.waitAppReady();
>>> 418:         } catch (Exception ex) {
>>> 419:             theApp.finishApp();
>> 
>> I'm worried about the output appearing twice in some tests:
>> 
>>         try {
>>             ...
>>             theApp = new LingeredAppWithLargeArray();
>>             LingeredApp.startAppExactJvmOpts(theApp, vmArgs);
>>             attachAndDump(heapDumpFileName, theApp.getPid());
>>         } finally {
>>             LingeredApp.stopApp(theApp);
>>             heapDumpFile.delete();
>>         }
>> 
>> So we always call `stopApp()`:
>> 
>>     public void stopApp() throws IOException {
>>            ...
>>            if (appProcess != null) {
>>            ...
>>             finishApp();
>>            ...
>>         }
>>     }
>> 
>> 
>> Which means we always call `finishApp()`, which means we always print the output. With your changes won't we see the output twice when there is an exception?
>> 
>> In the CR it looks like `LingeredApp.startAppExactJvmOpts()` is being call from `JCmdTest.java`. I don't see this test. Is it under development? Does it need a `stopApp()` call?
>
> 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.

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

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