On Tue, 19 Jan 2021 19:55:22 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:
>> Severin Gehwolf has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains four additional >> commits since the last revision: >> >> - Merge test files into one >> - Adress review feedback from dholmes >> - Merge branch 'master' into JDK-8258836-check-jni-mbeanserver >> - 8258836: JNI local refs exceed capacity getDiagnosticCommandInfo > > src/jdk.management/share/native/libmanagement_ext/DiagnosticCommandImpl.c > line 113: > >> 111: return NULL; >> 112: } >> 113: (*env)->PopLocalFrame(env, obj); > > This needs to assign the result to `obj` in order to get the affect you are > looking for. Otherwise it is really no different than passing in `NULL.`, and > in the `SetObjectArrayElement` that follows you'll end up assigning a popped > localref. I'm surprised this didn't result in some sort of test failure. Are > you testing with `num_args > 1`? Perhaps the localref frames are not eagerly > reused. Doh! Thanks Chris! Fixed. It did result in test failures (for fastdebug build). For example https://github.com/jerboaa/jdk/runs/1727117374?check_suite_focus=true Sorry for missing this. > src/jdk.management/share/native/libmanagement_ext/DiagnosticCommandImpl.c > line 172: > >> 170: } >> 171: if (num_commands == 0) { >> 172: (*env)->PopLocalFrame(env, result); > > Also here you need to assign the result to `result`. Fixed. > src/jdk.management/share/native/libmanagement_ext/DiagnosticCommandImpl.c > line 222: > >> 220: return NULL; >> 221: } >> 222: (*env)->PopLocalFrame(env, obj); > > ...and assign to `obj` here. Fixed. > src/jdk.management/share/native/libmanagement_ext/DiagnosticCommandImpl.c > line 227: > >> 225: EXCEPTION_CHECK_AND_FREE(dcmd_info_array); >> 226: } >> 227: (*env)->PopLocalFrame(env, result); > > ...and assign to `result` here. Fixed now. ------------- PR: https://git.openjdk.java.net/jdk/pull/2130
