From f32c2f9e238ac3f8a8e8f741167d7b0507d84a4b Mon Sep 17 00:00:00 2001 From: matejcik Date: Mon, 25 May 2020 13:04:15 +0200 Subject: [PATCH] core: replace workflow.kill_default with workflow.close_others --- core/src/apps/common/mnemonic.py | 5 +-- core/src/apps/common/passphrase.py | 2 +- core/src/trezor/ui/__init__.py | 3 +- core/src/trezor/workflow.py | 66 +++++++++++------------------- 4 files changed, 30 insertions(+), 46 deletions(-) diff --git a/core/src/apps/common/mnemonic.py b/core/src/apps/common/mnemonic.py index 5fe54ef06..11f2fdcfd 100644 --- a/core/src/apps/common/mnemonic.py +++ b/core/src/apps/common/mnemonic.py @@ -57,9 +57,8 @@ def get_seed(passphrase: str = "", progress_bar: bool = True) -> bytes: def _start_progress() -> None: # Because we are drawing to the screen manually, without a layout, we - # should make sure that no other layout is running. At this point, only - # the homescreen should be on, so shut it down. - workflow.kill_default() + # should make sure that no other layout is running. + workflow.close_others() t = Text("Please wait", ui.ICON_CONFIG) ui.draw_simple(t) diff --git a/core/src/apps/common/passphrase.py b/core/src/apps/common/passphrase.py index d6d2422a6..dfe56f613 100644 --- a/core/src/apps/common/passphrase.py +++ b/core/src/apps/common/passphrase.py @@ -26,6 +26,7 @@ async def get(ctx: wire.Context) -> str: async def _request_from_user(ctx: wire.Context) -> str: + workflow.close_others() # request exclusive UI access if storage.device.get_passphrase_always_on_device(): passphrase = await _request_on_device(ctx) else: @@ -69,7 +70,6 @@ async def _request_on_device(ctx: wire.Context) -> str: def _entry_dialog() -> None: - workflow.kill_default() text = Text("Passphrase entry", ICON_CONFIG) text.normal("Please type your", "passphrase on the", "connected host.") draw_simple(text) diff --git a/core/src/trezor/ui/__init__.py b/core/src/trezor/ui/__init__.py index cf1911f71..daee3759e 100644 --- a/core/src/trezor/ui/__init__.py +++ b/core/src/trezor/ui/__init__.py @@ -168,7 +168,8 @@ def draw_simple(t: Component) -> None: # noqa: F405 This function bypasses the UI workflow engine, so other layouts will not know that something was drawn over them. In particular, if no other Layout is shown in a workflow, the homescreen will not redraw when the workflow is finished. - Use `workflow.kill_default()` if you need to avoid this situation. + Make sure you use `workflow.close_others()` before invoking this function + (note that `workflow.close_others()` is implicitly called with `button_request()`). """ backlight_fade(style.BACKLIGHT_DIM) display.clear() diff --git a/core/src/trezor/workflow.py b/core/src/trezor/workflow.py index 323c522ed..acb0913d7 100644 --- a/core/src/trezor/workflow.py +++ b/core/src/trezor/workflow.py @@ -3,7 +3,7 @@ import utime from trezor import log, loop if False: - from typing import Any, Callable, Dict, Optional, Set + from typing import Callable, Dict, Optional, Set IdleCallback = Callable[[], None] @@ -20,7 +20,7 @@ tasks = set() # type: Set[loop.spawn] # Default workflow task, if a default workflow is running. Default workflow # is not contained in the `tasks` set above. -default_task = None # type: Optional[loop.Task] +default_task = None # type: Optional[loop.spawn] # Constructor for the default workflow. Returns a workflow task. default_constructor = None # type: Optional[Callable[[], loop.Task]] @@ -78,12 +78,10 @@ def start_default() -> None: assert default_constructor is not None if not default_task: - default_task = default_constructor() + default_task = loop.spawn(default_constructor()) if __debug__: - log.debug(__name__, "start default: %s", default_task) - # Schedule the default task. Because the task can complete on its own, - # we need to reset the `default_task` global in a finalizer. - loop.schedule(default_task, None, None, _finalize_default) + log.debug(__name__, "start default: %s", default_task.task) + default_task.set_finalizer(_finalize_default) else: if __debug__: log.debug(__name__, "default already started") @@ -110,17 +108,19 @@ def kill_default() -> None: if __debug__: log.debug(__name__, "close default") # We let the `_finalize_default` reset the global. - loop.close(default_task) + default_task.close() def close_others() -> None: - """Shut down all running tasks, except the one that is currently executing, and - restart the default.""" - try: - kill_default() + """Request workflow (and UI) exclusivity: shut down all running tasks, except + the one that is currently executing. + + If this is called from outside a registered workflow, it is equivalent to "close + all tasks". In that case, the default task will be restarted afterwards. + """ + if default_task is not None and not default_task.is_running(): + default_task.close() # if no other tasks are running, start_default will run immediately - except ValueError: - pass # we need a local copy of tasks because processing task.close() modifies # the global instance @@ -131,39 +131,23 @@ def close_others() -> None: # if tasks were running, closing the last of them will run start_default -def _finalize_default(task: loop.Task, value: Any) -> None: +def _finalize_default(task: loop.spawn) -> None: """Finalizer for the default task. Cleans up globals and restarts the default in case no other task is running.""" global default_task - if default_task is task: - if __debug__: - log.debug(__name__, "default closed: %s", task) - default_task = None + assert default_task is task # finalizer is closing something other than default? + assert default_constructor is not None # it should always be configured - if not tasks: - # No registered workflows are running and we are in the default task - # finalizer, so when this function finished, nothing will be running. - # We must schedule a new instance of the default now. - if default_constructor is not None: - start_default() - else: - raise RuntimeError # no tasks and no default constructor + if __debug__: + log.debug(__name__, "default closed: %s", task.task) + default_task = None - else: - if __debug__: - log.warning( - __name__, - "default task does not match: task=%s, default_task=%s", - task, - default_task, - ) - - -# TODO -# If required, a function `shutdown_default` should be written, that clears the -# default constructor and shuts down the running default task. -# We currently do not need such function, so I'm just noting how it should work. + if not tasks: + # No registered workflows are running and we are in the default task + # finalizer, so when this function finished, nothing will be running. + # We must schedule a new instance of the default now. + start_default() class IdleTimer: