[jdk17] RFR: 8269661: JNI_GetStringCritical does not lock char array
Kim Barrett
kbarrett at openjdk.java.net
Sun Jul 4 10:59:03 UTC 2021
Please review this fix to jni_GetStringCritical. When object pinning is
being used it pins/unpins the String argument, but that's the wrong object.
It should be pinning the associated byte array (when !latin1), as it is a
reference into that array that will be returned and must be imobilized.
Presently only Shenandoah is affected by this bug, as it is the only GC that
uses object pinning (at the region level) rather than the GC locker. But
both G1 and ZGC plan to use object pinning in the future. But with region
pinning the problem is still going to be rare because the String and its
value array are likely to be allocated in the same region.
In addition, if pinning the String's value array, we also need to prevent
string deduplication from changing the array while in the critical section.
Otherwise, the unpin when the critical section is released will use the
wrong object for the unpin operation. We accomplish this by marking the
String as no longer subject to string deduplication. As that state is
sticky, when pinning is used a String can't be deduplicated after being used
in a critical section. We could add a critical section counter to String,
but for now we're going to guess that isn't worth the effort. (String has
at least 2 bytes available for the two string deduplication flags plus such
a counter (more in some configurations, but always at least 2 at present)).
As part of the restructuring to accomplish these changes, it proved to be
easy to avoid using the GC locker (or pinning) for the latin1 case, where a
copy of the value array will be used. Hence this change also addresses
JDK-8269650.
Testing:
Existing tests for GetStringCritical are in various vmTestbase/gc/lock
tests and vmTestbase/nsk/stress/jni/jnistress004.java.
Ran mach5 tier1, tier5. Tier5 is where the vmTestbase/gc/lock and
vmTestbase/nsk/stress/jni tests are run.
Locally (linux-x64) ran those tests with Shenandoah, with and without
string deduplication enabled.
-------------
Commit messages:
- conditionally lock the string value array
Changes: https://git.openjdk.java.net/jdk17/pull/209/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk17&pr=209&range=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8269661
Stats: 73 lines in 3 files changed: 60 ins; 10 del; 3 mod
Patch: https://git.openjdk.java.net/jdk17/pull/209.diff
Fetch: git fetch https://git.openjdk.java.net/jdk17 pull/209/head:pull/209
PR: https://git.openjdk.java.net/jdk17/pull/209
More information about the hotspot-dev
mailing list