Fail to register Java API for plugin that was not loaded (VPP-522)

Change-Id: Idb95888ae6fc7a9df197a919e3d3283f915f4a4c
Signed-off-by: Marek Gradzki <mgradzki@cisco.com>

7 files changed, 53 insertions, 47 deletions

diff --git a/plugins/ioam-plugin/ioam/jvpp/io/fd/vpp/jvpp/ioamtrace/test/ioamTraceApiTest.java b/plugins/ioam-plugin/ioam/jvpp/io/fd/vpp/jvpp/ioamtrace/test/IoamTraceApiTest.java
index ce8ff61218b..fa5c7ba4dc5 100644
--- a/plugins/ioam-plugin/ioam/jvpp/io/fd/vpp/jvpp/ioamtrace/test/ioamTraceApiTest.java
+++ b/plugins/ioam-plugin/ioam/jvpp/io/fd/vpp/jvpp/ioamtrace/test/IoamTraceApiTest.java
@@ -25,13 +25,12 @@ import io.fd.vpp.jvpp.ioamtrace.callback.TraceProfileAddCallback;
 import io.fd.vpp.jvpp.ioamtrace.dto.TraceProfileAdd;
 import io.fd.vpp.jvpp.ioamtrace.dto.TraceProfileAddReply;
 
