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

[Bug]: rust-analyzer-proc-macro-srv segfault #23087

Open
xJonathanLEI opened this issue Feb 1, 2025 · 3 comments
Open

[Bug]: rust-analyzer-proc-macro-srv segfault #23087

xJonathanLEI opened this issue Feb 1, 2025 · 3 comments
Labels
bug report Something is not working properly rust Any problem related to the Rust ecosystem (including cargo)

Comments

@xJonathanLEI
Copy link
Contributor

Problem description

I've run in to a very weird issue. rust-analyzer-proc-macro-srv, the program shipped with the rust package and used by rust-analyzer to resolve and expand proc macros, would crash with a segfault signal (11) when being used in a repo with a certain combination of crates.

I was first hit with this issue more than a week ago but it's been really hard to debug. I made a "minimal" reproducer here but it looks far from actually minimal. I couldn't trim it further as any single dependency or line of code removed would result in the crash not happening.

Note

Yes, the repo does not compile. But that's irrelevant. I originally encountered this issue on a much larger repo that compiles totally fine. Also, even on repos that do not compile, the process should apparently not segfault.

I managed to extract the hundreds of raw messages that rust-analyzer sends to rust-analyzer-proc-macro-srv and recreated the setup outside of the context of rust-analyzer. Then, I used a trial-and-error appraoch to filter out most messages to arrive at a sequence of 23 messages. Removing any of these 23 messages results in the crash not happening.

I'm not posting the "distilled" reproducer yet as it involves referencing the compiled binaries (needed by rust-analyzer-proc-macro-srv to run the expansion), and those depend strictly on Rust ABI versions, so using the full reproducer might be the easiest way to see the error.

Nevertheless, the fact that you have to send 23 messages in a sequence to crash the server is quite fascinating... and a huge headache to debug. The root cause might be quite deep.

What steps will reproduce the bug?

Run helix in this repo: xJonathanLEI/termux-proc-macro-srv-crash-repro and wait for rust-analyzer-proc-macro-srv to crash:

proc-macro server for workspace `xxxx/termux-proc-macro-srv-crash-repro/Cargo.toml` exited: proc-macro server exited with signal: 11 (SIGSEGV)

What is the expected behavior?

rust-analyzer-proc-macro-srv does not segfault.

System information

Termux Variables:
TERMUX_API_VERSION=0.50.1
TERMUX_APK_RELEASE=GITHUB
TERMUX_APP_PACKAGE_MANAGER=apt
TERMUX_APP_PID=8313
TERMUX_IS_DEBUGGABLE_BUILD=1
TERMUX_MAIN_PACKAGE_FORMAT=debian
TERMUX_VERSION=0.118.1
TERMUX__USER_ID=0
Packages CPU architecture:
aarch64
Subscribed repositories:
# sources.list
deb https://is.mirror.flokinet.net/termux/termux-main stable main
Updatable packages:
boost/stable 1:1.84.0 aarch64 [upgradable from: 1.84.0]
openssh-sftp-server/stable 9.9p1-5 aarch64 [upgradable from: 9.9p1-4]
openssh/stable 9.9p1-5 aarch64 [upgradable from: 9.9p1-4]
termux-tools version:
1.44.6
Android version:
14
Kernel build information:
Linux localhost 6.1.75-android14-11-29543898-abS9210ZHS4AXK6 #1 SMP PREEMPT Fri Nov 29 11:15:43 UTC 2024 aarch64 Android
Device manufacturer:
samsung
Device model:
SM-S9210
LD Variables:
LD_LIBRARY_PATH=
LD_PRELOAD=/data/data/com.termux/files/usr/lib/libtermux-exec.so
Installed termux plugins:
com.termux.api versionCode:51
@xJonathanLEI xJonathanLEI added bug report Something is not working properly untriaged labels Feb 1, 2025
@xJonathanLEI
Copy link
Contributor Author

Given the nature of segfault issues and the fact that it mysteriously takes a whole 23 messages to crash the server, it's quite unlikely that this can be trivially solved. I'm willing to put up a bounty for this if anyone is interested, as it does disrupts the Rust dev workflow on Termux devices.

Meanwhile, taking advantage of the exact fact that it takes so many messages to crash the server, and that proc macro expansions seem to be independent of each other, I've developed a very temporary workaround in rust-analyzer for anyone who runs into the same issue: xJonathanLEI/rust-analyzer#dev/termux.

The workaround simply uses a fresh child process for each proc macro expansion task. The child process is terminated immediately after each expansion. This is slower than reusing the child process, but given that Android devices aren't known for abundance in RAM it seems to be a wise choice to trade speed for less memory pressure to avoid OOM.

An actually proper workaround would be to implement child process restart logic for rust-analyzer, likely as a config option. Such a patch would then be reasonable to get upstreamed. For now though, I've tested my hacky workaround and it indeed works. I'm okay with some slight performance drop caused by it as this is so much better than screwing up all the proc macros...

@xJonathanLEI
Copy link
Contributor Author

xJonathanLEI commented Feb 1, 2025

Should we apply this workaround as a patch for our rust-analyzer package? The perf drop should be pretty small in most cases, and at least it prevents people from running into this extremely annoying issue.

@truboxl
Copy link
Contributor

truboxl commented Feb 4, 2025

PR is welcomed

@truboxl truboxl added rust Any problem related to the Rust ecosystem (including cargo) and removed untriaged labels Feb 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug report Something is not working properly rust Any problem related to the Rust ecosystem (including cargo)
Projects
None yet
Development

No branches or pull requests

2 participants