Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit f3fb8934 authored by Fabian Kozynski's avatar Fabian Kozynski
Browse files

Change BroadcastDispatcher to 1receiver/action

With the aggregated filter, as it had to be registered when any single
action changed (added for first time or removed the last receiver for
that action), if there was any action that got periodically
registered/unregistered, it would cause frequent cycles of
unregister/register with Context. This leads to interruption of service.

With this change, changes in one action would not affect receivers for
the others.

Test: manual, SystemUI works as expected
Test: manual, dumps and logs
Test: atest UserBroadcastDispatcherTest ActionReceiverTest
Bug: 148390204
Bug: 157165818

Change-Id: I7961f6c60600dfa0407615319c43bb72e43b4795
parent 81fa33a7
Loading
Loading
Loading
Loading
+133 −0
Original line number Diff line number Diff line
/*
 * Copyright (C) 2020 The Android Open Source Project
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

package com.android.systemui.broadcast

import android.content.BroadcastReceiver
import android.content.Context
import android.content.Intent
import android.content.IntentFilter
import android.util.ArraySet
import com.android.systemui.Dumpable
import com.android.systemui.broadcast.logging.BroadcastDispatcherLogger
import com.android.systemui.util.indentIfPossible
import java.io.FileDescriptor
import java.io.PrintWriter
import java.util.concurrent.Executor
import java.util.concurrent.atomic.AtomicInteger

/**
 * Receiver for a given action-userId pair to be used by [UserBroadcastDispatcher].
 *
 * Each object of this class will take care of a single Action. It will register if it has at least
 * one [BroadcastReceiver] added to it, and unregister when none are left.
 *
 * It will also re-register if filters with new categories are added. But this should not happen
 * often.
 *
 * This class has no sync controls, so make sure to only make modifications from the background
 * thread.
 */
class ActionReceiver(
    private val action: String,
    private val userId: Int,
    private val registerAction: BroadcastReceiver.(IntentFilter) -> Unit,
    private val unregisterAction: BroadcastReceiver.() -> Unit,
    private val bgExecutor: Executor,
    private val logger: BroadcastDispatcherLogger
) : BroadcastReceiver(), Dumpable {

    companion object {
        val index = AtomicInteger(0)
    }

    var registered = false
        private set
    private val receiverDatas = ArraySet<ReceiverData>()
    private val activeCategories = ArraySet<String>()

    @Throws(IllegalArgumentException::class)
    fun addReceiverData(receiverData: ReceiverData) {
        if (!receiverData.filter.hasAction(action)) {
            throw(IllegalArgumentException("Trying to attach to $action without correct action," +
                "receiver: ${receiverData.receiver}"))
        }
        val addedCategories = activeCategories
                .addAll(receiverData.filter.categoriesIterator()?.asSequence() ?: emptySequence())

        if (receiverDatas.add(receiverData) && receiverDatas.size == 1) {
            registerAction(createFilter())
            registered = true
        } else if (addedCategories) {
            unregisterAction()
            registerAction(createFilter())
        }
    }

    fun hasReceiver(receiver: BroadcastReceiver): Boolean {
        return receiverDatas.any { it.receiver == receiver }
    }

    private fun createFilter(): IntentFilter {
        val filter = IntentFilter(action)
        activeCategories.forEach(filter::addCategory)
        return filter
    }

    fun removeReceiver(receiver: BroadcastReceiver) {
        if (receiverDatas.removeAll { it.receiver == receiver } &&
                receiverDatas.isEmpty() && registered) {
            unregisterAction()
            registered = false
            activeCategories.clear()
        }
    }

    @Throws(IllegalStateException::class)
    override fun onReceive(context: Context, intent: Intent) {
        if (intent.action != action) {
            throw(IllegalStateException("Received intent for ${intent.action} " +
                "in receiver for $action}"))
        }
        val id = index.getAndIncrement()
        logger.logBroadcastReceived(id, userId, intent)
        // Immediately return control to ActivityManager
        bgExecutor.execute {
            receiverDatas.forEach {
                if (it.filter.matchCategories(intent.categories) == null) {
                    it.executor.execute {
                        it.receiver.pendingResult = pendingResult
                        it.receiver.onReceive(context, intent)
                        logger.logBroadcastDispatched(id, action, it.receiver)
                    }
                }
            }
        }
    }

    override fun dump(fd: FileDescriptor, pw: PrintWriter, args: Array<out String>) {
        pw.indentIfPossible {
            println("Registered: $registered")
            println("Receivers:")
            pw.indentIfPossible {
                receiverDatas.forEach {
                    println(it.receiver)
                }
            }
            println("Categories: ${activeCategories.joinToString(", ")}")
        }
    }
}
 No newline at end of file
