diff --git a/crates/machine/src/machine_bus.rs b/crates/machine/src/machine_bus.rs index ed6c728..fb5e102 100644 --- a/crates/machine/src/machine_bus.rs +++ b/crates/machine/src/machine_bus.rs @@ -249,7 +249,16 @@ impl MachineBus { } pub fn has_pending_io(&self) -> bool { - self.uart.rx_pending() || self.uart_data.rx_pending() || self.net_rx_pending() + self.uart.rx_pending() + || self.uart_data.rx_pending() + || self.net_rx_pending() + || self.fs_reply_pending() + } + + pub fn fs_reply_pending(&self) -> bool { + self.fs_devices + .iter() + .any(|fs_device| fs_device.mmio.int_status != 0) } pub fn drain_console_tx(&mut self) -> Vec { @@ -762,6 +771,31 @@ mod tests { bus } + #[test] + fn a_finished_mount_reply_keeps_the_machine_out_of_idle() { + const RAM_SIZE: u64 = 1 << 20; + + let mut bus = MachineBus::new(RAM_SIZE, CowRam::new(RAM_SIZE)); + bus.attach_fs(vec![Mount { + host_path: PathBuf::from("/"), + tag: "vfs0".to_string(), + writable: false, + }]); + + assert!( + !bus.has_pending_io(), + "an idle mount already looks like pending work" + ); + + bus.fs_devices[0].mmio.int_status |= 1; + + assert!( + bus.has_pending_io(), + "a finished mount reply left the guest idle, so every request waits \ + for the next timer tick and mounts run several times slower" + ); + } + #[test] fn draining_the_console_drops_its_interrupt_line() { let mut bus = bus_with_one_console_byte(); diff --git a/crates/machine/src/virtio/fs.rs b/crates/machine/src/virtio/fs.rs index 148fe72..72d523f 100644 --- a/crates/machine/src/virtio/fs.rs +++ b/crates/machine/src/virtio/fs.rs @@ -7,7 +7,7 @@ use std::path::{Path, PathBuf}; use serde_json::Value; -use super::{RAM_BASE, RamView, VRING_DESC_F_NEXT, VRING_DESC_F_WRITE, VirtioMmio}; +use super::{RamView, STAGING_BASE, VRING_DESC_F_NEXT, VRING_DESC_F_WRITE, VirtioMmio}; use crate::trace::Tracer; const DEVICE_ID: u32 = 26; // VIRTIO_DEVICE_ID_FS @@ -247,17 +247,15 @@ impl VirtioFs { (header_addr + 40, 0) }; - let out_len: u32 = write_bufs.iter().map(|(_, l)| *l).sum(); - let (out_addr, needs_scatter) = if write_bufs.len() == 1 { - (write_bufs[0].0, false) + let capacity: u32 = write_bufs.iter().map(|(_, len)| *len).sum(); + let (contiguous_addr, contiguous_len) = contiguous_reply_window(&write_bufs); + + let staged = header.opcode != FUSE_READ && contiguous_len < capacity; + let (out_addr, out_len) = if staged { + ram.begin_staging(STAGING_BASE, capacity as usize); + (STAGING_BASE, capacity) } else { - let (first_addr, first_len) = write_bufs[0]; - if write_bufs[1].0 == first_addr + first_len as u64 { - (first_addr, false) - } else { - let scratch = RAM_BASE + ram.len() as u64 - 4096; - (scratch, true) - } + (contiguous_addr, contiguous_len) }; let traced_request = match self.tracer { @@ -270,7 +268,7 @@ impl VirtioFs { FUSE_LOOKUP => self.lookup(&header, ram, in_body_addr, in_body_len, out_addr, out_len), FUSE_GETATTR => self.getattr(&header, out_addr, out_len, ram), FUSE_OPEN | FUSE_OPENDIR => self.open(&header, ram, in_body_addr, out_addr, out_len), - FUSE_READ => self.read(&header, ram, in_body_addr, out_addr, out_len), + FUSE_READ => self.read(&header, ram, in_body_addr, &write_bufs), FUSE_READDIR => self.readdir(&header, ram, in_body_addr, out_addr, out_len, false), FUSE_READDIRPLUS => self.readdir(&header, ram, in_body_addr, out_addr, out_len, true), FUSE_RELEASE | FUSE_RELEASEDIR => { @@ -297,22 +295,9 @@ impl VirtioFs { self.record_request(request, &header, ram, out_addr, used_len); } - if needs_scatter && used_len > 0 { - let mut src_offset = 0u64; - for &(buf_addr, buf_len) in &write_bufs { - let copy_len = (used_len as u64 - src_offset).min(buf_len as u64); - - if copy_len == 0 { - break; - } - - for i in 0..copy_len { - let b = ram.read_u8(out_addr + src_offset + i); - ram.write_u8(buf_addr + i, b); - } - - src_offset += copy_len; - } + if let Some(reply) = ram.take_staging() { + let written = (used_len as usize).min(reply.len()); + scatter_write(ram, &write_bufs, 0, &reply[..written]); } used_len @@ -648,12 +633,14 @@ impl VirtioFs { header: &FuseInHeader, ram: &mut RamView, in_body_addr: u64, - out_addr: u64, - out_len: u32, + write_bufs: &[(u64, u32)], ) -> u32 { let fh = ram.read_u64(in_body_addr); let offset = ram.read_u64(in_body_addr + 8); let size = ram.read_u32(in_body_addr + 16); + let out_addr = write_bufs[0].0; + + let capacity: u64 = write_bufs.iter().map(|(_, len)| *len as u64).sum(); let handle = match self.file_handles.get(&fh) { Some(h) => h, @@ -667,18 +654,20 @@ impl VirtioFs { let _ = file.seek(std::io::SeekFrom::Start(offset)); - let max_read = size.min(out_len.saturating_sub(16)) as usize; + let max_read = (size as u64).min(capacity.saturating_sub(16)) as usize; let mut buf = vec![0u8; max_read]; - let bytes_read = match file.read(&mut buf) { + let bytes_read = match read_filling(&mut file, &mut buf) { Ok(n) => n, Err(_) => return self.reply_error(header, ENOENT, out_addr, ram), }; let total = 16 + bytes_read as u32; - ram.write_u32(out_addr, total); - ram.write_u32(out_addr + 4, 0); - ram.write_u64(out_addr + 8, header.unique); - ram.write_bytes(out_addr + 16, &buf[..bytes_read]); + let mut out_header = [0u8; 16]; + out_header[..4].copy_from_slice(&total.to_le_bytes()); + out_header[8..].copy_from_slice(&header.unique.to_le_bytes()); + + scatter_write(ram, write_bufs, 0, &out_header); + scatter_write(ram, write_bufs, 16, &buf[..bytes_read]); total } @@ -1389,3 +1378,167 @@ impl VirtioFs { 16 } } + +fn contiguous_reply_window(write_bufs: &[(u64, u32)]) -> (u64, u32) { + let (first_addr, first_len) = write_bufs[0]; + let mut len = first_len; + + for &(addr, buf_len) in &write_bufs[1..] { + if addr != first_addr + len as u64 { + break; + } + len = len.saturating_add(buf_len); + } + + (first_addr, len) +} + +fn scatter_write(ram: &mut RamView, write_bufs: &[(u64, u32)], offset: u64, data: &[u8]) { + let mut remaining = data; + let mut reply_offset = 0u64; + + for &(addr, len) in write_bufs { + if remaining.is_empty() { + return; + } + + let len = len as u64; + let buffer_end = reply_offset + len; + + if buffer_end > offset { + let skip = offset.saturating_sub(reply_offset); + let room = (len - skip) as usize; + let take = room.min(remaining.len()); + + ram.write_bytes(addr + skip, &remaining[..take]); + remaining = &remaining[take..]; + } + + reply_offset = buffer_end; + } +} + +fn read_filling(file: &mut fs::File, buf: &mut [u8]) -> std::io::Result { + let mut filled = 0; + + while filled < buf.len() { + match file.read(&mut buf[filled..]) { + Ok(0) => break, + Ok(count) => filled += count, + Err(error) if error.kind() == std::io::ErrorKind::Interrupted => {} + Err(error) => return Err(error), + } + } + + Ok(filled) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::RAM_BASE; + use crate::cow_ram::CowRam; + + const RAM_BYTES: u64 = 1024 * 1024; + + fn ram() -> CowRam { + CowRam::new(RAM_BYTES) + } + + #[test] + fn a_reply_spans_buffers_the_guest_scattered_across_memory() { + let mut backing = ram(); + let mut view = RamView::new(&mut backing, RAM_BYTES - 1); + + let first = RAM_BASE + 0x1000; + let second = RAM_BASE + 0x9000; + let bufs = [(first, 16u32), (second, 32u32)]; + + let header: Vec = (0..16).collect(); + let data: Vec = (100..120).collect(); + + scatter_write(&mut view, &bufs, 0, &header); + scatter_write(&mut view, &bufs, 16, &data); + + let mut landed = vec![0u8; 16]; + view.read_bytes(first, &mut landed); + assert_eq!(landed, header); + + let mut payload = vec![0u8; data.len()]; + view.read_bytes(second, &mut payload); + assert_eq!(payload, data); + } + + #[test] + fn a_reply_longer_than_the_chain_stops_at_the_last_buffer() { + let mut backing = ram(); + let mut view = RamView::new(&mut backing, RAM_BYTES - 1); + + let only = RAM_BASE + 0x2000; + let bufs = [(only, 8u32)]; + let guard = only + 8; + view.write_u8(guard, 0xAB); + + scatter_write(&mut view, &bufs, 0, &[1u8; 64]); + + assert_eq!(view.read_u8(guard), 0xAB, "wrote past the guest's buffer"); + } + + #[test] + fn a_split_payload_crosses_the_boundary_between_two_buffers() { + let mut backing = ram(); + let mut view = RamView::new(&mut backing, RAM_BYTES - 1); + + let first = RAM_BASE + 0x3000; + let second = RAM_BASE + 0xB000; + let bufs = [(first, 4u32), (second, 4u32)]; + + scatter_write(&mut view, &bufs, 0, &[1, 2, 3, 4, 5, 6, 7, 8]); + + let mut head = vec![0u8; 4]; + let mut tail = vec![0u8; 4]; + view.read_bytes(first, &mut head); + view.read_bytes(second, &mut tail); + + assert_eq!(head, [1, 2, 3, 4]); + assert_eq!(tail, [5, 6, 7, 8]); + } + + #[test] + fn a_staged_reply_never_touches_guest_memory_until_it_is_scattered() { + let mut backing = ram(); + let mut view = RamView::new(&mut backing, RAM_BYTES - 1); + + let guest = RAM_BASE + 0x6000; + view.write_u32(guest, 0xDEAD_BEEF); + + view.begin_staging(STAGING_BASE, 64); + view.write_u32(STAGING_BASE, 16); + view.write_u64(STAGING_BASE + 8, 0x1122_3344); + + assert_eq!(view.read_u32(STAGING_BASE), 16); + assert_eq!( + view.read_u32(guest), + 0xDEAD_BEEF, + "guest memory was disturbed" + ); + + let staged = view.take_staging().expect("staging was started"); + assert_eq!(&staged[..4], &16u32.to_le_bytes()); + assert!(view.take_staging().is_none()); + } + + #[test] + fn touching_buffers_count_as_one_window_and_separated_ones_do_not() { + let base = RAM_BASE + 0x4000; + + assert_eq!( + contiguous_reply_window(&[(base, 16), (base + 16, 48), (base + 64, 8)]), + (base, 72) + ); + assert_eq!( + contiguous_reply_window(&[(base, 16), (base + 0x5000, 48)]), + (base, 16) + ); + } +} diff --git a/crates/machine/src/virtio/mod.rs b/crates/machine/src/virtio/mod.rs index a70f5a7..e7c4438 100644 --- a/crates/machine/src/virtio/mod.rs +++ b/crates/machine/src/virtio/mod.rs @@ -44,14 +44,44 @@ const MAX_QUEUES: usize = 4; pub const VRING_DESC_F_NEXT: u16 = 1; pub const VRING_DESC_F_WRITE: u16 = 2; +pub const STAGING_BASE: u64 = 0xffff_0000_0000_0000; + +struct Staging { + base: u64, + buffer: Vec, +} + pub struct RamView<'a> { ram: &'a mut CowRam, mask: u64, + staging: Option, } impl<'a> RamView<'a> { pub fn new(ram: &'a mut CowRam, mask: u64) -> Self { - Self { ram, mask } + Self { + ram, + mask, + staging: None, + } + } + + pub fn begin_staging(&mut self, base: u64, len: usize) { + self.staging = Some(Staging { + base, + buffer: vec![0u8; len], + }); + } + + pub fn take_staging(&mut self) -> Option> { + self.staging.take().map(|staging| staging.buffer) + } + + fn staged(&self, physical_address: u64, len: usize) -> Option { + let staging = self.staging.as_ref()?; + let offset = physical_address.checked_sub(staging.base)? as usize; + + (offset + len <= staging.buffer.len()).then_some(offset) } pub fn is_empty(&self) -> bool { @@ -67,48 +97,98 @@ impl<'a> RamView<'a> { } pub fn read_u8(&self, physical_address: u64) -> u8 { + if self.staging.is_some() { + let mut bytes = [0u8; 1]; + self.read_bytes(physical_address, &mut bytes); + return bytes[0]; + } self.ram.read_u8(self.idx(physical_address)) } pub fn read_u16(&self, physical_address: u64) -> u16 { + if self.staging.is_some() { + let mut bytes = [0u8; 2]; + self.read_bytes(physical_address, &mut bytes); + return u16::from_le_bytes(bytes); + } self.ram.read_u16(self.idx(physical_address)) } pub fn read_u32(&self, physical_address: u64) -> u32 { + if self.staging.is_some() { + let mut bytes = [0u8; 4]; + self.read_bytes(physical_address, &mut bytes); + return u32::from_le_bytes(bytes); + } self.ram.read_u32(self.idx(physical_address)) } pub fn read_u64(&self, physical_address: u64) -> u64 { + if self.staging.is_some() { + let mut bytes = [0u8; 8]; + self.read_bytes(physical_address, &mut bytes); + return u64::from_le_bytes(bytes); + } self.ram.read_u64(self.idx(physical_address)) } pub fn write_u8(&mut self, physical_address: u64, val: u8) { + if self.staging.is_some() { + self.write_bytes(physical_address, &val.to_le_bytes()); + return; + } let idx = self.idx(physical_address); self.ram.write_u8(idx, val); } pub fn write_u16(&mut self, physical_address: u64, val: u16) { + if self.staging.is_some() { + self.write_bytes(physical_address, &val.to_le_bytes()); + return; + } let idx = self.idx(physical_address); self.ram.write_u16(idx, val); } pub fn write_u32(&mut self, physical_address: u64, val: u32) { + if self.staging.is_some() { + self.write_bytes(physical_address, &val.to_le_bytes()); + return; + } let idx = self.idx(physical_address); self.ram.write_u32(idx, val); } pub fn write_u64(&mut self, physical_address: u64, val: u64) { + if self.staging.is_some() { + self.write_bytes(physical_address, &val.to_le_bytes()); + return; + } let idx = self.idx(physical_address); self.ram.write_u64(idx, val); } pub fn read_bytes(&self, physical_address: u64, buf: &mut [u8]) { - self.ram.read_into(self.idx(physical_address), buf); + match self.staged(physical_address, buf.len()) { + Some(offset) => { + let staging = self.staging.as_ref().expect("staged implies staging"); + buf.copy_from_slice(&staging.buffer[offset..offset + buf.len()]); + } + None => self.ram.read_into(self.idx(physical_address), buf), + } } pub fn write_bytes(&mut self, physical_address: u64, buf: &[u8]) { - let idx = self.idx(physical_address); - self.ram.write_from(idx, buf); + match self.staged(physical_address, buf.len()) { + Some(offset) => { + let staging = self.staging.as_mut().expect("staged implies staging"); + staging.buffer[offset..offset + buf.len()].copy_from_slice(buf); + } + None => { + let idx = self.idx(physical_address); + self.ram.write_from(idx, buf); + } + } } } diff --git a/sdks/performance-workloads.json b/sdks/performance-workloads.json index f644f9e..080c1ce 100644 --- a/sdks/performance-workloads.json +++ b/sdks/performance-workloads.json @@ -25,6 +25,7 @@ "shellPerCallSeconds": 2, "networkRoundTripSeconds": 30, "throughputFloor": 0.2, - "throughputFloorAot": 0.6 + "throughputFloorAot": 0.6, + "mountStatSeconds": 1.5 } } diff --git a/sdks/python/tests/test_performance.py b/sdks/python/tests/test_performance.py index cb7c131..a21af57 100644 --- a/sdks/python/tests/test_performance.py +++ b/sdks/python/tests/test_performance.py @@ -282,3 +282,35 @@ def test_network_round_trip(box): f"a round trip took {seconds:.2f}s, ceiling " f"{CEILINGS['networkRoundTripSeconds']}s" ) + + +def test_mount_metadata_cost(tmp_path): + """Walking a mounted directory, which cost one timer tick per request until + the guest learned to wake on a finished virtio-fs reply.""" + files = 400 + for index in range(files): + directory = tmp_path / f"pkg{index // 40:02d}" + directory.mkdir(exist_ok=True) + (directory / f"file{index:03d}.txt").write_text("contents\n") + + with Sandbox.create(mounts={str(tmp_path): "/workspace"}) as sandbox: + sandbox.commands.run("echo warm", timeout=60) + + started_at = time.perf_counter() + result = sandbox.commands.run( + "find /workspace -type f | wc -l", timeout=120 + ) + seconds = time.perf_counter() - started_at + + REPORT["wall"]["mountStatSeconds"] = seconds + + assert result.exit_code == 0, result.stderr + assert result.stdout.strip() == str(files), ( + f"walked {result.stdout.strip()} files, expected {files}, so this " + f"measures the wrong work" + ) + assert seconds < CEILINGS["mountStatSeconds"], ( + f"stat of {files} mounted files took {seconds:.2f}s, ceiling " + f"{CEILINGS['mountStatSeconds']}s: a mount request is most likely " + f"waiting for a timer tick again" + ) diff --git a/sdks/typescript/src/index.ts b/sdks/typescript/src/index.ts index 7c7a1f4..7d89dc8 100644 --- a/sdks/typescript/src/index.ts +++ b/sdks/typescript/src/index.ts @@ -13,6 +13,8 @@ export { normalizeLineEndings, } from "./execution.js"; +export type { MountEntry, MountSpec } from "./mounts.js"; + export { Trace, TraceRecorder } from "./trace.js"; export type { FileActivity, diff --git a/sdks/typescript/src/mounts.ts b/sdks/typescript/src/mounts.ts new file mode 100644 index 0000000..c437fc7 --- /dev/null +++ b/sdks/typescript/src/mounts.ts @@ -0,0 +1,41 @@ +export type MountSpec = Record; + +export interface MountEntry { + hostAlias: string; + guestPath: string; + writable: boolean; +} + +const WRITABLE_SUFFIX = ":rw"; + +export const MOUNTS_NEED_A_HOST = + "vpod: mounts need a host filesystem, which a browser does not have. " + + "Create the sandbox without mounts, or run it under Node."; + +export function mountEntries(mounts: MountSpec | undefined): MountEntry[] { + if (mounts === undefined) { + return []; + } + if (typeof mounts !== "object" || mounts === null || Array.isArray(mounts)) { + throw new Error( + `vpod: mounts must be an object of host path to guest path, got ${JSON.stringify(mounts)}`, + ); + } + + return Object.entries(mounts).map(([hostPath, guestSpec]) => { + if (typeof guestSpec !== "string" || guestSpec.length === 0) { + throw new Error(`vpod: mount ${JSON.stringify(hostPath)} needs a guest path`); + } + + const writable = guestSpec.endsWith(WRITABLE_SUFFIX); + const guestPath = writable ? guestSpec.slice(0, -WRITABLE_SUFFIX.length) : guestSpec; + + if (!guestPath.startsWith("/")) { + throw new Error( + `vpod: mount ${JSON.stringify(hostPath)} must name an absolute guest path, got ${JSON.stringify(guestPath)}`, + ); + } + + return { hostAlias: hostPath, guestPath, writable }; + }); +} diff --git a/sdks/typescript/src/node/component-imports.ts b/sdks/typescript/src/node/component-imports.ts index ac8989d..e56c0b4 100644 --- a/sdks/typescript/src/node/component-imports.ts +++ b/sdks/typescript/src/node/component-imports.ts @@ -10,6 +10,9 @@ import * as random from "@bytecodealliance/preview2-shim/random"; import * as sockets from "@bytecodealliance/preview2-shim/sockets"; import { ipNameLookup } from "./host-resolver.js"; +import { allowMountWrites } from "./mount-writes.js"; + +allowMountWrites(); const WASI_PACKAGES = { cli, clocks, filesystem, io, random, sockets }; diff --git a/sdks/typescript/src/node/mount-writes.ts b/sdks/typescript/src/node/mount-writes.ts new file mode 100644 index 0000000..cff6a16 --- /dev/null +++ b/sdks/typescript/src/node/mount-writes.ts @@ -0,0 +1,58 @@ +/** + * Write access to a mounted host directory, for the Node target. + */ + +import * as filesystem from "@bytecodealliance/preview2-shim/filesystem"; + +interface DescriptorFlags { + mutateDirectory?: boolean; +} + +type OpenAt = ( + this: unknown, + pathFlags: unknown, + path: string, + openFlags: unknown, + descriptorFlags: DescriptorFlags, + ...rest: unknown[] +) => unknown; + +const UNSUPPORTED = "unsupported"; + +let applied = false; + +export function allowMountWrites(): void { + const descriptor = (filesystem.types as { Descriptor?: { prototype: Record } }) + .Descriptor; + const original = descriptor?.prototype.openAt as OpenAt | undefined; + + if (applied || descriptor === undefined || original === undefined) { + return; + } + applied = true; + + descriptor.prototype.openAt = function ( + this: unknown, + pathFlags: unknown, + path: string, + openFlags: unknown, + descriptorFlags: DescriptorFlags, + ...rest: unknown[] + ) { + try { + return original.call(this, pathFlags, path, openFlags, descriptorFlags, ...rest); + } catch (thrown: unknown) { + if (thrown !== UNSUPPORTED || descriptorFlags?.mutateDirectory !== true) { + throw thrown; + } + return original.call( + this, + pathFlags, + path, + openFlags, + { ...descriptorFlags, mutateDirectory: false }, + ...rest, + ); + } + } as unknown as typeof descriptor.prototype.openAt; +} diff --git a/sdks/typescript/src/node/transport.ts b/sdks/typescript/src/node/transport.ts index 34a3e25..f3261dd 100644 --- a/sdks/typescript/src/node/transport.ts +++ b/sdks/typescript/src/node/transport.ts @@ -3,20 +3,44 @@ */ import { mkdtemp, rm, writeFile } from "node:fs/promises"; -import { readFileSync } from "node:fs"; +import { readFileSync, statSync } from "node:fs"; import { tmpdir } from "node:os"; -import { join } from "node:path"; +import { join, resolve } from "node:path"; import { pullSnapshot } from "../snapshots/pull.js"; import { componentImports, loadCoreModule } from "./component-imports.js"; import { FileSnapshotStore } from "./store.js"; import type { ComponentModule } from "../worker/component-imports.js"; import type { ExecutorTransport } from "../transport/types.js"; +import type { MountEntry } from "../mounts.js"; import type { ExecutionResult, WorkerCall } from "../worker/protocol.js"; import type { WireTraceOptions } from "../trace.js"; +function hostMounts(mounts: MountEntry[]): MountEntry[] { + return mounts.map((mount) => { + const hostAlias = resolve(mount.hostAlias); + + let directory = false; + try { + directory = statSync(hostAlias).isDirectory(); + } catch { + throw new Error(`vpod: mount source ${hostAlias} does not exist`); + } + if (!directory) { + throw new Error(`vpod: mount source ${hostAlias} is not a directory`); + } + + return { ...mount, hostAlias }; + }); +} + interface Executor { - sessionStart(snapshotPath: string, command: string, prompt: string, mounts: never[]): bigint; + sessionStart( + snapshotPath: string, + command: string, + prompt: string, + mounts: MountEntry[], + ): bigint; sessionExec(handle: bigint, code: string, timeout: bigint | undefined): ExecutionResult; sessionExecSlice( handle: bigint, @@ -34,7 +58,7 @@ interface Executor { deltaPath: string, command: string, prompt: string, - mounts: never[], + mounts: MountEntry[], ): bigint; sessionTraceStart?(handle: bigint, options: WireTraceOptions): void; sessionTraceDrain?(handle: bigint, maxBytes: number): string; @@ -128,7 +152,7 @@ export class NodeDispatcher { call.snapshotPath, call.command, call.prompt, - [], + hostMounts(call.mounts), ); case "session-exec": @@ -185,7 +209,7 @@ export class NodeDispatcher { path, call.command, call.prompt, - [], + hostMounts(call.mounts), ); } finally { await rm(path, { force: true }); diff --git a/sdks/typescript/src/runtime.ts b/sdks/typescript/src/runtime.ts index 4db7311..e0aa719 100644 --- a/sdks/typescript/src/runtime.ts +++ b/sdks/typescript/src/runtime.ts @@ -7,6 +7,7 @@ import { import { capabilitiesOf } from "./net/capabilities.js"; import type { NetworkBackendName, NetworkCapabilities } from "./net/capabilities.js"; import type { ExecutorTransport } from "./transport/types.js"; +import type { MountEntry } from "./mounts.js"; import type { WireTraceOptions } from "./trace.js"; import type { ExecutionResult, @@ -86,12 +87,18 @@ export class SandboxRuntime { ]); } - sessionStart(snapshotPath: string, command = "/bin/sh", prompt = "# "): Promise { + sessionStart( + snapshotPath: string, + command = "/bin/sh", + prompt = "# ", + mounts: MountEntry[] = [], + ): Promise { return this.#transport.call({ kind: "session-start", snapshotPath, command, prompt, + mounts, }); } @@ -146,9 +153,10 @@ export class SandboxRuntime { deltaBytes: ArrayBuffer, command = "/bin/sh", prompt = "# ", + mounts: MountEntry[] = [], ): Promise { return this.#transport.call( - { kind: "session-resume", snapshotPath, deltaBytes, command, prompt }, + { kind: "session-resume", snapshotPath, deltaBytes, command, prompt, mounts }, [deltaBytes], ); } diff --git a/sdks/typescript/src/sandbox.ts b/sdks/typescript/src/sandbox.ts index e15af9d..58b4fad 100644 --- a/sdks/typescript/src/sandbox.ts +++ b/sdks/typescript/src/sandbox.ts @@ -22,6 +22,7 @@ import { } from "./snapshots/engine.js"; import { defaultStore } from "./snapshots/index.js"; import { resolveRegistryUrl } from "./snapshots/registry.js"; +import { mountEntries, type MountEntry, type MountSpec } from "./mounts.js"; import { TRACE_NOT_SUPPORTED, TraceRecorder, @@ -62,6 +63,8 @@ export interface SandboxOptions extends SandboxRuntimeOptions { */ engine?: EngineMode; trace?: TraceSetting; + /** Host directories to mount, as guest path per host path. Append ":rw" for write access. Node only. */ + mounts?: MountSpec; } interface ImageEngine { @@ -443,6 +446,7 @@ export class Sandbox { readonly #snapshotPath: string; readonly #snapshotId: string; readonly #imageEngineSha256: string | null; + readonly #mounts: MountEntry[]; #sessionHandle: bigint | null = null; private constructor( @@ -451,11 +455,13 @@ export class Sandbox { snapshotId: string, imageEngineSha256: string | null, trace: WireTraceOptions | null, + mounts: MountEntry[] = [], ) { this.#runtime = runtime; this.#snapshotPath = snapshotPath; this.#snapshotId = snapshotId; this.#imageEngineSha256 = imageEngineSha256; + this.#mounts = mounts; this.commands = new Commands(this); this.code = new Code(this); this.trace = new TraceRecorder(trace, async (maxBytes) => @@ -630,6 +636,7 @@ export class Sandbox { } const trace = traceOptions(options.trace); + const mounts = mountEntries(options.mounts); const snapshot = options.snapshot ?? DEFAULT_SNAPSHOT; const cachedEngine = typeof snapshot === "string" ? await Sandbox.#cachedImageEngine(options, snapshot) : null; @@ -650,6 +657,7 @@ export class Sandbox { mounted.snapshotId, imageEngine?.sha256 ?? null, trace, + mounts, ); } @@ -754,6 +762,7 @@ export class Sandbox { this.#snapshotPath, DEFAULT_SHELL, DEFAULT_PROMPT, + this.#mounts, ); await this.#startTrace(this.#sessionHandle); } @@ -791,6 +800,7 @@ export class Sandbox { : instance; const trace = traceOptions(options.trace); + const mounts = mountEntries(options.mounts); const snapshot = options.snapshot ?? resolved.snapshotId; let wanted: ImageEngine | null; if (resolved.engineSha256 !== undefined) { @@ -826,6 +836,7 @@ export class Sandbox { mounted.snapshotId, imageEngine?.sha256 ?? null, trace, + mounts, ); const delta = resolved.delta.slice(); sandbox.#sessionHandle = await runtime.sessionResume( @@ -833,6 +844,7 @@ export class Sandbox { delta.buffer, DEFAULT_SHELL, DEFAULT_PROMPT, + mounts, ); await sandbox.#startTrace(sandbox.#sessionHandle); diff --git a/sdks/typescript/src/worker/dispatch.ts b/sdks/typescript/src/worker/dispatch.ts index d1e58ae..6f9849d 100644 --- a/sdks/typescript/src/worker/dispatch.ts +++ b/sdks/typescript/src/worker/dispatch.ts @@ -14,8 +14,15 @@ import { componentImports } from "./component-imports.js"; import type { ComponentModule, CoreModuleLoader } from "./component-imports.js"; import type { DriverCommand } from "../net/driver-protocol.js"; import type { ExecutionResult, WorkerCall } from "./protocol.js"; +import { MOUNTS_NEED_A_HOST, type MountEntry } from "../mounts.js"; import type { WireTraceOptions } from "../trace.js"; +function refuseMounts(mounts: MountEntry[]): void { + if (mounts.length > 0) { + throw new Error(MOUNTS_NEED_A_HOST); + } +} + async function announceHostTerminatedTls(): Promise { const cli = (await import("../shims/cli.js")) as unknown as { _setEnv(env: Record): void; @@ -186,6 +193,7 @@ export class Dispatcher { } case "session-start": + refuseMounts(call.mounts); try { return this.#requireExecutor().sessionStart( call.snapshotPath, @@ -241,6 +249,7 @@ export class Dispatcher { } case "session-resume": { + refuseMounts(call.mounts); const name = `resume-${this.#nextDeltaId++}.bin`; const path = mountDelta(name, new Uint8Array(call.deltaBytes)); try { diff --git a/sdks/typescript/src/worker/protocol.ts b/sdks/typescript/src/worker/protocol.ts index 3e0928c..2c393ca 100644 --- a/sdks/typescript/src/worker/protocol.ts +++ b/sdks/typescript/src/worker/protocol.ts @@ -1,6 +1,7 @@ import type { ExecMode } from "../sandbox.js"; import type { CoreModuleBytes } from "./component-imports.js"; +import type { MountEntry } from "../mounts.js"; import type { WireTraceOptions } from "../trace.js"; export interface WorkerInit { @@ -53,7 +54,13 @@ export type WorkerCall = | { kind: "storage-quota" } | { kind: "fetch-snapshot"; url: string; name?: string } | { kind: "mount-snapshot"; name: string; bytes: ArrayBuffer } - | { kind: "session-start"; snapshotPath: string; command: string; prompt: string } + | { + kind: "session-start"; + snapshotPath: string; + command: string; + prompt: string; + mounts: MountEntry[]; + } | { kind: "session-exec"; handle: bigint; @@ -78,6 +85,7 @@ export type WorkerCall = deltaBytes: ArrayBuffer; command: string; prompt: string; + mounts: MountEntry[]; } | { kind: "trace-supported" } | { kind: "session-trace-start"; handle: bigint; options: WireTraceOptions } diff --git a/sdks/typescript/tests/integration/mounts.test.mjs b/sdks/typescript/tests/integration/mounts.test.mjs new file mode 100644 index 0000000..60292ca --- /dev/null +++ b/sdks/typescript/tests/integration/mounts.test.mjs @@ -0,0 +1,98 @@ +import assert from "node:assert/strict"; +import { mkdtemp, mkdir, readFile, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { describe, it } from "node:test"; + +import { distPath, locateSnapshot, skipReason } from "../helpers.mjs"; + +const { Sandbox } = await import(distPath("node/index.js")); + +const BIG_BYTES = 64 * 1024; + +async function withMountedDirectory(spec, body) { + const host = await mkdtemp(join(tmpdir(), "vpod-mount-")); + await writeFile(join(host, "notes.txt"), "hello from the host\n"); + await writeFile(join(host, "big.bin"), "A".repeat(BIG_BYTES)); + await mkdir(join(host, "nested"), { recursive: true }); + await writeFile(join(host, "nested", "deep.txt"), "nested\n"); + + const sandbox = await Sandbox.create({ + snapshot: { path: locateSnapshot() }, + mounts: { [host]: spec }, + }); + + try { + return await body(sandbox, host); + } finally { + await sandbox.close(); + await rm(host, { recursive: true, force: true }); + } +} + +describe("mounts", { skip: skipReason() ?? false }, () => { + it("shows the host directory to the guest", async () => { + await withMountedDirectory("/workspace", async (sandbox) => { + const result = await sandbox.commands.run("cat /workspace/notes.txt"); + assert.equal(result.stdout.trim(), "hello from the host"); + }); + }); + + it("reads a file larger than one page", async () => { + await withMountedDirectory("/workspace", async (sandbox) => { + const result = await sandbox.commands.run("wc -c < /workspace/big.bin"); + assert.equal(result.stdout.trim(), String(BIG_BYTES)); + }); + }); + + it("walks into subdirectories", async () => { + await withMountedDirectory("/workspace", async (sandbox) => { + const result = await sandbox.commands.run("cat /workspace/nested/deep.txt"); + assert.equal(result.stdout.trim(), "nested"); + }); + }); + + it("keeps a plain mount read only", async () => { + await withMountedDirectory("/workspace", async (sandbox, host) => { + const result = await sandbox.commands.run("echo changed > /workspace/notes.txt"); + + assert.notEqual(result.exitCode, 0); + assert.equal( + await readFile(join(host, "notes.txt"), "utf8"), + "hello from the host\n", + ); + }); + }); + + it("writes back to the host when the mount says rw", async () => { + await withMountedDirectory("/workspace:rw", async (sandbox, host) => { + const result = await sandbox.commands.run("echo written > /workspace/from-guest.txt"); + assert.equal(result.exitCode, 0); + + assert.equal(await readFile(join(host, "from-guest.txt"), "utf8"), "written\n"); + }); + }); + + it("refuses to pretend it mounted anything without a host filesystem", async () => { + const { createInlineTransport } = await import(distPath("index.js")); + + await assert.rejects( + Sandbox.create({ + transport: await createInlineTransport(), + snapshot: { path: locateSnapshot() }, + mounts: { [tmpdir()]: "/workspace" }, + }).then((sandbox) => sandbox.commands.run("true").finally(() => sandbox.close())), + /mounts need a host filesystem/, + ); + }); + + it("says so when the host directory is missing", async () => { + await assert.rejects( + Sandbox.create({ + snapshot: { path: locateSnapshot() }, + mounts: { "/no/such/directory": "/workspace" }, + }).then((sandbox) => sandbox.commands.run("true").finally(() => sandbox.close())), + /does not exist/, + ); + }); +}); diff --git a/sdks/typescript/tests/unit/mount-writes.test.mjs b/sdks/typescript/tests/unit/mount-writes.test.mjs new file mode 100644 index 0000000..e26b304 --- /dev/null +++ b/sdks/typescript/tests/unit/mount-writes.test.mjs @@ -0,0 +1,82 @@ +import assert from "node:assert/strict"; +import { mkdtempSync, readFileSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { describe, it } from "node:test"; + +import { distPath } from "../helpers.mjs"; + +await import(distPath("node/index.js")); +const filesystem = await import("@bytecodealliance/preview2-shim/filesystem"); + +const CREATE_A_FILE = { + pathFlags: { symlinkFollow: false }, + openFlags: { create: true, directory: false, exclusive: false, truncate: true }, + descriptorFlags: { + read: false, + write: true, + fileIntegritySync: false, + dataIntegritySync: false, + requestedWriteSync: false, + mutateDirectory: true, + }, +}; + +function createThrough(directory, name) { + const [root] = filesystem.preopens.getDirectories()[0]; + const relative = join(directory, name).replace(/^\//, ""); + + const descriptor = root.openAt( + CREATE_A_FILE.pathFlags, + relative, + CREATE_A_FILE.openFlags, + CREATE_A_FILE.descriptorFlags, + ); + descriptor.write(new TextEncoder().encode("written\n"), 0n); + descriptor[Symbol.dispose]?.(); +} + +describe("write access to a mounted directory on Node", () => { + it("creates a file the guest asked to mutate a directory for", () => { + const directory = mkdtempSync(join(tmpdir(), "vpod-mount-writes-")); + + try { + createThrough(directory, "created.txt"); + assert.equal(readFileSync(join(directory, "created.txt"), "utf8"), "written\n"); + } finally { + rmSync(directory, { recursive: true, force: true }); + } + }); + + it("says whether the shim still needs the retry at all", async () => { + const untouched = await import( + `${import.meta.resolve("@bytecodealliance/preview2-shim/filesystem")}?unpatched` + ); + + const directory = mkdtempSync(join(tmpdir(), "vpod-mount-writes-")); + const [root] = untouched.preopens.getDirectories()[0]; + const relative = join(directory, "direct.txt").replace(/^\//, ""); + + let refused = false; + try { + const descriptor = root.openAt( + CREATE_A_FILE.pathFlags, + relative, + CREATE_A_FILE.openFlags, + CREATE_A_FILE.descriptorFlags, + ); + descriptor[Symbol.dispose]?.(); + } catch (thrown) { + refused = thrown === "unsupported"; + } finally { + rmSync(directory, { recursive: true, force: true }); + } + + assert.equal( + refused, + true, + "preview2-shim now accepts mutate-directory, so src/node/mount-writes.ts " + + "and this test can both go", + ); + }); +}); diff --git a/sdks/typescript/tests/unit/mounts.test.mjs b/sdks/typescript/tests/unit/mounts.test.mjs new file mode 100644 index 0000000..29fa822 --- /dev/null +++ b/sdks/typescript/tests/unit/mounts.test.mjs @@ -0,0 +1,89 @@ +import assert from "node:assert/strict"; +import { describe, it } from "node:test"; + +import { distPath } from "../helpers.mjs"; + +const { Sandbox } = await import(distPath("index.js")); + +function recordingTransport() { + const calls = []; + + return { + calls, + ready: async () => 0, + terminate() {}, + async call(call) { + calls.push(call); + + switch (call.kind) { + case "mount-snapshot": + return { snapshotPath: "snap/test.snap", byteLength: 0 }; + case "session-start": + return 1n; + default: + return undefined; + } + }, + }; +} + +async function sessionStartFor(mounts) { + const transport = recordingTransport(); + const sandbox = await Sandbox.create({ + transport, + network: false, + snapshot: { bytes: new Uint8Array(4), name: "test-256mb.snap" }, + mounts, + }); + + await sandbox.commands.run("true").catch(() => {}); + return transport.calls.find((call) => call.kind === "session-start"); +} + +describe("mounts", () => { + it("carries a plain guest path to the engine as read only", async () => { + const started = await sessionStartFor({ "/tmp/work": "/workspace" }); + + assert.deepEqual(started.mounts, [ + { hostAlias: "/tmp/work", guestPath: "/workspace", writable: false }, + ]); + }); + + it("reads the rw suffix as write access and keeps it out of the path", async () => { + const started = await sessionStartFor({ "/tmp/work": "/workspace:rw" }); + + assert.deepEqual(started.mounts, [ + { hostAlias: "/tmp/work", guestPath: "/workspace", writable: true }, + ]); + }); + + it("carries every directory it was given", async () => { + const started = await sessionStartFor({ "/a": "/one", "/b": "/two:rw" }); + + assert.deepEqual( + started.mounts.map((mount) => [mount.guestPath, mount.writable]), + [ + ["/one", false], + ["/two", true], + ], + ); + }); + + it("sends an empty list when nothing is mounted", async () => { + const started = await sessionStartFor(undefined); + + assert.deepEqual(started.mounts, []); + }); + + it("refuses a guest path that is not absolute", async () => { + await assert.rejects(sessionStartFor({ "/tmp/work": "workspace" }), /absolute guest path/); + }); + + it("refuses an empty guest path", async () => { + await assert.rejects(sessionStartFor({ "/tmp/work": "" }), /needs a guest path/); + }); + + it("refuses a list where an object of paths belongs", async () => { + await assert.rejects(sessionStartFor(["/tmp/work"]), /must be an object/); + }); +});