Remove custom listener when activity stops

Issue

I am building a Custom View in a library module that every 30 seconds displays a question (obtained from a REST Api) and allows the user to select one of the possible answers.

Also, I need to make use of that library in an app, displaying a video underneath the question.

All the business and UI logic should be handled in the library.

I created an ErrorListener interface to be able to show the error messages from the library in the app's Activity.

Do I need to have a method in my Custom view to remove the listener if the activity is paused or stopped? Is there anything else I should have into account for when the activity is paused/stopped or destroyed? (like stopping the intervalsHandler, countDownTimer, etc.)

My Custom View

class BuffView @JvmOverloads constructor(context: Context, attrs: AttributeSet? = null)
    : LinearLayout(context, attrs) {

    private val apiErrorHandler = ApiErrorHandler()
    private val getBuffUseCase = GetBuffUseCase(apiErrorHandler)

    private val intervalsHandler = Handler()

    private val buffView: LinearLayout = inflate(context, R.layout.buff_view, this) as LinearLayout

    private var errorListener: ErrorListener? = null

    private var countDownTimer: CountDownTimer? = null

    private var buffIdCount = 1
    private var getBuffs = false
    
    fun init() {
        getBuffs = true
        getBuff()
    }

    private fun getBuff() {
        if (!getBuffs) return
        getBuffUseCase.invoke(Params(buffIdCount.toLong()), object : UseCaseResponse<Buff> {
            override fun onSuccess(result: Buff) {
                if (isDataValid(result)) displayBuff(result) else hideBuff()
            }

            override fun onError(errorModel: ErrorModel?) {
                errorListener?.onError(errorModel?.message ?: context.getString(R.string.generic_error_message))
                hideBuff()
            }
        })

        if (buffIdCount < TOTAL_BUFFS ) {
            intervalsHandler.postDelayed({
                buffIdCount++
                getBuff()
                stopCountDownTimer()
            }, REQUEST_BUFF_INTERVAL_TIME)
        }
    }

    private fun isDataValid(buff: Buff): Boolean {
        if (buff.author.firstName.isEmpty() && buff.author.lastName.isEmpty()) {
            showErrorInvalidData(context.getString(R.string.author_reason_error_message))
            return false
        }

        if (buff.question.title.isEmpty()) {
            showErrorInvalidData(context.getString(R.string.question_reason_error_message))
            return false
        }

        if (buff.timeToShow == null || buff.timeToShow < 0) {
            showErrorInvalidData(context.getString(R.string.timer_reason_error_message))
            return false
        }

        if (buff.answers == null) {
            showErrorInvalidData(context.getString(R.string.answers_reason_error_message))
            return false
        }
        return true
    }

    private fun showErrorInvalidData(reason: String) {
        errorListener?.onError(context.getString(R.string.data_incomplete_error_message, reason))
    }

    private fun displayBuff(buff: Buff) {
        setQuestion(buff.question.title)
        setAuthor(buff.author)
        setAnswer(buff.answers!!)
        setProgressBar(buff.timeToShow!!)
        setCloseButton()
        invalidate()
        showBuff()
    }

    private fun setQuestion(questionText: String) {
        question_text.text = questionText
    }

    private fun setAuthor(author: Buff.Author) {
        val firstName = author.firstName
        val lastName = author.lastName
        sender_name.text = "$firstName $lastName"

        Glide.with(context)
            .load(author.image)
            .into(sender_image)
    }

    private fun setAnswer(answers: List<Buff.Answer>) {
        val answersContainer = findViewById<LinearLayout>(R.id.answersContainer)
        answersContainer.removeAllViews()
        for(answer in answers) {
            val answerView: View = LayoutInflater.from(answersContainer.context).inflate(
                R.layout.buff_answer,
                answersContainer,
                false
            )

            answer.answerImage?.x0?.url?.let {
                Glide.with(context)
                    .load(it)
                    .into(answerView.answer_image)
            }

            answerView.setOnClickListener {
                answerView.background = ContextCompat.getDrawable(
                    context,
                    R.drawable.answer_selected_bg
                )
                answerView.answer_text.setTextColor(
                    ContextCompat.getColor(
                        context,
                        android.R.color.white
                    )
                )
                //freeze timer
                stopCountDownTimer()

                //hideView() after 2 seconds
                it.postDelayed({
                    hideBuff()
                }, HIDE_BUFF_AFTER_SELECTED_ANSWER_DURATION)
            }

            answerView.answer_text?.text = answer.title
            answersContainer.addView(answerView)
        }
    }

    private fun setProgressBar(timeToShow: Int) {
        question_time_progress.max = timeToShow
        countDownTimer = object : CountDownTimer(
            timeToShow * ONE_SECOND_INTERVAL,
            ONE_SECOND_INTERVAL
        ) {
            override fun onTick(millisUntilFinished: Long) {
                question_time.text = (millisUntilFinished / ONE_SECOND_INTERVAL).toString()
                question_time_progress.progress = timeToShow - (millisUntilFinished / ONE_SECOND_INTERVAL).toInt()
            }

            override fun onFinish() {
               hideBuff()
            }
        }.start()
    }

    private fun showBuff() {
        buffView.visibility = VISIBLE
    }

    private fun hideBuff() {
        buffView.visibility = GONE
    }

    private fun stopCountDownTimer() {
        countDownTimer?.cancel()
    }

    private fun setCloseButton() {
        buff_close.setOnClickListener {
            hideBuff()
            stopCountDownTimer()
        }
    }

     fun addErrorListener(errorListener: ErrorListener) {
        this.errorListener = errorListener
    }
}

