[Issue #29] - Improve diagnostics of Process reading in the native code

[oleg-nenashev]

This change should significantly improve the diagnostics info for #29 in some corner cases. All the issues below may lead to #29 or to "Failed to read PEB" (e.g. JENKINS-8614 in Jenkins tests)

Native:

• Enable CRT in the Release build. It makes the DLL 5 times bigger (<10 KB => 50KB), but allows doing fine-grain logging. I can make it a non-default behavior if it is preferred
• Return explicit error if the process is no longer running
• Return explicit error if we try to poll the 64bit process from a 32bit JVM (probably it is a way to do it in WinP by dirty hacks, but I would not expect any other stuff in Jenkins to work reliably in such case - leads to org.jvnet.winp.WinpException: Failed to read environment variable - [JENKINS-30782] #29 or PEB error)
• Return explicit errors if the Process Memory region is not readable && we want to read EnvVars from there. For newly created processes it will lead to org.jvnet.winp.WinpException: Failed to read environment variable - [JENKINS-30782] #29 (Header is ready, but memory region is not ready)
• Return explicit error if the code is messed up, and the Environment pointer is incorrect (e.g. we read RTL_USER_PROCESS_PARAMETERS incorrectly, leads to org.jvnet.winp.WinpException: Failed to read environment variable - [JENKINS-30782] #29)

Java:

• Enable stacktraces and core dumps for the build failure
• More tests

Work is in progress, because AppVeyor JVM was crashing, need to investigate. likely it is due to the isWow64() method, where I oversimplified the pattern once the simple approach worked.

@reviewbybees

[oleg-nenashev]

So yes, I should not have removed the intelligent import code, which has been proposed for https://msdn.microsoft.com/en-us/library/ms684139.aspx . AppVeyor is not running builds as a desktop App, hence it captured the API misusage.

[oleg-nenashev]

Self-:bug:

[ghost]

This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation.

[oleg-nenashev]

@reviewbybees @kohsuke Ready for review. I would especially appreciate feedback regarding CRT inclusion feasibility. Effectively it is required for fine-grain logging only, which I could disable for the release build (and maybe make it configurable via system property). OTOH, 40KB should not be a problem in 2017 since it needs to be loaded only once.

[jglick]

👍 from what little I understand of it. Probably @kohsuke is the only one able to do a deep review.

[jglick]

Sigh.

[jglick]

Not using toSystemErrorCode?

[oleg-nenashev]

Nope, because it is a native stuff. Ideally I should create enum there as well, but I firstly need to think about fatal/not-fatal errors (E.g. Process memory block unavailability is not fatal, and it is one of potential JENKINS-30782 root causes)

[jglick]

Nice.

[oleg-nenashev]

@reviewbybees done

[oleg-nenashev]

@kohsuke said he is fine with CRT inclusion. @reviewbybees done => Merging