On 6/23/21 12:21 PM, Roman Kennke wrote:
Hi Simon,
my colleagues recently ran into a crash in libjdwp, when measuring
performance with OpenJDK 17 (early access build). The same crash was
observed with OpenJDK 16.0.1, but not with OpenJDK 15.
We are hoping to get a fix for the crash, before the official
OpenJDK 17
release. We've opened a RHEL bugzilla ticket for this (
https://bugzilla.redhat.com/show_bug.cgi?id=1972529), but we expect
this
won't be enough to resolve the crash before the release.
I wonder if this could be caused by calling commonRef_unpin on a ref
that is not "pinned"? Specifically, look at the updated weakenNode:
static jweak
weakenNode(JNIEnv *env, RefNode *node)
{
if (node->strongCount == 1) {
...
return weakRef;
} else {
node->strongCount--;
return node->ref;
}
}
if strongCount is 0, this will underflow and then delete node will
take the wrong path:
if (node->strongCount != 0) {
JNI_FUNC_PTR(env,DeleteGlobalRef)(env, node->ref);
} else {
JNI_FUNC_PTR(env,DeleteWeakGlobalRef)(env, node->ref);
}
The previous version of weakenNode looked like this:
static jweak
weakenNode(JNIEnv *env, RefNode *node)
{
if (node->isStrong) {
...
return weakRef;
} else {
return node->ref;
}
}
so a unbalanced unpin call would previously not fail in the same way.
Thank, Stefan, this seems like a plausible cause for the problem. It
seems to me that unbalanced pin and unpin can only be caused by
unbalanced DisableCollection/EnableCollection in the wire protocol,
and I guess we don't really have much control over this and should be
defensive.
Simon, can you test if this patch helps:
http://cr.openjdk.java.net/~rkennke/fixlibjdwp.patch
It basically protects from the underflow that Stefan found.
It would be good to have a test case for this. Is there a way to
construct tests that exercise JDWP commands?
Thanks,
Roman
https://bugs.openjdk.java.net/browse/JDK-8269232 has been filed. There
is more discussion there, including some caveats about the proposed fix.
I also suggest first adding an assert just to make sure you are indeed
triggering it. Eclipse should also look into why it likely has
unbalanced enable/disableCollection calls.
Chris
