RFR: 8325567: jspawnhelper without args fails with segfault

[Vladimir Petko]

On Mon, 4 Mar 2024 23:23:04 GMT, Roger Riggs <rriggs at openjdk.org> wrote:

>> This change is intended to address the segmentation fault issue that occurs when jspawnhelper is called without arguments, and it includes an updated test to verify the behavior in such cases.
>> 
>> Existing tests passes since it does not check behavior without args.
>> After test update the test fails without 
>> 
>>    if (argc != 2) {
>>         shutItDown();
>>     }
>> 
>> code block
>> 
>>> Test results: failed: 1
>>> Report written to /home/ec2-user/moe/ws/openjdk/jdk/build/linux-x86_64-server-fastdebug/test-results/jtreg_test_jdk_java_lang_ProcessBuilder_JspawnhelperProtocol_java/html/report.html
>>> Results written to /home/ec2-user/moe/ws/openjdk/jdk/build/linux-x86_64-server-fastdebug/test-support/jtreg_test_jdk_java_lang_ProcessBuilder_JspawnhelperProtocol_java
>>> Error: Some tests failed or other problems occurred.
>>> Finished running test 'jtreg:test/jdk/java/lang/ProcessBuilder/JspawnhelperProtocol.java'
>>> Test report is stored in build/linux-x86_64-server-fastdebug/test-results/jtreg_test_jdk_java_lang_ProcessBuilder_JspawnhelperProtocol_java
>>>  
>>> ==============================
>>> Test summary
>>> ==============================
>>>    TEST                                              TOTAL  PASS  FAIL ERROR   
>>>    jtreg:test/jdk/java/lang/ProcessBuilder/JspawnhelperProtocol.java
>>> >>                                                       1     0     1     0 <<
>>> ==============================
>>> TEST FAILURE
>> 
>> 
>> 
>> after added the code block back 
>> 
>>    if (argc != 2) {
>>         shutItDown();
>>     }
>> 
>> 
>> updated test passes
>> 
>> 
>> lang/ProcessBuilder/JspawnhelperProtocol.d:/home/ec2-user/moe/ws/openjdk/jdk/test/jdk/java/lang/ProcessBuilder:/home/ec2-user/moe/ws/openjdk/jdk/build/linux-x86_64-server-fastdebug/test-support/jtreg_test_jdk_java_lang_ProcessBuilder_JspawnhelperProtocol_java/classes/0/test/lib \\
>>         -Xmx768m \\
>>         -XX:MaxRAMPercentage=1.04167 \\
>>         -Dtest.boot.jdk=/home/ec2-user/moe/soft/jdk/jdk-21 \\
>>         -Djava.io.tmpdir=/home/ec2-user/moe/ws/openjdk/jdk/build/linux-x86_64-server-fastdebug/test-support/jtreg_test_jdk_java_lang_ProcessBuilder_JspawnhelperProtocol_java/tmp \\
>>         -ea \\
>>         -esa \\
>>         -Djava.library.path=/home/ec2-user/moe/ws/openjdk/jdk/build/linux-x86_64-server-fastdebug/images/test/jdk/jtreg/native \\
>>         com.sun.javatest.regtest.agent.MainWrapper /home/ec2-user/moe/ws/openjdk/jdk/build/linux-x86_64-server-fastdebug/test-support/jtreg_test_jdk_java_lang...
>
> test/jdk/java/lang/ProcessBuilder/JspawnhelperProtocol.java line 111:
> 
>> 109:         if (p.exitValue() != 1)
>> 110:             System.exit(ERROR + 2);
>> 111:         System.exit(0);
> 
> Its bad form to System.exit from not at the top level. Is that necessary.

This is in line with other tests in this file , e.g. see `parentCode()`. 

I agree that code would be cleaner if the test is refactored to return error code from the test methods and system.exit() in the main instead, but this might be a separate pr done before or after this one.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18112#discussion_r1511953089