Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Ctrl-C doesn't stop threads #78

Closed
Tired-Fox opened this issue Jul 19, 2022 · 10 comments
Closed

[BUG] Ctrl-C doesn't stop threads #78

Tired-Fox opened this issue Jul 19, 2022 · 10 comments
Labels
Bug Something isn't working

Comments

@Tired-Fox
Copy link
Contributor

Tired-Fox commented Jul 19, 2022

Describe the bug
When running any graphics loop, using the compositor, and you execute a keyboard interrupt the thread that draws the window keeps going. Mainly found on windows 10.

I tested this on Windows 11 and the issue doesn't exist.

To Reproduce

  1. Use a Windows 10 machine
  2. Use Windows Terminal or Alacritty
  3. Run any of the examples
  4. Press CTRL-C
  5. Program exits, but thread keeps drawing screen which traps the terminal in the window

Expected behavior
Program exits cleanly back to terminal prompt

Screenshots
LockedInDrawWindow
The above gif shows a flash of text from the interrupt then another draw call.

LockedInTerminalInterrupt
The above image shows the successful interrupt but the terminal is trapped because of a thread still being active.

System information

  • Windows 10
  • Alacritty
  • Windows Terminal
PyTermGUI version 6.4.0

System details:
    Python version: 3.10.5
    $TERM:          None
    $COLORTERM:     None
    Color support:  ColorSystem.STANDARD
    OS Platform:    Windows-10-10.0.19043-SP0

Possible cause
On Windows 10 a keyboard interrupt does not stop child threads and only stops the main thread.

Possible solution
Set thread to be a daemon. When a thread is set to be a daemon it will terminate when all non-daemon threads terminate. This means when the main thread terminates so will the child thread.

Add daemon=True to where the compositor draw thread is created

165        Thread(name="CompositorDrawLoop", target=self._draw_loop, daemon=True).start()

I have tested this on the machine that had this bug and it fixed the problem.

NOTE:
I would like to add that making the different threads daemons may solve some other issues as well.

@Tired-Fox Tired-Fox added the Bug Something isn't working label Jul 19, 2022
@bczsalba
Copy link
Owner

Will look into this, thank you!

I would like to add that making the different threads daemons may solve some other issues as well.

Do you have any specific things in mind for this?

@Tired-Fox
Copy link
Contributor Author

Tired-Fox commented Jul 20, 2022

Didn't read to deep, but it could help with issue #18.

Also thought with the ticket about SIGWINCH not working for windows (#33), there could be a daemon thread created that sends a signal when os.get_terminal_size() changes it's value. I have already started thinking about other ways of solving that ticket as well.

@bczsalba
Copy link
Owner

Look out for #25, AFAICR we also tried a thread-based implementation that caused the same issue. Would be grateful if you could help out!

@Tired-Fox
Copy link
Contributor Author

I duel boot windows and linux, so I don't mind going into the windows side and helping.

I may look into writing some Windows API hooks, let me know if you are interested in bringing this into the project.

@bczsalba
Copy link
Owner

I actually have been for a long time! Specifically this API would be greatly beneficial to the library, for seemingly very little cost.

@Tired-Fox
Copy link
Contributor Author

I saw that you linked that in #33. I wouldn't mind writing something that can translate between linux and windows calls to make it universal. I can start a discussion as to keep this issue about the problem listed above.

@bczsalba
Copy link
Owner

@Tired-Fox could you test out if daemon=True fixes the issue on Windows? It seems to not have any adverse effect on MacOS, so it should be fine on Linux too.

@Tired-Fox
Copy link
Contributor Author

I can test both Linux and Windows

@Tired-Fox
Copy link
Contributor Author

@bczsalba It seems that it has no adverse effects for Linux. It also works on windows with no obvious side affects.

@bczsalba
Copy link
Owner

bczsalba commented Jul 23, 2022

Can you test if this fixed the issue? 😄

P.S. Thank you for the testing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants