fix: support multiple listeners and report connectivity accurately

README.md

@@ -89,6 +89,8 @@ const connectionType = NitroNetworkInfo.connectionType
 
 Adds a listener for network status changes. Returns a function to unsubscribe the listener.
 
+Any number of listeners can be registered; each unsubscribe function removes only its own listener. A new listener is called once with the current status, then whenever the status changes — a Wi-Fi to cellular handover is reported as a change of `connectionType`, not as a disconnection.
+
 **Parameters:**
 
 - `listener: NetworkInfoListener` - Callback function that receives network status updates

android/src/main/java/com/nitronetworkinfo/HybridNitroNetworkInfo.kt

@@ -4,29 +4,39 @@ import com.margelo.nitro.NitroModules
 import com.margelo.nitro.nitronetworkinfo.ConnectionType
 import com.margelo.nitro.nitronetworkinfo.HybridNitroNetworkInfoSpec
 import com.margelo.nitro.nitronetworkinfo.NitroNetworkStatusInfo
+import java.util.concurrent.ConcurrentHashMap
+import java.util.concurrent.atomic.AtomicLong
 
 
 class HybridNitroNetworkInfo: HybridNitroNetworkInfoSpec(), NetworkInfoDelegate {
     private val context = NitroModules.applicationContext ?: throw Exception("Context is null")
     private val nitroNetworkInfoImpl = NitroNetworkInfoImpl(context, this)
-    private var networkInfoStatusListener: ((NitroNetworkStatusInfo) -> Unit)? = null
+    private val listeners = ConcurrentHashMap<Long, (NitroNetworkStatusInfo) -> Unit>()
+    private val nextListenerId = AtomicLong(0)
 
     override val isConnected: Boolean
-        get() = nitroNetworkInfoImpl.getIsConnection()
+        get() = nitroNetworkInfoImpl.status.isConnected
     override val connectionType: ConnectionType
-        get() = nitroNetworkInfoImpl.getConnectionType()
+        get() = nitroNetworkInfoImpl.status.connectionType
 
+    /**
+     * Adds a listener beside any others; the function returned removes only
+     * this one. The listener is called once with the current status, then on
+     * every change.
+     */
     override fun addListener(listener: (NitroNetworkStatusInfo) -> Unit): () -> Unit {
-        networkInfoStatusListener = listener
-        nitroNetworkInfoImpl.start()
-        listener(NitroNetworkStatusInfo(isConnected, connectionType))
-        return {
-            networkInfoStatusListener = null
-            nitroNetworkInfoImpl.stop()
-        }
+        val id = nextListenerId.getAndIncrement()
+        listeners[id] = listener
+        listener(nitroNetworkInfoImpl.status)
+        return { listeners.remove(id) }
     }
 
     override fun onNetworkInfoChanged(info: NitroNetworkStatusInfo) {
-        networkInfoStatusListener?.invoke(info)
+        listeners.values.forEach { it(info) }
+    }
+
+    override fun dispose() {
+        nitroNetworkInfoImpl.dispose()
+        super.dispose()
     }
 }

android/src/main/java/com/nitronetworkinfo/NitroNetworkInfoImpl.kt

@@ -16,88 +16,104 @@ interface NetworkInfoDelegate {
     fun onNetworkInfoChanged(info: NitroNetworkStatusInfo)
 }
 
