RFR(XS) 8020791: [TESTBUG] runtime/jsig/Test8017498.sh failed to compile
Daniel D. Daugherty
daniel.daugherty at oracle.com
Fri Jul 19 13:06:46 PDT 2013
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