Skip to content

Commit

Permalink
tui: fix an obscure freezing issue
Browse files Browse the repository at this point in the history
* Fixes an obscure issue where the applications main loop becomes
  suspended as the result of attempting to register an `on_press`
  callback which takes as an argument a Cylc client.
* Even though this callback is not actually called at this point,
  just passing the reference to it appears to be enough to cause
  thread deadlocking because Cylc clients are async based and
  not thread safe.
  • Loading branch information
oliver-sanders committed Oct 13, 2023
1 parent b175218 commit 6f3f170
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 28 deletions.
34 changes: 24 additions & 10 deletions cylc/flow/tui/data.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,12 @@
from functools import partial
from subprocess import Popen, PIPE

from cylc.flow.exceptions import ClientError
from cylc.flow.exceptions import (
ClientError,
ClientTimeout,
WorkflowStopped,
)
from cylc.flow.network.client_factory import get_client
from cylc.flow.id import Tokens
from cylc.flow.tui.util import (
extract_context
Expand Down Expand Up @@ -187,11 +192,25 @@ def generate_mutation(mutation, arguments):
'''


def list_mutations(client, selection):
def list_mutations(selection, is_running=True):
"""List mutations relevant to the provided selection.
Args:
selection:
The user selection.
is_running:
If False, then mutations which require the scheduler to be
running will be omitted.
Note, this is only relevant for workflow nodes because if a
workflow is stopped, then any tasks within it will be removed
anyway.
"""
context = extract_context(selection)
selection_type = list(context)[-1]
ret = []
if client:
if is_running:
# add the online mutations
ret.extend(MUTATIONS.get(selection_type, []))
# add the offline mutations
Expand Down Expand Up @@ -232,20 +251,15 @@ def context_to_variables(context):
return variables


def mutate(client, mutation, selection):
def mutate(mutation, selection):
if mutation in {
_mutation
for section in OFFLINE_MUTATIONS.values()
for _mutation in section
}:
offline_mutate(mutation, selection)
elif client:
online_mutate(client, mutation, selection)
else:
raise Exception(
f'Cannot peform command {mutation} on a stopped workflow'
' or invalid command.'
)
online_mutate(mutation, selection)


def online_mutate(mutation, selection):
Expand Down
34 changes: 16 additions & 18 deletions cylc/flow/tui/overlay.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,20 +37,13 @@
"""

from contextlib import suppress
from functools import partial
import re
import sys

import urwid

from cylc.flow.exceptions import (
ClientError,
ClientTimeout,
WorkflowStopped,
)
from cylc.flow.id import Tokens
from cylc.flow.network.client_factory import get_client
from cylc.flow.task_state import (
TASK_STATUSES_ORDERED,
TASK_STATUS_WAITING
Expand All @@ -61,7 +54,6 @@
TUI
)
from cylc.flow.tui.data import (
extract_context,
list_mutations,
mutate,
)
Expand Down Expand Up @@ -296,20 +288,23 @@ def context(app):
"""An overlay for context menus."""
value = app.tree_walker.get_focus()[0].get_node().get_value()
selection = [value['id_']] # single selection ATM
context = extract_context(selection)

client = None
if 'workflow' in context:
w_id = context['workflow'][0]
with suppress(WorkflowStopped, ClientError, ClientTimeout):
client = get_client(w_id)
is_running = True
if (
value['type_'] == 'workflow'
and value['data']['status'] not in {'running', 'paused'}
):
# this is a stopped workflow
# => don't display mutations only valid for a running workflow
is_running = False

def _mutate(mutation, _):
nonlocal app, client
nonlocal app, selection

app.open_overlay(partial(progress, text='Running Command'))
try:
mutate(client, mutation, selection)
except ClientError as exc:
mutate(mutation, selection)
except Exception as exc:
app.open_overlay(partial(error, text=str(exc)))
else:
app.close_topmost()
Expand All @@ -334,7 +329,10 @@ def _mutate(mutation, _):
mutation,
on_press=partial(_mutate, mutation)
)
for mutation in list_mutations(client, selection)
for mutation in list_mutations(
selection,
is_running,
)
]
)
)
Expand Down

0 comments on commit 6f3f170

Please sign in to comment.