Dan, Serguei: Thanks! On 28 mar 2014, at 19:23, [email protected] 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 <[email protected]> >>> 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 >> >
