Restarting workspace after feature installation does not take into account changes to eclipse.ini

[robstryker]

Take a feature installation that has a p2.inf file as follows:

instructions.configure=\
org.eclipse.equinox.p2.touchpoint.eclipse.addJvmArg(jvmArg:--add-opens=java.base/java.security=ALL-UNNAMED);

instructions.unconfigure=\
org.eclipse.equinox.p2.touchpoint.eclipse.removeJvmArg(jvmArg:--add-opens=java.base/java.security=ALL-UNNAMED);

When this feature is installed via the UI, eclipse will come up with a dialog asking you to restart the IDE to apply the software update. When you select "Restart Now", the workspace will restart. The expected changes listed above will be physically added to the installation's eclipse.ini file. But when the workspace starts, these specific add-opens will not be active.

If you then go restart the workbench one more time, the changes are applied to the running VM as expected.

This would indicate a bug in the p2's software update - restart action where it fails to properly recognize or apply changes to eclipse.ini.

[mickaelistria]

Was also reported as https://bugs.eclipse.org/bugs/show_bug.cgi?id=551378

[jonahgraham]

IIUC there is already some special handling to support changing the JVM and this change is to ensure other settings are done too.

As an implementation idea it may be best to fully restart the eclipse process rather than reusing the existing eclipse process with special handling for what changed. That would mean that all the eclipse.ini settings would be recalculated the same as a fresh launch, and items such as the correct splash screen will be displayed on restart (in cases of updating versions of Eclipse IDE).

Change to eclipse executable behaviour requires changes outside this repo as well.

[jonahgraham]

Note that Eclipse IDE WG Funded Development Effort #24 has been raised to resolve this problem.

[stephan-herrmann]

See also

• https://bugs.eclipse.org/bugs/show_bug.cgi?id=323565
• https://bugs.eclipse.org/bugs/show_bug.cgi?id=531112

[jonahgraham]

There is a lot of history here - thanks @stephan-herrmann for providing more of it.

One main thing that is unclear to me is why restart can't/doesn't do a full cold restart. Maybe no one interested in this in the past had the knowledge necessary to change the launcher, or perhaps a new "restart" mechanism (return code) needs to be added to do a cold-restart.

Answering the above will be part of what is needed to successfully resolve this issue.

[laeubi]

@jonahgraham not sure if it is relevant today but some things to consider regarding "cold" restart:

1. You can launch Equinox (!) applications without a launcher and therefore it should work at least without a native launcher as well, this is especially important if you run the application from within Eclipse
2. Killing the JVM process can have slightly higher times unless the application is up again
3. In general a JVM can host many different code (e.g. you can embed Equinox in a webcontainer) or even run multiple frameworks in parallel, having one of the "kill" the JVM might not be what one desire

Of course for a single deployed instance (e.g. Eclipse IDE) some or even all of the mentioned things are irrelevant, but the launcher code itself is generic code.

[jonahgraham]

Thanks @laeubi for the extra context - someone taking on the dev effort should be taking that into consideration to makes sure non Eclipse IDE style workflows continue to work as expected.

[jonahgraham]

@robstryker or an Equinox committer - the title of this issue has [linux] in it, but this issue is not limited to linux AFAIU. Can you remove the [linux] part from the title?

[mickaelistria]

Maybe we could start by not allowing "Restart", or by showing an error message "Restart function cannot be used safely because a configuration file was changed. The application will shut down. Please restart it from the usual launcher for the modifications to be taken into account" when restarting, in case we know the eclipse.ini has changed since startup? (comparing file modification date vs process start date should do the trick).

[merks]

@mickaelistria

That's a very constructive suggestion. It is easy to implement compared to trying to relaunch the application completely from scratch on three differ platforms...

[laeubi]

The message is way to complicated for a user to understand, I would make it less "technical" and simply mention that some changes require a shutdown of the application to take effect with the choice to "Only Restart now", "Shutdown Application now", "Do nothing and proceed" or similar.

[jonahgraham]

