duckduckgo/Android

[Bug] runBlocking calls from inside coroutine

bbrockbernd opened this issue · 2 comments

Describe the bug

There are two instances where a runBlocking builder is called from within a coroutine. Although there are rare occasions where blocking a coroutine is desired, in general this should be avoided. Since it can hurt performance and in bad cases result in deadlocks.

Instance 1
Function didJoinWaitList containing a runBlocking call:

private fun didJoinWaitlist(): Boolean {
return runBlocking { netPWaitlistRepository.getWaitlistToken() != null }
}

Is called from isTreated, which is a suspend function:

private suspend fun isTreated(): Boolean {
if (appBuildConfig.isInternalBuild()) {
// internal users are always treated
return true
}
// User is in beta already
if (didJoinBeta() || didJoinWaitlist()) {
return netPRemoteFeature.isWaitlistActive()

Fix 1
The didJoinWaitList can be turned into a suspend function since it is only called from other suspend functions, which in turn allows the runBlocking builder to be removed.

Instance 2
runBlocking call from within a flow operation (Flow.combine). The passed lambda is a suspend function.

.combine(appTrackerRepository.getManualAppExclusionListFlow()) { ddgExclusionList, manualList ->
logcat { "getProtectedApps flow" }
installedApps
.map {
val isExcluded = isExcludedFromAppTP(it, ddgExclusionList, manualList)
runBlocking {
TrackingProtectionAppInfo(
packageName = it.packageName,
name = packageManager.getApplicationLabel(it).toString(),
category = it.parseAppCategory(),
isExcluded = networkProtectionExclusionList.isExcluded(it.packageName) || isExcluded,
knownProblem = hasKnownIssue(it, ddgExclusionList),
userModified = isUserModified(it.packageName, manualList),
)
}
}

Fix 2
InstalledApps can be handled as a flow with Sequence.asFlow() allowing runBlocking to be removed.

How to Reproduce

Goto:
https://github.com/duckduckgo/Android/blob/dd6fbf0df4911ba085d2c37c05f861fb1e2958f3/network-protection/network-protection-impl/src/main/java/com/duckduckgo/networkprotection/impl/waitlist/NetworkProtectionWaitlistImpl.kt
and
https://github.com/duckduckgo/Android/blob/4d756eb80aa88e334db91951156783a0b0562383/app-tracking-protection/vpn-impl/src/main/java/com/duckduckgo/mobile/android/vpn/apps/TrackingProtectionAppsRepository.kt

Expected behavior

Unblocked coroutines ;)

Environment

- DDG App Version:
- Device:
- OS:

Thank you for opening an Issue in our Repository.
The issue has been forwarded to the team and we'll follow up as soon as we have time to investigate.
As stated in our Contribution Guidelines, requests for feedback should be addressed via the Feedback section in the Android app.

Thanks, we are actually removing some of this code soon and will be cleaning this out.