From e6b66074b9a1b45362342500b8880c8795f269cd Mon Sep 17 00:00:00 2001 From: Eric Froemling Date: Mon, 2 Nov 2020 16:43:57 -0600 Subject: [PATCH] Workarounds for reference loops from Enum constructors --- assets/src/ba_data/python/ba/__init__.py | 3 +- assets/src/ba_data/python/ba/_general.py | 59 ++++++++++++++++++- assets/src/ba_data/python/ba/_ui.py | 9 +-- .../python/bastd/ui/gather/__init__.py | 21 ++----- .../python/bastd/ui/gather/manualtab.py | 6 -- .../python/bastd/ui/gather/publictab.py | 4 ++ assets/src/ba_data/python/bastd/ui/party.py | 2 +- .../ba_data/python/bastd/ui/store/browser.py | 12 ++-- assets/src/ba_data/python/bastd/ui/watch.py | 18 ++---- docs/ba_module.md | 21 ++++++- src/ballistica/ballistica.cc | 2 +- src/ballistica/networking/networking_sys.h | 1 + 12 files changed, 105 insertions(+), 53 deletions(-) diff --git a/assets/src/ba_data/python/ba/__init__.py b/assets/src/ba_data/python/ba/__init__.py index 8e3a6e9f..1cdaf5ff 100644 --- a/assets/src/ba_data/python/ba/__init__.py +++ b/assets/src/ba_data/python/ba/__init__.py @@ -60,7 +60,8 @@ from ba._campaign import Campaign from ba._gameutils import (GameTip, animate, animate_array, show_damage_count, timestring, cameraflash) from ba._general import (WeakCall, Call, existing, Existable, - verify_object_death, storagename, getclass) + verify_object_death, storagename, getclass, + enum_by_value) from ba._keyboard import Keyboard from ba._level import Level from ba._lobby import Lobby, Chooser diff --git a/assets/src/ba_data/python/ba/_general.py b/assets/src/ba_data/python/ba/_general.py index a4ad9fdb..01ecf267 100644 --- a/assets/src/ba_data/python/ba/_general.py +++ b/assets/src/ba_data/python/ba/_general.py @@ -8,6 +8,7 @@ import types import weakref import random import inspect +from enum import Enum from typing import TYPE_CHECKING, TypeVar, Protocol from efro.terminal import Clr @@ -16,6 +17,7 @@ from ba._error import print_error, print_exception from ba._enums import TimeType if TYPE_CHECKING: + from types import FrameType from typing import Any, Type, Optional from efro.call import Call as Call # 'as Call' so we re-export. from weakref import ReferenceType @@ -34,6 +36,7 @@ class Existable(Protocol): ExistableType = TypeVar('ExistableType', bound=Existable) T = TypeVar('T') +ET = TypeVar('ET', bound=Enum) def existing(obj: Optional[ExistableType]) -> Optional[ExistableType]: @@ -306,18 +309,38 @@ def print_active_refs(obj: Any) -> None: Useful for tracking down cyclical references and causes for zombie objects. """ - from types import FrameType + # pylint: disable=too-many-nested-blocks + from types import FrameType, TracebackType refs = list(gc.get_referrers(obj)) print(f'{Clr.YLW}Active referrers to {obj}:{Clr.RST}') for i, ref in enumerate(refs): print(f'{Clr.YLW}#{i+1}:{Clr.BLU} {ref}{Clr.RST}') + # For certain types of objects such as stack frames, show what is # keeping *them* alive too. if isinstance(ref, FrameType): print(f'{Clr.YLW} Active referrers to #{i+1}:{Clr.RST}') refs2 = list(gc.get_referrers(ref)) for j, ref2 in enumerate(refs2): - print(f'{Clr.YLW} #{j+1}:{Clr.BLU} {ref2}{Clr.RST}') + print(f'{Clr.YLW} #a{j+1}:{Clr.BLU} {ref2}{Clr.RST}') + + # Can go further down the rabbit-hole if needed... + if bool(False): + if isinstance(ref2, TracebackType): + print(f'{Clr.YLW} ' + f'Active referrers to #a{j+1}:{Clr.RST}') + refs3 = list(gc.get_referrers(ref2)) + for k, ref3 in enumerate(refs3): + print(f'{Clr.YLW} ' + f'#b{k+1}:{Clr.BLU} {ref3}{Clr.RST}') + + if isinstance(ref3, BaseException): + print(f'{Clr.YLW} Active referrers to' + f' #b{k+1}:{Clr.RST}') + refs4 = list(gc.get_referrers(ref3)) + for x, ref4 in enumerate(refs4): + print(f'{Clr.YLW} #c{x+1}:{Clr.BLU}' + f' {ref4}{Clr.RST}') def _verify_object_death(wref: ReferenceType) -> None: @@ -376,3 +399,35 @@ def storagename(suffix: str = None) -> str: if suffix is not None: fullpath = f'{fullpath}_{suffix}' return fullpath.replace('.', '_') + + +def _gut_exception(exc: Exception) -> None: + assert exc.__traceback__ is not None + frame: Optional[FrameType] = exc.__traceback__.tb_frame + while frame is not None: + frame.clear() + frame = frame.f_back + + +def enum_by_value(cls: Type[ET], value: Any) -> ET: + """Create an enum from a value. + + Category: General Utility Functions + + This is basically the same as doing 'obj = EnumType(value)' except + that it works around an issue where a reference loop is created + if an exception is thrown due to an invalid value. Since we disable + the cyclic garbage collector for most of the time, such loops can lead + to our objects sticking around longer than we want. This workaround is + not perfect in that the destruction happens in the next cycle, but it is + better than never. + This issue has been submitted to Python as a bug so hopefully we can + remove this eventually if it gets fixed: https://bugs.python.org/issue42248 + """ + try: + return cls(value) + except Exception as exc: + # Blow away all stack frames in the exception which will break the + # cycle and allow it to be destroyed. + _ba.pushcall(_Call(_gut_exception, exc)) + raise diff --git a/assets/src/ba_data/python/ba/_ui.py b/assets/src/ba_data/python/ba/_ui.py index 0756ad4f..2e3350a3 100644 --- a/assets/src/ba_data/python/ba/_ui.py +++ b/assets/src/ba_data/python/ba/_ui.py @@ -123,12 +123,13 @@ class UISubsystem: # With our legacy main-menu system, the caller is responsible for # clearing out the old main menu window when assigning the new. # However there are corner cases where that doesn't happen and we get - # old windows stuck under the new main one. So let's guard against that - # However, we can't simply delete the existing main window when + # old windows stuck under the new main one. So let's guard against + # that. However, we can't simply delete the existing main window when # a new one is assigned because the user may transition the old out - # *after* the assignment. Sigh. So as a happy medium let's check in + # *after* the assignment. Sigh. So, as a happy medium, let's check in # on the old after a short bit of time and kill it if its still alive. - # That will be a bit ugly on screen but at least will un-break things. + # That will be a bit ugly on screen but at least should un-break + # things. def _delay_kill() -> None: import time if existing: diff --git a/assets/src/ba_data/python/bastd/ui/gather/__init__.py b/assets/src/ba_data/python/bastd/ui/gather/__init__.py index 7fb255a6..3420f6bf 100644 --- a/assets/src/ba_data/python/bastd/ui/gather/__init__.py +++ b/assets/src/ba_data/python/bastd/ui/gather/__init__.py @@ -133,7 +133,7 @@ class GatherWindow(ba.Window): pos=(tab_buffer_h * 0.5, self._height - 130 + tabs_top_extra), size=(self._width - tab_buffer_h, 50), - on_select_call=self._set_tab) + on_select_call=ba.WeakCall(self._set_tab)) # Now instantiate handlers for these tabs. tabtypes: Dict[GatherWindow.TabID, Type[GatherTab]] = { @@ -174,6 +174,7 @@ class GatherWindow(ba.Window): texture=ba.gettexture('scrollWidget'), model_transparent=ba.getmodel('softEdgeOutside')) self._tab_container: Optional[ba.Widget] = None + self._restore_state() def __del__(self) -> None: @@ -251,16 +252,11 @@ class GatherWindow(ba.Window): current_tab = self.TabID.ABOUT gather_tab_val = ba.app.config.get('Gather Tab') try: - stored_tab = self.TabID(gather_tab_val) + stored_tab = ba.enum_by_value(self.TabID, gather_tab_val) if stored_tab in self._tab_row.tabs: current_tab = stored_tab except ValueError: - # EWWWW; this exception causes a dependency loop that won't - # go away until the next cyclical collection, which can - # keep us alive. Perhaps should rethink our garbage - # collection strategy, but for now just explicitly running - # a cycle. - ba.pushcall(ba.garbage_collect) + pass self._set_tab(current_tab) if sel_name == 'Back': sel = self._back_button @@ -268,15 +264,10 @@ class GatherWindow(ba.Window): sel = self._tab_container elif isinstance(sel_name, str) and sel_name.startswith('Tab:'): try: - sel_tab_id = self.TabID(sel_name.split(':')[-1]) + sel_tab_id = ba.enum_by_value(self.TabID, + sel_name.split(':')[-1]) except ValueError: sel_tab_id = self.TabID.ABOUT - # EWWWW; this exception causes a dependency loop that won't - # go away until the next cyclical collection, which can - # keep us alive. Perhaps should rethink our garbage - # collection strategy, but for now just explicitly running - # a cycle. - ba.pushcall(ba.garbage_collect) sel = self._tab_row.tabs[sel_tab_id].button else: sel = self._tab_row.tabs[current_tab].button diff --git a/assets/src/ba_data/python/bastd/ui/gather/manualtab.py b/assets/src/ba_data/python/bastd/ui/gather/manualtab.py index 45dbb22f..64c0c2e3 100644 --- a/assets/src/ba_data/python/bastd/ui/gather/manualtab.py +++ b/assets/src/ba_data/python/bastd/ui/gather/manualtab.py @@ -184,12 +184,6 @@ class ManualGatherTab(GatherTab): try: port = int(cast(str, ba.textwidget(query=port_textwidget))) except ValueError: - # EWWWW; this exception causes a dependency loop that won't - # go away until the next cyclical collection, which can - # keep us alive. Perhaps should rethink our garbage - # collection strategy, but for now just explicitly running - # a cycle. - ba.pushcall(ba.garbage_collect) port = -1 if port > 65535 or port < 0: ba.screenmessage(ba.Lstr(resource='internal.invalidPortErrorText'), diff --git a/assets/src/ba_data/python/bastd/ui/gather/publictab.py b/assets/src/ba_data/python/bastd/ui/gather/publictab.py index 2b29537a..c77c73a5 100644 --- a/assets/src/ba_data/python/bastd/ui/gather/publictab.py +++ b/assets/src/ba_data/python/bastd/ui/gather/publictab.py @@ -842,6 +842,10 @@ class PublicGatherTab(GatherTab): key: val for key, val in list(self._parties.items()) if val.claimed } + + # Make sure we update the list immediately in response to this. + self._server_list_dirty = True + self._update_server_list() def _update(self) -> None: diff --git a/assets/src/ba_data/python/bastd/ui/party.py b/assets/src/ba_data/python/bastd/ui/party.py index 28330095..99fb7abd 100644 --- a/assets/src/ba_data/python/bastd/ui/party.py +++ b/assets/src/ba_data/python/bastd/ui/party.py @@ -71,7 +71,7 @@ class PartyWindow(ba.Window): info = _ba.get_connection_to_host_info() if info.get('name', '') != '': - title = info['name'] + title = ba.Lstr(value=info['name']) else: title = ba.Lstr(resource=self._r + '.titleText') diff --git a/assets/src/ba_data/python/bastd/ui/store/browser.py b/assets/src/ba_data/python/bastd/ui/store/browser.py index 3067dabc..b1bf4585 100644 --- a/assets/src/ba_data/python/bastd/ui/store/browser.py +++ b/assets/src/ba_data/python/bastd/ui/store/browser.py @@ -1025,16 +1025,11 @@ class StoreBrowserWindow(ba.Window): assert isinstance(sel_name, (str, type(None))) try: - current_tab = self.TabID(ba.app.config.get('Store Tab')) + current_tab = ba.enum_by_value(self.TabID, + ba.app.config.get('Store Tab')) except ValueError: current_tab = self.TabID.CHARACTERS - # EWWWW; this exception causes a dependency loop that won't - # go away until the next cyclical collection, which can keep - # us alive. Perhaps should rethink our garbage collection - # strategy, but for now just explicitly running a cycle. - ba.pushcall(ba.garbage_collect) - if self._show_tab is not None: current_tab = self._show_tab if sel_name == 'GetTickets' and self._get_tickets_button: @@ -1045,7 +1040,8 @@ class StoreBrowserWindow(ba.Window): sel = self._scrollwidget elif isinstance(sel_name, str) and sel_name.startswith('Tab:'): try: - sel_tab_id = self.TabID(sel_name.split(':')[-1]) + sel_tab_id = ba.enum_by_value(self.TabID, + sel_name.split(':')[-1]) except ValueError: sel_tab_id = self.TabID.CHARACTERS sel = self._tab_row.tabs[sel_tab_id].button diff --git a/assets/src/ba_data/python/bastd/ui/watch.py b/assets/src/ba_data/python/bastd/ui/watch.py index f27a1819..7f4ccea3 100644 --- a/assets/src/ba_data/python/bastd/ui/watch.py +++ b/assets/src/ba_data/python/bastd/ui/watch.py @@ -509,14 +509,9 @@ class WatchWindow(ba.Window): {}).get('sel_name') assert isinstance(sel_name, (str, type(None))) try: - current_tab = self.TabID(ba.app.config.get('Watch Tab')) + current_tab = ba.enum_by_value(self.TabID, + ba.app.config.get('Watch Tab')) except ValueError: - # EWWWW; this exception causes a dependency loop that won't - # go away until the next cyclical collection, which can - # keep us alive. Perhaps should rethink our garbage - # collection strategy, but for now just explicitly running - # a cycle. - ba.pushcall(ba.garbage_collect) current_tab = self.TabID.MY_REPLAYS self._set_tab(current_tab) @@ -526,14 +521,9 @@ class WatchWindow(ba.Window): sel = self._tab_container elif isinstance(sel_name, str) and sel_name.startswith('Tab:'): try: - sel_tab_id = self.TabID(sel_name.split(':')[-1]) + sel_tab_id = ba.enum_by_value(self.TabID, + sel_name.split(':')[-1]) except ValueError: - # EWWWW; this exception causes a dependency loop that won't - # go away until the next cyclical collection, which can - # keep us alive. Perhaps should rethink our garbage - # collection strategy, but for now just explicitly running - # a cycle. - ba.pushcall(ba.garbage_collect) sel_tab_id = self.TabID.MY_REPLAYS sel = self._tab_row.tabs[sel_tab_id].button else: diff --git a/docs/ba_module.md b/docs/ba_module.md index 25a9627a..45582c19 100644 --- a/docs/ba_module.md +++ b/docs/ba_module.md @@ -1,5 +1,5 @@ -

last updated on 2020-10-31 for Ballistica version 1.5.27 build 20236

+

last updated on 2020-11-02 for Ballistica version 1.5.27 build 20238

This page documents the Python classes and functions in the 'ba' module, which are the ones most relevant to modding in Ballistica. If you come across something you feel should be included here or could be better explained, please let me know. Happy modding!

@@ -86,6 +86,7 @@