Re: RFR: 8325567: jspawnhelper without args fails with segfault [v4]
On Thu, 7 Mar 2024 16:33:11 GMT, Elif Aslan 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@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
Re: RFR: 8325567: jspawnhelper without args fails with segfault [v4]
> 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_ProcessBuilder_JspawnhelperProtocol_java/java/lang/ProcessBuilder/JspawnhelperPr...
Elif Aslan has updated the pull request incrementally with one additional
commit since the last revision:
Revert JspawnhelperProtocol.java
-
Changes:
- all: https://git.openjdk.org/jdk/pull/18112/files
- new: https://git.openjdk.org/jdk/pull/18112/files/8fbf24e2..ef803b4c
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=18112&range=03
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=18112&range=02-03
Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
Patch: https://git.openjdk.org/jdk/pull/18112.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/18112/head:pull/18112
PR: https://git.openjdk.org/jdk/pull/18112
