2002506 - Enforce structured concurrency by replacing fire-and-forget launch with withContext in suspend functions

Status: NEW

(Firefox for Android :: Accounts and Sync, task)

Description

[Mihai Adrian Carare [:mcarare]]

Across the codebase, there are several instances where suspend functions use CoroutineScope.launch (or similar async builders without waiting) to perform background operations.

This pattern is problematic because:

1.Violation of Suspend Contract: A suspend function implies that the caller must wait for its work to complete. Using launch returns control to the caller immediately, breaking this contract and creating a race condition where the caller assumes work is done when it is still running.

2.Silent Failures: Exceptions thrown inside a detached launch (especially on Dispatchers.IO) may not propagate to the caller or the UI, leading to silent failures or crashes in the uncaught exception handler rather than being catchable.

3.Resource Leaks: Fire-and-forget coroutines can outlive the lifecycle of the component that requested them if not carefully managed, whereas withContext naturally scopes the work to the caller's lifecycle.

Proposed Solution: Conduct a review of suspend functions in the codebase and refactor them to use withContext(Dispatcher) instead of scope.launch(Dispatcher). ( or scope.launch { } )

Example:

// BAD: Breaks suspend contract
suspend fun saveData() {
    // Starts a background job and returns immediately.
    // The caller thinks save is done, but it's still running!
    scope.launch(Dispatchers.IO) { 
        db.insert(data)
    }
}

should be changed to:

// GOOD: Honors suspend contract
suspend fun saveData() {
    // Suspends execution until the insert is ACTUALLY finished.
    withContext(Dispatchers.IO) {
        db.insert(data)
    }
}

Note that there are specific scenarios where using launch (or async) inside a suspend function is valid and necessary. For example, parallel decomposition (coroutineScope): If you need to run multiple independent tasks in parallel and wait for all of them to finish before returning, you use coroutineScope (or supervisorScope) combined with launch or async. For example:

suspend fun loadUserProfile(userId: String): UserProfile = coroutineScope {
    // 1. Start fetching stats (runs in parallel)
    val statsJob = launch { 
        // populates a cache or similar side-effect
        repo.refreshStats(userId) 
    }

    // 2. Start fetching friends (runs in parallel)
    val friendsJob = async { 
        repo.getFriends(userId) 
    }

    // 3. Fetch main user info (runs in parallel)
    val userInfo = repo.getUserInfo(userId)

    // The function SUSPENDS here at the end of the block
    // until statsJob is done AND friendsJob is done.
    
    UserProfile(userInfo, friendsJob.await())
}