cylc-flow icon indicating copy to clipboard operation
cylc-flow copied to clipboard

tui: prevent hang on shutdown

Open oliver-sanders opened this issue 1 year ago • 1 comments

It has been reported that Tui can hang on shutdown.

The easiest way to reproduce this is to press "q" as soon as starting Tui. Sometimes this can cause it to hang.

$ cylc vip <workflow>
$ cylc tui <workflow>
q

This PR adds a timeout on the updater shutdown to prevent any hanging operation stopping Tui from exiting.

This isn't really the "right" way to solve the problem, getting the updater to exit cleanly in all circumstances would be nicer. However, it's a reasonable fail safe and since the updater is a read-only process (i.e. the only "resources" it's managing are Cylc clients which have timeouts built in) there are no ill effects of terminating it.

Check List

  • [x] I have read CONTRIBUTING.md and added my name as a Code Contributor.
  • [x] Contains logically grouped changes (else tidy your branch by rebase).
  • [x] Does not contain off-topic changes (use other PRs for other changes).
  • [x] Applied any dependency changes to both setup.cfg (and conda-environment.yml if present).
  • [ ] Not reasonably testable.
  • [x] CHANGES.md entry included if this is a change that can affect users
  • [x] Cylc-Doc pull request opened if required at cylc/cylc-doc/pull/XXXX.
  • [x] If this is a bug fix, PR should be raised against the relevant ?.?.x branch.

oliver-sanders avatar Jun 28 '24 09:06 oliver-sanders

@wxtim, the original bug isn't reproducing as easily as it originally did. To simulate this bug, try changing break to continue in this code:

https://github.com/cylc/cylc-flow/blob/67a6ba5f429efbf91c1610fba6bf602941a369fe/cylc/flow/tui/updater.py#L148-L159

That will prevent the updater from shutting down. Before this commit, Tui should hang indefinitely, after this PR it should exit after 4 seconds (due to the timeout).

oliver-sanders avatar Jul 02 '24 15:07 oliver-sanders

try changing break to continue in this code

Looks like I can now repeat the problem. Pleasingly this branch fixes it!

wxtim avatar Jul 03 '24 12:07 wxtim