On Tue, 23 Feb 2021 04:09:21 GMT, Yumin Qi <[email protected]> wrote:
>> 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.
I still think the `finishApp()` call should be removed from `startApp()` since
it is now in `startAppExactJvmOpts()`. Also move the `println()` to
`startAppExactJvmOpts()` and then `startApp()` can just be:
LingeredApp a = new LingeredApp();
startApp(a, additionalJvmOpts);
No need for the exception handling anymore since `startAppExactJvmOpts()` is
alread doing that.
-------------
PR: https://git.openjdk.java.net/jdk/pull/2679
