python-lsp / pylsp-mypy

Mypy plugin for the Python LSP Server.
MIT License
130 stars 37 forks source link

`dmypy` is not killed when python-lsp exits #88

Open itaranto opened 3 months ago

itaranto commented 3 months ago

I'm using Neovim with its native LSP client.

If I close the editor I still see an instance of dmypy up and running.

When editing a .py file I can see the follwing using pstree -lp:

image

It seems to be there's one instance of dmypy which in fact is a child process of pylsp, this instance gets killed when I close my editor (and by extension, pylsp too). But there's another instance which seems to be running in the background.

These are my settings:


    lspconfig.pylsp.setup {
      capabilities = capabilities,
      settings = {
        pylsp = {
          plugins = {
            autopep8 = { enabled = false },
            flake8 = { enabled = false },
            mccabe = { enabled = false },
            pycodestyle = { enabled = false },
            pydocstyle = { enabled = false },
            pyflakes = { enabled = false },
            pylint = { enabled = false },
            pylsp_mypy = {
              dmypy = true,
              enabled = true,
              live_mode = false,
            },
            yapf = { enabled = false },
          }
        }
      }
    }
itaranto commented 3 months ago

It seems running dmypy daemon is the way to go here as it always runs as a child process.

I tried hacking the code, I removed all the calls to "status" and "restart" and I replaced "run" with "daemon". I could make it to only run one instance as a child process but it doesn't seem to actually display diagnostics anymore.

This is what I did:

    if not dmypy:
        # ...
    else:
        # Run to use existing daemon or restart if required
        args = ["--status-file", dmypy_status_file, "daemon", "--"] + apply_overrides(
            args, overrides
        )

        if shutil.which("dmypy"):
            # dmypy exists on path
            # -> use mypy on path
            log.info("dmypy run args = %s via path", args)
            completed_process = subprocess.run(
                ["dmypy", *args], capture_output=True, **windows_flag, encoding="utf-8"
            )
            report = completed_process.stdout
            errors = completed_process.stderr
            exit_status = completed_process.returncode
        else:
            # dmypy does not exist on path, but must exist in the env pylsp-mypy is installed in
            # -> use dmypy via api
            log.info("dmypy run args = %s via api", args)
            report, errors, exit_status = mypy_api.run_dmypy(args)

This has also the side effect that now pylsp stays alive after closing the editor for some reason

Richardk2n commented 3 months ago

Where do you get the dmypy daemon from? I can't find any reference to that in the documentation. I do not know what dmypy does in the background, the two processes might be expected behavior. The deamon staying open is expected (this is what it is supposed to do). One could argue, that we should stop it atexit.

itaranto commented 3 months ago

Where do you get the dmypy daemon from? I can't find any reference to that in the documentation.

[I] ~ $ dmypy --help
usage: dmypy [-h] [--status-file STATUS_FILE] [-V] {start,restart,status,stop,kill,check,run,recheck,suggest,inspect,hang,daemon,help} ...

Client for mypy daemon mode

positional arguments:
  {start,restart,status,stop,kill,check,run,recheck,suggest,inspect,hang,daemon,help}
    start               Start daemon
    restart             Restart daemon (stop or kill followed by start)
    status              Show daemon status
    stop                Stop daemon (asks it politely to go away)
    kill                Kill daemon (kills the process)
    check               Check some files (requires daemon)
    run                 Check some files, [re]starting daemon if necessary
    recheck             Re-check the previous list of files, with optional modifications (requires daemon)
    suggest             Suggest a signature or show call sites for a specific function
    inspect             Locate and statically inspect expression(s)
    hang                Hang for 100 seconds
    daemon              Run daemon in foreground

options:
  -h, --help            show this help message and exit
  --status-file STATUS_FILE
                        status file to retrieve daemon details
  -V, --version         Show program's version number and exit

The deamon staying open is expected (this is what it is supposed to do). One could argue, that we should stop it atexit.

That could be a possibility, yes. It would be also better to run dmypy in such a way that's a child process of pylsp so when pylsp dies, dmypy dies too.

Richardk2n commented 3 months ago

Interestingly this is not in the doc. Also, Run daemon in foreground does not sound like the thing we want to me. Furthermore, we do run it either as a child process or in the same process (depending on venvs). It starting additional separated processes is a sensible design decision of mypy, which we cannot influence. Unless you can provide a working way to implement your preferred solution, I will take a look at stopping it atexit. BTW, what OS are you using? According to mypy src it might matter for this.

itaranto commented 3 months ago

Also, Run daemon in foreground does not sound like the thing we want to me. Furthermore, we do run it either as a child process or in the same process (depending on venvs).

Yes, it's confusing, I know. I just saw that option and ran it and then I saw how dmypy behaved. The results where that dmypy didn't spawn any "daemonized" processes, just the one started from the terminal.

It starting additional separated processes is a sensible design decision of mypy, which we cannot influence. Unless you can provide a working way to implement your preferred solution, I will take a look at stopping it atexit.

I can take a look there, yes. I can also file a bug/feature request to mypy to see if I get any response.

BTW, what OS are you using? According to mypy src it might matter for this.

I'm running Linux.