RFR: 8325567: jspawnhelper without args fails with segfault [v4]

Aleksey Shipilev shade at openjdk.org
Thu Mar 7 16:41:58 UTC 2024


On Thu, 7 Mar 2024 16:33:11 GMT, Elif Aslan <duke at openjdk.org> wrote:

>> This change is intended to address the segmentation fault issue that occurs when jspawnhelper is called without arguments,.
>> There is a new test added  to verify the behavior in such cases.
>> 
>> `[ec2-user at ip-172-16-0-10 jdk]$ make CONF=linux-x86_64-server-fastdebug test TEST=test/jdk/java/lang/ProcessBuilder/JspawnhelperWarnings.java`
>> 
>> 
>> 
>> ==============================
>> Test summary
>> ==============================
>>    TEST                                              TOTAL  PASS  FAIL ERROR
>>    jtreg:test/jdk/java/lang/ProcessBuilder/JspawnhelperWarnings.java
>>                                                          1     1     0     0
>> ==============================
>> TEST SUCCESS
>
> Elif Aslan has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Revert JspawnhelperProtocol.java

So, the test is "passing" with `argc == 2` because jspawnhelper shuts down on illegal `argv[1]`, right? This is probably fine.

More stylistic comments:

test/jdk/java/lang/ProcessBuilder/JspawnhelperWarnings.java line 31:

> 29:  * @requires (os.family == "linux") | (os.family == "aix")
> 30:  * @library /test/lib
> 31:  * @run main/othervm/timeout=300 JspawnhelperWarnings

This test does not have to be `othervm`, it could be `driver`:


  @run driver JspawnhelperWarnings

test/jdk/java/lang/ProcessBuilder/JspawnhelperWarnings.java line 61:

> 59:      public static void main(String[] args) throws Exception {
> 60:         for (int nArgs = 0; nArgs < 10; ++nArgs)
> 61:             jspawnhelperWithNArgs(nArgs);

Style: braces for loop body. There is also no point in using `++nArgs`, when `nArgs++` is sufficient.

test/jdk/java/lang/ProcessBuilder/JspawnhelperWarnings.java line 63:

> 61:             jspawnhelperWithNArgs(nArgs);
> 62:     }
> 63: }

Needs newline at the end of the file.

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

PR Review: https://git.openjdk.org/jdk/pull/18112#pullrequestreview-1922949264
PR Review Comment: https://git.openjdk.org/jdk/pull/18112#discussion_r1516478694
PR Review Comment: https://git.openjdk.org/jdk/pull/18112#discussion_r1516481779
PR Review Comment: https://git.openjdk.org/jdk/pull/18112#discussion_r1516482020


More information about the core-libs-dev mailing list