RFR: 8037345 com/sun/jdi/* tests can fail, with race condition on log files

serguei.spitsyn at oracle.com serguei.spitsyn at oracle.com
Fri Mar 28 18:23:06 UTC 2014


I'm Ok with fix as it is.

Thanks,
Serguei

On 3/28/14 7:09 AM, Daniel D. Daugherty wrote:
>
> On 3/28/14 8:02 AM, Staffan Larsen wrote:
>> On 28 mar 2014, at 14:16, Daniel D. Daugherty 
>> <daniel.daugherty at oracle.com> wrote:
>>
>>>> http://cr.openjdk.java.net/~sla/8037345/webrev.00/
>>> test/com/sun/jdi/ShellScaffold.sh
>>>     Wouldn't it be better to make grepForString() work for multiple
>>>     processes by making the temp file unique via '$$'? You'll have
>>>     to update the logic carefully since theFile can refer to either
>>>     a file you want to keep or the temp file.
>> I thought about that, but wanted to avoid making the code more complex.
>
> Your call, but I think fixing grepForString() is less complex than
> the in-line logic that you had to use to get around this.
>
>
>> (I also noticed that $$ had the same value for both calls to 
>> grepForString(). $BASHPID on the other hand had different values 
>> (when running with bash, which wasn’t the default shell). I’m not a 
>> good enough bash hacker to understand why.)
>
> That sounds very, very strange. The whole point of '$$' is to give you
> a unique value for each shell process. If that's broken, then we have
> much bigger problems.
>
> In any case, I'm OK with what you have if you don't want to chase this
> down any more.
>
> Dan
>
>
>>
>> /Staffan
>>
>>
>>> Dan
>>>
>>>
>>> On 3/28/14 5:56 AM, Staffan Larsen wrote:
>>>> Please review this fix for the com/sun/jdi tests.
>>>>
>>>> In grepForString(), the script sometimes creates a temporary file 
>>>> which is used for grepping in. This file is a copy of the jdb 
>>>> outputfile and is deleted at the end of grepForString(). If two 
>>>> processes execute grepForString at the same time, there is a race 
>>>> where one process may delete the temporary file that the other 
>>>> process is still using. grepForString() is not written to be used 
>>>> bu multiple processes. Normally grepForString is only called from 
>>>> the jdp process.
>>>>
>>>> In waitForFinish() the main process is waiting for the jdb process 
>>>> to finish. It does this in a loop checking if the pid still exists, 
>>>> and also checking for some errors in the jdb outputfile. This last 
>>>> checking is the problem since it uses jdbFailIfPresent (which calls 
>>>> grepForString( to do this. Now grepForString is called from two 
>>>> different processes and we have a race. This last usage was 
>>>> introduced by the fix for JDK-6946101.
>>>>
>>>> The solution is to not call grepForString from the waitForFinish 
>>>> loop, but instead revert to the old behavior of using grep directly.
>>>>
>>>> webrev: http://cr.openjdk.java.net/~sla/8037345/webrev.00/
>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8037345
>>>>
>>>> Thanks,
>>>> /Staffan
>



More information about the serviceability-dev mailing list