Re: RFR: 8325567: jspawnhelper without args fails with segfault [v3]

2024-03-07 Thread Aleksey Shipilev
On Thu, 7 Mar 2024 16:29:12 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:
> 
>   Use outputanalyzer

test/jdk/java/lang/ProcessBuilder/JspawnhelperProtocol.java line 238:

> 236: }
> 237: }
> 238: }

This looks like a leftover, please revert.

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

> 44: 
> 45: private static void jspawnhelperWithNArgs(int nArgs) throws Exception 
> {
> 46: System.out.println("Running jspawnhelper with "+nArgs+" args");

Style: spaces around `+` operator.

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

> 45: private static void jspawnhelperWithNArgs(int nArgs) throws Exception 
> {
> 46: System.out.println("Running jspawnhelper with "+nArgs+" args");
> 47: String[] args = new String[nArgs +1];

Style: spaces around `+` operator.

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

> 52: if (!p.waitFor(TIMEOUT, TimeUnit.SECONDS)) {
> 53: throw new RuntimeException("jspawnhelper timed out after " + 
> TIMEOUT + " seconds");
> 54: }

I don't think we need to wait here, the `shouldHaveExitValue` waits for us.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18112#discussion_r1516472478
PR Review Comment: https://git.openjdk.org/jdk/pull/18112#discussion_r1516472833
PR Review Comment: https://git.openjdk.org/jdk/pull/18112#discussion_r1516477172
PR Review Comment: https://git.openjdk.org/jdk/pull/18112#discussion_r1516476881


Re: RFR: 8325567: jspawnhelper without args fails with segfault [v3]

2024-03-07 Thread Elif Aslan
> 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:

  Use outputanalyzer

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18112/files
  - new: https://git.openjdk.org/jdk/pull/18112/files/3f382e74..8fbf24e2

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=18112&range=02
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18112&range=01-02

  Stats: 32 lines in 1 file changed: 1 ins; 21 del; 10 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