adoroszlai commented on code in PR #6073:
URL: https://github.com/apache/ozone/pull/6073#discussion_r1462988081
##########
hadoop-ozone/dist/src/shell/ozone/ozone-functions.sh:
##########
@@ -1433,7 +1433,7 @@ function ozone_set_module_access_args
OZONE_MODULE_ACCESS_ARGS="${OZONE_MODULE_ACCESS_ARGS} --add-exports
java.management/com.sun.jmx.mbeanserver=ALL-UNNAMED"
fi
if [[ "${JAVA_MAJOR_VERSION}" -ge 9 ]]; then
- OZONE_MODULE_ACCESS_ARGS="${OZONE_MODULE_ACCESS_ARGS} --add-opens
java.base/java.nio=ALL-UNNAMED"
+ OZONE_MODULE_ACCESS_ARGS="${OZONE_MODULE_ACCESS_ARGS}
--add-opens='java.base/java.nio=ALL-UNNAMED'
--add-opens='java.base/java.lang=ALL-UNNAMED'
--add-opens='java.base/java.lang.reflect=ALL-UNNAMED'
--add-opens='java.management/com.sun.jmx.mbeanserver=ALL-UNNAMED'
--add-exports='java.management/com.sun.jmx.mbeanserver=ALL-UNNAMED'
--add-exports='java.base/sun.net.dns=ALL-UNNAMED' "
Review Comment:
1. Please follow the existing pattern to append items to the variable, one
per line. Do not use `'`.
2. This `-ge 9` block also applies to Java 17+, so we should not have
duplicates in the two `if` blocks. If something is needed for 9+ is already
there in the `-ge 17` block, move it.
3. Is `java.base/sun.net.dns` required for fixing reflection?
##########
hadoop-hdds/test-utils/src/main/java/org/apache/ozone/test/GenericTestUtils.java:
##########
@@ -468,4 +470,48 @@ public static String anyHostWithFreePort() {
}
}
+ /**
+ * This class is a utility class for java reflection operations.
+ */
+ public static final class ReflectionUtils {
+
+ /**
+ * This method provides the modifiers field using reflection approach
which is compatible
+ * for both pre Java 9 and post java 9 versions.
+ * @return modifiers field
+ * @throws IllegalAccessException
+ * @throws NoSuchFieldException
+ */
+ public static Field getModifiersField() throws IllegalAccessException,
NoSuchFieldException {
+ Field modifiersField = null;
+ try {
+ modifiersField = Field.class.getDeclaredField("modifiers");
+ } catch (NoSuchFieldException e) {
+ try {
+ Method getDeclaredFields0 = Class.class.getDeclaredMethod(
+ "getDeclaredFields0", boolean.class);
+ boolean accessibleBeforeSet = getDeclaredFields0.isAccessible();
+ getDeclaredFields0.setAccessible(true);
+ Field[] fields = (Field[]) getDeclaredFields0.invoke(Field.class,
false);
+ getDeclaredFields0.setAccessible(accessibleBeforeSet);
+ for (Field field : fields) {
+ if ("modifiers".equals(field.getName())) {
+ modifiersField = field;
+ break;
+ }
+ }
+ if (modifiersField == null) {
+ throw e;
+ }
+ } catch (NoSuchMethodException ex) {
+ e.addSuppressed(ex);
+ throw e;
+ } catch (InvocationTargetException ex) {
+ e.addSuppressed(ex);
+ throw e;
+ }
Review Comment:
nit: can combine identical `catch` blocks?
```suggestion
} catch (InvocationTargetException | NoSuchMethodException ex) {
e.addSuppressed(ex);
throw e;
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
