diff options
author | 2016-11-14 16:14:58 +0100 | |
---|---|---|
committer | 2016-11-17 10:05:26 +0000 | |
commit | 94ea8b7ff3a429fd08c8316873497f35967ae635 (patch) | |
tree | a5248c91131cc969230b38fe1536a84f6d6eac55 /vpp-api/java/jvpp-registry | |
parent | c0f6cf36a519421cac89601a52a85aa792ddc20f (diff) |
VPP-533 Fix ping race condition in JVpp
Improper synchronization between ping_send and ping_reply_handle
Change-Id: I844c96bc3f5cd750a1c43188d3133c92f8f14e38
Signed-off-by: Maros Marsalek <mmarsale@cisco.com>
Diffstat (limited to 'vpp-api/java/jvpp-registry')
-rw-r--r-- | vpp-api/java/jvpp-registry/io/fd/vpp/jvpp/JVppRegistryImpl.java | 45 |
1 files changed, 28 insertions, 17 deletions
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 01578ce0326..8e101404434 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 @@ -25,7 +25,6 @@ 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; @@ -37,14 +36,16 @@ public final class JVppRegistryImpl implements JVppRegistry, ControlPingCallback private static final Logger LOG = Logger.getLogger(JVppRegistryImpl.class.getName()); private final VppJNIConnection connection; + // Unguarded concurrent map, no race conditions expected on top of that private final Map<String, JVppCallback> pluginRegistry; - private final ConcurrentMap<Integer, ControlPingCallback> pingCalls; + // Guarded by self + private final Map<Integer, ControlPingCallback> pingCalls; public JVppRegistryImpl(final String clientName) throws IOException { connection = new VppJNIConnection(clientName); connection.connect(); - pluginRegistry = new HashMap<>(); - pingCalls = new ConcurrentHashMap<>(); + pluginRegistry = new ConcurrentHashMap<>(); + pingCalls = new HashMap<>(); } @Override @@ -53,7 +54,7 @@ public final class JVppRegistryImpl implements JVppRegistry, ControlPingCallback } @Override - public synchronized void register(final JVpp jvpp, final JVppCallback callback) { + public 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(); @@ -67,14 +68,14 @@ public final class JVppRegistryImpl implements JVppRegistry, ControlPingCallback } @Override - public synchronized void unregister(final String name) { + public void unregister(final String name) { requireNonNull(name, "Plugin name should not be null"); final JVppCallback previous = pluginRegistry.remove(name); assertPluginWasRegistered(name, previous); } @Override - public synchronized JVppCallback get(final String name) { + public JVppCallback get(final String name) { requireNonNull(name, "Plugin name should not be null"); JVppCallback value = pluginRegistry.get(name); assertPluginWasRegistered(name, value); @@ -84,27 +85,33 @@ public final class JVppRegistryImpl implements JVppRegistry, ControlPingCallback private native int controlPing0() throws VppInvocationException; @Override - public synchronized int controlPing(final Class<? extends JVpp> clazz) throws VppInvocationException { + public int controlPing(final Class<? extends JVpp> clazz) throws VppInvocationException { connection.checkActive(); final String name = clazz.getName(); final ControlPingCallback callback = (ControlPingCallback) pluginRegistry.get(clazz.getName()); assertPluginWasRegistered(name, callback); - int context = controlPing0(); - if (context < 0) { - throw new VppInvocationException("controlPing", context); - } + synchronized (pingCalls) { + int context = controlPing0(); + if (context < 0) { + throw new VppInvocationException("controlPing", context); + } - pingCalls.put(context, callback); - return context; + pingCalls.put(context, callback); + return context; + } } @Override public void onControlPingReply(final ControlPingReply reply) { - final ControlPingCallback callback = pingCalls.get(reply.context); + final ControlPingCallback callback; + synchronized (pingCalls) { + callback = pingCalls.remove(reply.context); + } if (callback == null) { - LOG.log(Level.WARNING, "No callback was registered for reply id={0} ", reply.context); + LOG.log(Level.WARNING, "No callback was registered for reply context=" + reply.context + " Contexts waiting=" + + pingCalls.keySet()); return; } // pass the reply to the callback registered by the ping caller @@ -114,7 +121,11 @@ public final class JVppRegistryImpl implements JVppRegistry, ControlPingCallback @Override public void onError(final VppCallbackException ex) { final int ctxId = ex.getCtxId(); - final ControlPingCallback callback = pingCalls.get(ctxId); + final ControlPingCallback callback; + + synchronized (pingCalls) { + callback = pingCalls.get(ctxId); + } if (callback == null) { LOG.log(Level.WARNING, "No callback was registered for reply id={0} ", ctxId); return; |