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

Potential compiler memory leak in Nightly #75808

Closed
sazzer opened this issue Aug 22, 2020 · 11 comments
Closed

Potential compiler memory leak in Nightly #75808

sazzer opened this issue Aug 22, 2020 · 11 comments
Labels
A-async-await Area: Async & Await AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. C-bug Category: This is a bug. I-hang Issue: The compiler never terminates, due to infinite loops, deadlock, livelock, etc. ICEBreaker-Cleanup-Crew Helping to "clean up" bugs with minimal examples and bisections regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@sazzer
Copy link

sazzer commented Aug 22, 2020

(Apologies if I've done this wrong. I've used the Bug template and not the ICE one because I'm not getting the "Internal Compiler Error" error message. I've also had a bit of a search and not seen anyone else reporting this at all, but I can't see anything unusual that I'm doing.)

I have a project that recently stopped building, and it's taken me a while to work out why. I still don't know everything - sorry! - but what I do know is this:

  • I'm using Nightly. It does not affect Stable. Additionally, it does not affect Nightly from before 2020-08-13. The very first version that breaks is:
-> % rustc --version --verbose
rustc 1.47.0-nightly (81dc88f88 2020-08-13)
binary: rustc
commit-hash: 81dc88f88f92ba8ad7465f9cba10c12d3a7b70f1
commit-date: 2020-08-13
host: x86_64-apple-darwin
release: 1.47.0-nightly
LLVM version: 10.0
  • I'm getting this on macOS but it also affects Linux. I know because I first caught it on my Github Actions CI build.
  • When it happens, I can got to Activity Monitor and see the memory usage of rustc going up. I caught this one because the OS complained about running out of memory on one test when it reached 15GB of memory used by the compiler.
  • I'm using a Cargo Workspace to keep different components in my system isolated from each other. However, I can't see any reason why that would be the problem. The error I'm seeing happens deterministically in exactly one of the crates, and all of the others are fine.
  • I have one single line of code that I can comment out that will stop the problem happening. However, it's not anything special.

The error happens in the method:

#[get("/authentication/{provider}/complete")]
#[tracing::instrument(
    fields(http_method = "GET", http_path = "/authentication/:provider/complete"),
    skip(authentication_service, authorization_service)
)]
pub async fn complete(
    path: Path<(ProviderId,)>,
    query: Query<HashMap<String, String>>,
    authentication_service: Data<AuthenticationService>,
    authorization_service: Data<AuthorizationService>,
) -> HttpResponse {
    let user = authentication_service
        .complete_authentication(&path.0, &query.0)
        .await;
    todo!()
}

If I remove the .await call in there then the build completes. If I leave the .await in then it breaks.

However, I've got other Actix endpoints that use async/await and they work perfectly well.

Cheers

@sazzer sazzer added the C-bug Category: This is a bug. label Aug 22, 2020
@jyn514 jyn514 added I-hang Issue: The compiler never terminates, due to infinite loops, deadlock, livelock, etc. regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. labels Aug 22, 2020
@rustbot rustbot added the I-prioritize Indicates that prioritization has been requested for this issue label Aug 22, 2020
@jyn514
Copy link
Member

jyn514 commented Aug 22, 2020

@sazzer it would be great if you could reproduce this in a stand-alone example, or at least the commit of the project that causes the hang.

@jyn514 jyn514 added E-needs-mcve Call for participation: This issue needs a Minimal Complete and Verifiable Example A-async-await Area: Async & Await labels Aug 22, 2020
@sazzer
Copy link
Author

sazzer commented Aug 22, 2020

@jyn514 The project link would have helped, wouldn't it? Sorry! :(

(There are some other branches that I've been playing with other things on. It's the "rust" branch that is interesting here)

When doing either cargo test or cargo build it will go fine until you get to "mire-authentication" and that's the one that hangs. If you edit crates/authentication/src/endpoints/complete.rs to remove the .await on line 32 onwards then the build will work fine.

(Please don't judge me on the quality of the code - I'm still learning :) )

Cheers

@jyn514
Copy link
Member

jyn514 commented Aug 22, 2020

Thanks! Don't worry about the code quality, part of rustc's job is to give good errors for broken code, not just code that's already working :)

