Flush executables to disk after linkage

[nabijaczleweli]

A problem caused by not doing so in Chrome has been reported here.

File::sync_all() calls FlushFileBuffers() down the line, causing potentially unflushed buffers on high I/O-load systems to flush and preventing nasty non-reproducible bugs.

Closes #48545

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @michaelwoerister (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[michaelwoerister]

Thanks for the PR, @nabijaczleweli!

Could you please

• check that the process exited successfully,
• only do this on Windows,
• and add a comment on why we are doing this?

[nabijaczleweli]

Done

[michaelwoerister]

Looks great now, thank you! Please squash the commits into one, then this should be good to go.

[nabijaczleweli]

Done

[ollie27]

This would also need to be done here:

rust/src/librustc_trans/back/link.rs

Lines 837 to 838 in ae544ee

	match cmd.command().stdout(Stdio::piped()).stderr(Stdio::piped()).spawn() {
	Ok(child) => return child.wait_with_output(),

Although I'm not convinced that it wouldn't be better to rely on the linker to perform this mitigation.

[nabijaczleweli]

Done. As for "the linker" doing so – probably a good idea, but unreliable (henlo systems running 5-year-old ld).

[michaelwoerister]

Thanks @nabijaczleweli and @ollie27!

@bors r+

[bors]

📌 Commit 3787106 has been approved by michaelwoerister

[bors]

⌛ Testing commit 3787106 with merge 64f93f31b12940a6a4e083191fbaf5c1a479106a...

[bors]

💔 Test failed - status-appveyor

[nabijaczleweli]

Error seems to've originated outside of this PR's scope 🤔

[ollie27]

FlushFileBuffers requires that the file is opened with write permissions but fs::File::open is read-only.

[nabijaczleweli]

I was aware of that logically, but it's slipped my fingers. Anyway, fixed by using OpenOptions::write(true).

[michaelwoerister]

@bors r+

[bors]

📌 Commit e1d3c47 has been approved by michaelwoerister