Skip to content

Commit e6df7b2

Browse files
committed
Cap the maximum hard links per file
NTFS has a limit of 1023 hard links per file, and we have no guarantees about garbage collection latency on the disk. After a conservative number of links, make a new file to link to and use that instead.
1 parent c69c799 commit e6df7b2

File tree

1 file changed

+52
-28
lines changed

1 file changed

+52
-28
lines changed

tests/mock/clitools.rs

Lines changed: 52 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use std::fs;
99
use std::io::{self, Write};
1010
use std::path::{Path, PathBuf};
1111
use std::process::Command;
12-
use std::sync::Arc;
12+
use std::sync::{Arc, RwLock, RwLockWriteGuard};
1313
use std::time::Instant;
1414

1515
use enum_map::{enum_map, Enum, EnumMap};
@@ -117,6 +117,15 @@ struct ConstState {
117117
const_dist_dir: tempfile::TempDir,
118118
}
119119

120+
/// The lock to be used when creating test environments.
121+
///
122+
/// Essentially we use this in `.read()` mode to gate access to `fork()`
123+
/// new subprocesses, and in `.write()` mode to gate creation of new test
124+
/// environments. In doing this we can ensure that new test environment creation
125+
/// does not result in ETXTBSY because the FDs in question happen to be in
126+
/// newly `fork()`d but not yet `exec()`d subprocesses of other tests.
127+
pub static CMD_LOCK: Lazy<RwLock<usize>> = Lazy::new(|| RwLock::new(0));
128+
120129
impl ConstState {
121130
fn new(const_dist_dir: tempfile::TempDir) -> Self {
122131
Self {
@@ -187,9 +196,9 @@ pub fn setup_test_state(test_dist_dir: tempfile::TempDir) -> (tempfile::TempDir,
187196
}
188197

189198
let current_exe_path = env::current_exe().unwrap();
190-
let mut exe_dir = current_exe_path.parent().unwrap();
191-
if exe_dir.ends_with("deps") {
192-
exe_dir = exe_dir.parent().unwrap();
199+
let mut built_exe_dir = current_exe_path.parent().unwrap();
200+
if built_exe_dir.ends_with("deps") {
201+
built_exe_dir = built_exe_dir.parent().unwrap();
193202
}
194203
let test_dir = rustup_test::test_dir().unwrap();
195204

@@ -226,17 +235,47 @@ pub fn setup_test_state(test_dist_dir: tempfile::TempDir) -> (tempfile::TempDir,
226235
test_root_dir: test_dir.path().to_path_buf(),
227236
};
228237

229-
let build_path = exe_dir.join(format!("rustup-init{EXE_SUFFIX}"));
238+
let build_path = built_exe_dir.join(format!("rustup-init{EXE_SUFFIX}"));
230239

231240
let rustup_path = config.exedir.join(format!("rustup{EXE_SUFFIX}"));
232-
let setup_path = config.exedir.join(format!("rustup-init{EXE_SUFFIX}"));
241+
// Used to create dist servers. Perhaps should only link when needed?
242+
let init_path = config.exedir.join(format!("rustup-init{EXE_SUFFIX}"));
233243
let rustc_path = config.exedir.join(format!("rustc{EXE_SUFFIX}"));
234244
let cargo_path = config.exedir.join(format!("cargo{EXE_SUFFIX}"));
235245
let rls_path = config.exedir.join(format!("rls{EXE_SUFFIX}"));
236246
let rust_lldb_path = config.exedir.join(format!("rust-lldb{EXE_SUFFIX}"));
237247

238-
hard_link(build_path, &rustup_path).unwrap();
239-
hard_link(&rustup_path, setup_path).unwrap();
248+
const ESTIMATED_LINKS_PER_TEST: usize = 6 * 2;
249+
// NTFS has a limit of 1023 links per file; test setup creates 6 links, and
250+
// then some tests will link the cached installer to rustup/cargo etc,
251+
// adding more links
252+
const MAX_TESTS_PER_RUSTUP_EXE: usize = 1023 / ESTIMATED_LINKS_PER_TEST;
253+
// This returning-result inner structure allows failures without poisoning
254+
// cmd_lock.
255+
{
256+
fn link_or_copy(
257+
original: &Path,
258+
link: &Path,
259+
lock: &mut RwLockWriteGuard<usize>,
260+
) -> io::Result<()> {
261+
**lock += 1;
262+
if **lock < MAX_TESTS_PER_RUSTUP_EXE {
263+
hard_link(original, link)
264+
} else {
265+
// break the *original* so new tests form a new distinct set of
266+
// links. Do this by copying to link, breaking the source,
267+
// linking back.
268+
**lock = 0;
269+
fs::copy(original, link)?;
270+
fs::remove_file(original)?;
271+
hard_link(link, original)
272+
}
273+
}
274+
let mut lock = CMD_LOCK.write().unwrap();
275+
link_or_copy(&build_path, &rustup_path, &mut lock)
276+
}
277+
.unwrap();
278+
hard_link(&rustup_path, init_path).unwrap();
240279
hard_link(&rustup_path, rustc_path).unwrap();
241280
hard_link(&rustup_path, cargo_path).unwrap();
242281
hard_link(&rustup_path, rls_path).unwrap();
@@ -285,7 +324,8 @@ fn create_local_update_server(self_dist: &Path, exedir: &Path, version: &str) ->
285324

286325
fs::create_dir_all(dist_dir).unwrap();
287326
output_release_file(self_dist, "1", version);
288-
// TODO: should this hardlink?
327+
// TODO: should this hardlink since the modify-codepath presumes it has to
328+
// link break?
289329
fs::copy(rustup_bin, dist_exe).unwrap();
290330

291331
let root_url = format!("file://{}", self_dist.display());
@@ -693,7 +733,7 @@ impl Config {
693733

694734
let mut retries = 8;
695735
let out = loop {
696-
let lock = cmd_lock().read().unwrap();
736+
let lock = CMD_LOCK.read().unwrap();
697737
let out = cmd.output();
698738
drop(lock);
699739
match out {
@@ -783,22 +823,6 @@ pub fn env<E: rustup_test::Env>(config: &Config, cmd: &mut E) {
783823
config.env(cmd)
784824
}
785825

786-
use std::sync::RwLock;
787-
788-
/// Returns the lock to be used when creating test environments.
789-
///
790-
/// Essentially we use this in `.read()` mode to gate access to `fork()`
791-
/// new subprocesses, and in `.write()` mode to gate creation of new test
792-
/// environments. In doing this we can ensure that new test environment creation
793-
/// does not result in ETXTBSY because the FDs in question happen to be in
794-
/// newly `fork()`d but not yet `exec()`d subprocesses of other tests.
795-
pub fn cmd_lock() -> &'static RwLock<()> {
796-
lazy_static! {
797-
static ref LOCK: RwLock<()> = RwLock::new(());
798-
};
799-
&LOCK
800-
}
801-
802826
fn allow_inprocess<I, A>(name: &str, args: I) -> bool
803827
where
804828
I: IntoIterator<Item = A>,
@@ -1514,7 +1538,7 @@ fn create_custom_toolchains(customdir: &Path) {
15141538
}
15151539
}
15161540

1517-
pub fn hard_link<A, B>(a: A, b: B) -> io::Result<()>
1541+
pub fn hard_link<A, B>(original: A, link: B) -> io::Result<()>
15181542
where
15191543
A: AsRef<Path>,
15201544
B: AsRef<Path>,
@@ -1526,5 +1550,5 @@ where
15261550
}
15271551
fs::hard_link(a, b).map(drop)
15281552
}
1529-
inner(a.as_ref(), b.as_ref())
1553+
inner(original.as_ref(), link.as_ref())
15301554
}

0 commit comments

Comments
 (0)