Miscompilation when using ..Default::default()

[ISibboI]

When initializing a struct using ..Default::default() instead of writing out the struct, the compiler produces erroneous code.

This happens in the integration tests of a project of mine. When constructing the struct without the shorthand (but simply setting the values to their defaults), the integration tests fail in an expected way. If I use the shorthand ..Default::default(), the integration tests fail to open a TCP connection with an OsError 24, "too many open files". This appears to be reproducible, and other applications on my system don't complain about too many open files. So I suspect that the system could open more files, but the application has hit its limit (which it should not, since it should open just one TCP connection). I am running a standard ubuntu installation on a Thinkpad T480s.

When I first discovered the bug I got a stack overflow before any integration test could start. Even after a cargo clean.

I remember getting problems with the ..Default::default() shorthand even earlier when compiling a WASM library a year ago or so as well. Unfortunately though I do not have that project anymore.

Relevant code excerpt from here (not minimal example, sorry):

use reqwest::blocking::Client;

struct ClientWithCookies {
    client: Client,
    cookie: Option<String>,
}

impl Default for ClientWithCookies {
    fn default() -> Self {
        Self {
            client: Client::new(),
            // swap the two lines below to enable the error
            // ..Default::default()
            cookie: None,
        }
    }
}

This is the error I am getting on nightly when NOT using ..Default::default():

$ cargo +nightly test  --package rvoc-backend --test end_to_end test_signup_and_login
    Finished test [unoptimized + debuginfo] target(s) in 0.11s
     Running tests/end_to_end.rs (target/debug/deps/end_to_end-400bfc8a0b0dc22a)

running 1 test
test test_signup_and_login ... FAILED

failures:

---- test_signup_and_login stdout ----
Start test_signup_and_login
thread 'test_signup_and_login' panicked at 'called `Result::unwrap()` on an `Err` value: reqwest::Error { kind: Request, url: Url { scheme: "http", cannot_be_a_base: false, username: "", password: None, host: Some(Domain("localhost")), port: Some(2374), path: "/api/signup", query: None, fragment: None }, source: hyper::Error(Connect, ConnectError("tcp connect error", Os { code: 111, kind: ConnectionRefused, message: "Connection refused" })) }', tests/end_to_end.rs:60:10
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    test_signup_and_login

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 2 filtered out; finished in 0.01s

error: test failed, to rerun pass '-p rvoc-backend --test end_to_end'

And here the error when swapping the two lines highlighted above:

$ cargo +nightly test  --package rvoc-backend --test end_to_end test_signup_and_login
    Finished test [unoptimized + debuginfo] target(s) in 0.11s
     Running tests/end_to_end.rs (target/debug/deps/end_to_end-400bfc8a0b0dc22a)

running 1 test
test test_signup_and_login ... FAILED

failures:

---- test_signup_and_login stdout ----
Start test_signup_and_login
thread 'test_signup_and_login' panicked at 'Client::new(): reqwest::Error { kind: Builder, source: Os { code: 24, kind: Uncategorized, message: "Too many open files" } }', /home/sibbo/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/reqwest-0.11.8/src/blocking/client.rs:798:38
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    test_signup_and_login

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 2 filtered out; finished in 3.53s

error: test failed, to rerun pass '-p rvoc-backend --test end_to_end'

The difference between the error messages is obvious, and remember that this happens reproducibly on the same system. I have the repo checked out two times such that I can execute the two variants one after another without needing to wait for compilation. So I am very sure that the "Too many open files" is not caused by the system actually running out of file descriptors or so.

Further notice the difference in execution time. The correct version takes just 0.01 seconds typically, while the erroneous version takes around 3 seconds. This is also reproducible.

How to reproduce

# Check out two versions of the code
git clone --recursive https://github.com/ISibboI/vocabulary-learning-application
cd vocabulary-learning-application
git checkout 51f04c3d17339e3d740b5ee4bcdb0288f66d1f48
cd ..
cp -r vocabulary-learning-application vocabulary-learning-application-miscompilation

# Modify the miscompilation version to produce the miscompilation
sed -i 's\// ..Default::default()\..Default::default()\g' vocabulary-learning-application-miscompilation/backend/rvoc-backend/tests/end_to_end.rs
sed -i 's\cookie: None,\// cookie: None,\g' vocabulary-learning-application-miscompilation/backend/rvoc-backend/tests/end_to_end.rs

# Execute the two variants
# Ok variant
cargo +nightly test --manifest-path vocabulary-learning-application/backend/rvoc-backend/Cargo.toml --package rvoc-backend --test end_to_end test_signup_and_login
# Miscompilation variant
cargo +nightly test --manifest-path vocabulary-learning-application-miscompilation/backend/rvoc-backend/Cargo.toml --package rvoc-backend --test end_to_end test_signup_and_login

You can then arbitrarily rerun the last two commands, with the expected results.

Meta

I executed cargo clean and the problem persists.

The same thing happens on current beta and stable, even when executing cargo clean between switching toolchain.

rustc +nightly --version --verbose:

rustc 1.59.0-nightly (399ba6bb3 2022-01-03)
binary: rustc
commit-hash: 399ba6bb377ce02224b57c4d6e127e160fa76b34
commit-date: 2022-01-03
host: x86_64-unknown-linux-gnu
release: 1.59.0-nightly
LLVM version: 13.0.0

rustc +beta --version --verbose:

rustc 1.58.0-beta.2 (0e07bcb68 2021-12-04)
binary: rustc
commit-hash: 0e07bcb68b82b54c0c4ec6fe076e9d75b02109cf
commit-date: 2021-12-04
host: x86_64-unknown-linux-gnu
release: 1.58.0-beta.2
LLVM version: 13.0.0

rustc +stable --version --verbose:

rustc 1.57.0 (f1edd0429 2021-11-29)
binary: rustc
commit-hash: f1edd0429582dd29cccacaf50fd134b05593bd9c
commit-date: 2021-11-29
host: x86_64-unknown-linux-gnu
release: 1.57.0
LLVM version: 13.0.0

No panic, so no backtrace included.

[CosmicHorrorDev]

Just to note, this isn't a miscompilation. Your definition of default() will recurse infinitely which will normally show up as a stack overflow, but it looks like Client::new() is doing some logic that causes you to have too many open files first instead

[ehuss]

Yea, this is expected behavior. It is recursively calling Client::new() which involves opening some file descriptors and it eventually runs out.

Rustc should warn about this, but it currently doesn't. Closing as a duplicate of #78474.

[ISibboI]

Thanks. I did not actually know that the shorthand is supposed to behave this way.