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

Mishandling of process.write #60

Open
Guldoman opened this issue Apr 18, 2023 · 7 comments
Open

Mishandling of process.write #60

Guldoman opened this issue Apr 18, 2023 · 7 comments

Comments

@Guldoman
Copy link
Member

It looks like Server:write_request is happily truncating lots of data, causing LSP servers like sumneko to crash.
This is happening because process.write can sometimes not write the entire proposed buffer at once, even if the buffer is small.

For example, checking the non-chunked part of the function:

lite-xl-lsp/server.lua

Lines 1306 to 1314 in 20f33d0

while max_time > os.time() and written <= 0 do
written = self.proc:write(string.format(
'Content-Length: %d\r\n\r\n%s\r\n',
#data + 2,
data
))
if timeout == 0 then break end
end

this will exit even after only writing a single byte.

Also instead of exiting on timeout, it might be better to retry the write a bunch of times waiting a bit between retries. This will improve the chances that the buffer is consumed by the process. Kinda like what I'm doing in the primary_selection plugin:
https://github.com/lite-xl/lite-xl-plugins/blob/68a2a6ac694e66cc4bb005d00c9debaa751d8fe7/plugins/primary_selection.lua#L99-L119

Handling this correctly will render the chunked part of the function useless.

@jgmdev
Copy link
Member

jgmdev commented Apr 18, 2023

Could you provide some steps/project/configuration to reproduce?

@Guldoman
Copy link
Member Author

It is not always reproducible as it depends on how fast the language server is able to handle the data, but is usually enough to open a relatively big file.
Sumneko seems to be particularly susceptible to this issue, and it logs (in the /tmp/lua-language-server.XXX/log/....log file) errors like [warn] [#0:script/pub/report.lua:25]: Load proto error: Proto parse error: /usr/lib/lua-language-server/script/json.lua:348: ERROR: control character in string at line 1 col 6164.

It likely depends on system load too.

This could also be one of the causes of the sporadic desynchronization some people see.

You can try logging the size of the data to send, compared to written.
For example I'm getting things like

wrote	37603	out of	45795
wrote	40985	out of	50484
wrote	4498	out of	8594
wrote	8217	out of	10123

A simpler test would be done with the primary_selection plugin by for example removing the retries and the exponential back-off timeout, and checking what actually gets copied when selecting big files.

jgmdev added a commit that referenced this issue Apr 19, 2023
* Drops `requests_in_chunks` server option
* See issue #60 for more details
@jgmdev
Copy link
Member

jgmdev commented Apr 19, 2023

Ok, after some sleep properly read what you described, pushed a change that "should" fix the problem, let me know if it fixes it on your testing or if I introduced more bugs :)

@Guldoman
Copy link
Member Author

Guldoman commented Apr 19, 2023

I added a few comments to the commit.

EDIT:
This seems to fix the issue. I got an editor lockup tho, but it might have been caused by another plugin. I'll test more.

@jgmdev
Copy link
Member

jgmdev commented Apr 19, 2023

Ok, will implement your suggestions later and borrow more from your primary_selection code for the incremental yielding timeout + err checking (I thought about the process_write + yielding reuse but wasn't sure) , thanks for reviewing the commit!

Also, I haven't hit a lockup yet but I'm using luajit + threading branch and the default 1ms coroutine timeout change.

@jgmdev
Copy link
Member

jgmdev commented Apr 19, 2023

The other set of suggestions handled on this commit 27c241d except for the raw processing chunk chop out.

@jgmdev
Copy link
Member

jgmdev commented Apr 19, 2023

More suggested corrections 607cc85

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

No branches or pull requests

2 participants