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

Window does not close when stopping native mode with Ctrl-C on Windows #604

Closed
falkoschindler opened this issue Mar 22, 2023 · 13 comments · Fixed by #1732
Closed

Window does not close when stopping native mode with Ctrl-C on Windows #604

falkoschindler opened this issue Mar 22, 2023 · 13 comments · Fixed by #1732
Labels
bug Something isn't working
Milestone

Comments

@falkoschindler
Copy link
Contributor

As already mentioned on #584, the native application does not terminate correctly with Ctrl-C on Windows.

@falkoschindler falkoschindler added the bug Something isn't working label Mar 22, 2023
@falkoschindler falkoschindler added this to the v1.2.2 milestone Mar 22, 2023
@falkoschindler falkoschindler modified the milestones: v1.2.2, v1.2.3 Mar 24, 2023
@al-eax
Copy link
Contributor

al-eax commented Mar 26, 2023

uvicorn has similar issues:
encode/uvicorn#1872

@rodja rodja modified the milestones: v1.2.3, v1.2.4 Mar 30, 2023
@rodja
Copy link
Member

rodja commented Mar 30, 2023

We will wait a bit longer for the uvicorn issue to evlolve.

@falkoschindler falkoschindler modified the milestones: v1.2.4, v1.2.5 Apr 3, 2023
@rodja rodja modified the milestones: v1.2.5, 1.2.6 Apr 3, 2023
@falkoschindler falkoschindler modified the milestones: v1.2.6, v1.2.7 Apr 5, 2023
@falkoschindler
Copy link
Contributor Author

@rodja Issue encode/uvicorn#1872 is from Feb 21 and still tagged as "needs confirmation". Anyway, I'll tag our issue as "waiting" and move it to "Later". Feel free to revert.

@falkoschindler falkoschindler added the waiting Can not be fixed right now label Apr 6, 2023
@falkoschindler falkoschindler modified the milestones: v1.2.7, Later Apr 6, 2023
@ItsCubeTime
Copy link

Hai, you can implement a fix for this yourself!

if sys.platform == "win32":
    import win32api
    import os
    win32api.SetConsoleCtrlHandler(lambda a=None: os.kill(os.getpid(), 15)) # Enables ctrl+C to kill the terminal on Windows. 15 == signal.SIGTERM

@rodja
Copy link
Member

rodja commented May 14, 2023

@ItsCubeTime cool. Would you care to create a pull request to close this bug?

@falkoschindler
Copy link
Contributor Author

Good news for encode/uvicorn#1872:

xiaotushaoxia closed this as completed yesterday

So maybe we can resolve our issue soon by updating uvicorn.

@falkoschindler
Copy link
Contributor Author

Looks like encode/uvicorn#1872 has been closed based on PR encode/uvicorn#1584 which was merged already on April 13. According to the milestone and the release notes, it should have been released on April 28 with uvicorn 0.22.0.

@al-eax and @ItsCubeTime, can you check if the issue is gone with uvicorn 0.22.0? Thanks!

@falkoschindler falkoschindler removed the waiting Can not be fixed right now label Jul 12, 2023
@falkoschindler
Copy link
Contributor Author

Nope, the problem remains with uvicorn 0.22.0 and NiceGUI 1.3.5 on Windows. 😕

@falkoschindler
Copy link
Contributor Author

falkoschindler commented Jul 24, 2023

Looks like a pywebview issue. The example from https://pywebview.flowrl.com/#hello-world has the same problem:

import webview
webview.create_window('Hello world', 'https://pywebview.flowrl.com/')
webview.start()

@miningmanna
Copy link
Contributor

miningmanna commented Sep 29, 2023

As far as I understand, pywebview needs to run in the main thread, therefore eating the SIGINT/SIGTERM signal.
In nicegui it is already running in a different process. The uvicorn server also exits when you press ctrl-c in the console.
From my (maybe limited) testing, it could be as simple to fix as setting the pywebview process to be a daemon process.

native_mode.py (line 109)
process = mp.Process(target=open_window, args=args, daemon=True)

or have some kind of message one could send to the pywebview process, similar to how get_size and so on work, in order to cleanly terminate the process.

@falkoschindler
Copy link
Contributor Author

@miningmanna Have you tried setting daemon=True? If it worked, a pull request would be very welcome. We just can't test it ourselves, since we have no Windows machine at hand.

@miningmanna
Copy link
Contributor

@falkoschindler I tried it on a Windows 10 and Windows 11 machine. It works on both.
I will create a pull request.

@falkoschindler
Copy link
Contributor Author

Amazing! The argument daemon=False has been added to the code in the very first commit regarding native mode: c734cab#diff-e16537d6af67de700fc23c3bd55c1eb27592a57962272242cb148bae23275af0. Somehow it was never questioned.

For a bit more context, this is how ChatGPT describes its function:

The multiprocessing.Process with daemon=False means that the main program will wait for the open_window function to complete its execution in the separate process before the main program finishes.

So since the window process shouldn't run without the main process, setting daemon=True seems to be absolutely correct. 👍🏻

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
5 participants