Remove terminal service and create launch process

[awisniew90]

This PR removes the terminal service in favor of managing the system process as part of the Liberty Tools launch.

Also fixes #293, fixes #352

[mezarin]

I took a quick look and the console replacement code, had a couple of minor comments, but just a couple of observations as i ran the code:

• Running Liberty in dev mode says to exit dev mode with this: "press Ctrl-C or type 'q' and press Enter.". In this case only pressing q works.
• I cannot install a gradle app. It defaults to maven.
• Users in the past asked if there was a way to click on a link inside the terminal (Ctrl + click i believe). Is there a way to do that in the console?
• It seems possible to customize the title inside the console page. Can you make it look similar to what we had with the terminal: icon + app name? If not possible, perhaps move [Liberty] before the app name?
• With the terminal, if a user close the terminal tab, the liberty dev processes were terminated. Now the behavior is to leave the process running so that when you re-open the console, you get back where you left off before. This would be a change in behavior.
• If you have multiple apps running, and you hit the stop button on the console's toolbar, it ends dev mode. This in turn causes process objects to be leaked in ProcessController.java. So two options: Disable/remove the stop button or handle cleanup with a listener.

[mezarin]

Don't forget to update the last modified year in the copyright in this file and the test files.

[mezarin]

looks like colorOutput is no longer a parameter.

[mezarin]

I think you meant to do this: 2024, 2025

[awisniew90]

This was actually from a new file I merged earlier in January 2025.... I originally created the file in 2024 and forgot to update the year when I finally merged

[scottkurz]

@mezarin thx for trying this out and the detailed feedback. Some thoughts/questions back...

• Running Liberty in dev mode says to exit dev mode with this: "press Ctrl-C or type 'q' and press Enter.". In this case only pressing q works.

Hmm... I think we could live with this minor amt. of "misinformation" but I wonder if the approach we'd need to make links clickable could help filter this possibly? Would it be worth it if so?

• I cannot install a gradle app. It defaults to maven.

• Users in the past asked if there was a way to click on a link inside the terminal (Ctrl + click i believe). Is there a way to do that in the console?

THANK YOU for catching this!!! I think LDT/WDT uses something here maybe for this?

• With the terminal, if a user close the terminal tab, the liberty dev processes were terminated. Now the behavior is to leave the process running so that when you re-open the console, you get back where you left off before. This would be a change in behavior.

I think that's overall a good change and more what I imagine I might expect (if I could erase the last couple of years from my mind).

It seems possible to customize the title inside the console page. Can you make it look similar to what we had with the terminal: icon + app name? If not possible, perhaps move [Liberty] before the app name?

It seems nice having the pid as well. So you're just saying switch Liberty vs. the app name?

• If you have multiple apps running, and you hit the stop button on the console's toolbar, it ends dev mode. This in turn causes process objects to be leaked in ProcessController.java. So two options: Disable/remove the stop button or handle cleanup with a listener.

First, this issue applies just to any single app, right? The red square button on the Console toolbar is "Terminate", whereas there's a right-click menu option that provides "Terminate All". I think these behave as expected... is there actually a particular point here for multiple concurrent sessions?

I think we definitely want to keep the stop button active....the fact that we've lost Ctrl+C is just one more reason. If we can easily plug in a listener like you suggested, sure it'd be good to clean up. If not, though, it's not too bad... the use of the Process Map is guarded with a check on whether the Process isAlive(), so even if a single app were continually

[scottkurz]

Let me tack on to Ed's comments. I've noticed that doing a Terminate kills things more abruptly than Ctrl+C..

On Windows, at least, this is more likely to leave the .sRunning file, leading to issue OpenLiberty/ci.maven#1696

Since we don't have an option to Clean on server start this is even a bit more of a hassle (though even if we did have an option it might still be an annoyance if people didn't realize they should be using it, if it were only optional).

[mezarin]

I cannot install a gradle app. It defaults to maven.

What i meant there is really that when i install a Gradle app (app with just the gradle build stuff), it shows in the dashboard as a maven app. If i run it anyway, dev mode fails because there is no pom.xml.

It seems nice having the pid as well. So you're just saying switch Liberty vs. the app name?

Yeah that is fine, so something like this:
Either <Liberty icon> myAppName [pid 1234] or [Liberty] myAppName [pid:1234]

First, this issue applies just to any single app, right? The red square button on the Console toolbar is "Terminate", whereas there's a right-click menu option that provides "Terminate All". I think these behave as expected... is there actually a particular point here for multiple concurrent sessions?

The new process cleanup logic is not hooked up to the external stop (toolbar stop or Terminate All Ctx menu) or even something like issuing ctrl+c or q. So, if i have 10 apps, i run them in dev mode, and i stop them as mentioned above, the 10 entries that are created in the map will remain in the map. If i install more apps and continue on this pattern, at some point there will be an out of memory error. How long until the EOM or how plausible is this .. not sure. The listener route is the better route.

[scottkurz]

The listener route is the better route.

I don't think the OOM is too likely to be an issue. I suppose a WeakHashMap might allow cleanup a bit earlier. However, I do think the Terminate leading to OpenLiberty/ci.maven#1696 is significant enough that we should try to hook into Terminate.

I do think this could be handled as a follow-up issue/PR if we want to merge this though. Note 1696 only happens with an early Terminate, not a Terminate after the app has deployed, so not quite as prevalent.

Does this code in STS show a hook into Console Terminate?

[awisniew90]

Thanks for checking this out Ed!

1. For the console tab display, I'd say we are good in that we have <Liberty icon> myAppName [Liberty] [pid] already.

2. For the ProcessController map - terminate or q from dev mode effectively terminates the Java process that spawned dev mode, so I dont think we need to worry about a leak of Java Process objects (they are destroyed and the reference is now "null"). As far as an OOM for the map itself, I think we are ok there in that after each "terminate" we would just be left with a map entry of String/null... a very small footprint.

[awisniew90]

Disregard my last comment. The IProcess object that is created will "null" out its reference to the Java Process object, but the entry in our map is not nulled out. @mezarin - I added a terminate event listener that will get notified if our process terminates. It will then remove it from the map.

[scottkurz]

A lot of good feedback and changes here. I think I've looked at them all now and we will follow-up as needed.