+/**
+ * Watches every network that offers internet for as long as it exists.
+ *
+ * Reading the status never leaves the process: it is one volatile read of a
+ * value the network callback keeps current. Asking ConnectivityManager on each
+ * read would be two binder calls into system_server, synchronously on the JS
+ * thread, every time JavaScript touched `isConnected`.
+ *
+ * The status is derived from the whole set of live networks, not from the one
+ * that changed. A handover — Wi-Fi to cellular at a door — is a loss and a gain
+ * moments apart, and `onLost` for the Wi-Fi often arrives while cellular is
+ * already up; reporting that as "disconnected" was wrong.
+ */
 class NitroNetworkInfoImpl(context: Context, delegate: NetworkInfoDelegate? = null) {
     private val connectivityManager = context.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager
-    private var networkCallback: ConnectivityManager.NetworkCallback? = null
     private val mainHandler = Handler(Looper.getMainLooper())
     private val delegateRef = WeakReference(delegate)
 
-    @Synchronized
-    fun start() {
-        if (networkCallback != null) return
-
-        val request = NetworkRequest.Builder()
-            .addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET)
-            .build()
-
-        networkCallback = object : ConnectivityManager.NetworkCallback() {
-            override fun onAvailable(network: Network) {
-                Log.d(TAG, "Network is available")
-                emit(NitroNetworkStatusInfo(
-                    isConnected = true,
-                    connectionType = getConnectionType()
-                ))
-            }
-
-            override fun onLost(network: Network) {
-                Log.d(TAG, "Network is lost")
-                emit(NitroNetworkStatusInfo(
-                    isConnected = false,
-                    connectionType = ConnectionType.UNKNOWN
-                ))
-            }
+    // Written only on the connectivity callback thread, read from any thread.
+    @Volatile
+    var status: NitroNetworkStatusInfo = queryStatus()
+        private set
+
+    // Confined to the connectivity callback thread, which delivers every
+    // callback below in order, so it needs no lock.
+    private val networks = HashMap<Network, NetworkCapabilities?>()
+
+    private val callback = object : ConnectivityManager.NetworkCallback() {
+        override fun onAvailable(network: Network) {
+            // One binder call per network appearing, which is rare. Below API
+            // 26 no onCapabilitiesChanged is guaranteed to follow.
+            networks[network] = connectivityManager.getNetworkCapabilities(network)
+            recompute()
         }
-        networkCallback?.let {
-            connectivityManager.registerNetworkCallback(request, it)
+
+        override fun onCapabilitiesChanged(network: Network, networkCapabilities: NetworkCapabilities) {
+            // Cellular signal strength alone fires this every few seconds; the
+            // capabilities arrive with it, and recompute drops what changed
+            // nothing anybody reads.
+            networks[network] = networkCapabilities
+            recompute()
         }
-    }
 
-    fun emit(info: NitroNetworkStatusInfo){
-        mainHandler.post {
-            delegateRef.get()?.onNetworkInfoChanged(info)
+        override fun onLost(network: Network) {
+            networks.remove(network)
+            recompute()
         }
     }
 
-    fun getIsConnection(): Boolean {
-        val network = connectivityManager.activeNetwork
-        val capabilities = connectivityManager.getNetworkCapabilities(network)
-        return capabilities?.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) == true
+    init {
+        val request = NetworkRequest.Builder()
+            .addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET)
+            .build()
+        connectivityManager.registerNetworkCallback(request, callback)
     }
 
-    fun getConnectionType(): ConnectionType {
-        val network = connectivityManager.activeNetwork
-        val capabilities = connectivityManager.getNetworkCapabilities(network)
-        val connectionType = when (true) {
-            capabilities?.hasTransport(NetworkCapabilities.TRANSPORT_WIFI) -> {
-                ConnectionType.WIFI
-            }
-
-            capabilities?.hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR) -> {
-                ConnectionType.CELLULAR
-            }
-
-            capabilities?.hasTransport(NetworkCapabilities.TRANSPORT_ETHERNET) -> {
-                ConnectionType.ETHERNET
-            }
-
-            else -> ConnectionType.UNKNOWN
+    fun dispose() {
+        mainHandler.removeCallbacksAndMessages(null)
+        try {
+            connectivityManager.unregisterNetworkCallback(callback)
+        } catch (e: IllegalArgumentException) {
+            Log.w(TAG, "Network callback already unregistered", e)
         }
-        return connectionType
     }
 
-    fun stop() {
-        mainHandler.removeCallbacksAndMessages(null)
+    private fun recompute() {
+        val next = statusOf(networks.values)
+        if (next == status) return
+        status = next
+        mainHandler.post { delegateRef.get()?.onNetworkInfoChanged(next) }
+    }
 
-        networkCallback?.let {
-            try { connectivityManager.unregisterNetworkCallback(it) }
-            catch (e: IllegalArgumentException) {
-                Log.w(TAG, "Network callback already unregistered", e)
-            }
-        }
-        networkCallback = null
+    /** The starting status, asked once before the callback has reported. */
+    private fun queryStatus(): NitroNetworkStatusInfo {
+        val capabilities = connectivityManager.getNetworkCapabilities(connectivityManager.activeNetwork)
+            ?.takeIf { it.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) }
+        return statusOf(listOfNotNull(capabilities))
     }
 
     companion object {
         const val TAG = "NitroNetworkInfo"
+
+        /**
+         * Connected while any internet network is up. The type is the one
+         * Android itself prefers as the default — ethernet, then Wi-Fi, then
+         * cellular — so no binder call is needed to ask which that is.
+         */
+        private fun statusOf(capabilities: Collection<NetworkCapabilities?>): NitroNetworkStatusInfo {
+            if (capabilities.isEmpty()) {
+                return NitroNetworkStatusInfo(isConnected = false, connectionType = ConnectionType.UNKNOWN)
+            }
+            val type = when {
+                capabilities.any { it?.hasTransport(NetworkCapabilities.TRANSPORT_ETHERNET) == true } -> ConnectionType.ETHERNET
+                capabilities.any { it?.hasTransport(NetworkCapabilities.TRANSPORT_WIFI) == true } -> ConnectionType.WIFI
+                capabilities.any { it?.hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR) == true } -> ConnectionType.CELLULAR
+                else -> ConnectionType.UNKNOWN
+            }
+            return NitroNetworkStatusInfo(isConnected = true, connectionType = type)
+        }
     }
-}
+}

ios/HybridNitroNetworkInfo.swift

@@ -10,46 +10,64 @@ import Foundation
 typealias NetworkInfoListener = (NitroNetworkStatusInfo) -> Void
 
 class HybridNitroNetworkInfo: HybridNitroNetworkInfoSpec {
-    private var nitrogenNetworkInfoImpl = NitroNetworkInfoImpl()
-    private var networkInfoStatusListener: ((NitroNetworkStatusInfo) -> Void)? = nil
+    private let nitrogenNetworkInfoImpl = NitroNetworkInfoImpl()
+    private let lock = NSLock()
+    private var listeners: [UInt64: NetworkInfoListener] = [:]
+    private var nextListenerId: UInt64 = 0
+
+    override init() {
+        super.init()
+        nitrogenNetworkInfoImpl.delegate = self
+    }
 
     var isConnected: Bool {
-        get { nitrogenNetworkInfoImpl.getIsConnected() }
+        nitrogenNetworkInfoImpl.current.isConnected
     }
-    
+
     var connectionType: ConnectionType {
-        get { nitrogenNetworkInfoImpl.getConnectionType() }
+        nitrogenNetworkInfoImpl.current.connectionType
     }
-
-    func addListener(listener: @escaping (NitroNetworkStatusInfo) -> Void) throws -> () -> Void {
-        networkInfoStatusListener = listener
-        nitrogenNetworkInfoImpl.start()
+
+    /// Adds a listener beside any others; the function returned removes only
+    /// this one. The listener is called once with the current status, then on
+    /// every change.
+    func addListener(listener: @escaping NetworkInfoListener) throws -> () -> Void {
+        lock.lock()
+        let id = nextListenerId
+        nextListenerId += 1
+        listeners[id] = listener
+        lock.unlock()
+
+        // On main, behind any change already queued there, so the listener
+        // never sees an older status after a newer one. Before the monitor's
+        // first answer there is nothing to give, and nothing is waited for:
+        // the first path is delivered to every listener registered by then.
         DispatchQueue.main.async { [weak self] in
-            guard let self = self else { return }
-
-            self.networkInfoStatusListener?(
-                NitroNetworkStatusInfo(
-                    isConnected: self.isConnected,
-                    connectionType: self.connectionType
-                )
-            )
-
-            self.nitrogenNetworkInfoImpl.delegate = self
+            guard let self, self.isListening(id),
+                  let status = self.nitrogenNetworkInfoImpl.currentIfKnown else { return }
+            listener(status)
         }
 
         return { [weak self] in
-            self?.networkInfoStatusListener = nil
-            self?.nitrogenNetworkInfoImpl.stop()
+            guard let self else { return }
+            self.lock.lock()
+            self.listeners.removeValue(forKey: id)
+            self.lock.unlock()
         }
     }
-
-    deinit {
-        self.nitrogenNetworkInfoImpl.stop()
+
+    private func isListening(_ id: UInt64) -> Bool {
+        lock.lock()
+        defer { lock.unlock() }
+        return listeners[id] != nil
     }
 }
 
 extension HybridNitroNetworkInfo: NitroNetworkInfoDelegate {
     func nitroNetworkInfoDidChange(_ nitroNetworkInfo: NitroNetworkStatusInfo) {
-        self.networkInfoStatusListener?(nitroNetworkInfo)
+        lock.lock()
+        let current = Array(listeners.values)
+        lock.unlock()
+        current.forEach { $0(nitroNetworkInfo) }
     }
 }