Fix remaining memory leaks on Android

android/src/toga_android/widgets/base.py

@@ -1,3 +1,4 @@
+import contextlib
 from abc import ABC, abstractmethod
 from decimal import ROUND_HALF_EVEN, Decimal
 
@@ -194,6 +195,21 @@ def android_text_align(value):
     }[value]
 
 
+# In implementing certain widgets, weak back-references
+# are needed from the native widget back to the implementation
+# object, however, this can sometimes lead to functions
+# being called on the native object even after the implementation
+# has been garbage collected.  Since there is no implementation
+# to emit signals for, such ReferenceErrors we get are often
+# useless, so we suppress them.
+@contextlib.contextmanager
+def suppress_reference_error(return_value=None):
+    try:
+        yield
+    except ReferenceError:  # pragma: no cover
+        pass
+
+
 # The look and feel of Android widgets is sometimes implemented using background
 # Drawables like ColorDrawable, InsetDrawable and other animation effect Drawables
 # like RippleDrawable. Often when such effect Drawables are used, they are stacked

android/src/toga_android/widgets/canvas.py

@@ -1,4 +1,5 @@
 import itertools
+import weakref
 from math import degrees
 
 from android.graphics import (
@@ -18,36 +19,38 @@
 from toga.widgets.canvas import arc_to_bezier, sweepangle
 
 from ..colors import native_color
-from .base import Widget
+from .base import Widget, suppress_reference_error
 
 
 class DrawHandler(dynamic_proxy(IDrawHandler)):
     def __init__(self, impl):
         super().__init__()
-        self.impl = impl
-        self.interface = impl.interface
+        self.impl = weakref.proxy(impl)
+        self.interface = weakref.proxy(impl.interface)
 
     def handleDraw(self, canvas):
-        self.impl.reset_transform(canvas)
-        self.interface.context._draw(self.impl, path=Path(), canvas=canvas)
+        with suppress_reference_error():
+            self.impl.reset_transform(canvas)
+            self.interface.context._draw(self.impl, path=Path(), canvas=canvas)
 
 
 class TouchListener(dynamic_proxy(View.OnTouchListener)):
     def __init__(self, impl):
         super().__init__()
-        self.impl = impl
-        self.interface = impl.interface
+        self.impl = weakref.proxy(impl)
+        self.interface = weakref.proxy(impl.interface)
 
     def onTouch(self, canvas, event):
-        x, y = map(self.impl.scale_out, (event.getX(), event.getY()))
-        if (action := event.getAction()) == MotionEvent.ACTION_DOWN:
-            self.interface.on_press(x, y)
-        elif action == MotionEvent.ACTION_MOVE:
-            self.interface.on_drag(x, y)
-        elif action == MotionEvent.ACTION_UP:
-            self.interface.on_release(x, y)
-        else:  # pragma: no cover
-            return False
+        with suppress_reference_error():
+            x, y = map(self.impl.scale_out, (event.getX(), event.getY()))
+            if (action := event.getAction()) == MotionEvent.ACTION_DOWN:
+                self.interface.on_press(x, y)
+            elif action == MotionEvent.ACTION_MOVE:
+                self.interface.on_drag(x, y)
+            elif action == MotionEvent.ACTION_UP:
+                self.interface.on_release(x, y)
+            else:  # pragma: no cover
+                return False
         return True
 
 

android/src/toga_android/widgets/dateinput.py

@@ -1,3 +1,4 @@
+import weakref
 from datetime import date, datetime, time
 
 from android import R
@@ -6,6 +7,7 @@
 
 from toga_android.widgets.base import ContainedWidget
 
+from .base import suppress_reference_error
 from .internal.pickers import PickerBase
 
 
@@ -20,13 +22,14 @@ def native_date(py_date):
 class DatePickerListener(dynamic_proxy(DatePickerDialog.OnDateSetListener)):
     def __init__(self, impl):
         super().__init__()
-        self.impl = impl
+        self.impl = weakref.proxy(impl)
 
     def onDateSet(self, view, year, month_0, day):
-        # It should be impossible for the dialog to return an out-of-range value in
-        # normal use, but it can happen in the testbed, so go via the interface to clip
-        # the value.
-        self.impl.interface.value = date(year, month_0 + 1, day)
+        with suppress_reference_error():
+            # It should be impossible for the dialog to return an out-of-range value in
+            # normal use, but it can happen in the testbed, so go via the interface to
+            # clip the value.
+            self.impl.interface.value = date(year, month_0 + 1, day)
 
 
 class DateInput(PickerBase, ContainedWidget):

android/src/toga_android/widgets/detailedlist.py

@@ -1,3 +1,4 @@
+import weakref
 from dataclasses import dataclass
 
 from android import R
@@ -8,7 +9,7 @@
 from android.widget import ImageView, LinearLayout, RelativeLayout, ScrollView, TextView
 from java import dynamic_proxy
 
-from .base import Widget
+from .base import Widget, suppress_reference_error
 
 try:
     from androidx.swiperefreshlayout.widget import SwipeRefreshLayout
@@ -21,12 +22,13 @@
 class DetailedListOnClickListener(dynamic_proxy(View.OnClickListener)):
     def __init__(self, impl, row_number):
         super().__init__()
-        self.impl = impl
+        self.impl = weakref.proxy(impl)
         self.row_number = row_number
 
     def onClick(self, _view):
-        self.impl._set_selection(self.row_number)
-        self.impl.interface.on_select()
+        with suppress_reference_error():
+            self.impl._set_selection(self.row_number)
+            self.impl.interface.on_select()
 
 
 @dataclass
@@ -39,37 +41,38 @@ class Action:
 class DetailedListOnLongClickListener(dynamic_proxy(View.OnLongClickListener)):
     def __init__(self, impl, row_number):
         super().__init__()
-        self.impl = impl
-        self.interface = impl.interface
+        self.impl = weakref.proxy(impl)
+        self.interface = weakref.proxy(impl.interface)
         self.row_number = row_number
 
     def onLongClick(self, _view):
-        self.impl._set_selection(self.row_number)
-        self.impl.interface.on_select()
-
-        actions = [
-            action
-            for action in [
-                Action(
-                    self.interface._primary_action,
-                    self.interface.on_primary_action,
-                    self.impl._primary_action_enabled,
-                ),
-                Action(
-                    self.interface._secondary_action,
-                    self.interface.on_secondary_action,
-                    self.impl._secondary_action_enabled,
-                ),
+        with suppress_reference_error():
+            self.impl._set_selection(self.row_number)
+            self.impl.interface.on_select()
+
+            actions = [
+                action
+                for action in [
+                    Action(
+                        self.interface._primary_action,
+                        self.interface.on_primary_action,
+                        self.impl._primary_action_enabled,
+                    ),
+                    Action(
+                        self.interface._secondary_action,
+                        self.interface.on_secondary_action,
+                        self.impl._secondary_action_enabled,
+                    ),
+                ]
+                if action.enabled
             ]
-            if action.enabled
-        ]
 
-        if actions:
-            row = self.interface.data[self.row_number]
-            AlertDialog.Builder(self.impl._native_activity).setItems(
-                [action.name for action in actions],
-                DetailedListActionListener(actions, row),
-            ).show()
+            if actions:
+                row = self.interface.data[self.row_number]
+                AlertDialog.Builder(self.impl._native_activity).setItems(
+                    [action.name for action in actions],
+                    DetailedListActionListener(actions, row),
+                ).show()
 
         return True
 
@@ -78,21 +81,23 @@ class DetailedListActionListener(dynamic_proxy(DialogInterface.OnClickListener))
     def __init__(self, actions, row):
         super().__init__()
         self.actions = actions
-        self.row = row
+        self.row = weakref.proxy(row)
 
     def onClick(self, dialog, which):
-        self.actions[which].handler(row=self.row)
+        with suppress_reference_error():
+            self.actions[which].handler(row=self.row)
 
 
 if SwipeRefreshLayout is not None:  # pragma: no cover
 
     class OnRefreshListener(dynamic_proxy(SwipeRefreshLayout.OnRefreshListener)):
         def __init__(self, interface):
             super().__init__()
-            self._interface = interface
+            self._interface = weakref.proxy(interface)
 
         def onRefresh(self):
-            self._interface.on_refresh()
+            with suppress_reference_error():
+                self._interface.on_refresh()
 
 
 class DetailedList(Widget):

android/src/toga_android/widgets/internal/pickers.py

@@ -1,3 +1,4 @@
+import weakref
 from abc import ABC, abstractmethod
 from decimal import ROUND_UP
 
@@ -6,16 +7,18 @@
 from java import dynamic_proxy
 from travertino.size import at_least
 
+from ..base import suppress_reference_error
 from ..label import TextViewWidget
 
 
 class TogaPickerClickListener(dynamic_proxy(View.OnClickListener)):
     def __init__(self, impl):
         super().__init__()
-        self.impl = impl
+        self.impl = weakref.proxy(impl)
 
     def onClick(self, _):
-        self.impl._dialog.show()
+        with suppress_reference_error():
+            self.impl._dialog.show()
 
 
 class PickerBase(TextViewWidget, ABC):

android/src/toga_android/widgets/internal/webview.py

@@ -4,45 +4,36 @@
 from java import Override, jboolean, jvoid, static_proxy
 from java.lang import String as jstring
 
+from ..base import suppress_reference_error
+
 
 class TogaWebClient(static_proxy(WebViewClient)):
     def __init__(self, impl):
-        self._interface_ref = weakref.ref(impl.interface)
-        self._impl_ref = weakref.ref(impl)
+        self.impl = weakref.proxy(impl)
+        self.interface = weakref.proxy(impl.interface)
         super().__init__()
 
-    @property
-    def interface(self):
-        return self._interface_ref()
-
-    @property
-    def impl(self):
-        return self._impl_ref()
-
     @Override(jboolean, [A_WebView, WebResourceRequest])
     def shouldOverrideUrlLoading(self, webview, webresourcerequest):
         allow = True
-        if self.interface and self.interface.on_navigation_starting._raw:
-            url = webresourcerequest.getUrl().toString()
-            result = self.interface.on_navigation_starting(url=url)
-            if isinstance(result, bool):
-                # on_navigation_starting handler is synchronous
-                allow = result
-            else:
-                # on_navigation_starting handler is asynchronous. Deny navigation until
-                # the user defined on_navigation_starting coroutine has completed.
-                allow = False
+        with suppress_reference_error():
+            if self.interface.on_navigation_starting._raw:
+                url = webresourcerequest.getUrl().toString()
+                result = self.interface.on_navigation_starting(url=url)
+                if isinstance(result, bool):
+                    # on_navigation_starting handler is synchronous
+                    allow = result
+                else:
+                    # on_navigation_starting handler is asynchronous. Deny navigation
+                    # until the user defined on_navigation_starting coroutine has
+                    # completed.
+                    allow = False
         return not allow
 
     @Override(jvoid, [A_WebView, jstring])
     def onPageFinished(self, webview, url):
-        # It's possible for this handler to be invoked *after* the interface/impl object
-        # has been destroyed. If the interface/impl doesn't exist there's no handler to
-        # invoke either, so ignore the edge case. This can't be reproduced reliably, so
-        # don't check coverage on the `is None` case.
-        if self.interface:  # pragma: no-branch
+        with suppress_reference_error():
             self.interface.on_webview_load()
-
-        if self.impl and self.impl.loaded_future:  # pragma: no-branch
-            self.impl.loaded_future.set_result(None)
-            self.impl.loaded_future = None
+            if self.impl.loaded_future:
+                self.impl.loaded_future.set_result(None)
+                self.impl.loaded_future = None

android/src/toga_android/widgets/mapview.py

@@ -1,3 +1,4 @@
+import weakref
 from pathlib import Path
 
 from android.graphics import BitmapFactory
@@ -19,19 +20,21 @@
 import toga
 from toga.types import LatLng
 
-from .base import Widget
+from .base import Widget, suppress_reference_error
 
 if OSMMapView is not None:  # pragma: no branch
 
     class TogaOnMarkerClickListener(dynamic_proxy(Marker.OnMarkerClickListener)):
         def __init__(self, map_impl):
             super().__init__()
-            self.map_impl = map_impl
+            self.map_impl = weakref.proxy(map_impl)
 
         def onMarkerClick(self, marker, map_view):
-            result = marker.onMarkerClickDefault(marker, map_view)
-            pin = self.map_impl.pins[marker]
-            self.map_impl.interface.on_select(pin=pin)
+            result = False
+            with suppress_reference_error():
+                result = marker.onMarkerClickDefault(marker, map_view)
+                pin = self.map_impl.pins[marker]
+                self.map_impl.interface.on_select(pin=pin)
             return result