Skip to content

Conversation

@Veykril
Copy link
Member

@Veykril Veykril commented Nov 26, 2025

Closes #21130

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 26, 2025
@Veykril
Copy link
Member Author

Veykril commented Nov 26, 2025

cc @Shourya742 seems like if we try to connect to a proc-macro server that does not support postcard we will just hang on it now crashing as expected, we'll need to figure out a different approach here.

@Veykril Veykril enabled auto-merge November 26, 2025 07:43
@Shourya742
Copy link
Contributor

cc @Shourya742 seems like if we try to connect to a proc-macro server that does not support postcard we will just hang on it now crashing as expected, we'll need to figure out a different approach here.

I guess I know the reason for this, the json framing uses read_line, which blocks until it sees a newline. That means if the server doesn’t send one, we end up stuck waiting indefinitely. The problematic bit is here:

fn read<'a, R: BufRead>(
    inp: &mut R,
    buf: &'a mut String,
) -> io::Result<Option<&'a mut String>> {
    loop {
        buf.clear();

        inp.read_line(buf)?;
        buf.pop(); // Remove trailing '\n'

When we intialize the client with postcard, we don't write new line as the delimitor. I’ll send a patch.

@Veykril
Copy link
Member Author

Veykril commented Nov 26, 2025

So looking at things, the --format flag does seem to validate its inputs on the latest nightly, so we might just want to hold off a bit. Do a toolchain version check and then retry things here. The postcard protocol not using lineendings seems fine to me after all

@Shourya742
Copy link
Contributor

So looking at things, the --format flag does seem to validate its inputs on the latest nightly, so we might just want to hold off a bit. Do a toolchain version check and then retry things here. The postcard protocol not using lineendings seems fine to me after all

I was thinking that if we don’t receive a response within a specified time, we should abort the process and fall back.

@Veykril Veykril force-pushed the push-olvztmtwzmvv branch 2 times, most recently from adccc76 to 3740979 Compare November 26, 2025 08:05
@Veykril
Copy link
Member Author

Veykril commented Nov 26, 2025

that would work too and is definitely something we should implement.

I'll revert some parts here for now to unblock the pre-releases. And I'll also include #21132. It was a mistake not naming both of these flags legacy i think since we do want to replace the protocol entirely at some point.

Copy link
Contributor

@Shourya742 Shourya742 left a comment

Choose a reason for hiding this comment

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

ACK

@Veykril Veykril added this pull request to the merge queue Nov 26, 2025
Merged via the queue into rust-lang:master with commit 0f8057c Nov 26, 2025
15 checks passed
@Veykril Veykril deleted the push-olvztmtwzmvv branch November 26, 2025 10:26
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 26, 2025
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

Successfully merging this pull request may close these issues.

rust-analyzer hangs for over 50 seconds

3 participants