[JENKINS-48785] getCmdLineAndEnvVars is not compatible with 32bit windows os

[Kapoetski3]

The getCmdLineAndEnvVars function seems to be incompatible with 32bit windows os.

We currently have a jenkins server that runs on a x64 windows machine.
This machine starts a java process on a jenkins slave that is running on a 32bit windows machine.

The process on the 32bit machine cannot be killed. The getCmdLineAndEnvVars checks if the process is wow64, which it is not according to the msdn documentation:

A pointer to a value that is set to TRUE if the process is running under WOW64. If the process is running under 32-bit Windows, the value is set to FALSE. If the process is a 64-bit application running under 64-bit Windows, the value is also set to FALSE.

This means the iswow64 check will always fail, causing an error. This also means the process will never get cleaned.

The check should also look for PROCESSOR_ARCHITECTURE_INTEL and not just wow64

[oleg-nenashev]

Could you please also create an issue in Jenkins JIRA for it?

[Kapoetski3]

I created an issue in the Jenkins Jira and can be found here:
https://issues.jenkins-ci.org/browse/JENKINS-48785

[oleg-nenashev]

IIUC the issue correctly, the concerned code is https://github.com/kohsuke/winp/blob/6731aa9c13cb81be5d52a1c35bcd2bf018b929e6/native/envvar-cmdline.cpp#L167-L188

I'd guess the pitfall is in the method check: https://msdn.microsoft.com/en-us/library/windows/desktop/ms684139(v=vs.85).aspx

"For compatibility with operating systems that do not support this function, call GetProcAddress to detect whether IsWow64Process is implemented in Kernel32.dll. If GetProcAddress succeeds, it is safe to call this function. Otherwise, WOW64 is not present. Note that this technique is not a reliable way to detect whether the operating system is a 64-bit version of Windows because the Kernel32.dll in current versions of 32-bit Windows also contains this function.

Likely that's how I missed it during testing - my 32bit Windows version likely didn't have the function

[oleg-nenashev]

The comment in the first highlighted line is just wrong tho

[Kapoetski3]

I think the flow is also incorrect. I don't think the availability of IsWow64Process will make a difference.
This is because a pure 32bit Windows OS will return false for IsWow64Process. The flow you will have on a 32bit Windows is that it goes into the else of the #ifdef _WIN64. Here it will call the IsWow64Process which returns false because it is not wow64. And then it will go into the error on line 182. The IsWow64Process cannot detect a pure 32bit Windows Os, only a 32bit process running on a 64bit windows. So i think you need to do an extra check there, and only go into the error on line 182 if the process is not wow64 and the processor type is not PROCESSOR_ARCHITECTURE_INTEL.

[oleg-nenashev]

@Kapoetski3 I am ready to propose a patch, but I do not have 32bit environment to verify the fix. Since you have clarified the changes which need to be done, maybe you would like to propose the PR on your own to reduce the number of test iterations. I am happy to do a blind fix as well, but I will need your help with testing anyway

[oleg-nenashev]

Sorry for the delay, I have been processing the Jenkins TODO queue. I hope to have more time for WinP/WinSW this month

[Kapoetski3]

I can make a PR for this fix. For testing, i also do not have access to a 32bit machine anymore, as i no longer work at the company where i found this issue. What i can do is contact some people there and see if they can test it, or if they can help me test it.

[oleg-nenashev]

@Kapoetski3 Thanks! I have recovered the development environment for this repo, so I can ship the release once the pull request is ready to go