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. (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.) /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 >
