From 2f088938c6c8c718d0a4ed639c18868257e354a1 Mon Sep 17 00:00:00 2001 From: Harsh Shandilya Date: Wed, 16 Sep 2020 15:30:15 +0530 Subject: ui: replace GlobalScope with a hand-rolled CoroutineScope GlobalScope has numerous problems[1] that make it unfit for use in most applications and making it behave correctly requires an excessive amount of verbosity that's alleviated simply by using any other scope. Since we run multiple operations in the context of the application's lifecycle, introduce a new scope that is created when our application is, and cancelled upon its termination. While at it, make the scope default to Dispatchers.IO to reduce pressure on the UI event loop. Tasks requiring access to the UI thread appropriately switch context making the change completely safe. 1: https://medium.com/@elizarov/the-reason-to-avoid-globalscope-835337445abc Signed-off-by: Harsh Shandilya --- ui/src/main/java/com/wireguard/android/Application.kt | 11 +++++++++-- .../main/java/com/wireguard/android/BootShutdownReceiver.kt | 5 ++--- ui/src/main/java/com/wireguard/android/QuickTileService.kt | 5 ++--- .../main/java/com/wireguard/android/model/ObservableTunnel.kt | 6 +++--- ui/src/main/java/com/wireguard/android/model/TunnelManager.kt | 10 +++++----- ui/src/main/java/com/wireguard/android/util/Extensions.kt | 4 ++++ 6 files changed, 25 insertions(+), 16 deletions(-) diff --git a/ui/src/main/java/com/wireguard/android/Application.kt b/ui/src/main/java/com/wireguard/android/Application.kt index f63e6270..537d5416 100644 --- a/ui/src/main/java/com/wireguard/android/Application.kt +++ b/ui/src/main/java/com/wireguard/android/Application.kt @@ -24,14 +24,17 @@ import com.wireguard.android.util.ModuleLoader import com.wireguard.android.util.RootShell import com.wireguard.android.util.ToolsInstaller import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.GlobalScope +import kotlinx.coroutines.Job +import kotlinx.coroutines.cancel import kotlinx.coroutines.launch import java.lang.ref.WeakReference import java.util.Locale class Application : android.app.Application(), OnSharedPreferenceChangeListener { private val futureBackend = CompletableDeferred() + private val coroutineScope = CoroutineScope(Job() + Dispatchers.Main.immediate) private var backend: Backend? = null private lateinit var moduleLoader: ModuleLoader private lateinit var rootShell: RootShell @@ -98,7 +101,7 @@ class Application : android.app.Application(), OnSharedPreferenceChangeListener } tunnelManager = TunnelManager(FileConfigStore(applicationContext)) tunnelManager.onCreate() - GlobalScope.launch(Dispatchers.IO) { + coroutineScope.launch(Dispatchers.IO) { try { backend = determineBackend() futureBackend.complete(backend!!) @@ -116,6 +119,7 @@ class Application : android.app.Application(), OnSharedPreferenceChangeListener override fun onTerminate() { sharedPreferences.unregisterOnSharedPreferenceChangeListener(this) + coroutineScope.cancel() super.onTerminate() } @@ -146,6 +150,9 @@ class Application : android.app.Application(), OnSharedPreferenceChangeListener @JvmStatic fun getTunnelManager() = get().tunnelManager + + @JvmStatic + fun getCoroutineScope() = get().coroutineScope } init { diff --git a/ui/src/main/java/com/wireguard/android/BootShutdownReceiver.kt b/ui/src/main/java/com/wireguard/android/BootShutdownReceiver.kt index 70899a0c..203350fe 100644 --- a/ui/src/main/java/com/wireguard/android/BootShutdownReceiver.kt +++ b/ui/src/main/java/com/wireguard/android/BootShutdownReceiver.kt @@ -9,13 +9,12 @@ import android.content.Context import android.content.Intent import android.util.Log import com.wireguard.android.backend.WgQuickBackend -import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.GlobalScope +import com.wireguard.android.util.applicationScope import kotlinx.coroutines.launch class BootShutdownReceiver : BroadcastReceiver() { override fun onReceive(context: Context, intent: Intent) { - GlobalScope.launch(Dispatchers.Main.immediate) { + applicationScope.launch { if (Application.getBackend() !is WgQuickBackend) return@launch val action = intent.action ?: return@launch val tunnelManager = Application.getTunnelManager() diff --git a/ui/src/main/java/com/wireguard/android/QuickTileService.kt b/ui/src/main/java/com/wireguard/android/QuickTileService.kt index 5989499f..cb740d18 100644 --- a/ui/src/main/java/com/wireguard/android/QuickTileService.kt +++ b/ui/src/main/java/com/wireguard/android/QuickTileService.kt @@ -20,9 +20,8 @@ import com.wireguard.android.activity.MainActivity import com.wireguard.android.activity.TunnelToggleActivity import com.wireguard.android.backend.Tunnel import com.wireguard.android.model.ObservableTunnel +import com.wireguard.android.util.applicationScope import com.wireguard.android.widget.SlashDrawable -import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.GlobalScope import kotlinx.coroutines.launch /** @@ -57,7 +56,7 @@ class QuickTileService : TileService() { tile.icon = if (tile.icon == iconOn) iconOff else iconOn tile.updateTile() } - GlobalScope.launch(Dispatchers.Main.immediate) { + applicationScope.launch { try { tunnel!!.setStateAsync(Tunnel.State.TOGGLE) updateTile() diff --git a/ui/src/main/java/com/wireguard/android/model/ObservableTunnel.kt b/ui/src/main/java/com/wireguard/android/model/ObservableTunnel.kt index 89ad9a67..8bf8bf76 100644 --- a/ui/src/main/java/com/wireguard/android/model/ObservableTunnel.kt +++ b/ui/src/main/java/com/wireguard/android/model/ObservableTunnel.kt @@ -11,9 +11,9 @@ import com.wireguard.android.BR import com.wireguard.android.backend.Statistics import com.wireguard.android.backend.Tunnel import com.wireguard.android.databinding.Keyed +import com.wireguard.android.util.applicationScope import com.wireguard.config.Config import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.GlobalScope import kotlinx.coroutines.launch import kotlinx.coroutines.withContext @@ -74,7 +74,7 @@ class ObservableTunnel internal constructor( get() { if (field == null) // Opportunistically fetch this if we don't have a cached one, and rely on data bindings to update it eventually - GlobalScope.launch(Dispatchers.Main.immediate) { + applicationScope.launch { try { manager.getTunnelConfig(this@ObservableTunnel) } catch (e: Throwable) { @@ -110,7 +110,7 @@ class ObservableTunnel internal constructor( get() { if (field == null || field?.isStale != false) // Opportunistically fetch this if we don't have a cached one, and rely on data bindings to update it eventually - GlobalScope.launch(Dispatchers.Main.immediate) { + applicationScope.launch { try { manager.getTunnelStatistics(this@ObservableTunnel) } catch (e: Throwable) { diff --git a/ui/src/main/java/com/wireguard/android/model/TunnelManager.kt b/ui/src/main/java/com/wireguard/android/model/TunnelManager.kt index 1f9bc6c0..f4a0f1b4 100644 --- a/ui/src/main/java/com/wireguard/android/model/TunnelManager.kt +++ b/ui/src/main/java/com/wireguard/android/model/TunnelManager.kt @@ -22,10 +22,10 @@ import com.wireguard.android.backend.Statistics import com.wireguard.android.backend.Tunnel import com.wireguard.android.configStore.ConfigStore import com.wireguard.android.databinding.ObservableSortedKeyedArrayList +import com.wireguard.android.util.applicationScope import com.wireguard.config.Config import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.GlobalScope import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.async import kotlinx.coroutines.awaitAll @@ -101,7 +101,7 @@ class TunnelManager(private val configStore: ConfigStore) : BaseObservable() { } fun onCreate() { - GlobalScope.launch(Dispatchers.Main.immediate) { + applicationScope.launch { try { onTunnelsLoaded(withContext(Dispatchers.IO) { configStore.enumerate() }, withContext(Dispatchers.IO) { getBackend().runningTunnelNames }) } catch (e: Throwable) { @@ -122,7 +122,7 @@ class TunnelManager(private val configStore: ConfigStore) : BaseObservable() { } private fun refreshTunnelStates() { - GlobalScope.launch(Dispatchers.Main.immediate) { + applicationScope.launch { try { val running = withContext(Dispatchers.IO) { getBackend().runningTunnelNames } for (tunnel in tunnelMap) @@ -139,7 +139,7 @@ class TunnelManager(private val configStore: ConfigStore) : BaseObservable() { val previouslyRunning = getSharedPreferences().getStringSet(KEY_RUNNING_TUNNELS, null) ?: return if (previouslyRunning.isEmpty()) return - GlobalScope.launch(Dispatchers.Main.immediate) { + applicationScope.launch { withContext(Dispatchers.IO) { try { tunnelMap.filter { previouslyRunning.contains(it.name) }.map { async(SupervisorJob()) { setTunnelState(it, Tunnel.State.UP) } }.awaitAll() @@ -232,7 +232,7 @@ class TunnelManager(private val configStore: ConfigStore) : BaseObservable() { else -> return } val tunnelName = intent.getStringExtra("tunnel") ?: return - GlobalScope.launch(Dispatchers.Main.immediate) { + applicationScope.launch { val tunnels = manager.getTunnels() val tunnel = tunnels[tunnelName] ?: return@launch manager.setTunnelState(tunnel, state) diff --git a/ui/src/main/java/com/wireguard/android/util/Extensions.kt b/ui/src/main/java/com/wireguard/android/util/Extensions.kt index c8892295..22ef8bd7 100644 --- a/ui/src/main/java/com/wireguard/android/util/Extensions.kt +++ b/ui/src/main/java/com/wireguard/android/util/Extensions.kt @@ -11,6 +11,7 @@ import androidx.annotation.AttrRes import androidx.fragment.app.Fragment import androidx.lifecycle.lifecycleScope import androidx.preference.Preference +import com.wireguard.android.Application import com.wireguard.android.activity.SettingsActivity import kotlinx.coroutines.CoroutineScope @@ -24,6 +25,9 @@ fun Fragment.requireTargetFragment(): Fragment { return requireNotNull(targetFragment) { "A target fragment should always be set for $this" } } +val Any.applicationScope: CoroutineScope + get() = Application.getCoroutineScope() + val Preference.activity: SettingsActivity get() = context as? SettingsActivity ?: throw IllegalStateException("Failed to resolve SettingsActivity") -- cgit v1.2.3