{"action":"created","comment":{"url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/pulls/comments/37443396","id":37443396,"diff_hunk":"@@ -68,119 +74,77 @@ public boolean isRunning(){\n         }\n     }\n     public void Stop(){\n-        if(mBTConnector != null){\n-            mBTConnector.Stop();\n-            mBTConnector = null;\n+\n+        BTConnector tmpCon = mBTConnector;\n+        mBTConnector = null;\n+        if(tmpCon != null){\n+            tmpCon.Stop();\n         }\n \n-        if (mServerSocketList != null) {\n-            for(int i = 0; i < mServerSocketList.size(); i++) {\n-                BtToServerSocket tmp = mServerSocketList.get(i);\n-                if (tmp != null) {\n-                    print_debug(\"Disconnect:::Stop : mBtToServerSocket :\" + tmp.getName());\n-                    tmp.Stop();\n-                    tmp = null;\n-                    mServerSocketList.set(i, tmp);\n-                }\n-            }\n+        //disconnect all incoming connections\n+        DisconnectIncomingConnections();\n+\n+        // disconnect outgoing connection\n+        Disconnect(\"\");\n+    }\n \n-            mServerSocketList.clear();\n+    // we only cut off our outgoing connections, incoming ones are cut off from the other end.\n+    // if we want to cut off whole communications, we'll do Stop\n+    public boolean Disconnect(String peerId){\n+        BtToRequestSocket tmpSoc = mBtToRequestSocket;\n+        if(tmpSoc == null) {\n+            return false;\n         }\n \n-        if(mBtToRequestSocket != null) {\n-            print_debug(\"Disconnect:::Stop : mBtToRequestSocket\");\n-            mBtToRequestSocket.Stop();\n+        String currentpeerId = tmpSoc.GetPeerId();\n+        print_debug(\"Disconnect : \" + peerId + \", current request : \" + currentpeerId);\n+        if (peerId.length() == 0 || peerId.equalsIgnoreCase(currentpeerId)) {\n+            print_debug(\"Disconnect:::Stop :\" + currentpeerId);\n+\n+            tmpSoc.Stop();\n             mBtToRequestSocket = null;\n+\n+            return true;\n+\n         }\n+        return false;\n     }\n \n-    public boolean Disconnect(String peerId){\n+    //test time implementation to simulate 'peer disappearing'\n+    public boolean  DisconnectIncomingConnections() {\n \n-        boolean ret = false;\n+        if (mServerSocketList == null) {\n+            return false;\n+        }\n \n-// we only cut off our outgoing connections, incoming ones are cut off from the other end.\n-// if we want to cut off whole communications, we'll do Stop\n-        if(mBtToRequestSocket != null) {\n-            String currentpeerId = mBtToRequestSocket.GetPeerId();\n-            print_debug(\"Disconnect : \" + peerId + \", current request : \" + currentpeerId);\n-            if(peerId.length() == 0 || peerId.equalsIgnoreCase(currentpeerId)) {\n-                print_debug(\"Disconnect:::Stop :\" + currentpeerId);\n-                mBtToRequestSocket.Stop();\n-                mBtToRequestSocket = null;\n-                ret = true;\n+        for (int i = 0; i < mServerSocketList.size(); i++) {","path":"src/android/java/io/jxcore/node/BtConnectorHelper.java","position":137,"original_position":137,"commit_id":"02ec9ff3aa6e94ff206c2e0175e9061467be1512","original_commit_id":"02ec9ff3aa6e94ff206c2e0175e9061467be1512","user":{"login":"yaronyg","id":1480964,"avatar_url":"https://avatars.githubusercontent.com/u/1480964?v=3","gravatar_id":"","url":"https://api.github.com/users/yaronyg","html_url":"https://github.com/yaronyg","followers_url":"https://api.github.com/users/yaronyg/followers","following_url":"https://api.github.com/users/yaronyg/following{/other_user}","gists_url":"https://api.github.com/users/yaronyg/gists{/gist_id}","starred_url":"https://api.github.com/users/yaronyg/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/yaronyg/subscriptions","organizations_url":"https://api.github.com/users/yaronyg/orgs","repos_url":"https://api.github.com/users/yaronyg/repos","events_url":"https://api.github.com/users/yaronyg/events{/privacy}","received_events_url":"https://api.github.com/users/yaronyg/received_events","type":"User","site_admin":false},"body":"It is super unsafe to iterate on a thread safe object using a counter since the membership can change during the iteration. You have to model this stuff in your head. On each method call you have to ask yourself \"what happens if we have multiple simultaneous calls?\" If multiple people can work on mServerSocketList at the same time then certainly while we are iterating on its membership that membership can change! Also whenever using a new class you should always read its docs. The docs on CopyOnWriteArrayList (see [here](http://docs.oracle.com/javase/7/docs/api/java/util/concurrent/CopyOnWriteArrayList.html)) explicitly calls out the fact that it uses a snapshot style iterator to handle exactly this problem. So what you need to do is to call either iterator() or listIterator() in order to get a thread safe snap shot of the list's contents and iterate on that.\r\n\r\nAlso using that iterator should simplify the code below since, for example, you don't need to check for null. You should never be putting nulls into the list. Instead when you want to remove a member you should call remove. Keep in mind, btw, that you should NEVER remove by index. It's never safe. I'm shocked they even offer it as an option.\r\n\r\nIn fact, if I were you, since order doesn't matter to either lastAvailableList or mServerSocketList I would urge you to consider changing both variables to be of type CopyOnWriteArraySet. This will remove most of the really unsafe methods and make it easier to write correct code.","created_at":"2015-08-19T17:36:46Z","updated_at":"2015-08-19T17:36:46Z","html_url":"https://github.com/thaliproject/Thali_CordovaPlugin/pull/69#discussion_r37443396","pull_request_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/pulls/69","_links":{"self":{"href":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/pulls/comments/37443396"},"html":{"href":"https://github.com/thaliproject/Thali_CordovaPlugin/pull/69#discussion_r37443396"},"pull_request":{"href":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/pulls/69"}}},"pull_request":{"url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/pulls/69","id":42263779,"html_url":"https://github.com/thaliproject/Thali_CordovaPlugin/pull/69","diff_url":"https://github.com/thaliproject/Thali_CordovaPlugin/pull/69.diff","patch_url":"https://github.com/thaliproject/Thali_CordovaPlugin/pull/69.patch","issue_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/issues/69","number":69,"state":"open","locked":false,"title":"Patching replication manager","user":{"login":"yaronyg","id":1480964,"avatar_url":"https://avatars.githubusercontent.com/u/1480964?v=3","gravatar_id":"","url":"https://api.github.com/users/yaronyg","html_url":"https://github.com/yaronyg","followers_url":"https://api.github.com/users/yaronyg/followers","following_url":"https://api.github.com/users/yaronyg/following{/other_user}","gists_url":"https://api.github.com/users/yaronyg/gists{/gist_id}","starred_url":"https://api.github.com/users/yaronyg/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/yaronyg/subscriptions","organizations_url":"https://api.github.com/users/yaronyg/orgs","repos_url":"https://api.github.com/users/yaronyg/repos","events_url":"https://api.github.com/users/yaronyg/events{/privacy}","received_events_url":"https://api.github.com/users/yaronyg/received_events","type":"User","site_admin":false},"body":"<a href='#crh-start'></a><a href='#crh-data-%7B%22processed%22%3A%20%5B%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r36887935%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r36888127%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r36888210%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r36888421%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r36889852%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r36893413%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r36893551%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r36893563%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r36893593%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r36991433%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r36991828%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r36991922%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r36991973%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r36993367%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37000055%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37000208%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37000558%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37000591%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37127600%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37127746%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37338586%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37338654%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37338687%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37338749%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37353843%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37354140%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37354248%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37354529%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37355139%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37355581%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37355798%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37356404%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37356582%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37357316%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37357377%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37357503%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37357657%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37358143%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37358226%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37358447%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37358570%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37358650%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37359026%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37359460%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37359584%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37359656%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37359820%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37360002%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37360169%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37364290%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37364572%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37364857%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37365290%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37365990%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37438236%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37438768%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37440232%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37441413%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37442588%22%2C%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37442700%22%5D%2C%20%22comments%22%3A%20%7B%22Pull%206d929e4c702ac4914b30b94f77618ea4d6083ab5%20src/android/java/io/jxcore/node/BtConnectorHelper.java%20179%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37359584%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22%40DrJukka%20I%27m%20guessing%20line%20225%20below%20isn%27t%20correct%20since%20it%20presumes%20we%20can%20only%20connect%20to%20one%20peer%20at%20a%20time%3F%22%2C%20%22created_at%22%3A%20%222015-08-18T22%3A04%3A39Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20src/android/java/io/jxcore/node/BtConnectorHelper.java%3AL194-226%22%7D%2C%20%22Pull%206d929e4c702ac4914b30b94f77618ea4d6083ab5%20src/android/java/io/jxcore/node/BtConnectorHelper.java%20131%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37357657%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22I%20don%27t%20understand%20the%20intended%20semantics%20of%20%27ret%27.%20It%20seems%20to%20mean%20%5C%22I%20found%20and%20closed%20at%20least%20one%20server%20socket%5C%22.%20But%20why%20is%20that%20useful%3F%20I%20would%20have%20thought%20that%20you%20would%20only%20want%20to%20return%20true%20if%20you%20successfully%20closed%20all%20the%20sockets%3F%20And%20if%20there%20were%20no%20sockets%20then%20shouldn%27t%20you%20still%20return%20true%20since%20you%20did%20close%20all%20sockets%2C%20there%20just%20weren%27t%20any%3F%22%2C%20%22created_at%22%3A%20%222015-08-18T21%3A44%3A46Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20src/android/java/io/jxcore/node/BtConnectorHelper.java%3AL116-139%22%7D%2C%20%22Pull%206d929e4c702ac4914b30b94f77618ea4d6083ab5%20src/android/java/io/jxcore/node/BtConnectorHelper.java%20130%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37357503%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22Here%20you%20will%20want%20to%20use%20remove%28%29%20with%20the%20non-null%20tmp%20so%20you%20make%20sure%20you%20remove%20the%20right%20object.%22%2C%20%22created_at%22%3A%20%222015-08-18T21%3A43%3A26Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20src/android/java/io/jxcore/node/BtConnectorHelper.java%3AL116-139%22%7D%2C%20%22Pull%206d929e4c702ac4914b30b94f77618ea4d6083ab5%20src/android/java/io/jxcore/node/BtConnectorHelper.java%20147%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37358447%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22%40DrJukka%20Can%20you%20please%20use%20a%20more%20descriptive%20name%20like%20localMBtToRequestSocket%3F%22%2C%20%22created_at%22%3A%20%222015-08-18T21%3A52%3A36Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20src/android/java/io/jxcore/node/BtConnectorHelper.java%3AL194-226%22%7D%2C%20%22Pull%206d929e4c702ac4914b30b94f77618ea4d6083ab5%20src/android/java/io/jxcore/node/BtConnectorHelper.java%20113%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37356582%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22%40DrJukka%20As%20mentioned%20above%20this%20isn%27t%20safe.%20If%20there%20are%20any%20more%20examples%20of%20this%20I%20won%27t%20call%20them%20out.%20I%27m%20going%20to%20assume%20that%20you%20will%20search%20for%20them.%22%2C%20%22created_at%22%3A%20%222015-08-18T21%3A34%3A31Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20src/android/java/io/jxcore/node/BtConnectorHelper.java%3AL116-139%22%7D%2C%20%22Pull%206d929e4c702ac4914b30b94f77618ea4d6083ab5%20src/android/java/io/jxcore/node/BtConnectorHelper.java%20138%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37358143%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22%40DrJukka%20Sorry%20to%20stick%20this%20comment%20here%20but%20I%20can%27t%20add%20comments%20to%20GetBluetoothAddress%2C%20GetDeviceName%20or%20SetDeviceName.%20In%20both%20cases%20shouldn%27t%20the%20return%20value%20be%20null%20if%20we%20can%27t%20get%20the%20default%20adapter%3F%20Also%20rather%20than%20using%3A%5Cr%5Cn%5Cr%5Cn%60%60%60java%5Cr%5Cnif%20%28bluetooth%20%21%3D%20null%29%20%7B%5Cr%5Cn%20%20%20ret%20%3D%20bluetooth.getName%28%29%3B%5Cr%5Cn%7D%5Cr%5Cn%5Cr%5Cnreturn%20ret%3B%5Cr%5Cn%60%60%60%5Cr%5Cn%5Cr%5CnWouldn%27t%20it%20be%20cleaner%20to%20just%20use%3A%5Cr%5Cn%60%60%60java%5Cr%5Cnreturn%20bluetooth%20%3D%3D%20null%20%3F%20null%20%3A%20bluetooth.getName%28%29%3B%5Cr%5Cn%60%60%60%5Cr%5Cn%22%2C%20%22created_at%22%3A%20%222015-08-18T21%3A49%3A30Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%2C%20%7B%22body%22%3A%20%22%40DrJukka%20Also%20sorry%20to%20stick%20this%20comment%20here%20but%20I%27m%20limited%20by%20how%20GitHub%20reviews%20work.%20In%20any%20case%2C%20why%20is%20GetKeyValue%20and%20SetKeyValue%20still%20around%3F%20I%20thought%20we%20were%20going%20to%20get%20rid%20of%20these%3F%20Sorry%20if%20I%27ve%20lost%20my%20mind%20on%20this%20one%20but%20I%20thought%20this%20was%20functionality%20we%20removed%3F%22%2C%20%22created_at%22%3A%20%222015-08-18T21%3A50%3A20Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20src/android/java/io/jxcore/node/BtConnectorHelper.java%3AL116-139%22%7D%2C%20%22Pull%206d929e4c702ac4914b30b94f77618ea4d6083ab5%20src/android/java/io/jxcore/node/BtConnectorHelper.java%2084%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37355798%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22%40DrJukka%20This%20method%20really%20confuses%20me.%20I%20thought%20that%20you%20said%20that%20BlueTooth%20supports%20connecting%20to%20multiple%20different%20devices%20simultaneously.%20But%20this%20Disconnect%20method%20seems%20to%20assume%20that%20we%20will%20only%20connect%20to%20a%20single%20peer%20at%20a%20time.%20What%20am%20I%20missing%3F%22%2C%20%22created_at%22%3A%20%222015-08-18T21%3A27%3A19Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20src/android/java/io/jxcore/node/BtConnectorHelper.java%3AL76-94%22%7D%2C%20%22Pull%206d929e4c702ac4914b30b94f77618ea4d6083ab5%20src/android/java/io/jxcore/node/BtConnectorHelper.java%20156%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37358650%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22%40DrJukka%20I%20would%20suggest%20that%20the%20code%20would%20be%20easier%20to%20read%20if%20you%20just%20stuck%20a%20return%20after%20line%20204.%20That%20would%20allow%20you%20to%20get%20rid%20of%20the%20next%20else%20and%20indent%20all%20of%20that%20code%20one%20tab%20space%20to%20the%20left.%20It%27s%20easier%20to%20read%20code%20that%20isn%27t%20deeply%20nested.%22%2C%20%22created_at%22%3A%20%222015-08-18T21%3A54%3A45Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20src/android/java/io/jxcore/node/BtConnectorHelper.java%3AL194-226%22%7D%2C%20%22Pull%206d929e4c702ac4914b30b94f77618ea4d6083ab5%20src/android/java/io/jxcore/node/BtConnectorHelper.java%20155%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37358570%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22%40DrJukka%20This%20assumes%20that%20we%20can%20connect%20to%20only%20one%20peer%20at%20a%20time.%20But%20I%20thought%20you%20said%20that%20Bluetooth%20supports%20multiple%20simultaneous%20connections%20to%20different%20peers%3F%22%2C%20%22created_at%22%3A%20%222015-08-18T21%3A53%3A54Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20src/android/java/io/jxcore/node/BtConnectorHelper.java%3AL194-226%22%7D%2C%20%22Pull%206d929e4c702ac4914b30b94f77618ea4d6083ab5%20thali/thalireplicationmanager.js%2049%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37364572%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22It%20seems%20like%20this%20code%20is%20assuming%20that%20PEER_AVAILABILITY_CHANGED%20will%20always%20return%20all%20peers%20that%20are%20current%20visible%3F%20But%20in%20looking%20%5Bhere%5D%28https%3A//github.com/thaliproject/Thali_CordovaPlugin/blob/story_0/doc/api/connectivity.md%29%20it%20only%20says%20that%20the%20event%20will%20be%20called%20when%20a%20peer%27s%20availability%20changes.%20Which%20implies%20that%20if%20a%20peer%27s%20availability%20hasn%27t%20changed%20then%20it%20won%27t%20be%20included%20in%20the%20event.%20In%20which%20case%20we%20just%20overwrite%20this._peers%20with%20an%20incomplete%20list%20of%20who%20is%20available%21%20That%20doesn%27t%20sound%20right.%20What%20am%20I%20missing%3F%22%2C%20%22created_at%22%3A%20%222015-08-18T23%3A07%3A26Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20thali/thalireplicationmanager.js%3AL117-149%22%7D%2C%20%22Pull%206d929e4c702ac4914b30b94f77618ea4d6083ab5%20src/android/java/io/jxcore/node/BtConnectorHelper.java%2088%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37355581%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22%40DrJukka%20I%20don%27t%20believe%20this%20is%20safe.%20You%20can%27t%20synchronize%20on%20an%20object%20that%20can%20become%20null%2C%20I%20believe%20it%20will%20cause%20an%20exception.%20A%20much%20simpler%20approach%20is%20to%20just%20make%20a%20local%20copy%20of%20mBtToRequestSocket%20and%20then%20test%20if%20the%20local%20copy%20is%20null%20and%20issue%20methods%20based%20on%20that.%20This%20assumes%2C%20of%20course%20that%20all%20of%20mBtToRequestSocket%27s%20methods%20are%20thread%20safe%20but%20of%20course%20they%20should%20be%20given%20our%20asynchronous%20environment.%22%2C%20%22created_at%22%3A%20%222015-08-18T21%3A25%3A05Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20src/android/java/io/jxcore/node/BtConnectorHelper.java%3AL97-112%22%7D%2C%20%22Pull%20303ae2ac9921143683c569d7046cd50f0da23b12%20thali/thalireplicationmanager.js%2062%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r36888127%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22/sockiet/socket/%22%2C%20%22created_at%22%3A%20%222015-08-12T17%3A26%3A39Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%2C%20%7B%22body%22%3A%20%22Fixed%22%2C%20%22created_at%22%3A%20%222015-08-12T18%3A09%3A30Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/11578097%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/mpodwysocki%22%7D%7D%2C%20%7B%22body%22%3A%20%22%3A%2B1%3A%20%22%2C%20%22created_at%22%3A%20%222015-08-13T16%3A02%3A33Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20thali/thalireplicationmanager.js%3AL202-246%22%7D%2C%20%22Pull%2002ec9ff3aa6e94ff206c2e0175e9061467be1512%20src/android/java/io/jxcore/node/BtConnectorHelper.java%20123%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37442700%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22If%20we%20make%20mServerSocketList%20final%20then%20it%20can%20never%20be%20null%2C%20only%20empty.%22%2C%20%22created_at%22%3A%20%222015-08-19T17%3A31%3A01Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20src/android/java/io/jxcore/node/BtConnectorHelper.java%3AL74-151%22%7D%2C%20%22Pull%2002ec9ff3aa6e94ff206c2e0175e9061467be1512%20src/android/java/io/jxcore/node/BtConnectorHelper.java%20106%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37441413%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22The%20use%20of%20an%20empty%20string%20to%20mean%20disconnect%20really%20worries%20me.%20Mostly%20because%20this%20means%20that%20if%20someone%20in%20node.js%20land%20calls%20disconnect%20with%20an%20empty%20string%20that%20will%20also%20cause%20the%20current%20peer%20to%20disconnect.%20That%20is%20a%20great%20way%20to%20create%20really%20bad%20bugs.%20We%20should%20always%20make%20the%20programmer%20be%20explicit%20about%20their%20desire.%20So%20I%20would%20remove%20the%20%5C%22%5C%22%20flag%20%28which%20anyway%20is%20ambiguous%29%20and%20create%20a%20private%20method%20%5C%22DisconnectAll%5C%22%20that%20Stop%20can%20call%20and%20make%20Disconnect%20%5C%22safe%5C%22%20by%20requiring%20an%20exact%20peerid%20match.%22%2C%20%22created_at%22%3A%20%222015-08-19T17%3A19%3A55Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20src/android/java/io/jxcore/node/BtConnectorHelper.java%3AL74-151%22%7D%2C%20%22Pull%206d929e4c702ac4914b30b94f77618ea4d6083ab5%20src/android/java/io/jxcore/node/BtConnectorHelper.java%20187%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37359656%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22%40DrJukka%20On%20line%20236%20above%20/conenctivity/connectivity/%22%2C%20%22created_at%22%3A%20%222015-08-18T22%3A05%3A24Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%2C%20%7B%22body%22%3A%20%22%40DrJukka%20On%20line%20248%20what%20happens%20if%20bluetoothSocket%20is%20null%3F%20Currently%20the%20code%20does%20nothing.%20That%20is%20fine%20but%20in%20that%20case%20please%20rewrite%20the%20code%20to%20check%20for%20null%20and%20return%20and%20then%20out%20indent%20the%20rest%20of%20the%20code.%20I%27ll%20stop%20making%20this%20comment%20about%20%27if%27%20styling%20%28e.g.%20don%27t%20indent%20things%20like%20crazy%2C%20instead%20use%20return%29%20and%20assume%20you%20will%20check%20all%20your%20files%20for%20it%20and%20please%20fix.%22%2C%20%22created_at%22%3A%20%222015-08-18T22%3A07%3A05Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%2C%20%7B%22body%22%3A%20%22%40DrJukka%20The%20connected%20method%20is%2C%20I%20think%2C%20unnecessarily%20hard%20to%20read.%20Could%20you%20please%20refactor%20it%20into%20two%20methods%2C%20one%20called%20ConnectedIncoming%20and%20another%20called%20ConnectedOutgoing%20and%20whomever%20calls%20this%20can%20then%20switch%20between%20the%20two%20based%20on%20the%20incoming%20value%3F%20It%20would%20make%20the%20code%20much%20easier%20to%20read.%22%2C%20%22created_at%22%3A%20%222015-08-18T22%3A08%3A49Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20src/android/java/io/jxcore/node/BtConnectorHelper.java%3AL241-262%22%7D%2C%20%22Pull%206d929e4c702ac4914b30b94f77618ea4d6083ab5%20src/android/java/io/jxcore/node/BtConnectorHelper.java%2031%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37353843%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22I%20am%20very%20concerned%20that%20the%20approach%20of%20wrapping%20lastAvailableList%20in%20a%20synchronized%20is%20too%20error%20prone.%20If%20someone%2C%20somewhere%20ever%20forgets%20to%20protect%20lastAvailableList%20it%20all%20goes%20bad.%20Please%20replace%20this%20use%20of%20synchronized%20and%20make%20lastAvailableList%20into%20a%20CopyOnWriteArrayList%20or%20equivalent.%20The%20overhead%20of%20copying%20the%20array%27s%20contents%20shouldn%27t%20matter%20given%20the%20physical%20limitations%20on%20how%20many%20devices%20we%20can%20potentially%20discover.%22%2C%20%22created_at%22%3A%20%222015-08-18T21%3A09%3A13Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%2C%20%7B%22body%22%3A%20%22%40DrJukka%20Just%20adding%20your%20tag%20sign%20so%20you%20get%20notified%20since%20this%20comment%20is%20in%20your%20code.%22%2C%20%22created_at%22%3A%20%222015-08-18T21%3A11%3A51Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20src/android/java/io/jxcore/node/BtConnectorHelper.java%3AL55-75%22%7D%2C%20%22Pull%20303ae2ac9921143683c569d7046cd50f0da23b12%20thali/thalireplicationmanager.js%2036%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r36887935%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22What%20if%20we%20didn%27t%20get%20an%20error%20on%20the%20disconnect%3F%20In%20that%20case%20won%27t%20err%20be%20null%3F%20Shouldn%27t%20we%20check%20for%20that%20and%20only%20run%20line%20169%20and%20170%20if%20there%20is%20an%20actual%20error%3F%22%2C%20%22created_at%22%3A%20%222015-08-12T17%3A25%3A16Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%2C%20%7B%22body%22%3A%20%22Fixed%20as%20per%20latest%20commit%22%2C%20%22created_at%22%3A%20%222015-08-12T18%3A09%3A43Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/11578097%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/mpodwysocki%22%7D%7D%2C%20%7B%22body%22%3A%20%22%3A%2B1%3A%20%22%2C%20%22created_at%22%3A%20%222015-08-13T15%3A59%3A07Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20thali/thalireplicationmanager.js%3AL166-185%22%7D%2C%20%22Pull%206d929e4c702ac4914b30b94f77618ea4d6083ab5%20thali/thalireplicationmanager.js%20153%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37365290%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22Isn%27t%20this%20assuming%20that%20we%20can%20only%20be%20in%20retry%20with%20a%20single%20peer%20at%20a%20time%3F%20What%20if%20we%20have%20multiple%20peers%20we%20are%20simultaneously%20connecting%20to%20and%20we%20are%20in%20retry%20with%20one%20or%20more%20of%20them%3F%20Shouldn%27t%20isInRetry%20really%20be%20an%20array%20indexed%20by%20peerID%3F%5Cr%5Cn%5Cr%5CnAlso%20while%20on%20the%20topic%20can%20we%20take%20clients%20and%20replications%20and%20the%20rest%20and%20collapse%20them%20into%20one%20array%20with%20an%20object%3F%20It%20would%20make%20it%20much%20easier%20to%20think%20about%20threading%20issues%20since%20all%20the%20places%20state%20can%20blow%20up%20are%20in%20one%20place%3F%22%2C%20%22created_at%22%3A%20%222015-08-18T23%3A19%3A11Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20thali/thalireplicationmanager.js%3AL185-308%22%7D%2C%20%22Pull%206d929e4c702ac4914b30b94f77618ea4d6083ab5%20src/android/java/io/jxcore/node/BtConnectorHelper.java%20122%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37357316%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22%40DrJukka%20Instead%20of%20synchronizing%20on%20mServerSocketList%20we%20should%20use%20CopyOnWriteArrayList%20or%20equivalent.%22%2C%20%22created_at%22%3A%20%222015-08-18T21%3A41%3A10Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20src/android/java/io/jxcore/node/BtConnectorHelper.java%3AL116-139%22%7D%2C%20%22Pull%206d929e4c702ac4914b30b94f77618ea4d6083ab5%20src/android/java/io/jxcore/node/BtConnectorHelper.java%20124%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37357377%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22%40DrJukka%20Note%20that%20when%20you%20switch%20to%20CopyOnWriteArrayList%20or%20equivalent%20you%20will%20want%20to%20use%20methods%20like%20elements%28%29%20to%20get%20a%20consistent%20set%20of%20contents%20to%20iterate%20over.%22%2C%20%22created_at%22%3A%20%222015-08-18T21%3A41%3A49Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20src/android/java/io/jxcore/node/BtConnectorHelper.java%3AL116-139%22%7D%2C%20%22Pull%20c10d2e6341ce7528d45b8d1c82c423e712987b9c%20thali/thalireplicationmanager.js%2077%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37000591%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22If%20we%20got%20an%20error%20on%20the%20clientSocket%20then%20hasn%27t%20it%20already%20destroyed%20itself%3F%22%2C%20%22created_at%22%3A%20%222015-08-13T17%3A23%3A54Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%2C%20%7B%22body%22%3A%20%22Destroying%20a%20socket%20is%20idempotent%20or%20at%20least%20should%20be%20according%20to%20my%20tests%22%2C%20%22created_at%22%3A%20%222015-08-18T18%3A57%3A03Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/11578097%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/mpodwysocki%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20thali/thalireplicationmanager.js%3AL206-250%22%7D%2C%20%22Pull%20c10d2e6341ce7528d45b8d1c82c423e712987b9c%20thali/thalireplicationmanager.js%2072%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37000558%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22If%20we%20get%20an%20error%20on%20the%20stream%20don%27t%20we%20need%20to%20shut%20down%20the%20clientSocket%3F%22%2C%20%22created_at%22%3A%20%222015-08-13T17%3A23%3A39Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%2C%20%7B%22body%22%3A%20%22Fixed%20as%20per%20suggestion%22%2C%20%22created_at%22%3A%20%222015-08-18T18%3A56%3A32Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/11578097%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/mpodwysocki%22%7D%7D%2C%20%7B%22body%22%3A%20%22I%27m%20confused.%20Now%20we%20don%27t%20seem%20to%20be%20tracking%20any%20errors%20what%20so%20ever%20on%20stream.%20That%20doesn%27t%20sound%20right.%22%2C%20%22created_at%22%3A%20%222015-08-18T23%3A29%3A46Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20thali/thalireplicationmanager.js%3AL206-250%22%7D%2C%20%22Pull%206d929e4c702ac4914b30b94f77618ea4d6083ab5%20src/android/java/io/jxcore/node/BtConnectorHelper.java%20102%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37356404%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22I%20would%20suggest%20having%20a%20mBtToRequestSocket%20class%20that%20contains%20all%20the%20peers%20we%20are%20connected%20to%20and%20contains%20a%20method%20%27disconnectPeer%28peerId%29%27%20that%20will%20use%20a%20concurrent%20safe%20list%20with%20an%20atomic%20update%20and%20in%20that%20atomic%20update%20you%20would%20then%20check%20to%20see%20if%20the%20peerid%20exists%2C%20call%20close%20on%20the%20mBtToRequestSocket%20and%20then%20return%20null%20for%20the%20peer%20ID%27s%20state.%20I%20suspect%20this%20will%20be%20the%20most%20robust%20implementation%20that%20doesn%27t%20require%20a%20bunch%20of%20fiddly%20synchronized%20calls.%22%2C%20%22created_at%22%3A%20%222015-08-18T21%3A32%3A59Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20src/android/java/io/jxcore/node/BtConnectorHelper.java%3AL97-112%22%7D%2C%20%22Pull%2002ec9ff3aa6e94ff206c2e0175e9061467be1512%20src/android/java/io/jxcore/node/BtConnectorHelper.java%20119%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37442588%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22This%20comment%20is%20misleading%20since%20the%20method%20is%20also%20used%20by%20stop.%22%2C%20%22created_at%22%3A%20%222015-08-19T17%3A30%3A06Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20src/android/java/io/jxcore/node/BtConnectorHelper.java%3AL74-151%22%7D%2C%20%22Pull%20303ae2ac9921143683c569d7046cd50f0da23b12%20thali/thalireplicationmanager.js%2074%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r36889852%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22/sockiet/socket/%22%2C%20%22created_at%22%3A%20%222015-08-12T17%3A40%3A05Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%2C%20%7B%22body%22%3A%20%22Fixed%22%2C%20%22created_at%22%3A%20%222015-08-12T18%3A08%3A23Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/11578097%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/mpodwysocki%22%7D%7D%2C%20%7B%22body%22%3A%20%22%3A%2B1%3A%20%22%2C%20%22created_at%22%3A%20%222015-08-13T16%3A03%3A46Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20thali/thalireplicationmanager.js%3AL202-246%22%7D%2C%20%22Pull%206d929e4c702ac4914b30b94f77618ea4d6083ab5%20thali/thalireplicationmanager.js%2033%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37364290%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22Purely%20as%20an%20example%20of%20defensive%20programming%20would%20it%20make%20sense%20to%20put%20a%20return%20after%20line%20114%3F%22%2C%20%22created_at%22%3A%20%222015-08-18T23%3A03%3A31Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20thali/thalireplicationmanager.js%3AL106-116%22%7D%2C%20%22Pull%206d929e4c702ac4914b30b94f77618ea4d6083ab5%20thali/thalireplicationmanager.js%2058%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37364857%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22Could%20you%20please%20use%20a%20slightly%20more%20informative%20variable%20name%20than%20p%3F%20Like%20existingPeerReplication%3F%20I%27m%20slow%20and%20keep%20getting%20confused%20between%20peer%20and%20p.%22%2C%20%22created_at%22%3A%20%222015-08-18T23%3A12%3A15Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20thali/thalireplicationmanager.js%3AL117-149%22%7D%2C%20%22Pull%204ad4e709a8fb1ed9fce29ac6135b57a16b488c6f%20thali/thalireplicationmanager.js%2073%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37127600%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22Shouldn%27t%20this%20be%20before%20line%20206%3F%20I%20realize%20that%20given%20the%20single%20threaded%20nature%20of%20Javascript%20it%20shouldn%27t%20theoretically%20be%20possible%20for%20the%20event%20to%20be%20emitted%20before%20the%20handler%20is%20registered%20but%20boy%20do%20I%20hate%20tempting%20the%20gods.%5Cr%5Cn%5Cr%5CnAlso%20shouldn%27t%20this%20be%20once%20not%20on%3F%20Otherwise%20won%27t%20this%20will%20get%20called%20on%20EVERY%20stopped%20event%20which%20means%20that%20theoretically%20it%20isn%27t%20possible%20to%20ever%20stop%20the%20replication%20manager%3F%5Cr%5Cn%5Cr%5CnOh%20and%20what%20happens%20if%20we%20get%20a%20STOP_ERROR%3F%20Should%20we%20really%20not%20try%20to%20start%20again%3F%22%2C%20%22created_at%22%3A%20%222015-08-14T23%3A11%3A38Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%2C%20%7B%22body%22%3A%20%22Fixed%20as%20per%20suggestion%22%2C%20%22created_at%22%3A%20%222015-08-18T18%3A57%3A18Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/11578097%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/mpodwysocki%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20thali/thalireplicationmanager.js%3AL202-264%22%7D%2C%20%22Pull%20303ae2ac9921143683c569d7046cd50f0da23b12%20thali/thalireplicationmanager.js%2070%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r36888421%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22Why%20do%20you%20destroy%20the%20incomingClientSocket%20on%20error%20but%20not%20on%20end%3F%20%20Of%20course%20given%20my%20previous%20question%20about%20destroying%20%5C%22end%5C%22ed%20sockets%20maybe%20this%20is%20correct%20and%20line%20207%20is%20the%20problem%3F%22%2C%20%22created_at%22%3A%20%222015-08-12T17%3A28%3A54Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%2C%20%7B%22body%22%3A%20%22I%27m%20on%20the%20assumption%20that%20end%20is%20perfectly%20normal%20but%20error%20is%20not.%20%20Pretty%20sure%20we%20can%20get%20rid%20of%20that%20code%20for%20destroying%20the%20socket%20because%20an%20error%20should%20do%20that%20already.%20%20However%2C%20on%20stream%20error%20we%20might%20want%20to%20tear%20down%20the%20incoming%20client%20socket%22%2C%20%22created_at%22%3A%20%222015-08-13T16%3A17%3A06Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/11578097%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/mpodwysocki%22%7D%7D%2C%20%7B%22body%22%3A%20%22%3A%2B1%3A%20Please%20tell%20me%20there%20are%20tests%20to%20be%20sure%20you%20are%20right%3F%22%2C%20%22created_at%22%3A%20%222015-08-13T17%3A20%3A45Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20thali/thalireplicationmanager.js%3AL202-246%22%7D%2C%20%22Pull%20303ae2ac9921143683c569d7046cd50f0da23b12%20thali/thalireplicationmanager.js%2058%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r36888210%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22If%20we%20got%20an%20end%20event%20doesn%27t%20that%20mean%20the%20socket%20is%20already%20destroyed%3F%20Any%20danger%20from%20destroying%20a%20socket%20twice%3F%22%2C%20%22created_at%22%3A%20%222015-08-12T17%3A27%3A17Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%2C%20%7B%22body%22%3A%20%22I%20haven%27t%20noticed%20any%20exceptions%20being%20thrown%20from%20this%20code%2C%20so%20I%27m%20not%20sure%22%2C%20%22created_at%22%3A%20%222015-08-12T18%3A09%3A22Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/11578097%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/mpodwysocki%22%7D%7D%2C%20%7B%22body%22%3A%20%22Why%20not%20just%20write%20a%20quick%20test%20to%20be%20sure%3F%20E.g.%20open%20a%20socket%2C%20destroy%20it%20and%20then%20destroy%20it%20again.%20It%27s%20best%20to%20be%20sure.%22%2C%20%22created_at%22%3A%20%222015-08-13T16%3A03%3A21Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%2C%20%7B%22body%22%3A%20%22%3A%2B1%3A%20Per%20our%20conversation%22%2C%20%22created_at%22%3A%20%222015-08-13T17%3A19%3A24Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20thali/thalireplicationmanager.js%3AL202-246%22%7D%2C%20%22Pull%206d929e4c702ac4914b30b94f77618ea4d6083ab5%20src/android/java/io/jxcore/node/BtConnectorHelper.java%20195%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37359026%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22%40DrJukka%20This%20comment%20is%20in%20regards%20to%20line%20239%20above%20where%20you%20return%20%5C%22Device%20Address%20for%20....%20not%20found%20from%20Discovered%20device%20list%5C%22.%20The%20code%2C%20I%20believe%2C%20would%20be%20easier%20to%20read%20if%20you%20instead%20put%20in%5Cr%5Cn%5Cr%5Cn%60%60%60java%5Cr%5Cnif%20%28selectedDevice%20%3D%3D%20null%29%20%7B%5Cr%5Cn%20%20tmpCallback.ConnectionStatusUpdate%28%5C%22Device%20Address%20for%20%5C%22%20%2B%20toPeerId%20%2B%20%5C%22%20not%20found%20from%20Discovered%20device%20list.%5C%22%2C-1%29%3B%5Cr%5Cn%20%20return%3B%5Cr%5Cn%7D%5Cr%5Cn%60%60%60%5Cr%5CnThen%20you%20can%20remove%20the%20%60if%20%28selectedDevice%20%21%3D%20null%29%60%20and%20out%20indent%20that%20code.%22%2C%20%22created_at%22%3A%20%222015-08-18T21%3A58%3A14Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20src/android/java/io/jxcore/node/BtConnectorHelper.java%3AL241-262%22%7D%2C%20%22Pull%206d929e4c702ac4914b30b94f77618ea4d6083ab5%20src/android/java/io/jxcore/node/BtConnectorHelper.java%2026%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37355139%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22Does%20it%20ever%20make%20sense%20for%20start%20to%20be%20called%20twice%20on%20the%20same%20BtConnectorHelper%20object%3F%20If%20not%20then%20I%20would%20suggest%20that%20we%20can%20get%20rid%20of%20synchronized%20and%20instead%20just%20use%20AtomicBoolean%20to%20stick%20in%20a%20guard%20and%20call%20it%20a%20day.%22%2C%20%22created_at%22%3A%20%222015-08-18T21%3A20%3A56Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20src/android/java/io/jxcore/node/BtConnectorHelper.java%3AL55-75%22%7D%2C%20%22Pull%2002ec9ff3aa6e94ff206c2e0175e9061467be1512%20src/android/java/io/jxcore/node/BtConnectorHelper.java%2029%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37438768%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22You%20really%20want%20to%20make%20this%20final%20as%20well.%20%20Life%20is%20much%20easier%20if%20a%20variable%20can%20never%20go%20null.%20In%20the%20case%20of%20lastAvailableList%2C%20for%20example%2C%20the%20only%20place%20you%20ever%20reset%20it%20is%20in%20AddPeerIfNotDiscovered%20and%20you%20can%20just%20call%20the%20clear%20method%20on%20the%20variable.%5Cr%5Cn%5Cr%5CnTo%20be%20clear%2C%20you%20do%20NOT%20have%20a%20bug%20here.%20The%20code%20is%20correct.%20The%20use%20of%20final%20and%20clear%20is%20just%20meant%20as%20a%20defensive%20coding%20technique.%22%2C%20%22created_at%22%3A%20%222015-08-19T16%3A57%3A30Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20src/android/java/io/jxcore/node/BtConnectorHelper.java%3AL32-44%22%7D%2C%20%22Pull%206d929e4c702ac4914b30b94f77618ea4d6083ab5%20src/android/java/io/jxcore/node/BtConnectorHelper.java%20146%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37359460%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22%40DrJukka%20What%20happens%20if%20BeginConnectPeer%20is%20called%20twice%20in%20parallel%3F%20This%20is%20perfectly%20reasonable%20if%20we%20are%20trying%20to%20connect%20to%20two%20different%20peers.%20I%27m%20fairly%20confident%20that%20making%20the%20method%20synchronized%20is%20not%20the%20right%20solution.%20This%2C%20for%20example%2C%20won%27t%20solve%20what%20happens%20if%20someone%20tries%20to%20simultaneously%20connect%20and%20disconnect%20since%20synchronized%20methods%20apply%20to%20repeated%20calls%20to%20the%20method%2C%20not%20the%20object.%20I%5C%22m%20not%20worried%20about%20race%20conditions%20as%20much%20as%20I%27m%20worried%20about%20us%20trying%20to%20connect%20to%20the%20same%20peer%20twice.%20Will%20Bluetooth%27s%20layer%20stop%20that%3F%20Or%20can%20you%20open%20two%20Bluetooth%20client%20sockets%20to%20the%20same%20device%3F%22%2C%20%22created_at%22%3A%20%222015-08-18T22%3A02%3A54Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20src/android/java/io/jxcore/node/BtConnectorHelper.java%3AL194-226%22%7D%2C%20%22Pull%2002ec9ff3aa6e94ff206c2e0175e9061467be1512%20src/android/java/io/jxcore/node/BtConnectorHelper.java%2044%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37440232%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22Here%20is%20a%20fun%20example%20of%20why%20sometimes%20you%20have%20to%20use%20synchronized%20methods.%20Ask%20yourself%20the%20following%20question%20-%20what%20happens%20if%20start%20and%20stop%20both%20get%20called%20at%20the%20same%20time%20from%20different%20threads%3F%5Cr%5Cn%5Cr%5CnBut%20here%20is%20a%20better%20question%20-%20is%20it%20possible%20for%20start%20and%20stop%20to%20be%20called%20at%20the%20same%20time%20from%20different%20threads%3F%20I%20don%27t%20think%20so.%20But%20we%20should%20be%20sure.%20Here%20is%20my%20reasoning%20-%20The%20only%20entity%20that%20should%20EVER%20call%20BtConnectorHelper%20is%20JXcoreExtension%20and%20JXcore%20guarantees%20that%20JXcoreExtension%20will%20always%20be%20called%20on%20the%20same%20thread%20%28at%20least%20I%20think%20it%20guarantees%20that%2C%20we%20should%20check%20with%20Oguz%29.%20In%20that%20case%20methods%20like%20start%20and%20stop%20can%20only%20be%20called%20on%20a%20single%20thread%20since%20only%20JXcoreExtension%20can%20call%20them.%20If%20that%20is%20all%20true%20then%20this%20is%20worth%20some%20kind%20of%20comment%20at%20the%20top%20of%20the%20file%20pointing%20out%20that%20we%20are%20depending%20on%20the%20fact%20that%20other%20than%20in%20callbacks%20all%20the%20methods%20in%20BtConnectorHelper%20will%20only%20be%20called%20by%20JXcoreExtension%20and%20JXcoreExtension%20will%20only%20be%20called%20from%20the%20same%20thread.%22%2C%20%22created_at%22%3A%20%222015-08-19T17%3A10%3A04Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20src/android/java/io/jxcore/node/BtConnectorHelper.java%3AL52-70%22%7D%2C%20%22Pull%2002ec9ff3aa6e94ff206c2e0175e9061467be1512%20src/android/java/io/jxcore/node/BtConnectorHelper.java%2023%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37438236%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22On%20line%2028%20below%20you%20set%20context%20to%20null.%20Which%20is%20fine%20but%20since%20context%20is%20only%20set%20in%20the%20constructor%20you%20can%20make%20it%20final.%20It%27s%20always%20good%20habit%20in%20multi-threaded%20code%20to%20make%20anything%20that%20can%20be%20final%2C%20into%20final%2C%20it%20prevents%20mistakes.%20So%20i%20would%20review%20all%20your%20state%20variables%20and%20see%20which%20ones%20you%20can%20make%20final.%22%2C%20%22created_at%22%3A%20%222015-08-19T16%3A52%3A48Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20src/android/java/io/jxcore/node/BtConnectorHelper.java%3AL5-25%22%7D%2C%20%22Pull%204ad4e709a8fb1ed9fce29ac6135b57a16b488c6f%20thali/thalireplicationmanager.js%2096%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37127746%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22These%20multiple%20calls%20to%20restartMuxServerBridge%20here%20and%20in%20two%20more%20places%20below%20make%20me%20really%20nervous.%20It%20is%20at%20least%20theoretically%20possible%20for%20say%20both%20the%20incomingClientsocket%20and%20the%20server%20to%20error%20out%20at%20the%20same%20time.%20In%20that%20case%20we%20could%20end%20up%20calling%20restartMuxServerBridge%20multiple%20times.%20This%20would%20seem%20bad%20as%20we%20would%20try%20to%20stop%20and%20start%20things%20multiple%20times.%20Shouldn%27t%20we%20be%20protecting%20against%20multiple%20calls%20to%20restartMuxServerBridge%3F%20Or%2C%20more%20to%20the%20point%2C%20shouldn%27t%20subsequent%20calls%20when%20we%20already%20have%20one%20restart%20in%20progress%20be%20treated%20as%20thunks%3F%22%2C%20%22created_at%22%3A%20%222015-08-14T23%3A14%3A57Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%2C%20%7B%22body%22%3A%20%22Fixed%20with%20a%20flag%20to%20determine%20whether%20the%20cleanup%20has%20been%20called%20before%20so%20it%20can%20only%20be%20called%20once.%22%2C%20%22created_at%22%3A%20%222015-08-18T18%3A57%3A43Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/11578097%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/mpodwysocki%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20thali/thalireplicationmanager.js%3AL202-264%22%7D%2C%20%22Pull%206d929e4c702ac4914b30b94f77618ea4d6083ab5%20src/android/java/io/jxcore/node/BtConnectorHelper.java%2041%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37354248%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22%40DrJukka%20Why%20is%20this%20method%20synchronized%3F%20At%20worst%20it%20can%20return%20an%20outdated%20response%20but%20that%20is%20always%20possible.%22%2C%20%22created_at%22%3A%20%222015-08-18T21%3A12%3A50Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20src/android/java/io/jxcore/node/BtConnectorHelper.java%3AL55-75%22%7D%2C%20%22Pull%206d929e4c702ac4914b30b94f77618ea4d6083ab5%20src/android/java/io/jxcore/node/BtConnectorHelper.java%20203%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37360169%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22%40DrJukka%20Line%20271%20below%20is%20not%20thread%20safe%20because%20between%20the%20null%20check%20on%20271%20and%20the%20stop%20on%20272%20someone%20else%20could%20have%20changed%20mBtToRequestSocket%20to%20be%20null.%20Please%20first%20copy%20to%20a%20local%20variable%20and%20then%20check%20for%20null%20and%20call%20stop.%22%2C%20%22created_at%22%3A%20%222015-08-18T22%3A10%3A45Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20src/android/java/io/jxcore/node/BtConnectorHelper.java%3AL241-262%22%7D%2C%20%22Pull%206d929e4c702ac4914b30b94f77618ea4d6083ab5%20src/android/java/io/jxcore/node/BtConnectorHelper.java%2048%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/thaliproject/Thali_CordovaPlugin/pull/69%23discussion_r37354529%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22%40DrJukka%20Why%20isn%27t%20this%20synchronized%3F%20You%20can%20still%20have%20a%20race%20condition%20where%20the%20same%20mBTConnector%20has%20its%20stop%20method%20called%20twice.%20Does%20that%20not%20matter%3F%22%2C%20%22created_at%22%3A%20%222015-08-18T21%3A15%3A36Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1480964%3Fv%3D3%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/yaronyg%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20src/android/java/io/jxcore/node/BtConnectorHelper.java%3AL76-94%22%7D%7D%7D'></a>\n<a href='https://www.codereviewhub.com/'><img src='http://www.codereviewhub.com/site/github-bar.png' height=40></a>\n- [ ] <a href='#crh-comment-Pull c10d2e6341ce7528d45b8d1c82c423e712987b9c thali/thalireplicationmanager.js 72'></a> <img src='http://www.codereviewhub.com/site/github-remaining.png' height=16 width=60>&nbsp;<b><a href='https://github.com/thaliproject/Thali_CordovaPlugin/pull/69#discussion_r37000558'>File: thali/thalireplicationmanager.js:L206-250</a></b>\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> If we get an error on the stream don't we need to shut down the clientSocket?\n- <a href='https://github.com/mpodwysocki'><img border=0 src='https://avatars.githubusercontent.com/u/11578097?v=3' height=16 width=16'></a> Fixed as per suggestion\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> I'm confused. Now we don't seem to be tracking any errors what so ever on stream. That doesn't sound right.\n- [ ] <a href='#crh-comment-Pull c10d2e6341ce7528d45b8d1c82c423e712987b9c thali/thalireplicationmanager.js 77'></a> <img src='http://www.codereviewhub.com/site/github-remaining.png' height=16 width=60>&nbsp;<b><a href='https://github.com/thaliproject/Thali_CordovaPlugin/pull/69#discussion_r37000591'>File: thali/thalireplicationmanager.js:L206-250</a></b>\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> If we got an error on the clientSocket then hasn't it already destroyed itself?\n- <a href='https://github.com/mpodwysocki'><img border=0 src='https://avatars.githubusercontent.com/u/11578097?v=3' height=16 width=16'></a> Destroying a socket is idempotent or at least should be according to my tests\n- [ ] <a href='#crh-comment-Pull 4ad4e709a8fb1ed9fce29ac6135b57a16b488c6f thali/thalireplicationmanager.js 73'></a> <img src='http://www.codereviewhub.com/site/github-remaining.png' height=16 width=60>&nbsp;<b><a href='https://github.com/thaliproject/Thali_CordovaPlugin/pull/69#discussion_r37127600'>File: thali/thalireplicationmanager.js:L202-264</a></b>\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> Shouldn't this be before line 206? I realize that given the single threaded nature of Javascript it shouldn't theoretically be possible for the event to be emitted before the handler is registered but boy do I hate tempting the gods.\nAlso shouldn't this be once not on? Otherwise won't this will get called on EVERY stopped event which means that theoretically it isn't possible to ever stop the replication manager?\nOh and what happens if we get a STOP_ERROR? Should we really not try to start again?\n- <a href='https://github.com/mpodwysocki'><img border=0 src='https://avatars.githubusercontent.com/u/11578097?v=3' height=16 width=16'></a> Fixed as per suggestion\n- [ ] <a href='#crh-comment-Pull 4ad4e709a8fb1ed9fce29ac6135b57a16b488c6f thali/thalireplicationmanager.js 96'></a> <img src='http://www.codereviewhub.com/site/github-remaining.png' height=16 width=60>&nbsp;<b><a href='https://github.com/thaliproject/Thali_CordovaPlugin/pull/69#discussion_r37127746'>File: thali/thalireplicationmanager.js:L202-264</a></b>\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> These multiple calls to restartMuxServerBridge here and in two more places below make me really nervous. It is at least theoretically possible for say both the incomingClientsocket and the server to error out at the same time. In that case we could end up calling restartMuxServerBridge multiple times. This would seem bad as we would try to stop and start things multiple times. Shouldn't we be protecting against multiple calls to restartMuxServerBridge? Or, more to the point, shouldn't subsequent calls when we already have one restart in progress be treated as thunks?\n- <a href='https://github.com/mpodwysocki'><img border=0 src='https://avatars.githubusercontent.com/u/11578097?v=3' height=16 width=16'></a> Fixed with a flag to determine whether the cleanup has been called before so it can only be called once.\n- [ ] <a href='#crh-comment-Pull 6d929e4c702ac4914b30b94f77618ea4d6083ab5 src/android/java/io/jxcore/node/BtConnectorHelper.java 31'></a> <img src='http://www.codereviewhub.com/site/github-remaining.png' height=16 width=60>&nbsp;<b><a href='https://github.com/thaliproject/Thali_CordovaPlugin/pull/69#discussion_r37353843'>File: src/android/java/io/jxcore/node/BtConnectorHelper.java:L55-75</a></b>\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> I am very concerned that the approach of wrapping lastAvailableList in a synchronized is too error prone. If someone, somewhere ever forgets to protect lastAvailableList it all goes bad. Please replace this use of synchronized and make lastAvailableList into a CopyOnWriteArrayList or equivalent. The overhead of copying the array's contents shouldn't matter given the physical limitations on how many devices we can potentially discover.\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> @DrJukka Just adding your tag sign so you get notified since this comment is in your code.\n- [ ] <a href='#crh-comment-Pull 6d929e4c702ac4914b30b94f77618ea4d6083ab5 src/android/java/io/jxcore/node/BtConnectorHelper.java 41'></a> <img src='http://www.codereviewhub.com/site/github-remaining.png' height=16 width=60>&nbsp;<b><a href='https://github.com/thaliproject/Thali_CordovaPlugin/pull/69#discussion_r37354248'>File: src/android/java/io/jxcore/node/BtConnectorHelper.java:L55-75</a></b>\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> @DrJukka Why is this method synchronized? At worst it can return an outdated response but that is always possible.\n- [ ] <a href='#crh-comment-Pull 6d929e4c702ac4914b30b94f77618ea4d6083ab5 src/android/java/io/jxcore/node/BtConnectorHelper.java 48'></a> <img src='http://www.codereviewhub.com/site/github-remaining.png' height=16 width=60>&nbsp;<b><a href='https://github.com/thaliproject/Thali_CordovaPlugin/pull/69#discussion_r37354529'>File: src/android/java/io/jxcore/node/BtConnectorHelper.java:L76-94</a></b>\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> @DrJukka Why isn't this synchronized? You can still have a race condition where the same mBTConnector has its stop method called twice. Does that not matter?\n- [ ] <a href='#crh-comment-Pull 6d929e4c702ac4914b30b94f77618ea4d6083ab5 src/android/java/io/jxcore/node/BtConnectorHelper.java 26'></a> <img src='http://www.codereviewhub.com/site/github-remaining.png' height=16 width=60>&nbsp;<b><a href='https://github.com/thaliproject/Thali_CordovaPlugin/pull/69#discussion_r37355139'>File: src/android/java/io/jxcore/node/BtConnectorHelper.java:L55-75</a></b>\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> Does it ever make sense for start to be called twice on the same BtConnectorHelper object? If not then I would suggest that we can get rid of synchronized and instead just use AtomicBoolean to stick in a guard and call it a day.\n- [ ] <a href='#crh-comment-Pull 6d929e4c702ac4914b30b94f77618ea4d6083ab5 src/android/java/io/jxcore/node/BtConnectorHelper.java 88'></a> <img src='http://www.codereviewhub.com/site/github-remaining.png' height=16 width=60>&nbsp;<b><a href='https://github.com/thaliproject/Thali_CordovaPlugin/pull/69#discussion_r37355581'>File: src/android/java/io/jxcore/node/BtConnectorHelper.java:L97-112</a></b>\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> @DrJukka I don't believe this is safe. You can't synchronize on an object that can become null, I believe it will cause an exception. A much simpler approach is to just make a local copy of mBtToRequestSocket and then test if the local copy is null and issue methods based on that. This assumes, of course that all of mBtToRequestSocket's methods are thread safe but of course they should be given our asynchronous environment.\n- [ ] <a href='#crh-comment-Pull 6d929e4c702ac4914b30b94f77618ea4d6083ab5 src/android/java/io/jxcore/node/BtConnectorHelper.java 84'></a> <img src='http://www.codereviewhub.com/site/github-remaining.png' height=16 width=60>&nbsp;<b><a href='https://github.com/thaliproject/Thali_CordovaPlugin/pull/69#discussion_r37355798'>File: src/android/java/io/jxcore/node/BtConnectorHelper.java:L76-94</a></b>\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> @DrJukka This method really confuses me. I thought that you said that BlueTooth supports connecting to multiple different devices simultaneously. But this Disconnect method seems to assume that we will only connect to a single peer at a time. What am I missing?\n- [ ] <a href='#crh-comment-Pull 6d929e4c702ac4914b30b94f77618ea4d6083ab5 src/android/java/io/jxcore/node/BtConnectorHelper.java 102'></a> <img src='http://www.codereviewhub.com/site/github-remaining.png' height=16 width=60>&nbsp;<b><a href='https://github.com/thaliproject/Thali_CordovaPlugin/pull/69#discussion_r37356404'>File: src/android/java/io/jxcore/node/BtConnectorHelper.java:L97-112</a></b>\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> I would suggest having a mBtToRequestSocket class that contains all the peers we are connected to and contains a method 'disconnectPeer(peerId)' that will use a concurrent safe list with an atomic update and in that atomic update you would then check to see if the peerid exists, call close on the mBtToRequestSocket and then return null for the peer ID's state. I suspect this will be the most robust implementation that doesn't require a bunch of fiddly synchronized calls.\n- [ ] <a href='#crh-comment-Pull 6d929e4c702ac4914b30b94f77618ea4d6083ab5 src/android/java/io/jxcore/node/BtConnectorHelper.java 113'></a> <img src='http://www.codereviewhub.com/site/github-remaining.png' height=16 width=60>&nbsp;<b><a href='https://github.com/thaliproject/Thali_CordovaPlugin/pull/69#discussion_r37356582'>File: src/android/java/io/jxcore/node/BtConnectorHelper.java:L116-139</a></b>\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> @DrJukka As mentioned above this isn't safe. If there are any more examples of this I won't call them out. I'm going to assume that you will search for them.\n- [ ] <a href='#crh-comment-Pull 6d929e4c702ac4914b30b94f77618ea4d6083ab5 src/android/java/io/jxcore/node/BtConnectorHelper.java 122'></a> <img src='http://www.codereviewhub.com/site/github-remaining.png' height=16 width=60>&nbsp;<b><a href='https://github.com/thaliproject/Thali_CordovaPlugin/pull/69#discussion_r37357316'>File: src/android/java/io/jxcore/node/BtConnectorHelper.java:L116-139</a></b>\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> @DrJukka Instead of synchronizing on mServerSocketList we should use CopyOnWriteArrayList or equivalent.\n- [ ] <a href='#crh-comment-Pull 6d929e4c702ac4914b30b94f77618ea4d6083ab5 src/android/java/io/jxcore/node/BtConnectorHelper.java 124'></a> <img src='http://www.codereviewhub.com/site/github-remaining.png' height=16 width=60>&nbsp;<b><a href='https://github.com/thaliproject/Thali_CordovaPlugin/pull/69#discussion_r37357377'>File: src/android/java/io/jxcore/node/BtConnectorHelper.java:L116-139</a></b>\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> @DrJukka Note that when you switch to CopyOnWriteArrayList or equivalent you will want to use methods like elements() to get a consistent set of contents to iterate over.\n- [ ] <a href='#crh-comment-Pull 6d929e4c702ac4914b30b94f77618ea4d6083ab5 src/android/java/io/jxcore/node/BtConnectorHelper.java 130'></a> <img src='http://www.codereviewhub.com/site/github-remaining.png' height=16 width=60>&nbsp;<b><a href='https://github.com/thaliproject/Thali_CordovaPlugin/pull/69#discussion_r37357503'>File: src/android/java/io/jxcore/node/BtConnectorHelper.java:L116-139</a></b>\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> Here you will want to use remove() with the non-null tmp so you make sure you remove the right object.\n- [ ] <a href='#crh-comment-Pull 6d929e4c702ac4914b30b94f77618ea4d6083ab5 src/android/java/io/jxcore/node/BtConnectorHelper.java 131'></a> <img src='http://www.codereviewhub.com/site/github-remaining.png' height=16 width=60>&nbsp;<b><a href='https://github.com/thaliproject/Thali_CordovaPlugin/pull/69#discussion_r37357657'>File: src/android/java/io/jxcore/node/BtConnectorHelper.java:L116-139</a></b>\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> I don't understand the intended semantics of 'ret'. It seems to mean \"I found and closed at least one server socket\". But why is that useful? I would have thought that you would only want to return true if you successfully closed all the sockets? And if there were no sockets then shouldn't you still return true since you did close all sockets, there just weren't any?\n- [ ] <a href='#crh-comment-Pull 6d929e4c702ac4914b30b94f77618ea4d6083ab5 src/android/java/io/jxcore/node/BtConnectorHelper.java 138'></a> <img src='http://www.codereviewhub.com/site/github-remaining.png' height=16 width=60>&nbsp;<b><a href='https://github.com/thaliproject/Thali_CordovaPlugin/pull/69#discussion_r37358143'>File: src/android/java/io/jxcore/node/BtConnectorHelper.java:L116-139</a></b>\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> @DrJukka Sorry to stick this comment here but I can't add comments to GetBluetoothAddress, GetDeviceName or SetDeviceName. In both cases shouldn't the return value be null if we can't get the default adapter? Also rather than using:\n```java\nif (bluetooth != null) {\nret = bluetooth.getName();\n}\nreturn ret;\n```\nWouldn't it be cleaner to just use:\n```java\nreturn bluetooth == null ? null : bluetooth.getName();\n```\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> @DrJukka Also sorry to stick this comment here but I'm limited by how GitHub reviews work. In any case, why is GetKeyValue and SetKeyValue still around? I thought we were going to get rid of these? Sorry if I've lost my mind on this one but I thought this was functionality we removed?\n- [ ] <a href='#crh-comment-Pull 6d929e4c702ac4914b30b94f77618ea4d6083ab5 src/android/java/io/jxcore/node/BtConnectorHelper.java 147'></a> <img src='http://www.codereviewhub.com/site/github-remaining.png' height=16 width=60>&nbsp;<b><a href='https://github.com/thaliproject/Thali_CordovaPlugin/pull/69#discussion_r37358447'>File: src/android/java/io/jxcore/node/BtConnectorHelper.java:L194-226</a></b>\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> @DrJukka Can you please use a more descriptive name like localMBtToRequestSocket?\n- [ ] <a href='#crh-comment-Pull 6d929e4c702ac4914b30b94f77618ea4d6083ab5 src/android/java/io/jxcore/node/BtConnectorHelper.java 155'></a> <img src='http://www.codereviewhub.com/site/github-remaining.png' height=16 width=60>&nbsp;<b><a href='https://github.com/thaliproject/Thali_CordovaPlugin/pull/69#discussion_r37358570'>File: src/android/java/io/jxcore/node/BtConnectorHelper.java:L194-226</a></b>\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> @DrJukka This assumes that we can connect to only one peer at a time. But I thought you said that Bluetooth supports multiple simultaneous connections to different peers?\n- [ ] <a href='#crh-comment-Pull 6d929e4c702ac4914b30b94f77618ea4d6083ab5 src/android/java/io/jxcore/node/BtConnectorHelper.java 156'></a> <img src='http://www.codereviewhub.com/site/github-remaining.png' height=16 width=60>&nbsp;<b><a href='https://github.com/thaliproject/Thali_CordovaPlugin/pull/69#discussion_r37358650'>File: src/android/java/io/jxcore/node/BtConnectorHelper.java:L194-226</a></b>\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> @DrJukka I would suggest that the code would be easier to read if you just stuck a return after line 204. That would allow you to get rid of the next else and indent all of that code one tab space to the left. It's easier to read code that isn't deeply nested.\n- [ ] <a href='#crh-comment-Pull 6d929e4c702ac4914b30b94f77618ea4d6083ab5 src/android/java/io/jxcore/node/BtConnectorHelper.java 195'></a> <img src='http://www.codereviewhub.com/site/github-remaining.png' height=16 width=60>&nbsp;<b><a href='https://github.com/thaliproject/Thali_CordovaPlugin/pull/69#discussion_r37359026'>File: src/android/java/io/jxcore/node/BtConnectorHelper.java:L241-262</a></b>\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> @DrJukka This comment is in regards to line 239 above where you return \"Device Address for .... not found from Discovered device list\". The code, I believe, would be easier to read if you instead put in\n```java\nif (selectedDevice == null) {\ntmpCallback.ConnectionStatusUpdate(\"Device Address for \" + toPeerId + \" not found from Discovered device list.\",-1);\nreturn;\n}\n```\nThen you can remove the `if (selectedDevice != null)` and out indent that code.\n- [ ] <a href='#crh-comment-Pull 6d929e4c702ac4914b30b94f77618ea4d6083ab5 src/android/java/io/jxcore/node/BtConnectorHelper.java 146'></a> <img src='http://www.codereviewhub.com/site/github-remaining.png' height=16 width=60>&nbsp;<b><a href='https://github.com/thaliproject/Thali_CordovaPlugin/pull/69#discussion_r37359460'>File: src/android/java/io/jxcore/node/BtConnectorHelper.java:L194-226</a></b>\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> @DrJukka What happens if BeginConnectPeer is called twice in parallel? This is perfectly reasonable if we are trying to connect to two different peers. I'm fairly confident that making the method synchronized is not the right solution. This, for example, won't solve what happens if someone tries to simultaneously connect and disconnect since synchronized methods apply to repeated calls to the method, not the object. I\"m not worried about race conditions as much as I'm worried about us trying to connect to the same peer twice. Will Bluetooth's layer stop that? Or can you open two Bluetooth client sockets to the same device?\n- [ ] <a href='#crh-comment-Pull 6d929e4c702ac4914b30b94f77618ea4d6083ab5 src/android/java/io/jxcore/node/BtConnectorHelper.java 179'></a> <img src='http://www.codereviewhub.com/site/github-remaining.png' height=16 width=60>&nbsp;<b><a href='https://github.com/thaliproject/Thali_CordovaPlugin/pull/69#discussion_r37359584'>File: src/android/java/io/jxcore/node/BtConnectorHelper.java:L194-226</a></b>\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> @DrJukka I'm guessing line 225 below isn't correct since it presumes we can only connect to one peer at a time?\n- [ ] <a href='#crh-comment-Pull 6d929e4c702ac4914b30b94f77618ea4d6083ab5 src/android/java/io/jxcore/node/BtConnectorHelper.java 187'></a> <img src='http://www.codereviewhub.com/site/github-remaining.png' height=16 width=60>&nbsp;<b><a href='https://github.com/thaliproject/Thali_CordovaPlugin/pull/69#discussion_r37359656'>File: src/android/java/io/jxcore/node/BtConnectorHelper.java:L241-262</a></b>\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> @DrJukka On line 236 above /conenctivity/connectivity/\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> @DrJukka On line 248 what happens if bluetoothSocket is null? Currently the code does nothing. That is fine but in that case please rewrite the code to check for null and return and then out indent the rest of the code. I'll stop making this comment about 'if' styling (e.g. don't indent things like crazy, instead use return) and assume you will check all your files for it and please fix.\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> @DrJukka The connected method is, I think, unnecessarily hard to read. Could you please refactor it into two methods, one called ConnectedIncoming and another called ConnectedOutgoing and whomever calls this can then switch between the two based on the incoming value? It would make the code much easier to read.\n- [ ] <a href='#crh-comment-Pull 6d929e4c702ac4914b30b94f77618ea4d6083ab5 src/android/java/io/jxcore/node/BtConnectorHelper.java 203'></a> <img src='http://www.codereviewhub.com/site/github-remaining.png' height=16 width=60>&nbsp;<b><a href='https://github.com/thaliproject/Thali_CordovaPlugin/pull/69#discussion_r37360169'>File: src/android/java/io/jxcore/node/BtConnectorHelper.java:L241-262</a></b>\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> @DrJukka Line 271 below is not thread safe because between the null check on 271 and the stop on 272 someone else could have changed mBtToRequestSocket to be null. Please first copy to a local variable and then check for null and call stop.\n- [ ] <a href='#crh-comment-Pull 6d929e4c702ac4914b30b94f77618ea4d6083ab5 thali/thalireplicationmanager.js 33'></a> <img src='http://www.codereviewhub.com/site/github-remaining.png' height=16 width=60>&nbsp;<b><a href='https://github.com/thaliproject/Thali_CordovaPlugin/pull/69#discussion_r37364290'>File: thali/thalireplicationmanager.js:L106-116</a></b>\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> Purely as an example of defensive programming would it make sense to put a return after line 114?\n- [ ] <a href='#crh-comment-Pull 6d929e4c702ac4914b30b94f77618ea4d6083ab5 thali/thalireplicationmanager.js 49'></a> <img src='http://www.codereviewhub.com/site/github-remaining.png' height=16 width=60>&nbsp;<b><a href='https://github.com/thaliproject/Thali_CordovaPlugin/pull/69#discussion_r37364572'>File: thali/thalireplicationmanager.js:L117-149</a></b>\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> It seems like this code is assuming that PEER_AVAILABILITY_CHANGED will always return all peers that are current visible? But in looking [here](https://github.com/thaliproject/Thali_CordovaPlugin/blob/story_0/doc/api/connectivity.md) it only says that the event will be called when a peer's availability changes. Which implies that if a peer's availability hasn't changed then it won't be included in the event. In which case we just overwrite this._peers with an incomplete list of who is available! That doesn't sound right. What am I missing?\n- [ ] <a href='#crh-comment-Pull 6d929e4c702ac4914b30b94f77618ea4d6083ab5 thali/thalireplicationmanager.js 58'></a> <img src='http://www.codereviewhub.com/site/github-remaining.png' height=16 width=60>&nbsp;<b><a href='https://github.com/thaliproject/Thali_CordovaPlugin/pull/69#discussion_r37364857'>File: thali/thalireplicationmanager.js:L117-149</a></b>\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> Could you please use a slightly more informative variable name than p? Like existingPeerReplication? I'm slow and keep getting confused between peer and p.\n- [ ] <a href='#crh-comment-Pull 6d929e4c702ac4914b30b94f77618ea4d6083ab5 thali/thalireplicationmanager.js 153'></a> <img src='http://www.codereviewhub.com/site/github-remaining.png' height=16 width=60>&nbsp;<b><a href='https://github.com/thaliproject/Thali_CordovaPlugin/pull/69#discussion_r37365290'>File: thali/thalireplicationmanager.js:L185-308</a></b>\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> Isn't this assuming that we can only be in retry with a single peer at a time? What if we have multiple peers we are simultaneously connecting to and we are in retry with one or more of them? Shouldn't isInRetry really be an array indexed by peerID?\nAlso while on the topic can we take clients and replications and the rest and collapse them into one array with an object? It would make it much easier to think about threading issues since all the places state can blow up are in one place?\n- [ ] <a href='#crh-comment-Pull 02ec9ff3aa6e94ff206c2e0175e9061467be1512 src/android/java/io/jxcore/node/BtConnectorHelper.java 23'></a> <img src='http://www.codereviewhub.com/site/github-remaining.png' height=16 width=60>&nbsp;<b><a href='https://github.com/thaliproject/Thali_CordovaPlugin/pull/69#discussion_r37438236'>File: src/android/java/io/jxcore/node/BtConnectorHelper.java:L5-25</a></b>\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> On line 28 below you set context to null. Which is fine but since context is only set in the constructor you can make it final. It's always good habit in multi-threaded code to make anything that can be final, into final, it prevents mistakes. So i would review all your state variables and see which ones you can make final.\n- [ ] <a href='#crh-comment-Pull 02ec9ff3aa6e94ff206c2e0175e9061467be1512 src/android/java/io/jxcore/node/BtConnectorHelper.java 29'></a> <img src='http://www.codereviewhub.com/site/github-remaining.png' height=16 width=60>&nbsp;<b><a href='https://github.com/thaliproject/Thali_CordovaPlugin/pull/69#discussion_r37438768'>File: src/android/java/io/jxcore/node/BtConnectorHelper.java:L32-44</a></b>\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> You really want to make this final as well.  Life is much easier if a variable can never go null. In the case of lastAvailableList, for example, the only place you ever reset it is in AddPeerIfNotDiscovered and you can just call the clear method on the variable.\nTo be clear, you do NOT have a bug here. The code is correct. The use of final and clear is just meant as a defensive coding technique.\n- [ ] <a href='#crh-comment-Pull 02ec9ff3aa6e94ff206c2e0175e9061467be1512 src/android/java/io/jxcore/node/BtConnectorHelper.java 44'></a> <img src='http://www.codereviewhub.com/site/github-remaining.png' height=16 width=60>&nbsp;<b><a href='https://github.com/thaliproject/Thali_CordovaPlugin/pull/69#discussion_r37440232'>File: src/android/java/io/jxcore/node/BtConnectorHelper.java:L52-70</a></b>\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> Here is a fun example of why sometimes you have to use synchronized methods. Ask yourself the following question - what happens if start and stop both get called at the same time from different threads?\nBut here is a better question - is it possible for start and stop to be called at the same time from different threads? I don't think so. But we should be sure. Here is my reasoning - The only entity that should EVER call BtConnectorHelper is JXcoreExtension and JXcore guarantees that JXcoreExtension will always be called on the same thread (at least I think it guarantees that, we should check with Oguz). In that case methods like start and stop can only be called on a single thread since only JXcoreExtension can call them. If that is all true then this is worth some kind of comment at the top of the file pointing out that we are depending on the fact that other than in callbacks all the methods in BtConnectorHelper will only be called by JXcoreExtension and JXcoreExtension will only be called from the same thread.\n- [ ] <a href='#crh-comment-Pull 02ec9ff3aa6e94ff206c2e0175e9061467be1512 src/android/java/io/jxcore/node/BtConnectorHelper.java 106'></a> <img src='http://www.codereviewhub.com/site/github-remaining.png' height=16 width=60>&nbsp;<b><a href='https://github.com/thaliproject/Thali_CordovaPlugin/pull/69#discussion_r37441413'>File: src/android/java/io/jxcore/node/BtConnectorHelper.java:L74-151</a></b>\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> The use of an empty string to mean disconnect really worries me. Mostly because this means that if someone in node.js land calls disconnect with an empty string that will also cause the current peer to disconnect. That is a great way to create really bad bugs. We should always make the programmer be explicit about their desire. So I would remove the \"\" flag (which anyway is ambiguous) and create a private method \"DisconnectAll\" that Stop can call and make Disconnect \"safe\" by requiring an exact peerid match.\n- [ ] <a href='#crh-comment-Pull 02ec9ff3aa6e94ff206c2e0175e9061467be1512 src/android/java/io/jxcore/node/BtConnectorHelper.java 119'></a> <img src='http://www.codereviewhub.com/site/github-remaining.png' height=16 width=60>&nbsp;<b><a href='https://github.com/thaliproject/Thali_CordovaPlugin/pull/69#discussion_r37442588'>File: src/android/java/io/jxcore/node/BtConnectorHelper.java:L74-151</a></b>\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> This comment is misleading since the method is also used by stop.\n- [ ] <a href='#crh-comment-Pull 02ec9ff3aa6e94ff206c2e0175e9061467be1512 src/android/java/io/jxcore/node/BtConnectorHelper.java 123'></a> <img src='http://www.codereviewhub.com/site/github-remaining.png' height=16 width=60>&nbsp;<b><a href='https://github.com/thaliproject/Thali_CordovaPlugin/pull/69#discussion_r37442700'>File: src/android/java/io/jxcore/node/BtConnectorHelper.java:L74-151</a></b>\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> If we make mServerSocketList final then it can never be null, only empty.\n- [x] <a href='#crh-comment-Pull 303ae2ac9921143683c569d7046cd50f0da23b12 thali/thalireplicationmanager.js 36'></a> <img src='http://www.codereviewhub.com/site/github-completed.png' height=16 width=60>&nbsp;<b><a href='https://github.com/thaliproject/Thali_CordovaPlugin/pull/69#discussion_r36887935'>File: thali/thalireplicationmanager.js:L166-185</a></b>\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> What if we didn't get an error on the disconnect? In that case won't err be null? Shouldn't we check for that and only run line 169 and 170 if there is an actual error?\n- <a href='https://github.com/mpodwysocki'><img border=0 src='https://avatars.githubusercontent.com/u/11578097?v=3' height=16 width=16'></a> Fixed as per latest commit\n- [x] <a href='#crh-comment-Pull 303ae2ac9921143683c569d7046cd50f0da23b12 thali/thalireplicationmanager.js 62'></a> <img src='http://www.codereviewhub.com/site/github-completed.png' height=16 width=60>&nbsp;<b><a href='https://github.com/thaliproject/Thali_CordovaPlugin/pull/69#discussion_r36888127'>File: thali/thalireplicationmanager.js:L202-246</a></b>\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> /sockiet/socket/\n- <a href='https://github.com/mpodwysocki'><img border=0 src='https://avatars.githubusercontent.com/u/11578097?v=3' height=16 width=16'></a> Fixed\n- [x] <a href='#crh-comment-Pull 303ae2ac9921143683c569d7046cd50f0da23b12 thali/thalireplicationmanager.js 58'></a> <img src='http://www.codereviewhub.com/site/github-completed.png' height=16 width=60>&nbsp;<b><a href='https://github.com/thaliproject/Thali_CordovaPlugin/pull/69#discussion_r36888210'>File: thali/thalireplicationmanager.js:L202-246</a></b>\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> If we got an end event doesn't that mean the socket is already destroyed? Any danger from destroying a socket twice?\n- <a href='https://github.com/mpodwysocki'><img border=0 src='https://avatars.githubusercontent.com/u/11578097?v=3' height=16 width=16'></a> I haven't noticed any exceptions being thrown from this code, so I'm not sure\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> Why not just write a quick test to be sure? E.g. open a socket, destroy it and then destroy it again. It's best to be sure.\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> :+1: Per our conversation\n- [x] <a href='#crh-comment-Pull 303ae2ac9921143683c569d7046cd50f0da23b12 thali/thalireplicationmanager.js 70'></a> <img src='http://www.codereviewhub.com/site/github-completed.png' height=16 width=60>&nbsp;<b><a href='https://github.com/thaliproject/Thali_CordovaPlugin/pull/69#discussion_r36888421'>File: thali/thalireplicationmanager.js:L202-246</a></b>\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> Why do you destroy the incomingClientSocket on error but not on end?  Of course given my previous question about destroying \"end\"ed sockets maybe this is correct and line 207 is the problem?\n- <a href='https://github.com/mpodwysocki'><img border=0 src='https://avatars.githubusercontent.com/u/11578097?v=3' height=16 width=16'></a> I'm on the assumption that end is perfectly normal but error is not.  Pretty sure we can get rid of that code for destroying the socket because an error should do that already.  However, on stream error we might want to tear down the incoming client socket\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> :+1: Please tell me there are tests to be sure you are right?\n- [x] <a href='#crh-comment-Pull 303ae2ac9921143683c569d7046cd50f0da23b12 thali/thalireplicationmanager.js 74'></a> <img src='http://www.codereviewhub.com/site/github-completed.png' height=16 width=60>&nbsp;<b><a href='https://github.com/thaliproject/Thali_CordovaPlugin/pull/69#discussion_r36889852'>File: thali/thalireplicationmanager.js:L202-246</a></b>\n- <a href='https://github.com/yaronyg'><img border=0 src='https://avatars.githubusercontent.com/u/1480964?v=3' height=16 width=16'></a> /sockiet/socket/\n- <a href='https://github.com/mpodwysocki'><img border=0 src='https://avatars.githubusercontent.com/u/11578097?v=3' height=16 width=16'></a> Fixed\n\n\n<a href='https://www.codereviewhub.com/thaliproject/Thali_CordovaPlugin/pull/69?mark_as_completed=1'><img src='http://www.codereviewhub.com/site/github-mark-as-completed.png' height=26></a>&nbsp;<a href='https://www.codereviewhub.com/thaliproject/Thali_CordovaPlugin/pull/69?approve=1'><img src='http://www.codereviewhub.com/site/github-approve.png' height=26></a>&nbsp;<a href='https://github.com/thaliproject/Thali_CordovaPlugin/pull/69'><img src='http://www.codereviewhub.com/site/github-refresh.png' height=26></a>\n<a href='#crh-end'></a>","created_at":"2015-08-12T17:07:47Z","updated_at":"2015-08-19T17:36:46Z","closed_at":null,"merged_at":null,"merge_commit_sha":"56538061110314f4981955d1eaa5269c9509518a","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/pulls/69/commits","review_comments_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/pulls/69/comments","review_comment_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/pulls/comments{/number}","comments_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/issues/69/comments","statuses_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/statuses/02ec9ff3aa6e94ff206c2e0175e9061467be1512","head":{"label":"thaliproject:story_0_patch0","ref":"story_0_patch0","sha":"02ec9ff3aa6e94ff206c2e0175e9061467be1512","user":{"login":"thaliproject","id":7918300,"avatar_url":"https://avatars.githubusercontent.com/u/7918300?v=3","gravatar_id":"","url":"https://api.github.com/users/thaliproject","html_url":"https://github.com/thaliproject","followers_url":"https://api.github.com/users/thaliproject/followers","following_url":"https://api.github.com/users/thaliproject/following{/other_user}","gists_url":"https://api.github.com/users/thaliproject/gists{/gist_id}","starred_url":"https://api.github.com/users/thaliproject/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/thaliproject/subscriptions","organizations_url":"https://api.github.com/users/thaliproject/orgs","repos_url":"https://api.github.com/users/thaliproject/repos","events_url":"https://api.github.com/users/thaliproject/events{/privacy}","received_events_url":"https://api.github.com/users/thaliproject/received_events","type":"Organization","site_admin":false},"repo":{"id":36732493,"name":"Thali_CordovaPlugin","full_name":"thaliproject/Thali_CordovaPlugin","owner":{"login":"thaliproject","id":7918300,"avatar_url":"https://avatars.githubusercontent.com/u/7918300?v=3","gravatar_id":"","url":"https://api.github.com/users/thaliproject","html_url":"https://github.com/thaliproject","followers_url":"https://api.github.com/users/thaliproject/followers","following_url":"https://api.github.com/users/thaliproject/following{/other_user}","gists_url":"https://api.github.com/users/thaliproject/gists{/gist_id}","starred_url":"https://api.github.com/users/thaliproject/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/thaliproject/subscriptions","organizations_url":"https://api.github.com/users/thaliproject/orgs","repos_url":"https://api.github.com/users/thaliproject/repos","events_url":"https://api.github.com/users/thaliproject/events{/privacy}","received_events_url":"https://api.github.com/users/thaliproject/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/thaliproject/Thali_CordovaPlugin","description":"Thali p2p plugin ","fork":false,"url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin","forks_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/forks","keys_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/keys{/key_id}","collaborators_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/teams","hooks_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/hooks","issue_events_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/issues/events{/number}","events_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/events","assignees_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/assignees{/user}","branches_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/branches{/branch}","tags_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/tags","blobs_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/git/refs{/sha}","trees_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/git/trees{/sha}","statuses_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/statuses/{sha}","languages_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/languages","stargazers_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/stargazers","contributors_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/contributors","subscribers_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/subscribers","subscription_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/subscription","commits_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/commits{/sha}","git_commits_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/git/commits{/sha}","comments_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/comments{/number}","issue_comment_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/issues/comments{/number}","contents_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/contents/{+path}","compare_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/compare/{base}...{head}","merges_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/merges","archive_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/downloads","issues_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/issues{/number}","pulls_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/pulls{/number}","milestones_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/milestones{/number}","notifications_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/labels{/name}","releases_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/releases{/id}","created_at":"2015-06-02T12:45:51Z","updated_at":"2015-08-06T22:35:27Z","pushed_at":"2015-08-19T15:35:42Z","git_url":"git://github.com/thaliproject/Thali_CordovaPlugin.git","ssh_url":"git@github.com:thaliproject/Thali_CordovaPlugin.git","clone_url":"https://github.com/thaliproject/Thali_CordovaPlugin.git","svn_url":"https://github.com/thaliproject/Thali_CordovaPlugin","homepage":null,"size":1665,"stargazers_count":8,"watchers_count":8,"language":"Objective-C","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":2,"mirror_url":null,"open_issues_count":43,"forks":2,"open_issues":43,"watchers":8,"default_branch":"master"}},"base":{"label":"thaliproject:story_0","ref":"story_0","sha":"191d8da24d38e83c982f9d2fbae68ca10d2edb35","user":{"login":"thaliproject","id":7918300,"avatar_url":"https://avatars.githubusercontent.com/u/7918300?v=3","gravatar_id":"","url":"https://api.github.com/users/thaliproject","html_url":"https://github.com/thaliproject","followers_url":"https://api.github.com/users/thaliproject/followers","following_url":"https://api.github.com/users/thaliproject/following{/other_user}","gists_url":"https://api.github.com/users/thaliproject/gists{/gist_id}","starred_url":"https://api.github.com/users/thaliproject/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/thaliproject/subscriptions","organizations_url":"https://api.github.com/users/thaliproject/orgs","repos_url":"https://api.github.com/users/thaliproject/repos","events_url":"https://api.github.com/users/thaliproject/events{/privacy}","received_events_url":"https://api.github.com/users/thaliproject/received_events","type":"Organization","site_admin":false},"repo":{"id":36732493,"name":"Thali_CordovaPlugin","full_name":"thaliproject/Thali_CordovaPlugin","owner":{"login":"thaliproject","id":7918300,"avatar_url":"https://avatars.githubusercontent.com/u/7918300?v=3","gravatar_id":"","url":"https://api.github.com/users/thaliproject","html_url":"https://github.com/thaliproject","followers_url":"https://api.github.com/users/thaliproject/followers","following_url":"https://api.github.com/users/thaliproject/following{/other_user}","gists_url":"https://api.github.com/users/thaliproject/gists{/gist_id}","starred_url":"https://api.github.com/users/thaliproject/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/thaliproject/subscriptions","organizations_url":"https://api.github.com/users/thaliproject/orgs","repos_url":"https://api.github.com/users/thaliproject/repos","events_url":"https://api.github.com/users/thaliproject/events{/privacy}","received_events_url":"https://api.github.com/users/thaliproject/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/thaliproject/Thali_CordovaPlugin","description":"Thali p2p plugin ","fork":false,"url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin","forks_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/forks","keys_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/keys{/key_id}","collaborators_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/teams","hooks_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/hooks","issue_events_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/issues/events{/number}","events_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/events","assignees_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/assignees{/user}","branches_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/branches{/branch}","tags_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/tags","blobs_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/git/refs{/sha}","trees_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/git/trees{/sha}","statuses_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/statuses/{sha}","languages_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/languages","stargazers_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/stargazers","contributors_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/contributors","subscribers_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/subscribers","subscription_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/subscription","commits_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/commits{/sha}","git_commits_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/git/commits{/sha}","comments_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/comments{/number}","issue_comment_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/issues/comments{/number}","contents_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/contents/{+path}","compare_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/compare/{base}...{head}","merges_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/merges","archive_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/downloads","issues_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/issues{/number}","pulls_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/pulls{/number}","milestones_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/milestones{/number}","notifications_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/labels{/name}","releases_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/releases{/id}","created_at":"2015-06-02T12:45:51Z","updated_at":"2015-08-06T22:35:27Z","pushed_at":"2015-08-19T15:35:42Z","git_url":"git://github.com/thaliproject/Thali_CordovaPlugin.git","ssh_url":"git@github.com:thaliproject/Thali_CordovaPlugin.git","clone_url":"https://github.com/thaliproject/Thali_CordovaPlugin.git","svn_url":"https://github.com/thaliproject/Thali_CordovaPlugin","homepage":null,"size":1665,"stargazers_count":8,"watchers_count":8,"language":"Objective-C","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":2,"mirror_url":null,"open_issues_count":43,"forks":2,"open_issues":43,"watchers":8,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/pulls/69"},"html":{"href":"https://github.com/thaliproject/Thali_CordovaPlugin/pull/69"},"issue":{"href":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/issues/69"},"comments":{"href":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/issues/69/comments"},"review_comments":{"href":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/pulls/69/comments"},"review_comment":{"href":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/pulls/69/commits"},"statuses":{"href":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/statuses/02ec9ff3aa6e94ff206c2e0175e9061467be1512"}}}}