RFR: JDK-8051349: nsk/jvmti/scenarios/sampling/SP06/sp06t003 fails in nightly

[David Holmes]

<trimming>

On 14/12/2018 11:28 pm, Gary Adams wrote:
> On 12/13/18, 11:51 AM, Chris Plummer wrote:
> BTW, another failure has been detected in sp06t001. This time the 
> threads are suspended,
> but I believe there is a race between thread start and the call to 
> interrupt() the thread.
> I think there may be some confusion over which thread is invoking the 
> interrupt() call.
> It is running on the main thread from the code after a call to start the 
> thread, but the
> thread may not have run when the interrupt is requested.
> 
> public class sp06t001 extends DebugeeClass {
> ...
> 
>          // create threads list
>          threads = new sp06t001Thread[] {
>              new sp06t001ThreadRunning("threadRunning", log),
>              new sp06t001ThreadEntering("threadEntering", log),
>              new sp06t001ThreadWaiting("threadWaiting", log),
>              new sp06t001ThreadSleeping("threadSleeping", log),
>              new 
> sp06t001ThreadRunningInterrupted("threadRunningInterrupted", log),
>              new sp06t001ThreadRunningNative("threadRunningNative", log)
>          };
> 
>          // run threads
>          log.display("Starting tested threads");
>          try {
>              synchronized (endingMonitor) {
>                  // start threads (except first one)
>                  for (int i = 0; i < threads.length; i++) {
>                      threads[i].start();
>                      if (!threads[i].checkReady()) {
>                          throw new Failure("Unable to prepare thread ..."
> ...
> 
> class sp06t001ThreadRunningInterrupted extends sp06t001Thread {
> ...
>      public boolean checkReady() {
>          // interrupt thread on wait()
>          synchronized (waitingMonitor) {
>              interrupt();
>          }
>          return true;
>      }

checkReady is broken. The synchronization is completely pointless as it 
doesn't ensure the target got to the wait() as the calling thread could 
call checkReady before the target has run sufficient to acquire the 
waitingMonitor monitor.

That said as long as the interrupt() occurs after the thread has been 
started it will cause the wait() to terminate. Though the wait() itself 
could return early (in theory) due to spurious wakeup.

The coordination/synchronization in these tests is really flawed.

David
-----

>>
>> thanks,
>>
>> Chris
>>
>> On 12/13/18 5:25 AM, Gary Adams wrote:
>>> While testing I ran into another of the related failures that were
>>> associated with the original bug.
>>> The following fake exception stacktrace is for failure analysis.
>>> nsk.share.Fake_Exception_for_RULE_Creation: (sp02t003.cpp:313) jvmti->GetFrameLocation(threadsDesc[i].thread, j, &qMethod, &qLocation)
>>> 	at nsk_lvcomplain(nsk_tools.cpp:172)
>>> # ERROR: sp02t003.cpp, 313: jvmti->GetFrameLocation(threadsDesc[i].thread, j, &qMethod, &qLocation)
>>> #   jvmti error: code=31, name=JVMTI_ERROR_NO_MORE_FRAMES
>>> - sp02t003.cpp, 310:       3 frame: method: 0x7f225042b2d8, location: 16
>>> # ERROR: sp02t003.cpp, 313: jvmti->GetFrameLocation(threadsDesc[i].thread, j, &qMethod, &qLocation)
>>> #   jvmti error: code=31, name=JVMTI_ERROR_NO_MORE_FRAMES
>>> # ERROR: sp02t003.cpp, 350: No expected method frame for not suspended thread #4 (threadRunningInterrupted)
>>> In this particular failure, the GetFrameLocation call failed, because 
>>> the frame was no longer
>>> accessible.
>>>
>>> If the threads are not suspended, the GetFrameLocation may fail,
>>> or if it passes, the qMethod and qLocation could belong to another
>>> frame.
>>>
>>> I've updated the webrev to allow the call to GetFrameLocation, but to 
>>> only
>>> report an error if the threads are suspended. Similarly, the checks 
>>> to compare
>>> qMethod and qLocation will be skipped, if the threads are not suspended.
>>> And the final comparison that the method was found will only be an error
>>> if the threads are suspended.
>>>
>>>   Webrev: http://cr.openjdk.java.net/~gadams/8051349/webrev.01/
>>>
>>> On 12/12/18, 11:54 AM, Gary Adams wrote:
>>>> After some testing with nsk verbose messages enabled,
>>>> it is clear that this test is failing in checkThreads when the
>>>> location did not match between the call to GetStackTrace
>>>> and GetFrameLocation. For the tests that are run when the threads
>>>> have not been suspended, there is no guarantee the locations
>>>> will match.
>>>>
>>>>   Issue: https://bugs.openjdk.java.net/browse/JDK-8051349
>>>>   Webrev: http://cr.openjdk.java.net/~gadams/8051349/webrev.00/
>>>>
>>>
>>
>