RFR(XS) 8020791: [TESTBUG] runtime/jsig/Test8017498.sh failed to compile

Calvin Cheung calvin.cheung at oracle.com
Fri Jul 19 14:38:36 PDT 2013


Sure. I'll add it before push.

Calvin

On 7/19/2013 2:27 PM, Daniel D. Daugherty wrote:
> Thumbs up.
>
> Forgot to ask you to add an @bug for this bug ID (8020791).
>
> Dan
>
>
> On 7/19/13 3:16 PM, Calvin Cheung wrote:
>> Thanks to Dan's help, the .sh script is now simplified by:
>> - using the timeout jtreg keyword so that the while loop isn't 
>> necessary;
>> - using eval so that the LD_PRELOAD can be passed in the command line 
>> without having to export the setting.
>>
>> webrev at the same location:
>>     http://cr.openjdk.java.net/~ccheung/8020791/webrev/
>>
>> Calvin
>>
>> On 7/19/2013 1:06 PM, Daniel D. Daugherty wrote:
>>>
>>> On 7/19/13 1:07 PM, Yumin Qi wrote:
>>>>
>>>> On 7/19/2013 11:48 AM, Calvin Cheung wrote:
>>>>> Thanks Dan and David for the reveiw.
>>>>>
>>>>> Dan,
>>>>>
>>>>> I've made all the changes as you've suggested except for the 
>>>>> LD_PRELOAD="$MY_LD_PRELOAD" before the java command. The shell 
>>>>> tried to execute the LD_PRELOAD="$MY_LD_PRELOAD" as a command and 
>>>>> resulting in "No such file or directory" error.
>>>>> So I'm keeping the "export" and "unset" of LD_PRELOAD for now.
>>>>>
>>>> You can put LD_PRELOAD="$MY_LD_PRELOAD" after command, that is
>>>> shell cmd LD_PRELOAD="$MY_LD_PRELOAD"
>>>
>>> Yumin,
>>>
>>> I don't think that will work. The
>>>
>>>     VAR=<value> cmd ....
>>>
>>> is interpreted by the shell (sh in this case). Saying
>>>
>>>     cmd ... VAR=<value>
>>>
>>> will just pass "VAR=<value>" as an option to the cmd and
>>> not set an environment variable.
>>>
>>> Dan
>>>
>>>
>>>>
>>>> Thanks
>>>> Yumin
>>>>> updated webrev is at the same location:
>>>>>     http://cr.openjdk.java.net/~ccheung/8020791/webrev/
>>>>>
>>>>> thanks,
>>>>> Calvin
>>>>>
>>>>> On 7/19/2013 7:22 AM, Daniel D. Daugherty wrote:
>>>>>> On 7/18/13 6:12 PM, Calvin Cheung wrote:
>>>>>>> Please review this small fix for a testcase.
>>>>>>>
>>>>>>> webrev:
>>>>>>> http://cr.openjdk.java.net/~ccheung/8020791/webrev/
>>>>>>
>>>>>> test/runtime/jsig/Test8017498.sh
>>>>>>     line 54: export LD_PRELOAD=${LD_PRELOAD}
>>>>>>         This export will affect every program run after this point.
>>>>>>
>>>>>>         Please change the variable name to MY_LD_PRELOAD" and don't
>>>>>>         export it.
>>>>>>
>>>>>>     line 57: NULL=NUL
>>>>>>     line 58: PS=";"
>>>>>>     line 59: FS="\\"
>>>>>>         These are not needed for your default case and they are
>>>>>>         Windows specific. Your default case is non-Linux so these
>>>>>>         are wrong in any case.
>>>>>>
>>>>>>     line 70: gcc -DLINUX -fPIC -shared -o 
>>>>>> ${TESTSRC}${FS}libTestJNI.so ...
>>>>>>         Please check the exit status of 'gcc' and do _something_
>>>>>>         if the compile fails. You could even pass the test if there
>>>>>>         is no compiler.
>>>>>>
>>>>>>     line 72: # run the java test in the background
>>>>>>         Why do you have to run the test in the background?
>>>>>>
>>>>>>     line 73: echo ${TESTJAVA}${FS}bin${FS}java 
>>>>>> -Djava.library.path=${TESTSRC}${FS} -server TestJNI 100 > 
>>>>>> test.out 2>&1 &
>>>>>>     line 74: ${TESTJAVA}${FS}bin${FS}java 
>>>>>> -Djava.library.path=${TESTSRC}${FS} -server TestJNI 100 > 
>>>>>> test.out 2>&1 &
>>>>>>     line 75:
>>>>>>     line 76: # obtain the process id
>>>>>>     line 77: C_PID=$!
>>>>>>
>>>>>>         line 73 will run the echo in background and you're 
>>>>>> running java in
>>>>>>         the background. That might lead to a race for the C_PID 
>>>>>> setting on
>>>>>>         line 77.
>>>>>>
>>>>>>         Lines 73 and 74 would be better as:
>>>>>>
>>>>>>         cmd="LD_PRELOAD="$MY_LD_PRELOAD" \
>>>>>>             ${TESTJAVA}${FS}bin${FS}java \
>>>>>>             -Djava.library.path=${TESTSRC}${FS} -server TestJNI 100"
>>>>>>         echo "$cmd > test.out 2>&1 &"
>>>>>>         $cmd > test.out 2>&1 &
>>>>>>
>>>>>>     line 80: sleep 1
>>>>>>         One second? This is just asking for intermittent failures of
>>>>>>         this test. If you decide that you have to run this test in
>>>>>>         background then something like this will be better:
>>>>>>
>>>>>>         count=0
>>>>>>         # give the test 60 seconds to pass
>>>>>>         while [ "$count" -lt 12 ]; do
>>>>>>             sleep 5
>>>>>>             grep "old handler" test.out > ${NULL}
>>>>>>             if [ $? = 0 ]; then
>>>>>>                 echo "Test Passed"
>>>>>>                 exit 0
>>>>>>             fi
>>>>>>             count=`expr $count + 1`
>>>>>>         done
>>>>>>         kill -9 ${C_PID}
>>>>>>         echo "Test Failed"
>>>>>>         exit 1
>>>>>>
>>>>>>     line 82: # reset LD_PRELOAD
>>>>>>     line 83: unset LD_PRELOAD
>>>>>>         You don't need this if you switch to the MY_LD_PRELOAD logic
>>>>>>         that I gave you above.
>>>>>>
>>>>>> test/runtime/jsig/TestJNI.c
>>>>>>     line 45: pthread_attr_t attr;
>>>>>>     line 46: stack_t stack;
>>>>>>         These variables are not used.
>>>>>>
>>>>>> Dan
>>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> Bug:
>>>>>>> https://jbs.oracle.com/bugs/browse/JDK-8020791
>>>>>>> http://bugs.sun.com/view_bug.do?bug_id=8020791
>>>>>>>
>>>>>>> When the testcase was developed, it was using an incomplete 
>>>>>>> jni_md.h
>>>>>>> without the following:
>>>>>>> #if defined(SOLARIS) || defined(LINUX) || defined(_ALLBSD_SOURCE)
>>>>>>> The default typedef long jlong will be used during compilation 
>>>>>>> of the
>>>>>>> native code of the testcase on linux_x64.
>>>>>>>
>>>>>>> With the above #if defined in the jni_md.h, if "LINUX" isn't 
>>>>>>> defined,
>>>>>>> the following typedef will be used and causing the compile error.
>>>>>>>     typedef __int64 jlong;
>>>>>>>
>>>>>>> The fix is to define LINUX (-DLINUX) in the gcc command line.
>>>>>>> The change in TestJNI.c is just a minor cleanup.
>>>>>>>
>>>>>>> Test:
>>>>>>>    Ran jtreg on the testcase.
>>>>>>>
>>>>>>> thanks,
>>>>>>> Calvin
>>>>>>
>>>>>
>>>>
>>>
>>
>



More information about the hotspot-runtime-dev mailing list