search

microsoft / openjdk

Microsoft Build of OpenJDK
https://www.microsoft.com/openjdk
MIT License
323 stars 19 forks source link

'PATH' feature requires a reboot. The MSI installer should therefore set a `ScheduleReboot` after `InstallFinalize` #627

Open cwegener opened 1 week ago

cwegener commented 1 week ago

'PATH' feature requires a reboot. The MSI installer should therefore set a ScheduleReboot after InstallFinalize

OS & version Windows 10/11

MSFT Build of OpenJDK Version: 21.0.4

To Reproduce: Steps to reproduce the behavior:

  1. Download OpenJDK MSI
  2. Install MSI with default options ('PATH' feature is selected)
  3. See that no reboot is scheduled

Expected behavior A reboot should be scheduled when installing the 'PATH' feature.

Screenshots, Logs etc None

Additional context To confirm that the PATH variable has not updated after installation, run psexec -s cmd /c set PATH and inspect the output.

KalleOlaviNiemitalo commented 1 week ago

Isn't there a WM_SETTINGCHANGE message posted to indicate that the system environment variables changed? But I suppose window messages cannot be received by noninteractive services like whatever PsExec is using.

cwegener commented 1 week ago

Isn't there a WM_SETTINGCHANGE message posted to indicate that the system environment variables changed? But I suppose window messages cannot be received by noninteractive services like whatever PsExec is using.

Good question! I will verify the WM_SETTINGCHANGE. I suppose WM_SETTINGCHANGE is something that is related to interactive desktop session?

the psexec -s simulates my actual use case by running a new process under the service manager (inside svchost.exe). The service manager is the component that will only get a copy of the full new environment after a reboot.

I just looked up WM_SETTINGCHANGE and that is a Desktop application message from winuser.h, so yeah it only affects desktop applications. Not many Java applications are desktop applications in my experience (but I could be wrong)

cwegener commented 1 week ago

Good question! I will verify the WM_SETTINGCHANGE. I suppose WM_SETTINGCHANGE is something that is related to interactive desktop session?

I confirmed that WM_SETTINGCHANGE works fine for desktop applications. Any newly spawned process that the current logged in user's shell launches will have an up-to-date copy of the system environment.

So, in summary, the WM_SETTINGCHANGE works of a very specific scenario which is interactive desktop java applications on a physical desktop (WM_SETTINGCHANGE won't work on Terminal Server I believe. From past experience Terminal Server session like used by RDS are children of a System service and they inherit the system environment from the Terminal Server service)

My specific use case is that I am distributing a Java application that runs as a service on Windows.

When my customers perform the OpenJDK installation and my Java application MSI installation without performing a system reboot, they will see an error in the eventlog from my Windows Server wrapper (which is responsible to spawn a java.exe -jar) that java.exe is not on the PATH.

Which is confusing for my customers because they won't be aware of these intricacies of how the system environment works in Windows and it won't be obvious to them that a simple computer reboot is required.

karianna commented 1 week ago

@brunoborges & @d3r3kk for triage

d3r3kk commented 1 week ago

Same issue as adoptium/installer#133, we will investigate this quarter and decide by Jan 2025 release whether or not to include this. One potential workaround (in the interim) could be to have the service's installer install the JDK and then dictate this behaviour, but that may not be applicable to every scenario either.

KalleOlaviNiemitalo commented 1 week ago

my Windows Server wrapper (which is responsible to spawn a java.exe -jar)

Can you make the wrapper use OpenProcessToken and CreateEnvironmentBlock to get the updated PATH environment variable, and use that when starting the java.exe process? Similar to how Windows Terminal used to do it in https://github.com/microsoft/terminal/blob/95a79c22620396ce26aabece7942342f461690be/src/types/Environment.cpp#L13-L24. (The pseudo-handle returned by GetCurrentProcessToken function doesn't have the TOKEN_DUPLICATE access right that CreateEnvironmentBlock requires.)

cwegener commented 1 week ago

One potential workaround (in the interim) could be to have the service's installer install the JDK and then dictate this behaviour, but that may not be applicable to every scenario either.

Yes. I did consider something like this.

I'm not overly in love with that idea though. It seems that this would make my installer much smarter than it needs to be. I do prefer to have installers to be independent and composable and not have to know anything about external dependencies.

Can you make the wrapper use OpenProcessToken and CreateEnvironmentBlock to get the updated PATH environment variable, and use that when starting the java.exe process?

Thanks @KalleOlaviNiemitalo , I will need to investigate that option. My wrapper is a Golang app and currently uses the exec package of the Go std library. My gut feel is that I probably would need to look at CFFI for this functionality. I'll have a look and see if that might be an option.