diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/global_install_concurrent/prepare.mjs b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/global_install_concurrent/prepare.mjs new file mode 100644 index 0000000000..9447fcb902 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/global_install_concurrent/prepare.mjs @@ -0,0 +1,20 @@ +import { access, writeFile } from 'node:fs/promises'; +import { join } from 'node:path'; +import { setTimeout } from 'node:timers/promises'; + +const { VP_TEST_BARRIER: barrier, VP_TEST_PARTICIPANT: participant } = process.env; +if (barrier) { + await writeFile(join(barrier, participant), 'ready'); + const other = participant === 'first' ? 'second' : 'first'; + const deadline = Date.now() + 20_000; + while (true) { + try { + await access(join(barrier, other)); + break; + } catch (error) { + if (error.code !== 'ENOENT') throw error; + if (Date.now() >= deadline) throw new Error('npm preparation was serialized'); + await setTimeout(10); + } + } +} diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/global_install_concurrent/snapshots.toml b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/global_install_concurrent/snapshots.toml new file mode 100644 index 0000000000..242b0fd652 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/global_install_concurrent/snapshots.toml @@ -0,0 +1,8 @@ +[[case]] +name = "global_install_concurrent" +vp = "global" +comment = "Concurrent npm preparation must remain parallel, and activation must leave an executable active installation. Forced replacement must not deadlock." +env = { npm_config_offline = "true", npm_config_audit = "false" } +steps = [ + { argv = ["node", "verify.mjs"], tty = false }, +] diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/global_install_concurrent/snapshots/global_install_concurrent.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/global_install_concurrent/snapshots/global_install_concurrent.md new file mode 100644 index 0000000000..a169d48ffa --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/global_install_concurrent/snapshots/global_install_concurrent.md @@ -0,0 +1,10 @@ +# global_install_concurrent + +Concurrent npm preparation must remain parallel, and activation must leave an executable active installation. Forced replacement must not deadlock. + +## `node verify.mjs` + +``` +Concurrent preparation completed; the active installation and command survived. +Forced replacement and removal completed without deadlock. +``` diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/global_install_concurrent/verify.mjs b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/global_install_concurrent/verify.mjs new file mode 100644 index 0000000000..d79a0b6a40 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/global_install_concurrent/verify.mjs @@ -0,0 +1,64 @@ +import assert from 'node:assert/strict'; +import { spawn } from 'node:child_process'; +import { mkdir, readFile, readdir, writeFile } from 'node:fs/promises'; +import { join, resolve } from 'node:path'; + +const home = process.env.VP_HOME; +const executable = (name) => join(home, 'bin', name + (process.platform === 'win32' ? '.exe' : '')); +const vp = executable('vp'); +const packages = join(home, 'packages'); +const name = 'global-lock-demo'; +const barrier = resolve('barrier'); +await mkdir(barrier); + +async function makePackage(directory, packageName, version) { + await mkdir(directory); + await writeFile(join(directory, 'package.json'), JSON.stringify({ + name: packageName, + version, + bin: { [name]: 'cli.js' }, + scripts: { postinstall: 'node ../prepare.mjs' }, + })); + await writeFile(join(directory, 'cli.js'), `#!/usr/bin/env node\nconsole.log('${version}');\n`); +} + +function run(binary, args, env = {}) { + return new Promise((resolve, reject) => { + const child = spawn(binary, args, { + env: { ...process.env, ...env }, + stdio: ['ignore', 'pipe', 'pipe'], + timeout: 30_000, + killSignal: 'SIGKILL', + }); + let output = ''; + child.stdout.on('data', (chunk) => { output += chunk; }); + child.stderr.on('data', (chunk) => { output += chunk; }); + child.on('error', reject); + child.on('close', (code) => { + if (code === 0) resolve(output.trim()); + else reject(new Error(`${args.join(' ')} exited ${code}: ${output}`)); + }); + }); +} + +await makePackage('first', name, '1.0.0'); +await makePackage('second', name, '2.0.0'); +await Promise.all(['first', 'second'].map((participant) => run(vp, ['install', '-g', `./${participant}`], { + VP_TEST_BARRIER: barrier, + VP_TEST_PARTICIPANT: participant, +}))); + +const metadata = JSON.parse(await readFile(join(packages, `${name}.json`), 'utf8')); +const installations = (await readdir(join(packages, name), { withFileTypes: true })) + .filter((entry) => entry.isDirectory()).map((entry) => entry.name); +assert.deepEqual(installations, [metadata.installId]); +assert.equal(await run(executable(name), []), metadata.version); +console.log('Concurrent preparation completed; the active installation and command survived.'); + +await makePackage('replacement', 'global-lock-replacement', '3.0.0'); +await run(vp, ['install', '-g', '--force', './replacement']); +assert.equal(await run(executable(name), []), '3.0.0'); +await assert.rejects(readFile(join(packages, `${name}.json`)), { code: 'ENOENT' }); +await run(vp, ['uninstall', '-g', 'global-lock-replacement']); +await assert.rejects(readFile(join(packages, 'global-lock-replacement.json')), { code: 'ENOENT' }); +console.log('Forced replacement and removal completed without deadlock.'); diff --git a/crates/vp_global_cli/src/commands/global/bin_lock.rs b/crates/vp_global_cli/src/commands/global/bin_lock.rs new file mode 100644 index 0000000000..30d61a702c --- /dev/null +++ b/crates/vp_global_cli/src/commands/global/bin_lock.rs @@ -0,0 +1,154 @@ +//! Serialize global bin ownership checks, activation, rollback, and cleanup. + +use std::{ + fs::{File, OpenOptions, TryLockError}, + time::{Duration, Instant}, +}; + +use crate::{commands::env::config::get_bin_dir, error::Error}; + +const WAIT: Duration = Duration::from_mins(5); +const POLL_INTERVAL: Duration = Duration::from_millis(50); + +fn open_lock_file() -> Result { + let bin_dir = get_bin_dir()?; + std::fs::create_dir_all(&bin_dir)?; + // Keep the file in place so all writers lock the same inode. + Ok(OpenOptions::new() + .read(true) + .write(true) + .create(true) + .truncate(false) + .open(bin_dir.join(".vp-global-bin.lock"))?) +} + +fn try_lock(file: &File, deadline: Instant) -> Result { + match file.try_lock() { + Ok(()) => Ok(true), + Err(TryLockError::WouldBlock) if Instant::now() < deadline => Ok(false), + Err(TryLockError::WouldBlock) => Err(Error::Other( + "Timed out waiting for another global package operation to finish.".into(), + )), + Err(TryLockError::Error(error)) => Err(error.into()), + } +} + +/// Hold the returned file until all shared-state changes and cleanup finish. +pub(crate) async fn acquire() -> Result { + let file = open_lock_file()?; + let deadline = Instant::now() + WAIT; + while !try_lock(&file, deadline)? { + tokio::time::sleep(POLL_INTERVAL).await; + } + Ok(file) +} + +/// Synchronous counterpart for npm shim dispatch, outside prompts and npm execution. +pub(crate) fn acquire_sync() -> Result { + let file = open_lock_file()?; + let deadline = Instant::now() + WAIT; + while !try_lock(&file, deadline)? { + std::thread::sleep(POLL_INTERVAL); + } + Ok(file) +} + +#[cfg(test)] +mod tests { + use vp_shared::{EnvConfig, env_vars}; + + use super::*; + + #[tokio::test] + async fn async_wait_does_not_block_and_drop_releases_lock() { + let temp = tempfile::tempdir().unwrap(); + EnvConfig::with_vars_async([(env_vars::VP_HOME, temp.path())], |_| async { + let first = acquire_sync().unwrap(); + let second = acquire(); + tokio::pin!(second); + assert!(tokio::time::timeout(Duration::from_millis(150), &mut second).await.is_err()); + drop(first); + let second = + tokio::time::timeout(Duration::from_secs(5), second).await.unwrap().unwrap(); + let contender = open_lock_file().unwrap(); + assert!(try_lock(&contender, Instant::now()).is_err()); + drop(second); + assert!(try_lock(&contender, Instant::now()).unwrap()); + }) + .await; + } + + #[test] + fn lock_child_process() { + let Some(marker_dir) = std::env::var_os("VP_TEST_GLOBAL_LOCK_MARKERS") else { return }; + let marker_dir = std::path::PathBuf::from(marker_dir); + std::fs::write(marker_dir.join("started"), b"").unwrap(); + let _lock = acquire_sync().unwrap(); + std::fs::write(marker_dir.join("acquired"), b"").unwrap(); + let mut byte = [0]; + std::io::Read::read(&mut std::io::stdin(), &mut byte).unwrap(); + } + + #[tokio::test] + async fn serializes_processes_and_releases_lock_after_kill() { + let temp = tempfile::tempdir().unwrap(); + let home = temp.path().join("home"); + let spawn = |name: &str| { + let markers = temp.path().join(name); + std::fs::create_dir_all(&markers).unwrap(); + let child = tokio::process::Command::new(std::env::current_exe().unwrap()) + .args(["--exact", "commands::global::bin_lock::tests::lock_child_process"]) + .env(env_vars::VP_HOME, &home) + .env("VP_TEST_GLOBAL_LOCK_MARKERS", &markers) + .stdin(std::process::Stdio::piped()) + .stdout(std::process::Stdio::null()) + .kill_on_drop(true) + .spawn() + .unwrap(); + (child, markers) + }; + let wait_for = |path: std::path::PathBuf| async move { + tokio::time::timeout(Duration::from_secs(10), async { + while !path.exists() { + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await + .unwrap(); + }; + let (mut first, first_markers) = spawn("first"); + wait_for(first_markers.join("acquired")).await; + let (mut second, second_markers) = spawn("second"); + wait_for(second_markers.join("started")).await; + assert!( + tokio::time::timeout( + Duration::from_millis(150), + wait_for(second_markers.join("acquired")) + ) + .await + .is_err() + ); + first.kill().await.unwrap(); + wait_for(second_markers.join("acquired")).await; + drop(second.stdin.take()); + assert!( + tokio::time::timeout(Duration::from_secs(5), second.wait()) + .await + .unwrap() + .unwrap() + .success() + ); + } + + #[test] + fn contention_times_out_without_taking_lock() { + let temp = tempfile::tempdir().unwrap(); + EnvConfig::with_vars([(env_vars::VP_HOME, temp.path())], |_| { + let _first = acquire_sync().unwrap(); + let second = open_lock_file().unwrap(); + let error = try_lock(&second, Instant::now()).unwrap_err(); + assert!(error.to_string().contains("Timed out waiting")); + assert!(matches!(second.try_lock(), Err(TryLockError::WouldBlock))); + }); + } +} diff --git a/crates/vp_global_cli/src/commands/global/install.rs b/crates/vp_global_cli/src/commands/global/install.rs index 7e753f801e..94f8bdfaca 100644 --- a/crates/vp_global_cli/src/commands/global/install.rs +++ b/crates/vp_global_cli/src/commands/global/install.rs @@ -26,7 +26,7 @@ use crate::{ package_metadata::{PackageMetadata, is_legacy_install_id, is_nested_install_id}, }, global::{ - LEGACY_PACKAGE_MANAGER_PACKAGES, is_local_package_spec, parse_package_spec, + LEGACY_PACKAGE_MANAGER_PACKAGES, bin_lock, is_local_package_spec, parse_package_spec, update_version_spec, }, }, @@ -274,7 +274,6 @@ pub async fn install( progress.finish_and_clear(); // 5. Finalize installed packages. - let mut bin_owners = HashMap::::new(); for (index, (package_name, Package { spec, install })) in packages.into_iter().enumerate() { let lock_file = install_locks.remove(&package_name); let Some(InstalledPackage { @@ -288,6 +287,18 @@ pub async fn install( continue; }; + // Re-read ownership under the shared lock and hold it through rollback and cleanup. + let activation_lock = match bin_lock::acquire().await { + Ok(lock) => lock, + Err(error) => { + let _ = cleanup_failed_install(&install_dir).await; + if first_error.is_none() { + first_error = Some(package_error(&package_name, error)); + } + continue; + } + }; + // Load previous metadata once for stale-bin detection below. let previous_metadata = match PackageMetadata::load(&package_name).await { Ok(metadata) => metadata, @@ -337,13 +348,6 @@ pub async fn install( // 5.1 Detect binary ownership conflicts before writing metadata. for bin_name in &bin_names { - if let Some(owner) = bin_owners.get(bin_name) - && owner != &package_name - { - conflicts.push((bin_name.clone(), owner.clone())); - continue; - } - match BinConfig::load(bin_name).await { Ok(Some(config)) => { if config.package != package_name { @@ -376,7 +380,7 @@ pub async fn install( "Uninstalling {} (conflicts with {})...", pkg, package_name )); - if let Err(error) = Box::pin(uninstall(&pkg, false)).await { + if let Err(error) = uninstall_locked(&pkg, false).await { let _ = cleanup_failed_install(&install_dir).await; if first_error.is_none() { first_error = Some(package_error(&package_name, error)); @@ -484,13 +488,10 @@ pub async fn install( continue; } - for bin_name in &bin_names { - bin_owners.insert(bin_name.clone(), package_name.clone()); - } - // 5.6 Remove stale installations for this package. cleanup_stale_installations(&package_name, &install_id).await; drop(lock_file); + drop(activation_lock); // 5.7 Print success message output::raw(&format!( @@ -1071,6 +1072,12 @@ async fn stale_bin_names_for_package( /// 1. Try to use PackageMetadata for binary list /// 2. Fallback to scanning BinConfig files for orphaned binaries pub async fn uninstall(package_name: &str, dry_run: bool) -> Result<(), Error> { + let _lock = bin_lock::acquire().await?; + uninstall_locked(package_name, dry_run).await +} + +// Called directly by forced activation, which already holds the global bin lock. +async fn uninstall_locked(package_name: &str, dry_run: bool) -> Result<(), Error> { if is_local_package_spec(package_name) { // We can't resolve local packages for uninstall, follow npm's behavior return Err(Error::Other( @@ -1402,6 +1409,110 @@ mod tests { trampoline } + #[tokio::test] + async fn test_uninstall_waits_for_global_activation() { + let temp = tempfile::tempdir().unwrap(); + vp_shared::EnvConfig::with_vars_async([(env_vars::VP_HOME, temp.path())], |_| async { + let metadata = PackageMetadata::new( + "test-package".into(), + "1.0.0".into(), + "22.18.0".into(), + None, + vec![], + HashSet::new(), + "npm".into(), + ); + metadata.save().await.unwrap(); + let bin_dir = get_bin_dir().unwrap(); + std::fs::create_dir_all(&bin_dir).unwrap(); + let lock = OpenOptions::new() + .read(true) + .write(true) + .create(true) + .truncate(false) + .open(bin_dir.join(".vp-global-bin.lock")) + .unwrap(); + lock.lock().unwrap(); + + let removal = uninstall("test-package", false); + tokio::pin!(removal); + assert!(tokio::time::timeout(Duration::from_millis(150), &mut removal).await.is_err()); + assert!(PackageMetadata::load("test-package").await.unwrap().is_some()); + drop(lock); + tokio::time::timeout(Duration::from_secs(5), removal).await.unwrap().unwrap(); + assert!(PackageMetadata::load("test-package").await.unwrap().is_none()); + }) + .await; + } + + #[cfg(unix)] + #[tokio::test] + async fn test_concurrent_installs_prepare_outside_lock_and_keep_active_installation() { + use std::os::unix::fs::PermissionsExt; + + let temp = tempfile::tempdir().unwrap(); + vp_shared::EnvConfig::with_vars_async([(env_vars::VP_HOME, temp.path())], |env| async move { + // A local npm stand-in exercises activation without downloads or lifecycle scripts. + let node_bin = env.dirs.data.join("js_runtime/node/22.18.0/bin"); + std::fs::create_dir_all(&node_bin).unwrap(); + std::fs::write(node_bin.join("node"), b"").unwrap(); + let npm = node_bin.join("npm"); + std::fs::write(&npm, r#"#!/bin/sh +set -eu +package="$npm_config_prefix/lib/node_modules/test-package" +mkdir -p "$package" +printf '%s' '{"name":"test-package","version":"1.0.0","bin":{"test-cli":"cli"}}' > "$package/package.json" +printf '#!/bin/sh\nprintf working' > "$package/cli" +touch "$npm_config_prefix/prepared" +"#).unwrap(); + std::fs::set_permissions(&npm, std::fs::Permissions::from_mode(0o755)).unwrap(); + + let lock = bin_lock::acquire().await.unwrap(); + let specs = vec!["test-package".to_string()]; + let options = || InstallOptions { + node_version: Some("22.18.0"), + force: false, + ignore_scripts: true, + concurrency: 5, + update: false, + }; + let mut installs = Box::pin(async { + tokio::join!(install(&specs, options()), install(&specs, options())) + }); + let package_dir = PackageMetadata::installation_dir_for("test-package", "").unwrap(); + let prepared = async { + loop { + let count = std::fs::read_dir(&package_dir).into_iter().flatten().flatten() + .filter(|entry| entry.path().join("prepared").exists()).count(); + if count == 2 { + break; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + }; + tokio::select! { + _ = &mut installs => panic!("installation finished while activation was locked"), + result = tokio::time::timeout(Duration::from_secs(10), prepared) => result.unwrap(), + } + assert!(tokio::time::timeout(Duration::from_millis(150), &mut installs).await.is_err()); + assert!(PackageMetadata::load("test-package").await.unwrap().is_none()); + drop(lock); + let (first, second) = tokio::time::timeout(Duration::from_secs(10), installs).await.unwrap(); + first.unwrap(); + second.unwrap(); + let active = PackageMetadata::load("test-package").await.unwrap().unwrap(); + let installation = active.installation_dir().unwrap(); + let binary = get_node_modules_dir(&installation, "test-package").join("cli"); + let output = Command::new(binary.as_path()).output().await.unwrap(); + assert!(output.status.success()); + assert_eq!(output.stdout, b"working"); + let installations: Vec<_> = std::fs::read_dir(&package_dir).unwrap() + .map(Result::unwrap).filter(|entry| entry.file_type().unwrap().is_dir()).collect(); + assert_eq!(installations.len(), 1); + assert_eq!(installations[0].path(), installation.as_path()); + }).await; + } + #[test] fn test_default_shims_are_protected() { for shim in crate::shim::DEFAULT_SHIM_TOOLS.iter().chain([&"vp"]) { diff --git a/crates/vp_global_cli/src/commands/global/mod.rs b/crates/vp_global_cli/src/commands/global/mod.rs index 3beef79e6e..5e851fa2e8 100644 --- a/crates/vp_global_cli/src/commands/global/mod.rs +++ b/crates/vp_global_cli/src/commands/global/mod.rs @@ -12,6 +12,7 @@ use vt_path::{AbsolutePathBuf, current_dir}; use crate::{commands::env::config::resolve_version, error::Error}; +pub(crate) mod bin_lock; pub mod install; pub mod outdated; pub mod packages; diff --git a/crates/vp_global_cli/src/shim/dispatch.rs b/crates/vp_global_cli/src/shim/dispatch.rs index 526b99b2b3..97555c7642 100644 --- a/crates/vp_global_cli/src/shim/dispatch.rs +++ b/crates/vp_global_cli/src/shim/dispatch.rs @@ -21,7 +21,10 @@ use crate::{ package_manager, package_metadata::PackageMetadata, }, - global::install::{extract_binaries, is_protected_shim}, + global::{ + bin_lock, + install::{extract_binaries, is_protected_shim}, + }, }, error::Error, }; @@ -234,6 +237,13 @@ fn check_npm_global_install_result( } let is_interactive = vp_shared::is_stdin_terminal(); + let lock = match bin_lock::acquire_sync() { + Ok(lock) => lock, + Err(error) => { + output::error(&error.to_string()); + return; + } + }; // (bin_name, source_path, package_name) let mut missing_bins: Vec<(String, AbsolutePathBuf, String)> = Vec::new(); let mut managed_conflicts: Vec<(String, String)> = Vec::new(); @@ -326,6 +336,9 @@ fn check_npm_global_install_result( } } + // Never hold the lock while waiting for a user to answer the prompt. + drop(lock); + // Deduplicate by bin_name so that when two packages declare the same binary, // only the last one is linked (matching npm's "last writer wins" behavior). let missing_bins = dedup_missing_bins(missing_bins); @@ -364,8 +377,49 @@ fn check_npm_global_install_result( }; if should_link { - for (bin_name, source_path, package_name) in &missing_bins { - create_bin_link(&bin_dir, bin_name, source_path, package_name, node_version); + link_missing_npm_bins(&bin_dir, &missing_bins, node_version); + } +} + +fn link_missing_npm_bins( + bin_dir: &AbsolutePath, + missing_bins: &[(String, AbsolutePathBuf, String)], + node_version: &str, +) { + let _lock = match bin_lock::acquire_sync() { + Ok(lock) => lock, + Err(error) => { + output::error(&error.to_string()); + return; + } + }; + for (bin_name, source_path, package_name) in missing_bins { + // A different operation may have claimed this name while the prompt was open. + match BinConfig::load_sync(bin_name) { + Ok(Some(config)) if config.source == BinSource::Vp => { + output::warn(&vt_str::format!( + "Skipped '{bin_name}': managed by `vp install -g {}`.", + config.package + )); + continue; + } + Err(error) => { + output::error(&error.to_string()); + continue; + } + _ => {} + } + if std::fs::symlink_metadata(bin_dir.join(bin_name)).is_ok() { + continue; + } + #[cfg(windows)] + if ["exe", "cmd"].iter().any(|extension| { + std::fs::symlink_metadata(bin_dir.join(format!("{bin_name}.{extension}"))).is_ok() + }) { + continue; + } + if source_path.as_path().exists() { + create_bin_link(bin_dir, bin_name, source_path, package_name, node_version); } } } @@ -469,6 +523,13 @@ fn remove_npm_global_uninstall_links( npm_prefix: &AbsolutePath, ) { let Ok(bin_dir) = config::get_bin_dir() else { return }; + let _lock = match bin_lock::acquire_sync() { + Ok(lock) => lock, + Err(error) => { + output::error(&error.to_string()); + return; + } + }; for (bin_name, package_name, package_dir) in bin_entries { // Skip protected shims: a stale Npm BinConfig must not let @@ -1829,6 +1890,41 @@ mod tests { }); } + #[test] + fn test_npm_link_rechecks_owner_after_prompt() { + let temp = TempDir::new().unwrap(); + vp_shared::EnvConfig::with_vars([(env_vars::VP_HOME, temp.path())], |_| { + let bin_dir = config::get_bin_dir().unwrap(); + std::fs::create_dir_all(&bin_dir).unwrap(); + let source = bin_dir.join("source.js"); + std::fs::write(&source, "console.log('test');").unwrap(); + let candidates = vec![("test-cli".into(), source, "npm-package".into())]; + + // Another activation claims the name while npm's link prompt is open. + BinConfig::new( + "test-cli".into(), + "managed-package".into(), + "1.0.0".into(), + "22.18.0".into(), + ) + .save_sync() + .unwrap(); + link_missing_npm_bins(&bin_dir, &candidates, "22.18.0"); + assert_eq!( + BinConfig::load_sync("test-cli").unwrap().unwrap().package, + "managed-package" + ); + for name in ["test-cli", "test-cli.cmd", "test-cli.exe"] { + assert!(std::fs::symlink_metadata(bin_dir.join(name)).is_err()); + } + + BinConfig::delete_sync("test-cli").unwrap(); + link_missing_npm_bins(&bin_dir, &candidates, "22.18.0"); + assert_eq!(BinConfig::load_sync("test-cli").unwrap().unwrap().package, "npm-package"); + assert!(bin_dir.join("test-cli").as_path().exists()); + }); + } + #[test] fn test_npm_uninstall_preserves_links_until_package_is_removed() { let temp = TempDir::new().unwrap();