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

Flycheck subprocess group is forcefully terminated after it has sent EOF #17902

Closed
tmandry opened this issue Aug 15, 2024 · 0 comments · Fixed by #17903
Closed

Flycheck subprocess group is forcefully terminated after it has sent EOF #17902

tmandry opened this issue Aug 15, 2024 · 0 comments · Fixed by #17903
Labels
C-bug Category: bug

Comments

@tmandry
Copy link
Member

tmandry commented Aug 15, 2024

rust-analyzer version: (eg. output of "rust-analyzer: Show RA Version" command, accessible in VSCode via Ctrl/⌘+Shift+P)

rust-analyzer 0.0.0 (b562603 2024-05-15)

editor or extension: (eg. VSCode, Vim, Emacs, etc. For VSCode users, specify your extension version; for users of other editors, provide the distribution if applicable)

VSCode

relevant settings: (eg. client settings, or environment variables like CARGO, RUSTC, RUSTUP_HOME or CARGO_HOME)

Using rust-analyzer.check.overrideCommand


We're using a custom check command on our repo. This command optionally collects duration metrics, so we can improve our developer experience. These metrics are collected in the background after the process completes.

Unfortunately, as soon as rust-analyzer gets an EOF from the check command it sends a SIGKILL to the entire process group. This prevents the metrics from being collected.

I don't think such aggressive killing is warranted. If the check is being rerun then it's fine to kill it as a way of cancelling quickly. But if it completes normally, it should be allowed to exit normally.

@tmandry tmandry added the C-bug Category: bug label Aug 15, 2024
bors added a commit that referenced this issue Aug 16, 2024
Allow flycheck process to exit gracefully

Assuming it isn't cancelled. Closes #17902.

The only place CommandHandle::join() is used is when the flycheck command
finishes, so this commit changes the behavior of the method itself.

The only reason I can see for the existing behavior is if the command is somehow holding onto a build lock longer than it should, this would force it to be released. But it would be a pretty heavy-handed way to solve that issue. I'm not aware of this occurring in practice.
@bors bors closed this as completed in da34676 Aug 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant