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 <me@msfjarvis.dev>
This commit is contained in:
Harsh Shandilya 2020-09-16 15:30:15 +05:30 committed by Jason A. Donenfeld
parent 53adb0e9a6
commit 2f088938c6
6 changed files with 25 additions and 16 deletions

View File

@ -24,14 +24,17 @@ import com.wireguard.android.util.ModuleLoader
import com.wireguard.android.util.RootShell import com.wireguard.android.util.RootShell
import com.wireguard.android.util.ToolsInstaller import com.wireguard.android.util.ToolsInstaller
import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CompletableDeferred
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.GlobalScope import kotlinx.coroutines.Job
import kotlinx.coroutines.cancel
import kotlinx.coroutines.launch import kotlinx.coroutines.launch
import java.lang.ref.WeakReference import java.lang.ref.WeakReference
import java.util.Locale import java.util.Locale
class Application : android.app.Application(), OnSharedPreferenceChangeListener { class Application : android.app.Application(), OnSharedPreferenceChangeListener {
private val futureBackend = CompletableDeferred<Backend>() private val futureBackend = CompletableDeferred<Backend>()
private val coroutineScope = CoroutineScope(Job() + Dispatchers.Main.immediate)
private var backend: Backend? = null private var backend: Backend? = null
private lateinit var moduleLoader: ModuleLoader private lateinit var moduleLoader: ModuleLoader
private lateinit var rootShell: RootShell private lateinit var rootShell: RootShell
@ -98,7 +101,7 @@ class Application : android.app.Application(), OnSharedPreferenceChangeListener
} }
tunnelManager = TunnelManager(FileConfigStore(applicationContext)) tunnelManager = TunnelManager(FileConfigStore(applicationContext))
tunnelManager.onCreate() tunnelManager.onCreate()
GlobalScope.launch(Dispatchers.IO) { coroutineScope.launch(Dispatchers.IO) {
try { try {
backend = determineBackend() backend = determineBackend()
futureBackend.complete(backend!!) futureBackend.complete(backend!!)
@ -116,6 +119,7 @@ class Application : android.app.Application(), OnSharedPreferenceChangeListener
override fun onTerminate() { override fun onTerminate() {
sharedPreferences.unregisterOnSharedPreferenceChangeListener(this) sharedPreferences.unregisterOnSharedPreferenceChangeListener(this)
coroutineScope.cancel()
super.onTerminate() super.onTerminate()
} }
@ -146,6 +150,9 @@ class Application : android.app.Application(), OnSharedPreferenceChangeListener
@JvmStatic @JvmStatic
fun getTunnelManager() = get().tunnelManager fun getTunnelManager() = get().tunnelManager
@JvmStatic
fun getCoroutineScope() = get().coroutineScope
} }
init { init {

View File

@ -9,13 +9,12 @@ import android.content.Context
import android.content.Intent import android.content.Intent
import android.util.Log import android.util.Log
import com.wireguard.android.backend.WgQuickBackend import com.wireguard.android.backend.WgQuickBackend
import kotlinx.coroutines.Dispatchers import com.wireguard.android.util.applicationScope
import kotlinx.coroutines.GlobalScope
import kotlinx.coroutines.launch import kotlinx.coroutines.launch
class BootShutdownReceiver : BroadcastReceiver() { class BootShutdownReceiver : BroadcastReceiver() {
override fun onReceive(context: Context, intent: Intent) { override fun onReceive(context: Context, intent: Intent) {
GlobalScope.launch(Dispatchers.Main.immediate) { applicationScope.launch {
if (Application.getBackend() !is WgQuickBackend) return@launch if (Application.getBackend() !is WgQuickBackend) return@launch
val action = intent.action ?: return@launch val action = intent.action ?: return@launch
val tunnelManager = Application.getTunnelManager() val tunnelManager = Application.getTunnelManager()

View File

@ -20,9 +20,8 @@ import com.wireguard.android.activity.MainActivity
import com.wireguard.android.activity.TunnelToggleActivity import com.wireguard.android.activity.TunnelToggleActivity
import com.wireguard.android.backend.Tunnel import com.wireguard.android.backend.Tunnel
import com.wireguard.android.model.ObservableTunnel import com.wireguard.android.model.ObservableTunnel
import com.wireguard.android.util.applicationScope
import com.wireguard.android.widget.SlashDrawable import com.wireguard.android.widget.SlashDrawable
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.GlobalScope
import kotlinx.coroutines.launch import kotlinx.coroutines.launch
/** /**
@ -57,7 +56,7 @@ class QuickTileService : TileService() {
tile.icon = if (tile.icon == iconOn) iconOff else iconOn tile.icon = if (tile.icon == iconOn) iconOff else iconOn
tile.updateTile() tile.updateTile()
} }
GlobalScope.launch(Dispatchers.Main.immediate) { applicationScope.launch {
try { try {
tunnel!!.setStateAsync(Tunnel.State.TOGGLE) tunnel!!.setStateAsync(Tunnel.State.TOGGLE)
updateTile() updateTile()

View File

@ -11,9 +11,9 @@ import com.wireguard.android.BR
import com.wireguard.android.backend.Statistics import com.wireguard.android.backend.Statistics
import com.wireguard.android.backend.Tunnel import com.wireguard.android.backend.Tunnel
import com.wireguard.android.databinding.Keyed import com.wireguard.android.databinding.Keyed
import com.wireguard.android.util.applicationScope
import com.wireguard.config.Config import com.wireguard.config.Config
import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.GlobalScope
import kotlinx.coroutines.launch import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext import kotlinx.coroutines.withContext
@ -74,7 +74,7 @@ class ObservableTunnel internal constructor(
get() { get() {
if (field == null) if (field == null)
// Opportunistically fetch this if we don't have a cached one, and rely on data bindings to update it eventually // 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 { try {
manager.getTunnelConfig(this@ObservableTunnel) manager.getTunnelConfig(this@ObservableTunnel)
} catch (e: Throwable) { } catch (e: Throwable) {
@ -110,7 +110,7 @@ class ObservableTunnel internal constructor(
get() { get() {
if (field == null || field?.isStale != false) 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 // 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 { try {
manager.getTunnelStatistics(this@ObservableTunnel) manager.getTunnelStatistics(this@ObservableTunnel)
} catch (e: Throwable) { } catch (e: Throwable) {

View File

@ -22,10 +22,10 @@ import com.wireguard.android.backend.Statistics
import com.wireguard.android.backend.Tunnel import com.wireguard.android.backend.Tunnel
import com.wireguard.android.configStore.ConfigStore import com.wireguard.android.configStore.ConfigStore
import com.wireguard.android.databinding.ObservableSortedKeyedArrayList import com.wireguard.android.databinding.ObservableSortedKeyedArrayList
import com.wireguard.android.util.applicationScope
import com.wireguard.config.Config import com.wireguard.config.Config
import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CompletableDeferred
import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.GlobalScope
import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.SupervisorJob
import kotlinx.coroutines.async import kotlinx.coroutines.async
import kotlinx.coroutines.awaitAll import kotlinx.coroutines.awaitAll
@ -101,7 +101,7 @@ class TunnelManager(private val configStore: ConfigStore) : BaseObservable() {
} }
fun onCreate() { fun onCreate() {
GlobalScope.launch(Dispatchers.Main.immediate) { applicationScope.launch {
try { try {
onTunnelsLoaded(withContext(Dispatchers.IO) { configStore.enumerate() }, withContext(Dispatchers.IO) { getBackend().runningTunnelNames }) onTunnelsLoaded(withContext(Dispatchers.IO) { configStore.enumerate() }, withContext(Dispatchers.IO) { getBackend().runningTunnelNames })
} catch (e: Throwable) { } catch (e: Throwable) {
@ -122,7 +122,7 @@ class TunnelManager(private val configStore: ConfigStore) : BaseObservable() {
} }
private fun refreshTunnelStates() { private fun refreshTunnelStates() {
GlobalScope.launch(Dispatchers.Main.immediate) { applicationScope.launch {
try { try {
val running = withContext(Dispatchers.IO) { getBackend().runningTunnelNames } val running = withContext(Dispatchers.IO) { getBackend().runningTunnelNames }
for (tunnel in tunnelMap) for (tunnel in tunnelMap)
@ -139,7 +139,7 @@ class TunnelManager(private val configStore: ConfigStore) : BaseObservable() {
val previouslyRunning = getSharedPreferences().getStringSet(KEY_RUNNING_TUNNELS, null) val previouslyRunning = getSharedPreferences().getStringSet(KEY_RUNNING_TUNNELS, null)
?: return ?: return
if (previouslyRunning.isEmpty()) return if (previouslyRunning.isEmpty()) return
GlobalScope.launch(Dispatchers.Main.immediate) { applicationScope.launch {
withContext(Dispatchers.IO) { withContext(Dispatchers.IO) {
try { try {
tunnelMap.filter { previouslyRunning.contains(it.name) }.map { async(SupervisorJob()) { setTunnelState(it, Tunnel.State.UP) } }.awaitAll() 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 else -> return
} }
val tunnelName = intent.getStringExtra("tunnel") ?: return val tunnelName = intent.getStringExtra("tunnel") ?: return
GlobalScope.launch(Dispatchers.Main.immediate) { applicationScope.launch {
val tunnels = manager.getTunnels() val tunnels = manager.getTunnels()
val tunnel = tunnels[tunnelName] ?: return@launch val tunnel = tunnels[tunnelName] ?: return@launch
manager.setTunnelState(tunnel, state) manager.setTunnelState(tunnel, state)

View File

@ -11,6 +11,7 @@ import androidx.annotation.AttrRes
import androidx.fragment.app.Fragment import androidx.fragment.app.Fragment
import androidx.lifecycle.lifecycleScope import androidx.lifecycle.lifecycleScope
import androidx.preference.Preference import androidx.preference.Preference
import com.wireguard.android.Application
import com.wireguard.android.activity.SettingsActivity import com.wireguard.android.activity.SettingsActivity
import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.CoroutineScope
@ -24,6 +25,9 @@ fun Fragment.requireTargetFragment(): Fragment {
return requireNotNull(targetFragment) { "A target fragment should always be set for $this" } return requireNotNull(targetFragment) { "A target fragment should always be set for $this" }
} }
val Any.applicationScope: CoroutineScope
get() = Application.getCoroutineScope()
val Preference.activity: SettingsActivity val Preference.activity: SettingsActivity
get() = context as? SettingsActivity ?: throw IllegalStateException("Failed to resolve SettingsActivity") get() = context as? SettingsActivity ?: throw IllegalStateException("Failed to resolve SettingsActivity")