The Eclipse IDE WG Funded Development Effort #24 is to implement the complete solution - i.e. not just blocking restart if eclipse.ini is modified. If there ends up being reasons or time delays that prevent getting the full solution implemented and integrated then starting with not allowing restart as discussed in the last few comments makes sense.

[merks]

During various discussed I also saw comments about editing the *.ini manually and then restarting. Probably that doesn't work either, does it? It would certainly be very nice (and expected) that a restart properly re-reads everything including the eclipse.ini and the config.ini, regardless of whether it was p2 that made changes or the user.

I'm just noting this because I'm not sure that the restart problem is actually a p2 problem rather than purely an equinox launcher problem.

[laeubi]

During various discussed I also saw comments about editing the *.ini manually and then restarting. Probably that doesn't work either, does it?

If you mean File > Restart then probably no, but honestly I never use this functionality but always close the program, make modifications and start it then again ...

[umairsair]

All, kindly check the linked PRs.

[merks]

@umairsair

I'm not sure if it's related, but I suspect so. It can no longer restart a debug launched RCP/IDE application. Trying to restart just terminates the application rather than starting a newly-launched debug session.

On the plus side, when testing updating a 2023-12 installation to 2023-04, the splash screen on restart displayed the updated splash screen, so that really great!! 🏆

[umairsair]

@merks

1. can you please share the exact eclipse command that you are invoking?
2. which host you are trying?
3. Did it happen in a case when the update from older version to newer version was performed and then IDE is restarted or you are seeing it consistently?

[merks]

I'm on Windows. I think it's reproducible with any launch:

E.g., from this from setup:

https://github.com/eclipse-platform/.github/blob/main/CONTRIBUTING.md#creating-an-eclipse-development-environment

It launches a process like this:

"C:\Program Files\Java\jdk-17.0.9+9\bin\javaw.exe" -XX:+ShowCodeDetailsInExceptionMessages -agentlib:jdwp=transport=dt_socket,suspend=y,address=localhost:63244 -Xms256m -Xmx2048m -Duser.home=D:\Users\merks -Declipse.pde.launch=true --add-modules=ALL-SYSTEM -Djava.security.manager=allow -Dfile.encoding=UTF-8 -Dstdout.encoding=UTF-8 -Dstderr.encoding=UTF-8 -classpath D:\Users\merks\platform-sdk-4.30\git\equinox\bundles\org.eclipse.equinox.launcher\bin; org.eclipse.equinox.launcher.Main -launcher D:\Users\merks\platform-sdk-4.30\ws.metadata.plugins\org.eclipse.pde.core.install_folders\1706880906456\eclipse.exe -name Eclipse -showsplash 600 -product org.eclipse.sdk.ide -data D:\Users\merks\platform-sdk-4.30\ws/../runtime-workspace -configuration "file:D:/Users/merks/platform-sdk-4.30/ws/.metadata/.plugins/org.eclipse.pde.core/Runtime Workspace/" -dev "file:D:/Users/merks/platform-sdk-4.30/ws/.metadata/.plugins/org.eclipse.pde.core/Runtime Workspace/dev.properties" -os win32 -ws win32 -arch x86_64 -nl en_US -consoleLog

But I debugged further and it's clear that it's intended that the process exits with exit code 24 whereas formerly it was exit code 23. I.e., I debugged an older Eclipse version and saw that here too the process terminates completely but with a different exit code and of course the java process is launched directly, so no native launcher is involved. So I suspect that it is the debug framework that needs to consider the new exit code value.

I'll have a look where that code might be. At this point I think there is nothing wrong with your changes but rather downstream needs to be adapted. And, as I said, the new behavior seeing the updated splash screen after an update is really great!

[merks]

FYI, I am looking at these as potentially needing further attention:

Only the one is in the launcher and maybe that should consider value 24 as well, but when I debug the exit logic it does end up setting 24 as the exit code, which looks correct and intended:

[umairsair]

@merks ,

I have reproduced the issue.

I'll debug on my end. It should not ever exit with exit code 24 in PDE workflow.

[merks]

We might be good to revert the other changes if that's the case...

[umairsair]

@merks ,

Kindly check following PR and see if it handles all the concerns?

eclipse-platform/eclipse.platform.ui#1622