RFR: 8352533: Report useful IOExceptions when jspawnhelper fails [v4]

[Aleksey Shipilev]

On Tue, 6 May 2025 11:23:32 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> Aleksey Shipilev has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains six additional commits since the last revision:
>> 
>>  - Add bug ID
>>  - Merge branch 'master' into JDK-8352533-jspawnhelper-ioexceptions
>>  - Pull message into a field
>>  - Test cases
>>  - Merge branch 'master' into JDK-8352533-jspawnhelper-ioexceptions
>>  - Initial fix
>>    
>>    Good spawnhelper failure message
>>    
>>    Trying tests
>>    
>>    Print helper message on all IOExceptions
>>    
>>    Final
>
> src/java.base/unix/native/libjava/ProcessImpl_md.c line 346:
> 
>> 344:         if (ret != EINVAL)
>> 345:             detail = tmpbuf;
>> 346:     }
> 
> Pre-existing, possibly as follow-up: 
> 
> I wonder whether we can do more. I don't like how we either only print out the errno string, or only print out the "Default" detail (I don't like the "default" prefix since it's really useful context information about this errno). 
> 
> For a problem on this side, we mostly pass in errno, then swallow the "defaultdetail". E.g. for posix_spawn failure, we'd print e.g. "12:Out of memory\nPossible reasons: ... " (or whatever localized string strerror returns).
> 
> For errors that happen inside the jspawnhelper (see line 790ff where we pass 0 for the errno), we print out "0:Bad code from spawn helper\nPossible reasons: ... ", so we swallow the errno we got from the jspawnhelper.
> 
> Could we print out instead the default, then the errno string, then the Possible reasons text? E.g.
> "posix_spawn failed (12:Out of memory)\nPossible reasons: ..."
> or
> "Bad code from spawn helper (12:Out of memory)\nPossible reasons: ..." 
> 
> There is an issue of errnum values from the far side possibly intersecting with valid errno. Could be solved simply by assigning negative values to ERR_MALLOC, ERR_PIPE and ERR_ARGS. I am not aware of any errno values being negative. The errno printing would need to recognise these three values in addition to errno values. We also could remove the +3 workaround in the test.
> 
> The only argument I see against is some vague form of backward compatibility, in case people get confused about the new information, or that existing tools parse this output.

Yeah, I dislike `defaultDetail` naming and the fact we swallow it. I see good reason to print it unconditionally. I think we can do this with just a little code massaging. See new commit. It prints the message like:


Recursively executing 'JspawnhelperProtocol simulateCrashInChild4'
posix_spawn:0
java.io.IOException: Cannot run program "pwd": Failed to exec spawn helper: pid: 1405770, exit code: 4, error: 0 (none) 
Possible reasons:
  - Spawn helper ran into JDK version mismatch
  - Spawn helper ran into unexpected internal error
  - Spawn helper was terminated by another process
Possible solutions:
  - Restart JVM, especially after in-place JDK updates
  - Check system logs for JDK-related errors
  - Re-install JDK to fix permission/versioning problems
  - Switch to legacy launch mechanism with -Djdk.lang.Process.launchMechanism=VFORK

	at java.base/java.lang.ProcessBuilder.start(ProcessBuilder.java:1110)
	at java.base/java.lang.ProcessBuilder.start(ProcessBuilder.java:1044)
	at java.base/java.lang.Runtime.exec(Runtime.java:605)
	at java.base/java.lang.Runtime.exec(Runtime.java:470)
	at JspawnhelperProtocol.parentCode(JspawnhelperProtocol.java:58)
	at JspawnhelperProtocol.main(JspawnhelperProtocol.java:232)

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/24149#discussion_r2086480179