(the commit that failed was https://github.com/sazzer/mire/commit/0bfad66a070673c58c1e98d334c5500712c77e63)

@sazzer
Copy link
Author

sazzer commented Aug 22, 2020

Just to clarify btw - it's not the commit that fails. That commit works fine on an earlier nightly toolchain, and on the stable toolchain. I've also not seen which of the earlier commits also fail on the newer nightly toolchain - though I can do if it would be useful? I suspect that it starts from https://github.com/sazzer/mire/commit/589c1b7.

@sazzer
Copy link
Author

sazzer commented Aug 22, 2020

Ok - the first commit that shows the problem appears to be https://github.com/sazzer/mire/commit/c927b77.

@LeSeulArtichaut LeSeulArtichaut added the E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc label Aug 23, 2020
@LeSeulArtichaut
Copy link
Contributor

LeSeulArtichaut commented Aug 23, 2020

It would be great to do some bisection and try to find an MCVE.
@rustbot ping cleanup

@rustbot rustbot added the ICEBreaker-Cleanup-Crew Helping to "clean up" bugs with minimal examples and bisections label Aug 23, 2020
@rustbot
Copy link
Collaborator

rustbot commented Aug 23, 2020

Hey Cleanup Crew ICE-breakers! This bug has been identified as a good
"Cleanup ICE-breaking candidate". In case it's useful, here are some
instructions for tackling these sorts of bugs. Maybe take a look?
Thanks! <3

cc @AminArria @camelid @chrissimpkins @contrun @DutchGhost @elshize @ethanboxx @h-michael @HallerPatrick @hdhoang @hellow554 @imtsuki @kanru @KarlK90 @LeSeulArtichaut @MAdrianMattocks @matheus-consoli @mental32 @nmccarty @Noah-Kennedy @pard68 @PeytonT @pierreN @Redblueflame @RobbieClarken @RobertoSnap @robjtede @SarthakSingh31 @senden9 @shekohex @sinato @spastorino @turboladen @woshilapin @yerke

@tmandry tmandry added this to On deck in wg-async work via automation Aug 25, 2020
@tmandry tmandry added the AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. label Aug 25, 2020
@spastorino spastorino added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Aug 26, 2020
@spastorino
Copy link
Member

spastorino commented Aug 26, 2020

Adding T-compiler label so this can be picked up by our automated prioritization tool.

@sazzer
Copy link
Author

sazzer commented Aug 27, 2020

Just an FYI that my CI build just auto-updated to the new stable 1.46.0 and this is now happening there too.

A build at 0822 BST worked successfully on rustc 1.45.2, and a build at 2033 BST failed, now using rustc 1.46.0. There were zero changes to the Rust code between those two builds, and the only difference is that because I'm using actions-rs/toolchain@v1 which auto-installs the latest Stable then it built with a different version of the compiler.

Cheers

@Skepfyr
Copy link

Skepfyr commented Aug 29, 2020

Minimized (as best I could) to:

use actix_web::{web::Data, HttpResponse};
use tokio_postgres::Client;

pub trait HttpServiceFactory {
    fn register(self);
}
pub struct Complete;
impl HttpServiceFactory for Complete {
    fn register(self) {
        pub async fn complete(client: Data<Client>) -> HttpResponse {
            async { create(&client).await }.await;
            HttpResponse::NotFound().finish()
        }
        actix_web::Resource::new("/").to(complete);
    }
}
async fn create(client: &Client) {
    async move {
        get_by_authentication(client).await;
        client.query_one("", &[]).await.unwrap();
    }
    .await
}
async fn get_by_authentication(client: &Client) {
    async move {
        client.query_one("", &[]).await.unwrap();
    }
    .await
}

Bisected to find the regression in #75443, all of which suggests it's another take on #75992.

@jyn514 jyn514 removed E-needs-mcve Call for participation: This issue needs a Minimal Complete and Verifiable Example E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc labels Aug 29, 2020
@jyn514
Copy link
Member

jyn514 commented Sep 2, 2020

Closing as duplicate of #75992

@jyn514 jyn514 closed this as completed Sep 2, 2020
wg-async work automation moved this from On deck to Done Sep 2, 2020
@jyn514 jyn514 removed the I-prioritize Indicates that prioritization has been requested for this issue label Sep 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-async-await Area: Async & Await AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. C-bug Category: This is a bug. I-hang Issue: The compiler never terminates, due to infinite loops, deadlock, livelock, etc. ICEBreaker-Cleanup-Crew Helping to "clean up" bugs with minimal examples and bisections regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
Development

No branches or pull requests

7 participants