[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR: 8269268: JDWP: Properly fix thread lookup assert in findThread() [v2]
From:       David Holmes <dholmes () openjdk ! java ! net>
Date:       2021-06-30 1:14:06
Message-ID: H4xleK_xkf_pI9AQOWIQVJdHwmt1ouUVJxbTPcKfLb0=.b9b4f363-a596-4344-954f-b46bd242eab7 () github ! com
[Download RAW message or body]

On Thu, 24 Jun 2021 16:51:59 GMT, Chris Plummer <cjplummer@openjdk.org> wrote:

> > Re-enable the assert that was disabled (with some overhead) by \
> > [JDK-8265683](https://bugs.openjdk.java.net/browse/JDK-8265683). Explanation is \
> > in the CR and also in comments included with the changes. 
> > I tested by running `vmTestbase/nsk/jdb/suspend/suspend001/suspend001.java` and \
> > `vmTestbase/nsk/jdb/wherei/wherei001/wherei001.java` 100's of times, and did not \
> > see any failures. I also verified the original issue was still reproducible by \
> > temporarily not setting `gdata->handlingVMDeath = JNI_TRUE`, which did trigger \
> > the assert as expected.
> 
> Chris Plummer has updated the pull request incrementally with one additional commit \
> since the last revision: 
> Rename flag and make some minor adjustments.

Just to follow up:

> As far as any other synchronizing or memory barriers being needed, I'm a bit \
> unclear on that part. The only thing that matters is that \
> gdata->jvmtiCallBacksCleared being set true is visible to other threads before the \
> callbacks get cleared. It seems the call to SetEventCallbacks() should ensure this.

In practice this should be fine as `SetEventCallbacks` acquires a monitor/mutex; and \
`findThread` is also called while holding a monitor, so although different monitors, \
in practice the acquire/release semantics should make this work. Additionally the \
complexity of the interaction between the two threads involved will introduce other \
synchronization actions.

-------------

PR: https://git.openjdk.java.net/jdk/pull/4580


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic