8318631: GetStackTraceSuspendedStressTest.java failed with: check_jvmti_status: JVMTI function returned error: JVMTI_ERROR_THREAD_NOT_ALIVE (15)

[sspitsyn]

It is a fix of a minor test issue.
The test should not fail when the JVMTI function SetEventNotificationMode() returns errors codes:

• JVMTI_ERROR_THREAD_NOT_ALIVE
• JVMTI_ERROR_WRONG_PHASE

Tested the fix locally and with mach5 test runs.

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8318631: GetStackTraceSuspendedStressTest.java failed with: check_jvmti_status: JVMTI function returned error: JVMTI_ERROR_THREAD_NOT_ALIVE (15) (Bug - P4)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/16488/head:pull/16488
$ git checkout pull/16488

Update a local copy of the PR:
$ git checkout pull/16488
$ git pull https://git.openjdk.org/jdk.git pull/16488/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 16488

View PR using the GUI difftool:
$ git pr show -t 16488

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/16488.diff

Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back sspitsyn! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@sspitsyn The following label will be automatically applied to this pull request:

• serviceability

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 01: Full - Incremental (4f9d30c0)
• 00: Full (58609b44)

[alexmenkov]

WRONG_PHASE looks good to me, but why THREAD_NOT_ALIVE is considered expected for suspended thread?
if the thread was terminated, SuspendThread should return THREAD_NOT_ALIVE, but once SuspendThread returns ERROR_NONE, how the thread can terminates before ResumeThread?

[dholmes-ora]

My question exactly. I'm not even sure why wrong phase is allowed here.

[AlanBateman]

Is the issue here that agent thread started by Debuggee.checkStatus is racing with the test? The producer/consumer thread do 1000 put/take ops and it looks like it can complete and VM commence shutdown while the agent thread is observing, do I read this correctly?

[sspitsyn]

Thank you for the comments.
Alex, you are right.
I've misread this code at line 201:

199         err = jvmti->SuspendThread(vthread);
200         if (err == JVMTI_ERROR_THREAD_NOT_ALIVE) {
201           continue;
202         }
203         check_jvmti_status(jni, err, "Error in SuspendThread");
204         // LOG("Agent: suspended vt: %s ct: %s\n", vname, cname);
205 
206         check_vthread_consistency_suspended(jvmti, jni, vthread);

The function check_vthread_consistency_suspended() is not called when if the SuspendThread returned JVMTI_ERROR_THREAD_NOT_ALIVE error code.

It does not look as a test bug then.

[sspitsyn]

David and Alan,
Yes, there is always a race between the app and the JVMTI agent execution. This was always a general issue with the JVMTI. The agent code executing JVMTI functions and events is not protected from the JVMTI_ERROR_WRONG_PHASE errors. This error code can be returned by JVMTI functions even in the middle of the event callback execution. We had this problem in the JDWP agent and fixed it by adding proper synchronization into the agent itself.

[bridgekeeper]

@sspitsyn This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

[bridgekeeper]

@sspitsyn This pull request has been inactive for more than 8 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the /open pull request command.