RFR: JDK-8190187: C++ code calling JNI_CreateJavaVM can be killed by Java

[Alan Bateman]

On 27/02/2018 15:04, Adam Farley8 wrote:
> Resending. Bump. :)
>
> On 14/02/2018 14:13, Adam Farley8 wrote:
> >> Hi All,
> >>
> >> -- Short version --
> >>
> >> Could a committer please take the fix for JDK-8190187 (full code 
> included
> >> in the bug) and:
> >>
> >> 1) Complete the CSR process for the new JNI Return code.
> >> 2) Commit the changes that contain (a) the new return code, and (b) the
> >> non-Hotspot code that handles the new code.
> > The patches attached to the JIRA issue are missing the changes to the
> > JVM TI spec (jvmti.xml).
>
> I'm not seeing the JNI return codes in that file. Are you after one of 
> those
> dated updates near the bottom?
> e.g.
> ```
> <changedate=/"14 February 2018"/version=/"11.0.0"/>
> Added JNI_SILENT_EXIT return code for early exits without errors.
> java.c's ParseArguments function now sets pret value to 2 for a NULL 
> pwhat.
>    This allows us to clearly identify when no class was set, but no 
> other error has occurred.
>    This is undone in java.c's ContinueInNewThread method, so the 
> surface behaviour does not change.
> </change>
> ```
The "Agent Start-Up" section is the section to look at. The important 
part is:

"The return value from|Agent_OnLoad|or|Agent_OnLoad_<agent-lib-name>|is 
used to indicate an error. Any value other than zero indicates an error 
and causes termination of the VM."

If there is special return value to mean "VM terminates without error" 
then this part of the spec will need to be adjusted. An additional point 
is that you can start several agents from the command line, does the VM 
terminate after it has started all agents or does it terminate when the 
first agent returns asks the VM to terminate quietly?



>
>
> > There is also text to be written for the JNI spec if this proposal goes
> > ahead.
>
> I assume you mean the "RETURNS" section of the JNI_CreateJavaVM
> bit on the invocation.html web page. Something like this?
>
> ```
> RETURNS:
> Returns JNI_OK on success; returns a suitable JNI error code (a 
> negative number) on failure.
>
> The sole exception is a silent exit, which returns JNI error code 
> JNI_SILENT_EXIT.
> This indicates that the VM cannot be used, but that this is the 
> intended behaviour for the
> arguments used. E.g. -Xlog:help (which prints help output and then 
> destroys the VM)
> ```
Yes, this is the place that will need changes.


>
>
> >
> > I don't agree that the launcher should be looking for
> > "-agentlib:jdwp=help" in the command line as it's just one of many ways
> > that the debugger agent might be started (e.g. -Xrunjdwp:<options>,
> > _JAVA_TOOLS_OPTIONS, ...).
>
> We can avoid that by finding a way around this line in 
> ContinueInNewThread (java.c):
>
> ```
> return (ret != 0) ? ret : rslt;
> ```
>
> I have devised a means to do this, as outlined in the jvmti.xml change 
> above. I put the
> changes into a recent clone of jdk/jdk, and attached the hg diff, 
> along with an improved
> test.
The launcher should only need to look at the return value from JNI 
CraeateJavaVM. I don't think it should do any special handling for the 
JDWP or other agents (there are just too many ways to inject command 
lines and the launcher cannot be expected to handle all of them).

-Alan