Simplify allocation of JNI global classes and exceptions (#1110)

Helps ensure they get freed properly (We have had a few cases before where this wasn't the case).

cscore/src/main/native/cpp/jni/CameraServerJNI.cpp

@@ -29,6 +29,16 @@ static JException unsupportedEx;
 // Thread-attached environment for listener callbacks.
 static JNIEnv* listenerEnv = nullptr;
 
+static const JClassInit classes[] = {
+    {"edu/wpi/cscore/UsbCameraInfo", &usbCameraInfoCls},
+    {"edu/wpi/cscore/VideoMode", &videoModeCls},
+    {"edu/wpi/cscore/VideoEvent", &videoEventCls}};
+
+static const JExceptionInit exceptions[] = {
+    {"edu/wpi/cscore/VideoException", &videoEx},
+    {"java/lang/NullPointerException", &nullPointerEx},
+    {"java/lang/UnsupportedOperationException", &unsupportedEx}};
+
 static void ListenerOnStart() {
   if (!jvm) return;
   JNIEnv* env;
@@ -59,23 +69,15 @@ JNIEXPORT jint JNICALL JNI_OnLoad(JavaVM* vm, void* reserved) {
     return JNI_ERR;
 
   // Cache references to classes
-  usbCameraInfoCls = JClass{env, "edu/wpi/cscore/UsbCameraInfo"};
-  if (!usbCameraInfoCls) return JNI_ERR;
+  for (auto& c : classes) {
+    *c.cls = JClass(env, c.name);
+    if (!*c.cls) return JNI_ERR;
+  }
 
-  videoModeCls = JClass{env, "edu/wpi/cscore/VideoMode"};
-  if (!videoModeCls) return JNI_ERR;
-
-  videoEventCls = JClass{env, "edu/wpi/cscore/VideoEvent"};
-  if (!videoEventCls) return JNI_ERR;
-
-  videoEx = JException{env, "edu/wpi/cscore/VideoException"};
-  if (!videoEx) return JNI_ERR;
-
-  nullPointerEx = JException(env, "java/lang/NullPointerException");
-  if (!nullPointerEx) return JNI_ERR;
-
-  unsupportedEx = JException(env, "java/lang/UnsupportedOperationException");
-  if (!unsupportedEx) return JNI_ERR;
+  for (auto& c : exceptions) {
+    *c.cls = JException(env, c.name);
+    if (!*c.cls) return JNI_ERR;
+  }
 
   // Initial configuration of listener start/exit
   cs::SetListenerOnStart(ListenerOnStart);
@@ -89,12 +91,12 @@ JNIEXPORT void JNICALL JNI_OnUnload(JavaVM* vm, void* reserved) {
   if (vm->GetEnv(reinterpret_cast<void**>(&env), JNI_VERSION_1_6) != JNI_OK)
     return;
   // Delete global references
-  usbCameraInfoCls.free(env);
-  videoModeCls.free(env);
-  videoEventCls.free(env);
-  videoEx.free(env);
-  nullPointerEx.free(env);
-  unsupportedEx.free(env);
+  for (auto& c : classes) {
+    c.cls->free(env);
+  }
+  for (auto& c : exceptions) {
+    c.cls->free(env);
+  }
   jvm = nullptr;
 }
 

hal/src/main/native/cpp/jni/HALUtil.cpp

@@ -61,6 +61,29 @@ static JClass matchInfoDataCls;
 static JClass accumulatorResultCls;
 static JClass canDataCls;
 
+static const JClassInit classes[] = {
+    {"edu/wpi/first/wpilibj/PWMConfigDataResult", &pwmConfigDataResultCls},
+    {"edu/wpi/first/wpilibj/can/CANStatus", &canStatusCls},
+    {"edu/wpi/first/wpilibj/hal/MatchInfoData", &matchInfoDataCls},
+    {"edu/wpi/first/wpilibj/AccumulatorResult", &accumulatorResultCls},
+    {"edu/wpi/first/wpilibj/CANData", &canDataCls}};
+
+static const JExceptionInit exceptions[] = {
+    {"java/lang/RuntimeException", &runtimeExCls},
+    {"java/lang/IllegalArgumentException", &illegalArgExCls},
+    {"edu/wpi/first/wpilibj/util/BoundaryException", &boundaryExCls},
+    {"edu/wpi/first/wpilibj/util/AllocationException", &allocationExCls},
+    {"edu/wpi/first/wpilibj/util/HalHandleException", &halHandleExCls},
+    {"edu/wpi/first/wpilibj/can/CANInvalidBufferException",
+     &canInvalidBufferExCls},
+    {"edu/wpi/first/wpilibj/can/CANMessageNotFoundException",
+     &canMessageNotFoundExCls},
+    {"edu/wpi/first/wpilibj/can/CANMessageNotAllowedException",
+     &canMessageNotAllowedExCls},
+    {"edu/wpi/first/wpilibj/can/CANNotInitializedException",
+     &canNotInitializedExCls},
+    {"edu/wpi/first/wpilibj/util/UncleanStatusException", &uncleanStatusExCls}};
+
 namespace frc {
 
 void ThrowAllocationException(JNIEnv* env, int32_t minRange, int32_t maxRange,
@@ -273,59 +296,15 @@ JNIEXPORT jint JNICALL JNI_OnLoad(JavaVM* vm, void* reserved) {
   if (vm->GetEnv(reinterpret_cast<void**>(&env), JNI_VERSION_1_6) != JNI_OK)
     return JNI_ERR;
 
-  runtimeExCls = JException(env, "java/lang/RuntimeException");
-  if (!runtimeExCls) return JNI_ERR;
+  for (auto& c : classes) {
+    *c.cls = JClass(env, c.name);
+    if (!*c.cls) return JNI_ERR;
+  }
 
-  illegalArgExCls = JException(env, "java/lang/IllegalArgumentException");
-  if (!illegalArgExCls) return JNI_ERR;
-
-  boundaryExCls =
-      JException(env, "edu/wpi/first/wpilibj/util/BoundaryException");
-  if (!boundaryExCls) return JNI_ERR;
-
-  allocationExCls =
-      JException(env, "edu/wpi/first/wpilibj/util/AllocationException");
-  if (!allocationExCls) return JNI_ERR;
-
-  halHandleExCls =
-      JException(env, "edu/wpi/first/wpilibj/util/HalHandleException");
-  if (!halHandleExCls) return JNI_ERR;
-
-  canInvalidBufferExCls =
-      JException(env, "edu/wpi/first/wpilibj/can/CANInvalidBufferException");
-  if (!canInvalidBufferExCls) return JNI_ERR;
-
-  canMessageNotFoundExCls =
-      JException(env, "edu/wpi/first/wpilibj/can/CANMessageNotFoundException");
-  if (!canMessageNotFoundExCls) return JNI_ERR;
-
-  canMessageNotAllowedExCls = JException(
-      env, "edu/wpi/first/wpilibj/can/CANMessageNotAllowedException");
-  if (!canMessageNotAllowedExCls) return JNI_ERR;
-
-  canNotInitializedExCls =
-      JException(env, "edu/wpi/first/wpilibj/can/CANNotInitializedException");
-  if (!canNotInitializedExCls) return JNI_ERR;
-
-  uncleanStatusExCls =
-      JException(env, "edu/wpi/first/wpilibj/util/UncleanStatusException");
-  if (!uncleanStatusExCls) return JNI_ERR;
-
-  pwmConfigDataResultCls =
-      JClass(env, "edu/wpi/first/wpilibj/PWMConfigDataResult");
-  if (!pwmConfigDataResultCls) return JNI_ERR;
-
-  canStatusCls = JClass(env, "edu/wpi/first/wpilibj/can/CANStatus");
-  if (!canStatusCls) return JNI_ERR;
-
-  matchInfoDataCls = JClass(env, "edu/wpi/first/wpilibj/hal/MatchInfoData");
-  if (!matchInfoDataCls) return JNI_ERR;
-
-  accumulatorResultCls = JClass(env, "edu/wpi/first/wpilibj/AccumulatorResult");
-  if (!accumulatorResultCls) return JNI_ERR;
-
-  canDataCls = JClass(env, "edu/wpi/first/wpilibj/CANData");
-  if (!canDataCls) return JNI_ERR;
+  for (auto& c : exceptions) {
+    *c.cls = JException(env, c.name);
+    if (!*c.cls) return JNI_ERR;
+  }
 
   return sim::SimOnLoad(vm, reserved);
 }
@@ -337,21 +316,13 @@ JNIEXPORT void JNICALL JNI_OnUnload(JavaVM* vm, void* reserved) {
   if (vm->GetEnv(reinterpret_cast<void**>(&env), JNI_VERSION_1_6) != JNI_OK)
     return;
   // Delete global references
-  runtimeExCls.free(env);
-  illegalArgExCls.free(env);
-  boundaryExCls.free(env);
-  allocationExCls.free(env);
-  halHandleExCls.free(env);
-  canInvalidBufferExCls.free(env);
-  canMessageNotFoundExCls.free(env);
-  canMessageNotAllowedExCls.free(env);
-  canNotInitializedExCls.free(env);
-  uncleanStatusExCls.free(env);
-  pwmConfigDataResultCls.free(env);
-  canStatusCls.free(env);
-  matchInfoDataCls.free(env);
-  accumulatorResultCls.free(env);
-  canDataCls.free(env);
+
+  for (auto& c : classes) {
+    c.cls->free(env);
+  }
+  for (auto& c : exceptions) {
+    c.cls->free(env);
+  }
   jvm = nullptr;
 }
 

ntcore/src/main/native/cpp/jni/NetworkTablesJNI.cpp

@@ -43,6 +43,24 @@ static JException interruptedEx;
 static JException nullPointerEx;
 static JException persistentEx;
 
+static const JClassInit classes[] = {
+    {"java/lang/Boolean", &booleanCls},
+    {"edu/wpi/first/networktables/ConnectionInfo", &connectionInfoCls},
+    {"edu/wpi/first/networktables/ConnectionNotification",
+     &connectionNotificationCls},
+    {"java/lang/Double", &doubleCls},
+    {"edu/wpi/first/networktables/EntryInfo", &entryInfoCls},
+    {"edu/wpi/first/networktables/EntryNotification", &entryNotificationCls},
+    {"edu/wpi/first/networktables/LogMessage", &logMessageCls},
+    {"edu/wpi/first/networktables/RpcAnswer", &rpcAnswerCls},
+    {"edu/wpi/first/networktables/NetworkTableValue", &valueCls}};
+
+static const JExceptionInit exceptions[] = {
+    {"java/lang/IllegalArgumentException", &illegalArgEx},
+    {"java/lang/InterruptedException", &interruptedEx},
+    {"java/lang/NullPointerException", &nullPointerEx},
+    {"edu/wpi/first/networktables/PersistentException", &persistentEx}};
+
 extern "C" {
 
 JNIEXPORT jint JNICALL JNI_OnLoad(JavaVM* vm, void* reserved) {
@@ -53,47 +71,15 @@ JNIEXPORT jint JNICALL JNI_OnLoad(JavaVM* vm, void* reserved) {
     return JNI_ERR;
 
   // Cache references to classes
-  booleanCls = JClass(env, "java/lang/Boolean");
-  if (!booleanCls) return JNI_ERR;
+  for (auto& c : classes) {
+    *c.cls = JClass(env, c.name);
+    if (!*c.cls) return JNI_ERR;
+  }
 
-  connectionInfoCls = JClass(env, "edu/wpi/first/networktables/ConnectionInfo");
-  if (!connectionInfoCls) return JNI_ERR;
-
-  connectionNotificationCls =
-      JClass(env, "edu/wpi/first/networktables/ConnectionNotification");
-  if (!connectionNotificationCls) return JNI_ERR;
-
-  doubleCls = JClass(env, "java/lang/Double");
-  if (!doubleCls) return JNI_ERR;
-
-  entryInfoCls = JClass(env, "edu/wpi/first/networktables/EntryInfo");
-  if (!entryInfoCls) return JNI_ERR;
-
-  entryNotificationCls =
-      JClass(env, "edu/wpi/first/networktables/EntryNotification");
-  if (!entryNotificationCls) return JNI_ERR;
-
-  logMessageCls = JClass(env, "edu/wpi/first/networktables/LogMessage");
-  if (!logMessageCls) return JNI_ERR;
-
-  rpcAnswerCls = JClass(env, "edu/wpi/first/networktables/RpcAnswer");
-  if (!rpcAnswerCls) return JNI_ERR;
-
-  valueCls = JClass(env, "edu/wpi/first/networktables/NetworkTableValue");
-  if (!valueCls) return JNI_ERR;
-
-  illegalArgEx = JException(env, "java/lang/IllegalArgumentException");
-  if (!illegalArgEx) return JNI_ERR;
-
-  interruptedEx = JException(env, "java/lang/InterruptedException");
-  if (!interruptedEx) return JNI_ERR;
-
-  nullPointerEx = JException(env, "java/lang/NullPointerException");
-  if (!nullPointerEx) return JNI_ERR;
-
-  persistentEx =
-      JException(env, "edu/wpi/first/networktables/PersistentException");
-  if (!persistentEx) return JNI_ERR;
+  for (auto& c : exceptions) {
+    *c.cls = JException(env, c.name);
+    if (!*c.cls) return JNI_ERR;
+  }
 
   return JNI_VERSION_1_6;
 }
@@ -103,19 +89,12 @@ JNIEXPORT void JNICALL JNI_OnUnload(JavaVM* vm, void* reserved) {
   if (vm->GetEnv(reinterpret_cast<void**>(&env), JNI_VERSION_1_6) != JNI_OK)
     return;
   // Delete global references
-  booleanCls.free(env);
-  connectionInfoCls.free(env);
-  connectionNotificationCls.free(env);
-  doubleCls.free(env);
-  entryInfoCls.free(env);
-  entryNotificationCls.free(env);
-  logMessageCls.free(env);
-  rpcAnswerCls.free(env);
-  valueCls.free(env);
-  illegalArgEx.free(env);
-  interruptedEx.free(env);
-  nullPointerEx.free(env);
-  persistentEx.free(env);
+  for (auto& c : classes) {
+    c.cls->free(env);
+  }
+  for (auto& c : exceptions) {
+    c.cls->free(env);
+  }
   jvm = nullptr;
 }
 

wpiutil/src/main/native/include/wpi/jni_util.h

@@ -71,6 +71,11 @@ class JClass {
   jclass m_cls = nullptr;
 };
 
+struct JClassInit {
+  const char* name;
+  JClass* cls;
+};
+
 template <typename T>
 class JGlobal {
  public:
@@ -618,6 +623,11 @@ class JException : public JClass {
   jmethodID m_constructor = nullptr;
 };
 
+struct JExceptionInit {
+  const char* name;
+  JException* cls;
+};
+
 }  // namespace java
 }  // namespace wpi