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

[Staffan Larsen]

Dan, Serguei: Thanks!

On 28 mar 2014, at 19:23, serguei.spitsyn at oracle.com wrote:

> 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
>> 
>