+9 −4
Original line number Diff line number Diff line
@@ -29,6 +29,7 @@ import android.os.UserHandle
import android.text.TextUtils
import android.util.SparseArray
import com.android.internal.annotations.VisibleForTesting
import com.android.internal.util.IndentingPrintWriter
import com.android.systemui.Dumpable
import com.android.systemui.broadcast.logging.BroadcastDispatcherLogger
import com.android.systemui.dump.DumpManager
@@ -65,6 +66,7 @@ private const val DEBUG = true
open class BroadcastDispatcher constructor (
    private val context: Context,
    private val bgLooper: Looper,
    private val bgExecutor: Executor,
    private val dumpManager: DumpManager,
    private val logger: BroadcastDispatcherLogger
) : Dumpable, BroadcastReceiver() {
@@ -173,15 +175,18 @@ open class BroadcastDispatcher constructor (

    @VisibleForTesting
    protected open fun createUBRForUser(userId: Int) =
            UserBroadcastDispatcher(context, userId, bgLooper, logger)
            UserBroadcastDispatcher(context, userId, bgLooper, bgExecutor, logger)

    override fun dump(fd: FileDescriptor, pw: PrintWriter, args: Array<out String>) {
        pw.println("Broadcast dispatcher:")
        pw.println("  Current user: ${handler.currentUser}")
        val ipw = IndentingPrintWriter(pw, "  ")
        ipw.increaseIndent()
        ipw.println("Current user: ${handler.currentUser}")
        for (index in 0 until receiversByUser.size()) {
            pw.println("  User ${receiversByUser.keyAt(index)}")
            receiversByUser.valueAt(index).dump(fd, pw, args)
            ipw.println("User ${receiversByUser.keyAt(index)}")
            receiversByUser.valueAt(index).dump(fd, ipw, args)
        }
        ipw.decreaseIndent()
    }

    private val handler = object : Handler(bgLooper) {
+48 −136
Original line number Diff line number Diff line
@@ -18,8 +18,6 @@ package com.android.systemui.broadcast

import android.content.BroadcastReceiver
import android.content.Context
import android.content.Intent
import android.content.IntentFilter
import android.os.Handler
import android.os.Looper
import android.os.Message
@@ -31,11 +29,10 @@ import androidx.annotation.VisibleForTesting
import com.android.internal.util.Preconditions
import com.android.systemui.Dumpable
import com.android.systemui.broadcast.logging.BroadcastDispatcherLogger
import com.android.systemui.util.indentIfPossible
import java.io.FileDescriptor
import java.io.PrintWriter
import java.lang.IllegalArgumentException
import java.lang.IllegalStateException
import java.util.concurrent.atomic.AtomicBoolean
import java.util.concurrent.Executor
import java.util.concurrent.atomic.AtomicInteger

private const val MSG_REGISTER_RECEIVER = 0
@@ -48,16 +45,14 @@ private const val DEBUG = false
 *
 * Created by [BroadcastDispatcher] as needed by users. The value of [userId] can be
 * [UserHandle.USER_ALL].
 *
 * Each instance of this class will register itself exactly once with [Context]. Updates to the
 * [IntentFilter] will be done in the background thread.
 */
class UserBroadcastDispatcher(
open class UserBroadcastDispatcher(
    private val context: Context,
    private val userId: Int,
    private val bgLooper: Looper,
    private val bgExecutor: Executor,
    private val logger: BroadcastDispatcherLogger
) : BroadcastReceiver(), Dumpable {
) : Dumpable {

    companion object {
        // Used only for debugging. If not debugging, this variable will not be accessed and all
@@ -76,47 +71,16 @@ class UserBroadcastDispatcher(
        }
    }

    private val registered = AtomicBoolean(false)

    internal fun isRegistered() = registered.get()

    // Only modify in BG thread
    private val actionsToReceivers = ArrayMap<String, MutableSet<ReceiverData>>()
    private val receiverToReceiverData = ArrayMap<BroadcastReceiver, MutableSet<ReceiverData>>()
    @VisibleForTesting
    internal val actionsToActionsReceivers = ArrayMap<String, ActionReceiver>()
    private val receiverToActions = ArrayMap<BroadcastReceiver, MutableSet<String>>()

    @VisibleForTesting
    internal fun isReceiverReferenceHeld(receiver: BroadcastReceiver): Boolean {
        return receiverToReceiverData.contains(receiver) ||
                actionsToReceivers.any {
            it.value.any { it.receiver == receiver }
        }
    }

    // Only call on BG thread as it reads from the maps
    private fun createFilter(): IntentFilter {
        Preconditions.checkState(bgHandler.looper.isCurrentThread,
                "This method should only be called from BG thread")
        val categories = mutableSetOf<String>()
        receiverToReceiverData.values.flatten().forEach {
            it.filter.categoriesIterator()?.asSequence()?.let {
                categories.addAll(it)
            }
        }
        val intentFilter = IntentFilter().apply {
            // The keys of the arrayMap are of type String! so null check is needed
            actionsToReceivers.keys.forEach { if (it != null) addAction(it) else Unit }
            categories.forEach { addCategory(it) }
        }
        return intentFilter
    }

    override fun onReceive(context: Context, intent: Intent) {
        val id = index.getAndIncrement()
        if (DEBUG) Log.w(TAG, "[$id] Received $intent")
        logger.logBroadcastReceived(id, userId, intent)
        bgHandler.post(
                HandleBroadcastRunnable(
                        actionsToReceivers, context, intent, pendingResult, id, logger))
        return actionsToActionsReceivers.values.any {
            it.hasReceiver(receiver)
        } || (receiver in receiverToActions)
    }

    /**
@@ -137,109 +101,57 @@ class UserBroadcastDispatcher(
        Preconditions.checkState(bgHandler.looper.isCurrentThread,
                "This method should only be called from BG thread")
        if (DEBUG) Log.w(TAG, "Register receiver: ${receiverData.receiver}")
        receiverToReceiverData.getOrPut(receiverData.receiver, { ArraySet() }).add(receiverData)
        var changed = false
        // Index the BroadcastReceiver by all its actions, that way it's easier to dispatch given
        // a received intent.
        receiverToActions
                .getOrPut(receiverData.receiver, { ArraySet() })
                .addAll(receiverData.filter.actionsIterator()?.asSequence() ?: emptySequence())
        receiverData.filter.actionsIterator().forEach {
            actionsToReceivers.getOrPut(it) {
                changed = true
                ArraySet()
            }.add(receiverData)
            actionsToActionsReceivers
                    .getOrPut(it, { createActionReceiver(it) })
                    .addReceiverData(receiverData)
        }
        logger.logReceiverRegistered(userId, receiverData.receiver)
        if (changed) {
            createFilterAndRegisterReceiverBG()
    }

    @VisibleForTesting
    internal open fun createActionReceiver(action: String): ActionReceiver {
        return ActionReceiver(
                action,
                userId,
                {
                    context.registerReceiverAsUser(this, UserHandle.of(userId), it, null, bgHandler)
                    logger.logContextReceiverRegistered(userId, it)
                },
                {
                    try {
                        context.unregisterReceiver(this)
                        logger.logContextReceiverUnregistered(userId, action)
                    } catch (e: IllegalArgumentException) {
                        Log.e(TAG, "Trying to unregister unregistered receiver for user $userId, " +
                                "action $action",
                                IllegalStateException(e))
                    }
                },
                bgExecutor,
                logger
        )
    }

    private fun handleUnregisterReceiver(receiver: BroadcastReceiver) {
        Preconditions.checkState(bgHandler.looper.isCurrentThread,
                "This method should only be called from BG thread")
        if (DEBUG) Log.w(TAG, "Unregister receiver: $receiver")
        val actions = receiverToReceiverData.getOrElse(receiver) { return }
                .flatMap { it.filter.actionsIterator().asSequence().asIterable() }.toSet()
        receiverToReceiverData.remove(receiver)?.clear()
        var changed = false
        actions.forEach { action ->
            actionsToReceivers.get(action)?.removeIf { it.receiver == receiver }
            if (actionsToReceivers.get(action)?.isEmpty() ?: false) {
                changed = true
                actionsToReceivers.remove(action)
            }
        receiverToActions.getOrDefault(receiver, mutableSetOf()).forEach {
            actionsToActionsReceivers.get(it)?.removeReceiver(receiver)
        }
        receiverToActions.remove(receiver)
        logger.logReceiverUnregistered(userId, receiver)
        if (changed) {
            createFilterAndRegisterReceiverBG()
        }
    }

    // Only call this from a BG thread
    private fun createFilterAndRegisterReceiverBG() {
        val intentFilter = createFilter()
        bgHandler.post(RegisterReceiverRunnable(intentFilter))
    }

    override fun dump(fd: FileDescriptor, pw: PrintWriter, args: Array<out String>) {
        pw.println("  Registered=${registered.get()}")
        actionsToReceivers.forEach { (action, list) ->
            pw.println("    $action:")
            list.forEach { pw.println("      ${it.receiver}") }
        }
    }

    private class HandleBroadcastRunnable(
        val actionsToReceivers: Map<String, Set<ReceiverData>>,
        val context: Context,
        val intent: Intent,
        val pendingResult: PendingResult,
        val index: Int,
        val logger: BroadcastDispatcherLogger
    ) : Runnable {
        override fun run() {
            if (DEBUG) Log.w(TAG, "[$index] Dispatching $intent")
            actionsToReceivers.get(intent.action)
                    ?.filter {
                        it.filter.hasAction(intent.action) &&
                            it.filter.matchCategories(intent.categories) == null }
                    ?.forEach {
                        it.executor.execute {
                            if (DEBUG) Log.w(TAG,
                                    "[$index] Dispatching ${intent.action} to ${it.receiver}")
                            logger.logBroadcastDispatched(index, intent.action, it.receiver)
                            it.receiver.pendingResult = pendingResult
                            it.receiver.onReceive(context, intent)
                        }
                    }
        }
    }

    private inner class RegisterReceiverRunnable(val intentFilter: IntentFilter) : Runnable {

        /*
         * Registers and unregisters the BroadcastReceiver
         */
        override fun run() {
            if (registered.get()) {
                try {
                    context.unregisterReceiver(this@UserBroadcastDispatcher)
                    logger.logContextReceiverUnregistered(userId)
                } catch (e: IllegalArgumentException) {
                    Log.e(TAG, "Trying to unregister unregistered receiver for user $userId",
                            IllegalStateException(e))
                }
                registered.set(false)
            }
            // Short interval without receiver, this can be problematic
            if (intentFilter.countActions() > 0 && !registered.get()) {
                context.registerReceiverAsUser(
                        this@UserBroadcastDispatcher,
                        UserHandle.of(userId),
                        intentFilter,
                        null,
                        bgHandler)
                registered.set(true)
                logger.logContextReceiverRegistered(userId, intentFilter)
        pw.indentIfPossible {
            actionsToActionsReceivers.forEach { (action, actionReceiver) ->
                println("$action:")
                actionReceiver.dump(fd, pw, args)
            }
        }
    }
+3 −2
Original line number Diff line number Diff line
@@ -99,11 +99,12 @@ class BroadcastDispatcherLogger @Inject constructor(
        })
    }

    fun logContextReceiverUnregistered(user: Int) {
    fun logContextReceiverUnregistered(user: Int, action: String) {
        log(INFO, {
            int1 = user
            str1 = action
        }, {
            "Receiver unregistered with Context for user $int1."
            "Receiver unregistered with Context for user $int1, action $str1"
        })
    }

+5 −2
Original line number Diff line number Diff line
@@ -61,6 +61,8 @@ import com.android.systemui.statusbar.policy.DataSaverController;
import com.android.systemui.statusbar.policy.NetworkController;
import com.android.systemui.util.leak.LeakDetector;

import java.util.concurrent.Executor;

import javax.inject.Named;
import javax.inject.Singleton;

@@ -188,11 +190,12 @@ public class DependencyProvider {
    public BroadcastDispatcher providesBroadcastDispatcher(
            Context context,
            @Background Looper backgroundLooper,
            @Background Executor backgroundExecutor,
            DumpManager dumpManager,
            BroadcastDispatcherLogger logger
    ) {
        BroadcastDispatcher bD =
                new BroadcastDispatcher(context, backgroundLooper, dumpManager, logger);
        BroadcastDispatcher bD = new BroadcastDispatcher(context, backgroundLooper,
                backgroundExecutor, dumpManager, logger);
        bD.initialize();
        return bD;
    }
Loading