RFR 6946101: ShellScaffold.sh based tests can issue "write error: Broken pipe" messages

[Staffan Larsen]

Very nice change - looks good!

test/com/sun/jdi/ShellScaffold.sh 
  line 1000:     # mydojdbCmds() didn't finished because it waits for JDB message
    nit: finished -> finish

Just a note that this should be pushed through jdk9/dev and not jdk9/hs-comp (where the webrev was made).

Thanks,
/Staffan



On 27 feb 2014, at 23:47, Daniel D. Daugherty <daniel.daugherty at oracle.com> wrote:

> On 2/27/14 9:20 AM, Pavel Punegov wrote:
>> Please review the fix for:
>>   https://bugs.openjdk.java.net/browse/JDK-6946101
>> 
>> webrev:
>>   http://cr.openjdk.java.net/~iignatyev/ppunegov/6946101/webrev.00/
> 
> test/com/sun/jdi/ShellScaffold.sh
>    line 531: # allows JDB to exit"
>        stray double-quote at end of comment
> 
>    line 563: dofail "It's not allowed to send quit and exit commands from the test"
>        'and' should be 'or'
> 
>    line 819:  # Kill debugger, it could be hang
>        Typo: 'hang' -> 'hung'
> 
> test/com/sun/jdi/ArrayLengthDumpTest.sh
> test/com/sun/jdi/CatchAllTest.sh
> test/com/sun/jdi/CatchCaughtTest.sh
> test/com/sun/jdi/CatchPatternTest.sh
> test/com/sun/jdi/CommandCommentDelimiter.sh
> test/com/sun/jdi/DeferredStepTest.sh
> test/com/sun/jdi/DeoptimizeWalk.sh
> test/com/sun/jdi/EvalArgs.sh
> test/com/sun/jdi/GetLocalVariables3Test.sh
> test/com/sun/jdi/GetLocalVariables4Test.sh
> test/com/sun/jdi/JdbExprTest.sh
> test/com/sun/jdi/JdbLockTest.sh
> test/com/sun/jdi/JdbMethodExitTest.sh
> test/com/sun/jdi/JdbMissStep.sh
> test/com/sun/jdi/MixedSuspendTest.sh
> test/com/sun/jdi/NotAField.sh
> test/com/sun/jdi/NullLocalVariable.sh
> test/com/sun/jdi/Redefine-g.sh
> test/com/sun/jdi/RedefineAnnotation.sh
> test/com/sun/jdi/RedefineChangeClassOrder.sh
> test/com/sun/jdi/RedefineClasses.sh
> test/com/sun/jdi/RedefineException.sh
> test/com/sun/jdi/RedefineFinal.sh
> test/com/sun/jdi/RedefineImplementor.sh
> test/com/sun/jdi/RedefineIntConstantToLong.sh
> test/com/sun/jdi/RedefineMulti.sh
> test/com/sun/jdi/RedefinePop.sh
> test/com/sun/jdi/RedefineTTYLineNumber.sh
> test/com/sun/jdi/StringConvertTest.sh
> test/com/sun/jdi/WatchFramePop.sh
>    I _think_ I understand the new test driver style:
> 
>    - get rid of all explicit 'cmd quit' usages because mydojdbCmds()
>      now wraps the test's dojdbCmds with a 'quit' cmd
>    - any test that previously ended with a 'cmd cont' is presumed to
>      be OK of that 'cmd cont' caused jdb to execute off the end of
>      main(); sounds reasonable to me
>    - perfect example of the new logic to catch an errant run off the
>      end is  test/com/sun/jdi/WatchFramePop.sh
>      - the last jdb cmd is 'next'
>      - and jdb is NOT supposed to run off the end
>      - the new logic should catch this nicely; I _think_ the old
>        logic would only catch a run off the end if someone manually
>        checked the test result
> 
> 
> Thumbs up!
> 
> Dan
> 
> 
>> 
>> 
>> This change fixes two issues with the tests:
>>   1. Fix incorrect 'quit' command sending to JDB when JDB process was finished.
>>   2. Improve JDB unexpected exit detection and  process synchronization.
>> 
>> Description of fix:
>>   1. Add allowExit parameter to cmd() to show that the given command can finish
>>   JDB. E.g., 'cont' command make JDB execute debuggee to the end.
>>   If allowExit wasn't set for a command then  assume that it can't finish
>>   execution, and fail the test if it did.
>> 
>>   2. Make test fail if it tries to send 'quit' or 'exit' commands. This makes
>>   it impossible to send quit/exit from test by mistake. Scaffold will
>>   finish JDB by itself if JDB didn't finish before be a command with allowExit
>>   set. Add dofinish() function to be the only method that may exit JDB.
>> 
>>   3. Add proper synchronization into waitForFinish(). On all systems except
>>   SunOS use wait (from bash). On Solaris find the shell subprocess and wait for
>>   its finish. It replaces wait  used on all other systems, because it
>>   doesn't work on sh/ksh as in bash.
>> 
>>   4. Fix tests: add allowExit to tests where it's needed.
>> 
>