Node init work: separate Node construction and init

java/jni/ZT_jnicache.cpp

@@ -114,13 +114,6 @@ jmethodID VirtualNetworkType_fromInt_method;
 // Instance fields
 //
 
-jfieldID Node_configListener_field;
-jfieldID Node_eventListener_field;
-jfieldID Node_frameListener_field;
-jfieldID Node_getListener_field;
-jfieldID Node_pathChecker_field;
-jfieldID Node_putListener_field;
-jfieldID Node_sender_field;
 jfieldID PeerPhysicalPath_address_field;
 jfieldID PeerPhysicalPath_lastReceive_field;
 jfieldID PeerPhysicalPath_lastSend_field;
@@ -243,13 +236,6 @@ void setupJNICache(JavaVM *vm) {
     // Instance fields
     //
 
-    EXCEPTIONANDNULLCHECK(Node_configListener_field = env->GetFieldID(Node_class, "configListener", "Lcom/zerotier/sdk/VirtualNetworkConfigListener;"));
-    EXCEPTIONANDNULLCHECK(Node_eventListener_field = env->GetFieldID(Node_class, "eventListener", "Lcom/zerotier/sdk/EventListener;"));
-    EXCEPTIONANDNULLCHECK(Node_frameListener_field = env->GetFieldID(Node_class, "frameListener", "Lcom/zerotier/sdk/VirtualNetworkFrameListener;"));
-    EXCEPTIONANDNULLCHECK(Node_getListener_field = env->GetFieldID(Node_class, "getListener", "Lcom/zerotier/sdk/DataStoreGetListener;"));
-    EXCEPTIONANDNULLCHECK(Node_pathChecker_field = env->GetFieldID(Node_class, "pathChecker", "Lcom/zerotier/sdk/PathChecker;"));
-    EXCEPTIONANDNULLCHECK(Node_putListener_field = env->GetFieldID(Node_class, "putListener", "Lcom/zerotier/sdk/DataStorePutListener;"));
-    EXCEPTIONANDNULLCHECK(Node_sender_field = env->GetFieldID(Node_class, "sender", "Lcom/zerotier/sdk/PacketSender;"));
     EXCEPTIONANDNULLCHECK(PeerPhysicalPath_address_field = env->GetFieldID(PeerPhysicalPath_class, "address", "Ljava/net/InetSocketAddress;"));
     EXCEPTIONANDNULLCHECK(PeerPhysicalPath_lastReceive_field = env->GetFieldID(PeerPhysicalPath_class, "lastReceive", "J"));
     EXCEPTIONANDNULLCHECK(PeerPhysicalPath_lastSend_field = env->GetFieldID(PeerPhysicalPath_class, "lastSend", "J"));

java/jni/ZT_jnicache.h

@@ -83,13 +83,6 @@ extern jmethodID VirtualNetworkType_fromInt_method;
 // Instance fields
 //
 
-extern jfieldID Node_configListener_field;
-extern jfieldID Node_eventListener_field;
-extern jfieldID Node_frameListener_field;
-extern jfieldID Node_getListener_field;
-extern jfieldID Node_pathChecker_field;
-extern jfieldID Node_putListener_field;
-extern jfieldID Node_sender_field;
 extern jfieldID PeerPhysicalPath_address_field;
 extern jfieldID PeerPhysicalPath_lastReceive_field;
 extern jfieldID PeerPhysicalPath_lastSend_field;

java/jni/com_zerotierone_sdk_Node.cpp

@@ -54,6 +54,7 @@ namespace {
             , configListener(NULL)
             , pathChecker(NULL)
             , callbacks(NULL)
+            , inited()
         {
             callbacks = (ZT_Node_Callbacks*)malloc(sizeof(ZT_Node_Callbacks));
             memset(callbacks, 0, sizeof(ZT_Node_Callbacks));
@@ -91,6 +92,10 @@ namespace {
         jobject pathChecker;
 
         ZT_Node_Callbacks *callbacks;
+
+        bool inited;
+
+        bool finishInitializing();
     };
 
 
@@ -539,19 +544,81 @@ namespace {
     }
 
     typedef std::map<int64_t, JniRef*> NodeMap;
-    static NodeMap nodeMap;
+    NodeMap nodeMap;
     ZeroTier::Mutex nodeMapMutex;
 
+    bool isInited(int64_t nodeId) {
+
+        ZeroTier::Mutex::Lock lock(nodeMapMutex);
+        NodeMap::iterator found = nodeMap.find(nodeId);
+
+        if (found == nodeMap.end()) {
+
+            //
+            // not in map yet, or has been removed from map
+            //
+            return false;
+        }
+        
+        JniRef *ref = found->second;
+
+        assert(ref);
+
+        return ref->inited;
+    }
+
+    bool JniRef::finishInitializing() {
+
+        ZeroTier::Mutex::Lock lock(nodeMapMutex);
+        NodeMap::iterator found = nodeMap.find(id);
+
+        if (found != nodeMap.end()) {
+            //
+            // already in map
+            //
+            LOGE("Cannot finish initializing; node is already in map");
+            return false;
+        }
+
+        nodeMap.insert(std::make_pair(id, this));
+
+        assert(!inited);
+        inited = true;
+
+        return true;
+    }
+
     ZT_Node* findNode(int64_t nodeId)
     {
         ZeroTier::Mutex::Lock lock(nodeMapMutex);
         NodeMap::iterator found = nodeMap.find(nodeId);
-        if(found != nodeMap.end())
-        {
-            JniRef *ref = found->second;
-            return ref->node;
+
+        assert(found != nodeMap.end());
+
+        JniRef *ref = found->second;
+
+        assert(ref);
+
+        return ref->node;
+    }
+
+    JniRef *removeRef(int64_t nodeId) {
+
+        ZeroTier::Mutex::Lock lock(nodeMapMutex);
+
+        NodeMap::iterator found = nodeMap.find(nodeId);
+
+        if (found == nodeMap.end()) {
+            return nullptr;
         }
-        return NULL;
+
+        JniRef *ref = found->second;
+
+        assert(ref);
+
+        nodeMap.erase(nodeId);
+
+        return ref;
     }
 }
 
@@ -574,10 +641,13 @@ JNIEXPORT void JNICALL JNI_OnUnload(JavaVM *vm, void *reserved)
 /*
  * Class:     com_zerotier_sdk_Node
  * Method:    node_init
- * Signature: (J)Lcom/zerotier/sdk/ResultCode;
+ * Signature: (JLcom/zerotier/sdk/DataStoreGetListener;Lcom/zerotier/sdk/DataStorePutListener;Lcom/zerotier/sdk/PacketSender;Lcom/zerotier/sdk/EventListener;Lcom/zerotier/sdk/VirtualNetworkFrameListener;Lcom/zerotier/sdk/VirtualNetworkConfigListener;Lcom/zerotier/sdk/PathChecker;)Lcom/zerotier/sdk/ResultCode;
  */
 JNIEXPORT jobject JNICALL Java_com_zerotier_sdk_Node_node_1init(
-    JNIEnv *env, jobject obj, jlong now)
+    JNIEnv *env, jobject obj, jlong now, jobject dataStoreGetListener,
+    jobject dataStorePutListener, jobject packetSender, jobject eventListener,
+    jobject frameListener, jobject configListener,
+    jobject pathChecker)
 {
     LOGV("Creating ZT_Node struct");
     jobject resultObject = ResultCode_RESULT_OK_enum;
@@ -587,49 +657,42 @@ JNIEXPORT jobject JNICALL Java_com_zerotier_sdk_Node_node_1init(
     ref->id = (int64_t)now;
     env->GetJavaVM(&ref->jvm);
 
-    jobject dataStoreGetListener = env->GetObjectField(obj, Node_getListener_field);
     if(dataStoreGetListener == NULL)
     {
         return NULL;
     }
     ref->dataStoreGetListener = env->NewGlobalRef(dataStoreGetListener);
 
-    jobject dataStorePutListener = env->GetObjectField(obj, Node_putListener_field);
     if(dataStorePutListener == NULL)
     {
         return NULL;
     }
     ref->dataStorePutListener = env->NewGlobalRef(dataStorePutListener);
 
-    jobject packetSender = env->GetObjectField(obj, Node_sender_field);
     if(packetSender == NULL)
     {
         return NULL;
     }
     ref->packetSender = env->NewGlobalRef(packetSender);
 
-    jobject frameListener = env->GetObjectField(obj, Node_frameListener_field);
     if(frameListener == NULL)
     {
         return NULL;
     }
     ref->frameListener = env->NewGlobalRef(frameListener);
 
-    jobject configListener = env->GetObjectField(obj, Node_configListener_field);
     if(configListener == NULL)
     {
         return NULL;
     }
     ref->configListener = env->NewGlobalRef(configListener);
 
-    jobject eventListener = env->GetObjectField(obj, Node_eventListener_field);
     if(eventListener == NULL)
     {
         return NULL;
     }
     ref->eventListener = env->NewGlobalRef(eventListener);
 
-    jobject pathChecker = env->GetObjectField(obj, Node_pathChecker_field);
     if(pathChecker != NULL)
     {
         ref->pathChecker = env->NewGlobalRef(pathChecker);
@@ -669,13 +732,29 @@ JNIEXPORT jobject JNICALL Java_com_zerotier_sdk_Node_node_1init(
         return resultObject;
     }
 
-    ZeroTier::Mutex::Lock lock(nodeMapMutex);
+    //
+    // node is now updated
+    //
     ref->node = node;
-    nodeMap.insert(std::make_pair(ref->id, ref));
+
+    if (!ref->finishInitializing()) {
+        LOGE("finishInitializing() failed");
+        return ResultCode_RESULT_FATAL_ERROR_INTERNAL_enum;
+    }
 
     return resultObject;
 }
 
+/*
+ * Class:     com_zerotier_sdk_Node
+ * Method:    node_isInited
+ * Signature: (J)Z
+ */
+JNIEXPORT jboolean JNICALL Java_com_zerotier_sdk_Node_node_1isInited
+        (JNIEnv *env, jobject obj, jlong nodeId) {
+    return isInited(nodeId);
+}
+
 /*
  * Class:     com_zerotier_sdk_Node
  * Method:    node_delete
@@ -687,25 +766,15 @@ JNIEXPORT void JNICALL Java_com_zerotier_sdk_Node_node_1delete(
     LOGV("Destroying ZT_Node struct");
     int64_t nodeId = (int64_t)id;
 
-    NodeMap::iterator found;
-
-    ZeroTier::Mutex::Lock lock(nodeMapMutex);
-    found = nodeMap.find(nodeId);
+    JniRef *ref = removeRef(nodeId);
 
-    if(found != nodeMap.end())
-    {
-        JniRef *ref = found->second;
-        nodeMap.erase(found);
+    if (!ref) {
+        return;
+    }
 
-        ZT_Node_delete(ref->node);
+    ZT_Node_delete(ref->node);
 
-        delete ref;
-        ref = NULL;
-    }
-    else
-    {
-        LOGE("Attempted to delete a node that doesn't exist!");
-    }
+    delete ref;
 }
 
 /*
@@ -728,11 +797,6 @@ JNIEXPORT jobject JNICALL Java_com_zerotier_sdk_Node_processVirtualNetworkFrame(
     int64_t nodeId = (int64_t) id;
 
     ZT_Node *node = findNode(nodeId);
-    if(node == NULL)
-    {
-        // cannot find valid node.  We should  never get here.
-        return ResultCode_RESULT_FATAL_ERROR_INTERNAL_enum;
-    }
 
     unsigned int nbtd_len = env->GetArrayLength(out_nextBackgroundTaskDeadline);
     if(nbtd_len < 1)
@@ -794,12 +858,6 @@ JNIEXPORT jobject JNICALL Java_com_zerotier_sdk_Node_processWirePacket(
 {
     int64_t nodeId = (int64_t) id;
     ZT_Node *node = findNode(nodeId);
-    if(node == NULL)
-    {
-        // cannot find valid node.  We should  never get here.
-        LOGE("Couldn't find a valid node!");
-        return ResultCode_RESULT_FATAL_ERROR_INTERNAL_enum;
-    }
 
     unsigned int nbtd_len = (unsigned int)env->GetArrayLength(out_nextBackgroundTaskDeadline);
     if(nbtd_len < 1)
@@ -917,11 +975,6 @@ JNIEXPORT jobject JNICALL Java_com_zerotier_sdk_Node_processBackgroundTasks(
 {
     int64_t nodeId = (int64_t) id;
     ZT_Node *node = findNode(nodeId);
-    if(node == NULL)
-    {
-        // cannot find valid node.  We should  never get here.
-        return ResultCode_RESULT_FATAL_ERROR_INTERNAL_enum;
-    }
 
     unsigned int nbtd_len = env->GetArrayLength(out_nextBackgroundTaskDeadline);
     if(nbtd_len < 1)
@@ -951,12 +1004,6 @@ JNIEXPORT jobject JNICALL Java_com_zerotier_sdk_Node_join(
 {
     int64_t nodeId = (int64_t) id;
     ZT_Node *node = findNode(nodeId);
-    if(node == NULL)
-    {
-        // cannot find valid node.  We should  never get here.
-        return ResultCode_RESULT_FATAL_ERROR_INTERNAL_enum;
-    }
-
     uint64_t nwid = (uint64_t)in_nwid;
 
     ZT_ResultCode rc = ZT_Node_join(node, nwid, NULL, NULL);
@@ -974,11 +1021,6 @@ JNIEXPORT jobject JNICALL Java_com_zerotier_sdk_Node_leave(
 {
     int64_t nodeId = (int64_t) id;
     ZT_Node *node = findNode(nodeId);
-    if(node == NULL)
-    {
-        // cannot find valid node.  We should  never get here.
-        return ResultCode_RESULT_FATAL_ERROR_INTERNAL_enum;
-    }
 
     uint64_t nwid = (uint64_t)in_nwid;
 
@@ -1001,11 +1043,6 @@ JNIEXPORT jobject JNICALL Java_com_zerotier_sdk_Node_multicastSubscribe(
 {
     int64_t nodeId = (int64_t) id;
     ZT_Node *node = findNode(nodeId);
-    if(node == NULL)
-    {
-        // cannot find valid node.  We should  never get here.
-        return ResultCode_RESULT_FATAL_ERROR_INTERNAL_enum;
-    }
 
     uint64_t nwid = (uint64_t)in_nwid;
     uint64_t multicastGroup = (uint64_t)in_multicastGroup;
@@ -1031,11 +1068,6 @@ JNIEXPORT jobject JNICALL Java_com_zerotier_sdk_Node_multicastUnsubscribe(
 {
     int64_t nodeId = (int64_t) id;
     ZT_Node *node = findNode(nodeId);
-    if(node == NULL)
-    {
-        // cannot find valid node.  We should  never get here.
-        return ResultCode_RESULT_FATAL_ERROR_INTERNAL_enum;
-    }
 
     uint64_t nwid = (uint64_t)in_nwid;
     uint64_t multicastGroup = (uint64_t)in_multicastGroup;
@@ -1060,10 +1092,6 @@ JNIEXPORT jobject JNICALL Java_com_zerotier_sdk_Node_orbit(
 {
     int64_t nodeId = (int64_t)id;
     ZT_Node *node = findNode(nodeId);
-    if(node == NULL)
-    {
-        return ResultCode_RESULT_FATAL_ERROR_INTERNAL_enum;
-    }
 
     uint64_t moonWorldId = (uint64_t)in_moonWorldId;
     uint64_t moonSeed = (uint64_t)in_moonSeed;
@@ -1084,10 +1112,6 @@ JNIEXPORT jobject JNICALL Java_com_zerotier_sdk_Node_deorbit(
 {
     int64_t nodeId = (int64_t)id;
     ZT_Node *node = findNode(nodeId);
-    if(node == NULL)
-    {
-        return ResultCode_RESULT_FATAL_ERROR_INTERNAL_enum;
-    }
 
     uint64_t moonWorldId = (uint64_t)in_moonWorldId;
 
@@ -1105,11 +1129,6 @@ JNIEXPORT jlong JNICALL Java_com_zerotier_sdk_Node_address(
 {
     int64_t nodeId = (int64_t) id;
     ZT_Node *node = findNode(nodeId);
-    if(node == NULL)
-    {
-        // cannot find valid node.  We should  never get here.
-        return 0;
-    }
 
     uint64_t address = ZT_Node_address(node);
     return (jlong)address;
@@ -1142,11 +1161,6 @@ JNIEXPORT jobject JNICALL Java_com_zerotier_sdk_Node_networkConfig(
 {
     int64_t nodeId = (int64_t) id;
     ZT_Node *node = findNode(nodeId);
-    if(node == NULL)
-    {
-        // cannot find valid node.  We should  never get here.
-        return 0;
-    }
 
     ZT_VirtualNetworkConfig *vnetConfig = ZT_Node_networkConfig(node, nwid);
 
@@ -1184,11 +1198,6 @@ JNIEXPORT jobjectArray JNICALL Java_com_zerotier_sdk_Node_peers(
 {
     int64_t nodeId = (int64_t) id;
     ZT_Node *node = findNode(nodeId);
-    if(node == NULL)
-    {
-        // cannot find valid node.  We should  never get here.
-        return 0;
-    }
 
     ZT_PeerList *peerList = ZT_Node_peers(node);
 
@@ -1238,11 +1247,6 @@ JNIEXPORT jobjectArray JNICALL Java_com_zerotier_sdk_Node_networks(
 {
     int64_t nodeId = (int64_t) id;
     ZT_Node *node = findNode(nodeId);
-    if(node == NULL)
-    {
-        // cannot find valid node.  We should  never get here.
-        return 0;
-    }
 
     ZT_VirtualNetworkList *networkList = ZT_Node_networks(node);
     if(networkList == NULL)

java/jni/com_zerotierone_sdk_Node.h

@@ -10,9 +10,17 @@ extern "C" {
 /*
  * Class:     com_zerotier_sdk_Node
  * Method:    node_init
- * Signature: (J)Lcom/zerotier/sdk/ResultCode;
+ * Signature: (JLcom/zerotier/sdk/DataStoreGetListener;Lcom/zerotier/sdk/DataStorePutListener;Lcom/zerotier/sdk/PacketSender;Lcom/zerotier/sdk/EventListener;Lcom/zerotier/sdk/VirtualNetworkFrameListener;Lcom/zerotier/sdk/VirtualNetworkConfigListener;Lcom/zerotier/sdk/PathChecker;)Lcom/zerotier/sdk/ResultCode;
  */
 JNIEXPORT jobject JNICALL Java_com_zerotier_sdk_Node_node_1init
+  (JNIEnv *, jobject, jlong, jobject, jobject, jobject, jobject, jobject, jobject, jobject);
+
+/*
+ * Class:     com_zerotier_sdk_Node
+ * Method:    node_isInited
+ * Signature: (J)Z;
+ */
+JNIEXPORT jboolean JNICALL Java_com_zerotier_sdk_Node_node_1isInited
   (JNIEnv *, jobject, jlong);
 
 /*

java/src/com/zerotier/sdk/Node.java

@@ -63,60 +63,58 @@ public class Node {
     /**
      * Node ID for JNI purposes.
      * Currently set to the now value passed in at the constructor
-     * 
-     * -1 if the node has already been closed
      */
-    private long nodeId;
-
-    private final DataStoreGetListener getListener;
-    private final DataStorePutListener putListener;
-    private final PacketSender sender;
-    private final EventListener eventListener;
-    private final VirtualNetworkFrameListener frameListener;
-    private final VirtualNetworkConfigListener configListener;
-    private final PathChecker pathChecker;
+    private final long nodeId;
     
     /**
      * Create a new ZeroTier One node
      *
+     * @param now Current clock in milliseconds
+     */
+    public Node(long now) {
+        this.nodeId = now;
+    }
+
+    /**
+     * Init a new ZeroTier One node
+     *
      * <p>Note that this can take a few seconds the first time it's called, as it
      * will generate an identity.</p>
      *
-     * @param now Current clock in milliseconds
      * @param getListener User written instance of the {@link DataStoreGetListener} interface called to get objects from persistent storage.  This instance must be unique per Node object.
      * @param putListener User written instance of the {@link DataStorePutListener} interface called to put objects in persistent storage.  This instance must be unique per Node object.
-     * @param sender
+     * @param sender User written instance of the {@link PacketSender} interface to send ZeroTier packets out over the wire.
      * @param eventListener User written instance of the {@link EventListener} interface to receive status updates and non-fatal error notices.  This instance must be unique per Node object.
-     * @param frameListener 
+     * @param frameListener User written instance of the {@link VirtualNetworkFrameListener} interface to send a frame out to a virtual network port.
      * @param configListener User written instance of the {@link VirtualNetworkConfigListener} interface to be called when virtual LANs are created, deleted, or their config parameters change.  This instance must be unique per Node object.
      * @param pathChecker User written instance of the {@link PathChecker} interface. Not required and can be null.
      */
-	public Node(long now,
-                DataStoreGetListener getListener,
-                DataStorePutListener putListener,
-                PacketSender sender,
-                EventListener eventListener,
-                VirtualNetworkFrameListener frameListener,
-                VirtualNetworkConfigListener configListener,
-                PathChecker pathChecker) throws NodeException
-	{
-        this.nodeId = now;
-
-        this.getListener = getListener;
-        this.putListener = putListener;
-        this.sender = sender;
-        this.eventListener = eventListener;
-        this.frameListener = frameListener;
-        this.configListener = configListener;
-        this.pathChecker = pathChecker;
-
-        ResultCode rc = node_init(now);
-        if(rc != ResultCode.RESULT_OK)
-        {
-            // TODO: Throw Exception
+    public ResultCode init(
+            DataStoreGetListener getListener,
+            DataStorePutListener putListener,
+            PacketSender sender,
+            EventListener eventListener,
+            VirtualNetworkFrameListener frameListener,
+            VirtualNetworkConfigListener configListener,
+            PathChecker pathChecker) throws NodeException {
+        ResultCode rc = node_init(
+                nodeId,
+                getListener,
+                putListener,
+                sender,
+                eventListener,
+                frameListener,
+                configListener,
+                pathChecker);
+        if(rc != ResultCode.RESULT_OK) {
             throw new NodeException(rc.toString());
         }
-	}
+        return rc;
+    }
+
+    public boolean isInited() {
+        return node_isInited(nodeId);
+    }
 
     /**
       * Close this Node.
@@ -124,10 +122,7 @@ public class Node {
       * <p>The Node object can no longer be used once this method is called.</p>
       */
     public void close() {
-        if(nodeId != -1) {
-            node_delete(nodeId);
-            nodeId = -1;
-        }
+        node_delete(nodeId);
     }
 
     @Override
@@ -408,7 +403,17 @@ public class Node {
     //
     // function declarations for JNI
     //
-    private native ResultCode node_init(long now);
+    private native ResultCode node_init(
+            long nodeId,
+            DataStoreGetListener dataStoreGetListener,
+            DataStorePutListener dataStorePutListener,
+            PacketSender packetSender,
+            EventListener eventListener,
+            VirtualNetworkFrameListener virtualNetworkFrameListener,
+            VirtualNetworkConfigListener virtualNetworkConfigListener,
+            PathChecker pathChecker);
+
+    private native boolean node_isInited(long nodeId);
 
     private native void node_delete(long nodeId);