-public class ioamTraceApiTest {
+public class IoamTraceApiTest {
 
-
-    static class ioamTraceTestCallback implements TraceProfileAddCallback {
+    static class IoamTraceTestCallback implements TraceProfileAddCallback {
 
         @Override
-	public void onTraceProfileAddReply(final TraceProfileAddReply reply) {
+        public void onTraceProfileAddReply(final TraceProfileAddReply reply) {
             System.out.printf("Received TraceProfileAddReply reply: context=%d%n",
                 reply.context);
         }
@@ -48,29 +47,24 @@ public class ioamTraceApiTest {
     }
 
     private static void ioamTraceTestApi() throws Exception {
-
         System.out.println("Testing Java API for ioam trace plugin");
-        final JVppRegistry registry = new JVppRegistryImpl("ioamTraceApiTest");
-        final JVpp jvpp = new JVppIoamtraceImpl();
+        try (final JVppRegistry registry = new JVppRegistryImpl("ioamTraceApiTest");
+             final JVpp jvpp = new JVppIoamtraceImpl()) {
+            registry.register(jvpp, new IoamTraceTestCallback());
 
-        registry.register(jvpp, new ioamTraceTestCallback());
-	try{
-	    System.out.println("Sending ioam trace profile add request...");
-	    TraceProfileAdd request = new TraceProfileAdd();
-	    request.traceType = 0x1f;
-	    request.numElts = 4;
-	    request.nodeId = 1;
-	    request.traceTsp = 2;
-	    request.appData = 1234;
-	    final int result = jvpp.send(request);
-	    System.out.printf("TraceProfileAdd send result = %d%n", result);
+            System.out.println("Sending ioam trace profile add request...");
+            TraceProfileAdd request = new TraceProfileAdd();
+            request.traceType = 0x1f;
+            request.numElts = 4;
+            request.nodeId = 1;
+            request.traceTsp = 2;
+            request.appData = 1234;
+            final int result = jvpp.send(request);
+            System.out.printf("TraceProfileAdd send result = %d%n", result);
 
-	    Thread.sleep(1000);
-	}
-        finally {
-	    System.out.println("Disconnecting...");
-            registry.close();
             Thread.sleep(1000);
-	}
+
+            System.out.println("Disconnecting...");
+        }
     }
 }
diff --git a/plugins/ioam-plugin/ioam/jvpp/io/fd/vpp/jvpp/ioamtrace/test/Readme.txt b/plugins/ioam-plugin/ioam/jvpp/io/fd/vpp/jvpp/ioamtrace/test/Readme.txt
index 0246d76d1af..1913f098fc4 100644
--- a/plugins/ioam-plugin/ioam/jvpp/io/fd/vpp/jvpp/ioamtrace/test/Readme.txt
+++ b/plugins/ioam-plugin/ioam/jvpp/io/fd/vpp/jvpp/ioamtrace/test/Readme.txt
@@ -1 +1 @@
-sudo java -cp build-vpp_debug-native/vpp-api/java/jvpp-registry-16.12.jar:build-vpp_debug-native/vpp-api/java/jvpp-core-16.12.jar:build-vpp_debug-native/plugins/ioam-plugin/jvpp-ioam-trace-1.0.jar io.fd.vpp.jvpp.ioamtrace.test.ioamTraceApiTest 
+sudo java -cp build-vpp_debug-native/vpp-api/java/jvpp-registry-16.12.jar:build-vpp_debug-native/plugins/ioam-plugin/jvpp-ioam-trace-1.0.jar io.fd.vpp.jvpp.ioamtrace.test.IoamTraceApiTest
diff --git a/plugins/ioam-plugin/ioam/lib-trace/jvpp_ioam_trace.c b/plugins/ioam-plugin/ioam/lib-trace/jvpp_ioam_trace.c
index 3b573506ec6..1d878ea3e24 100644
--- a/plugins/ioam-plugin/ioam/lib-trace/jvpp_ioam_trace.c
+++ b/plugins/ioam-plugin/ioam/lib-trace/jvpp_ioam_trace.c
@@ -64,11 +64,15 @@ JNIEXPORT void JNICALL Java_io_fd_vpp_jvpp_ioamtrace_JVppIoamtraceImpl_init0
   plugin_main->my_client_index = my_client_index;
   plugin_main->vl_input_queue = (unix_shared_memory_queue_t *)queue_address;
 
-    name = format (0, "ioam_trace_%08x%c", api_version, 0);
-    plugin_main->msg_id_base = vl_client_get_first_plugin_msg_id ((char *) name);
+  name = format (0, "ioam_trace_%08x%c", api_version, 0);
+  plugin_main->msg_id_base = vl_client_get_first_plugin_msg_id ((char *) name);
 
-  plugin_main->callbackObject = (*env)->NewGlobalRef(env, callback);
-  plugin_main->callbackClass = (jclass)(*env)->NewGlobalRef(env, (*env)->GetObjectClass(env, callback));
+  if (plugin_main->msg_id_base == (u16) ~0) {
+    jclass exClass = (*env)->FindClass(env, "java/lang/IllegalStateException");
+    (*env)->ThrowNew(env, exClass, "ioam_trace plugin is not loaded in VPP");
+  } else {
+    plugin_main->callbackObject = (*env)->NewGlobalRef(env, callback);
+    plugin_main->callbackClass = (jclass)(*env)->NewGlobalRef(env, (*env)->GetObjectClass(env, callback));
 
     #define _(N,n)                                  \
         vl_msg_api_set_handlers(VL_API_##N + plugin_main->msg_id_base, #n,     \
@@ -79,6 +83,7 @@ JNIEXPORT void JNICALL Java_io_fd_vpp_jvpp_ioamtrace_JVppIoamtraceImpl_init0
                 sizeof(vl_api_##n##_t), 1);
         foreach_api_reply_handler;
     #undef _
+  }
 }
 
 JNIEXPORT void JNICALL Java_io_fd_vpp_jvpp_ioamtrace_JVppIoamtraceImpl_close0
diff --git a/plugins/snat-plugin/snat/jvpp/io/fd/vpp/jvpp/snat/test/CallbackApiTest.java b/plugins/snat-plugin/snat/jvpp/io/fd/vpp/jvpp/snat/test/CallbackApiTest.java
index e168d0560e7..32165d962ba 100644
--- a/plugins/snat-plugin/snat/jvpp/io/fd/vpp/jvpp/snat/test/CallbackApiTest.java
+++ b/plugins/snat-plugin/snat/jvpp/io/fd/vpp/jvpp/snat/test/CallbackApiTest.java
@@ -64,6 +64,5 @@ public class CallbackApiTest {
 
             System.out.println("Disconnecting...");
         }
-        Thread.sleep(1000);
     }
 }
diff --git a/plugins/snat-plugin/snat/jvpp/io/fd/vpp/jvpp/snat/test/Readme.txt b/plugins/snat-plugin/snat/jvpp/io/fd/vpp/jvpp/snat/test/Readme.txt
index ac1b8f3f787..dbd4c996265 100644
--- a/plugins/snat-plugin/snat/jvpp/io/fd/vpp/jvpp/snat/test/Readme.txt
+++ b/plugins/snat-plugin/snat/jvpp/io/fd/vpp/jvpp/snat/test/Readme.txt
@@ -1 +1 @@
-sudo java -cp build-vpp-native/vpp-api/java/jvpp-registry-16.12.jar:build-vpp-native/vpp-api/java/jvpp-core-16.12.jar:build-vpp-native/plugins/snat-plugin/jvpp-snat-1.0.jar io.fd.vpp.jvpp.snat.test.CallbackApiTest
+sudo java -cp build-vpp-native/vpp-api/java/jvpp-registry-16.12.jar:build-vpp-native/plugins/snat-plugin/jvpp-snat-1.0.jar io.fd.vpp.jvpp.snat.test.CallbackApiTest
diff --git a/plugins/snat-plugin/snat/jvpp_snat.c b/plugins/snat-plugin/snat/jvpp_snat.c
index 537299ae751..fd72ddb14b2 100644
--- a/plugins/snat-plugin/snat/jvpp_snat.c
+++ b/plugins/snat-plugin/snat/jvpp_snat.c
@@ -64,11 +64,15 @@ JNIEXPORT void JNICALL Java_io_fd_vpp_jvpp_snat_JVppSnatImpl_init0
   plugin_main->my_client_index = my_client_index;
   plugin_main->vl_input_queue = (unix_shared_memory_queue_t *)queue_address;
 
-    name = format (0, "snat_%08x%c", api_version, 0);
-    plugin_main->msg_id_base = vl_client_get_first_plugin_msg_id ((char *) name);
+  name = format (0, "snat_%08x%c", api_version, 0);
+  plugin_main->msg_id_base = vl_client_get_first_plugin_msg_id ((char *) name);
 
-  plugin_main->callbackObject = (*env)->NewGlobalRef(env, callback);
-  plugin_main->callbackClass = (jclass)(*env)->NewGlobalRef(env, (*env)->GetObjectClass(env, callback));
+  if (plugin_main->msg_id_base == (u16) ~0) {
+    jclass exClass = (*env)->FindClass(env, "java/lang/IllegalStateException");
+    (*env)->ThrowNew(env, exClass, "snat plugin is not loaded in VPP");
+  } else {
+    plugin_main->callbackObject = (*env)->NewGlobalRef(env, callback);
+    plugin_main->callbackClass = (jclass)(*env)->NewGlobalRef(env, (*env)->GetObjectClass(env, callback));
 
     #define _(N,n)                                  \
         vl_msg_api_set_handlers(VL_API_##N + plugin_main->msg_id_base, #n,     \
@@ -79,6 +83,7 @@ JNIEXPORT void JNICALL Java_io_fd_vpp_jvpp_snat_JVppSnatImpl_init0
                 sizeof(vl_api_##n##_t), 1);
         foreach_api_reply_handler;
     #undef _
+  }
 }
 
 JNIEXPORT void JNICALL Java_io_fd_vpp_jvpp_snat_JVppSnatImpl_close0
diff --git a/vpp-api/java/jvpp-registry/io/fd/vpp/jvpp/JVppRegistryImpl.java b/vpp-api/java/jvpp-registry/io/fd/vpp/jvpp/JVppRegistryImpl.java
index fb2ef1c047f..01578ce0326 100644
--- a/vpp-api/java/jvpp-registry/io/fd/vpp/jvpp/JVppRegistryImpl.java
+++ b/vpp-api/java/jvpp-registry/io/fd/vpp/jvpp/JVppRegistryImpl.java
@@ -18,14 +18,16 @@ package io.fd.vpp.jvpp;
 
 import static java.util.Objects.requireNonNull;
 
+import io.fd.vpp.jvpp.callback.ControlPingCallback;
+import io.fd.vpp.jvpp.callback.JVppCallback;
+import io.fd.vpp.jvpp.dto.ControlPingReply;
 import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 import java.util.logging.Level;
 import java.util.logging.Logger;
-import io.fd.vpp.jvpp.callback.ControlPingCallback;
-import io.fd.vpp.jvpp.callback.JVppCallback;
-import io.fd.vpp.jvpp.dto.ControlPingReply;
 
 /**
  * Default implementation of JVppRegistry.
@@ -35,13 +37,13 @@ public final class JVppRegistryImpl implements JVppRegistry, ControlPingCallback
     private static final Logger LOG = Logger.getLogger(JVppRegistryImpl.class.getName());
 
     private final VppJNIConnection connection;
-    private final ConcurrentMap<String, JVppCallback> pluginRegistry;
+    private final Map<String, JVppCallback> pluginRegistry;
     private final ConcurrentMap<Integer, ControlPingCallback> pingCalls;
 
     public JVppRegistryImpl(final String clientName) throws IOException {
         connection = new VppJNIConnection(clientName);
         connection.connect();
-        pluginRegistry = new ConcurrentHashMap<>();
+        pluginRegistry = new HashMap<>();
         pingCalls = new ConcurrentHashMap<>();
     }
 
@@ -51,26 +53,28 @@ public final class JVppRegistryImpl implements JVppRegistry, ControlPingCallback
     }
 
     @Override
-    public void register(final JVpp jvpp, final JVppCallback callback) {
+    public synchronized void register(final JVpp jvpp, final JVppCallback callback) {
         requireNonNull(jvpp, "jvpp should not be null");
         requireNonNull(callback, "Callback should not be null");
         final String name = jvpp.getClass().getName();
-        if (pluginRegistry.putIfAbsent(name, callback) != null) {
-            throw new IllegalArgumentException(String.format("Callback for plugin %s was already registered", name));
+        if (pluginRegistry.containsKey(name)) {
+            throw new IllegalArgumentException(
+                String.format("Callback for plugin %s was already registered", name));
         }
         jvpp.init(this, callback, connection.getConnectionInfo().queueAddress,
             connection.getConnectionInfo().clientIndex);
+        pluginRegistry.put(name, callback);
     }
 
     @Override
-    public void unregister(final String name) {
+    public synchronized void unregister(final String name) {
         requireNonNull(name, "Plugin name should not be null");
         final JVppCallback previous = pluginRegistry.remove(name);
         assertPluginWasRegistered(name, previous);
     }
 
     @Override
-    public JVppCallback get(final String name) {
+    public synchronized JVppCallback get(final String name) {
         requireNonNull(name, "Plugin name should not be null");
         JVppCallback value = pluginRegistry.get(name);
         assertPluginWasRegistered(name, value);
@@ -80,7 +84,7 @@ public final class JVppRegistryImpl implements JVppRegistry, ControlPingCallback
     private native int controlPing0() throws VppInvocationException;
 
     @Override
-    public int controlPing(final Class<? extends JVpp> clazz) throws VppInvocationException {
+    public synchronized int controlPing(final Class<? extends JVpp> clazz) throws VppInvocationException {
         connection.checkActive();
         final String name = clazz.getName();
 
@@ -96,7 +100,6 @@ public final class JVppRegistryImpl implements JVppRegistry, ControlPingCallback
         return context;
     }
 
-
     @Override
     public void onControlPingReply(final ControlPingReply reply) {
         final ControlPingCallback callback = pingCalls.get(reply.context);