exit when no more data available

[dimbleby]

Description (e.g. "Related to ...", etc.)

Fixes #173.

I agree with recent comments saying that receiving an empty header at readline tells you that you're done: per https://docs.python.org/3/tutorial/inputoutput.html#methods-of-file-objects

If the end of the file has been reached, f.read() will return an empty string ('') ...
... if f.readline() returns an empty string, the end of the file has been reached ...

So that's what this does.

If this looks good, I wonder whether it's time for a 0.10.3 release? There have been a handful of bug fixes that it would be nice to see published.

Code review checklist (for code reviewer to complete)

• Pull request represents a single change (i.e. not fixing disparate/unrelated things in a single PR)
• Title summarizes what is changing
• Commit messages are meaningful (see this for details)
• Tests have been included and/or updated, as appropriate
• Docstrings have been included and/or updated, as appropriate
• Standalone docs have been updated accordingly
• CONTRIBUTORS.md was updated, as appropriate
• Changelog has been updated, as needed (see CHANGELOG.md)

[danixeee]

It's not working properly on my end. I have tested it with the json-extension example (macOS Catalina).

Looks like header = await loop.run_in_executor(executor, rfile.readline) is blocking, so it never returns an empty string when the pipe is closed.

Maybe we need something like:

async def _read(loop, executor, read_fn, timeout=1):
    return await asyncio.wait_for(
        loop.run_in_executor(executor, read_fn),
        timeout=timeout)

and then use it as:

try:
    header = await _read(loop, executor, rfile.readline)
except asyncio.TimeoutError:
    continue # ?

and

try:
    body = await _read(loop, executor, partial(rfile.read, content_length))
except asyncio.TimeoutError:
    continue # ?

I tried but something is not working. I won't have time to look in detail this week, could you try it out?

If you want to run an example using pipes, just set this env to "false" and only launch the client. You can check logs to see if the event loop is closed.

[dimbleby]

That is surprising to me: I've found that it works live - and the testcase that I added is also somewhat convincing.

(My motivation for getting involved in this was a recent change in the neovim LSP client which left jedi-language-server orphaned. The neovim change has been reverted for now, but I verified meanwhile that this fix was sufficient to resolve the problem.)

What you are suggesting probably works I would expect, but I don't think it ought to be necessary. Not sure that I'll get to trying it any sooner than you, but will update if I do.

[dimbleby]

Actually a couple of questions:

• what problem exactly is it that this is not fixing it for you? it definitely does fix something for me - and I have the unit test to prove it - so we must be doing something different!

• isn't the effect of that environment variable to turn on the TCP server? This fix is for the stdio version.

[danixeee]

what problem exactly is it that this is not fixing it for you?

The log file created by json-extension is missing some shutdown that should be fired here.

Also, if I added a few log.debug lines around await loop.run_in_executor(... lines and by looking at the log file it seems that it's blocking for some reason.

Which platform do you use? I know that CI covers windows and Linux, so it's quite strange.

isn't the effect of that environment variable to turn on the TCP server? This fix is for the stdio version.

It is if it is set to "true".

Please take a look:

pygls-io.mov

I have tested both normal process shutdown and forced one in the video.

What is weird is that once I got BrokenPipeError when I forcefully stopped debugging, but I can't reproduce it again now...
Same as explained in #179.

[dimbleby]

I'm on linux (via WSL, though I don't suppose that matters).

I'm not familiar with vscode and can't tell what you're doing in that video. So I'm afraid I still don't understand what you're hitting and unhappy with! What is "forced shutdown"?

Whatever it is that you are seeing, I think it must be different from what I have fixed, and what I believe #173 describes.

My understanding of that bug is that it is all about what happens if the client unexpectedly exits ("killing emacs with SIGKILL"), and what I am aiming to do is make sure that the server also exits in that case.

I have verified that this works live (via the neovim client), and added unit test to prove it. And if you try the original reproducer in the opening comment of #173, the behaviour is now quite different, we just exit at once.

Another way to verify this: change into /examples/json-extension, run python -m server, and send EOF (Ctrl-D)

• before the fix, nothing
• after the fix, we exit

I'm tempted to suggest that this fix is good and should be merged, and track whatever the other thing is separately.

[danixeee]

Logs are not related to the VS Code, but I have checked the server process manually and it's being killed properly. I will merge PR and release the new version, thanks.

[danixeee]

Looks good.