|
On 12/14/18 5:28 AM, Gary Adams wrote:
BTW, I forgot to ask if you can fix the typo in the following
comment:
345 /* check frame equalaty */
On 12/13/18, 11:51 AM, Chris Plummer wrote:
Hi Gary,
Unfortunately GetStackTrace() returns the top frame as the
first frame in the array. Thus if the thread is not suspended
and later you call GetFrameLocation() for some depth, there's
no way to make sure the frame at that depth is the same frame
at that depth in the array returned by GetStackTrace() unless
you first suspend the frame before calling GetFrameLocation().
You would also need to call GetFrameCount() while suspended,
compare that with the number of frames returned by
GetStackTrace(), and make any needed depth adjustments. But
since I think the intent is to not suspend the thread here, it
probably does not make sense to do that, and instead accept
that there might be some errors as you have done.
I'm not totally comfortable with these attempted operations and
then
bypassing the test failure if the threads were not suspended.
Would it be better to only perform check thread operations on the
suspended threads and to totally skip the operations that can not
be safely performed when the thread is not suspended.
I was starting to think along those lines. What's the point of
testing something if you know it can fail and you can't distinguish
between expected and unexpected failure. The other option is to
suspend like I describe above to make sure you can accurately verify
the stack trace. I'm just not sure if it makes sense to do that. Why
is the test run unsuspended in the first place?
One improvement I would like to see for your fix is to only
ignore JVMTI_ERROR_NO_MORE_FRAMES rather than ignore all
failures when suspended == NSK_TRUE.
The current layering of the macros and verify routines do not lend
themselves
to this sort of selective error checking. I'd probably file a
follow up bug to
address specific error checking.
Can't you just bypass the use of NSK_VERIFY? Isn't JC getting rid of
it anyway and using his JNI exception mechanism? In that case you
would need to use the version that does not fail if there was an
exception.
Also, I don't think you want the suspended check here:
348 /* check if expected method frame found */
349 if ((suspended == NSK_TRUE) && (found
<= 0)) {
The check for finding the method is:
341 if (frameStack[j].method ==
threadsDesc[i].method) {
Since frameStack[] is returned by GetStackTrace(), it is not
impacted when not suspended, and the expected method should
always be in frameStack[] somewhere. The issue is only with
using GetFrameLocation() to correlate what is in the result of
GetStackTrace().
Agreed. The early continuation was bypassing the found method
checking.
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;
}
Certainly checkReady() could be called before the target thread has
started the wait(). Then the thread does the wait() and probably
never exits (or maybe the early interrupted is resulting in some
other unexpected behavior).
I think you need synchronize between the two threads. What if the
target thread does a synchronized (waitingMonitor), sets a flag, and
then a wait(). The main thread sleeps until the flag is set, does a
synchronized(waitingMonitor), and then the interrupt().
Chris
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/
|
