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

Add check to determine if a console window is present #2532

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

EliseevEgor
Copy link

Problem

Uvicorn hangs when launched under PyCharm general run configuration on Windows.
User use case: updating code that affects web page content.

Reason

The main Uvicorn process waits for its child process (Server.run()) to terminate.

Why does it hang?

As we can see in the Uvicorn code above for terminating the process on Windows, Uvicorn explicitly sends CTRL_C_EVENT. The problem is that CTRL_C_EVENT may be silently ignored in a number of cases.

Our case is that by default PyCharm runs programs as windowless, but it is not specific to PyCharm as it’s a default way to launch cmd line in Java (using java.lang.ProcessBuilder).

Proposed solution

The basic idea is to add a check to determine if a console window is present. It is a sufficient flag whether CTRL_C_EVENT would be ignored by a child process or not.

A few words about process creation on Windows

On Windows, processes are typically created using the Windows API or high-level abstractions provided by programming languages. The main function for process creation is CreateProcess(), which spawns a new process.

This function has dwCreationFlags arg. It contains three flags: CREATE_NEW_CONSOLE, CREATE_NO_WINDOW, and DETACHED_PROCESS which determine whether the process will have a console. For example, when we run a Python script from a console, all the flags are set to 0. It means that a process has a console with a window (inherit).

You can read more here

The Problem

If the process is launched with the CREATE_NO_WINDOW flag, its child processes lose their association with the console window and cannot process the CTRL_C_EVENT signal. This prevents us from properly terminating child processes. java.lang.ProcessBuilder (the default way to call CreateProcess()) runs CreateProcess() with CREATE_NO_WINDOW = 1 (link).

Steps to Reproduce

Below is a Python script example to reproduce the process creation behavior. Also Java file Main.java that runs get_console_flag.py using ProcessBuilder.

  1. Run the script from the standard terminal (PowerShell). You’ll get: "Console state is ConsoleState.NORMAL_CONSOLE".

  2. Run the script from PyCharm or using the Main.java file (the first arg is a path to python.exe, the second arg is a path to get_console_flag.py file). You’ll get: "Console state is ConsoleState.CONSOLE_WITHOUT_WINDOW."

The correct solution

It should be noted that if the parent process does not have a console, the child process will have a new console. This is a rare case, but for a perfect solution, this case must also be handled.

A hack is needed here: we check that there is a console but no window, and only in this case do we call terminate. (see the attached patch)

Files:

  1. get_console_flag.py
import enum
from ctypes import *

kernel32 = windll.kernel32

BUF_SIZE = 8192


class ConsoleState(enum.Enum):
    NORMAL_CONSOLE = 0,
    CONSOLE_WITHOUT_WINDOW = 1,
    NO_CONSOLE = 2


console_title_works = kernel32.GetConsoleTitleW(create_unicode_buffer(BUF_SIZE), BUF_SIZE) != 0
has_console_handle = kernel32.GetConsoleWindow() != 0

console_state = None
if console_title_works and has_console_handle:
    console_state = ConsoleState.NORMAL_CONSOLE
elif console_title_works and not has_console_handle:
    console_state = ConsoleState.CONSOLE_WITHOUT_WINDOW
else:
    console_state = ConsoleState.NO_CONSOLE

message = f"Console state is {console_state}. \n"
match console_state:
    case ConsoleState.NO_CONSOLE:
        message += "Congrats. Feel free to spawn a new process: it will create a new console"
    case ConsoleState.NORMAL_CONSOLE:
        message += "Congrats. Feel free to spawn a new process: it will inherit console that accepts CTRL+C"
    case ConsoleState.CONSOLE_WITHOUT_WINDOW:
        message += (
            "Condolences, you have console but no window. Please call `FreeConsole` or ask for `CREATE_NEW_CONSOLE` explicitly. \n"
            "Otherwise your child will ignore CTRL+C")

with open("log.txt", "w") as f:
    f.write(message)
print(message)
  1. Main.java
import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;

// First arg is path to python.exe, second is path to script
public class Main {
    public static void main(String[] args) {
        String pythonExePath = args[0];
        String scriptPath = args[1];

        try {
            ProcessBuilder processBuilder = new ProcessBuilder();
            processBuilder.command(pythonExePath, scriptPath);

            // Run process
            Process process = processBuilder.start();

            // Output
            BufferedReader reader = new BufferedReader(new InputStreamReader(process.getInputStream()));
            String line;
            System.out.println("Output:");
            while ((line = reader.readLine()) != null) {
                System.out.println(line);
            }

            // Errors
            BufferedReader errorReader = new BufferedReader(new InputStreamReader(process.getErrorStream()));
            System.out.println("Errors (if any):");
            while ((line = errorReader.readLine()) != null) {
                System.out.println(line);
            }

            int exitCode = process.waitFor();
            System.out.println("\nExited with code: " + exitCode);

        } catch (InterruptedException | IOException e) {
            e.printStackTrace();
        }
    }
}
  1. The correct solution patch
    Terminate_only_if_there_is_CREATE_NO_WINDOW.patch

 It is a sufficient flag whether CTRL_C_EVENT would be ignored by a child process or not.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant