Skip to content

Fix ruff server hanging after Neovim closes#11291

Merged
snowsignal merged 1 commit intomainfrom
jane/server/neovim-shutdown
May 5, 2024
Merged

Fix ruff server hanging after Neovim closes#11291
snowsignal merged 1 commit intomainfrom
jane/server/neovim-shutdown

Conversation

@snowsignal
Copy link
Copy Markdown
Contributor

Summary

A follow-up to #11222. ruff server stalls during shutdown with Neovim because after it receives an exit notification and closes the I/O thread, it attempts to log a success message to stderr. Removing this log statement fixes this issue.

Test Plan

Track the instances of ruff in the OS task manager as you open and close Neovim. A new instance should appear when Neovim starts and it should disappear once Neovim is closed.

@snowsignal snowsignal added bug Something isn't working server Related to the LSP server labels May 5, 2024
Copy link
Copy Markdown
Member

@charliermarsh charliermarsh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think this would work as expected if we were sending messages via the LSP protocol? Or same issue?

@snowsignal
Copy link
Copy Markdown
Contributor Author

Do you think this would work as expected if we were sending messages via the LSP protocol? Or same issue?

That wouldn't work because the I/O with the client has shut down at the point where we log this. However, it would at least fail instantly instead of just waiting forever.

@snowsignal snowsignal enabled auto-merge (squash) May 5, 2024 17:15
@snowsignal snowsignal merged commit a8a9729 into main May 5, 2024
@snowsignal snowsignal deleted the jane/server/neovim-shutdown branch May 5, 2024 17:15
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented May 5, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@MichaReiser
Copy link
Copy Markdown
Member

How can we avoid this in the future? It seems error prone to rely on tracing not being used after the main loop ends (it may even be a dependency that writes a trace, maybe even a third party dependency

@charliermarsh
Copy link
Copy Markdown
Member

Any ideas? I don’t understand the root of the problem well-enough.

@MichaReiser
Copy link
Copy Markdown
Member

Not understanding the root cause is what makes me wary of the fix. It's a good fix to unblock users but it seems fragile to me long term.

@snowsignal
Copy link
Copy Markdown
Contributor Author

I can do some further investigation into why the tracing log was blocking.

@ShIRannx
Copy link
Copy Markdown

ShIRannx commented May 6, 2024

the ruff server is not the subprocess of nvim. is this relevant to it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working server Related to the LSP server

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants