andreacavalli/netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Do not use GetPrimitiveArrayCritical(...) due multiple not-fixed bugs… (#8921)

Browse Source 
* Do not use GetPrimitiveArrayCritical(...) due multiple not-fixed bugs related to GCLocker

Motivation:

GetPrimitiveArrayCritical(...) may cause multiple not-fixed bugs related to the GCLocker while there is little gain for our use-case. We should just use GetByteArrayRegion(...) and copy into a small on-stack buffer.

See also:

- https://shipilev.net/jvm/anatomy-quarks/9-jni-critical-gclocker/#_g1
- https://bugs.openjdk.java.net/browse/JDK-8048556
- https://bugs.openjdk.java.net/browse/JDK-8057573
- https://bugs.openjdk.java.net/browse/JDK-8057586

Special thanks to @jayv @shipilev @apangin for the pointers.

Modifications:

Replace GetPrimitiveArrayCritical(...) with GetByteArrayRegion(...)

Result:

Less risks hitting GCLocker related bugs.
This commit is contained in:
Norman Maurer 2019-03-07 10:30:55 +01:00
parent 22128a85fe
commit 2689f28cb7
1 changed files with 16 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
transport-native-unix-common/src/main/c/netty_unix_socket.c
View File

@ -232,16 +232,25 @@ static jint _socket(JNIEnv* env, jclass clazz, int type) {
int netty_unix_socket_initSockaddr(JNIEnv* env, jbyteArray address, jint scopeId, jint jport, int netty_unix_socket_initSockaddr(JNIEnv* env, jbyteArray address, jint scopeId, jint jport,
const struct sockaddr_storage* addr, socklen_t* addrSize) { const struct sockaddr_storage* addr, socklen_t* addrSize) {
uint16_t port = htons((uint16_t) jport); uint16_t port = htons((uint16_t) jport);
// We use 16 bytes as this allows us to fit ipv6, ipv4 and ipv4 mapped ipv6 addresses in the array.
jbyte addressBytes[16];
// Use GetPrimitiveArrayCritical and ReleasePrimitiveArrayCritical to signal the VM that we really would like int len = (*env)->GetArrayLength(env, address);
// to not do a memory copy here. This is ok as we not do any blocking action here anyway.
// This is important as the VM may suspend GC for the time! if (len > 16) {
jbyte* addressBytes = (*env)->GetPrimitiveArrayCritical(env, address, 0); // This should never happen but let's guard against it anyway.
if (addressBytes == NULL) {
// No memory left ?!?!?
netty_unix_errors_throwOutOfMemoryError(env);
return -1; return -1;
} }
// We use GetByteArrayRegion(...) and copy into a small stack allocated buffer and NOT GetPrimitiveArrayCritical(...)
// as there are still multiple GCLocker related bugs which are not fixed yet.
//
// For example:
// https://bugs.openjdk.java.net/browse/JDK-8048556
// https://bugs.openjdk.java.net/browse/JDK-8057573
// https://bugs.openjdk.java.net/browse/JDK-8057586
(*env)->GetByteArrayRegion(env, address, 0, len, addressBytes);
if (socketType == AF_INET6) { if (socketType == AF_INET6) {
struct sockaddr_in6* ip6addr = (struct sockaddr_in6*) addr; struct sockaddr_in6* ip6addr = (struct sockaddr_in6*) addr;
*addrSize = sizeof(struct sockaddr_in6); *addrSize = sizeof(struct sockaddr_in6);
@ -262,8 +271,6 @@ int netty_unix_socket_initSockaddr(JNIEnv* env, jbyteArray address, jint scopeId
ipaddr->sin_port = port; ipaddr->sin_port = port;
memcpy(&(ipaddr->sin_addr.s_addr), addressBytes + 12, 4); memcpy(&(ipaddr->sin_addr.s_addr), addressBytes + 12, 4);
} }
(*env)->ReleasePrimitiveArrayCritical(env, address, addressBytes, JNI_ABORT);
return 0; return 0;
} }