MainActivity

class MainActivity : AppCompatActivity() {

    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        setContentView(R.layout.activity_main)
        supportActionBar?.setDisplayHomeAsUpEnabled(true)

        video.setVideoPath(url);
        video.setOnPreparedListener {
            video.start()
        }

        buff_view.addErrorListener(object : ErrorListener {
            override fun onError(msg: String) {
                Toast.makeText(this@MainActivity, msg, Toast.LENGTH_LONG).show()
            }

        })
        buff_view.init()
    }
}

activity_main.xml

<FrameLayout
    xmlns:android="http://schemas.android.com/apk/res/android"
    xmlns:tools="http://schemas.android.com/tools"
    android:layout_width="match_parent"
    android:layout_height="match_parent"
    android:background="@android:color/black"
    tools:context=".ui.MainActivity">

    <VideoView
        android:id="@+id/video"
        android:layout_width="match_parent"
        android:layout_height="match_parent"
        android:gravity="center"
        android:keepScreenOn="true"
        android:textSize="50sp"
        android:textStyle="bold" />

    <com.buffup.sdk.ui.BuffView
        android:id="@+id/buff_view"
        android:layout_gravity="bottom|start"
        android:padding="16dp"
        android:visibility="gone"
        android:layout_width="wrap_content"
        android:layout_height="wrap_content"/>

</FrameLayout>

Solution

Activities are very bulky data structures, so you really want to make sure the system is able to garbage collect them when they're no longer needed. So it's definitely a good idea to make sure that when the activity goes to the background (or gets destroyed etc) that nothing that's still alive is holding a reference to it.

So the first thing is your error listener

buff_view.addErrorListener(object : ErrorListener {
        override fun onError(msg: String) {
            Toast.makeText(this@MainActivity, msg, Toast.LENGTH_LONG).show()
        }
 })

that object that BuffView is holding onto, has a reference to the Activity that it needs to use in the Toast call. So as long as that callback object exists, the activity will be held in memory. And that callback object exists for as long as buff_view is holding onto it.

So you can clear the listener, so buff_view isn't holding onto it anymore and it can be GCed. But what's holding onto buff_view? What's keeping that in memory? If it's a view created by the activity in the first place, for its layout, then it will be destroyed when the activity is destroyed - there's nothing linking it to the rest of the app process.

But if buff_view is longer-lived (say it's a component held in a ViewModel or something, meant to outlive any activity that's displaying it) then you definitely need to make sure it doesn't hold onto a destroyed activity.

---

That's a general overview so you have an idea of the kinds of things you're looking for. If your implementation is all internal, and you're able to reason about what's holding onto what and what their lifespans are, it's fine to just let things hold a reference if you know they'll be cleared out anyway.

If you need to manage your references, you have two choices really

• have explicit register/unregister methods, and make sure you call them appropriately (e.g. when the activity is going into the background)
• use a WeakReference that doesn't keep the listener object in memory (once the app drops its reference to the activity, it's free to be cleaned up, and your WeakReference will become empty

(Unregistering is easier - it's more work, but it forces you to reason about your app and how it all fits together, and there's less chance of weird behaviour when you're explicitly managing things)

If you're creating a library, and you're accepting external listeners (i.e. it's not just your own internal state management, which you can be sure about) then you'll need to decide which of these approaches to take. In some ways that's simpler - every listener you take in, provide an unregister function for it, or make it a weak reference. Assume the worst, and either keep weak references or make it the user's responsibility to clean up. Whichever behaviour you choose though, document it so the user knows whether they need to clean up or if it's safe to just ignore it

---

I don't know what your CountdownTimer is doing, but the general rule is that when your app is in the background, it's "paused" (unless it's, like, a music player or something obviously!) so it should stop doing things. That means pausing worker threads, cancelling events posted on the handler. You don't want a toast popping up 30 seconds later while someone's browsing their Instagram feed, you know? So you need to handle pausing and resuming things - what exactly that involves depends on your app and what it's doing

---

While I'm doing a megapost, if you haven't already, it's a good idea to learn how to use the memory profiler. Specifically the Heap Dump, which shows you what's in memory.

You can read the link to see the details, but basically, you do something that creates a few objects (like rotating your screen so activities get destroyed and new ones get created), hit the GC button to garbage collect any loose objects, and then do a Heap Dump.

Sort stuff by package, find your app's package, then poke around your classes and see how many of each object are in memory. Like if you're only supposed to have 1 of CoolActivity in memory, and you have 2 (or more!) that's a sign that something is holding onto the old one. And you can look to see exactly what is doing that. It's a good thing to check now and then!

Answered By - cactustictacs