From d7f0d91c61ad3e287a7d90f50c3679d9c854410b Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 27 Jun 2026 23:39:54 +0000 Subject: [PATCH 01/15] console.table: invoke each cell getter once TablePrinter made two full passes over the rows (one to size the columns, one to render), so every cell property was read twice and the rendered value was the second read's. The second pass also re-ran the iteration protocol, so a one-shot iterable produced an empty table. Read and format each cell once, caching the rendered bytes and their visible width, and render from the cache. --- src/jsc/ConsoleObject.rs | 335 ++++++++-------------- test/js/bun/console/console-table.test.ts | 72 +++++ 2 files changed, 193 insertions(+), 214 deletions(-) diff --git a/src/jsc/ConsoleObject.rs b/src/jsc/ConsoleObject.rs index 1335268276bb..38caf4dccb59 100644 --- a/src/jsc/ConsoleObject.rs +++ b/src/jsc/ConsoleObject.rs @@ -647,6 +647,24 @@ enum RowKey { Num(u32), } +/// A cell value formatted to bytes, with its visible width cached. +struct FormattedCell { + text: Vec, + width: u32, +} + +/// One row's pre-formatted cells, collected while computing column widths so +/// the render loop does not re-read properties (which would re-invoke getters). +struct FormattedRow { + key: FormattedCell, + /// Indexed by `column_index - 1`. `None` means the row has no value for + /// that column. May be shorter than `columns.len() - 1` when later rows + /// added columns this row lacks. + cells: Vec>, + /// Cell for the trailing "Values" column (primitives / Map values). + values_cell: Option, +} + const PADDING: u32 = 1; impl<'a> TablePrinter<'a> { @@ -679,25 +697,14 @@ impl<'a> TablePrinter<'a> { } } -/// A `Write` sink that counts visible terminal columns instead of bytes. -struct VisibleCharacterCounter<'a> { - width: &'a mut usize, -} - -impl bun_io::Write for VisibleCharacterCounter<'_> { - fn write_all(&mut self, bytes: &[u8]) -> bun_io::Result<()> { - *self.width += strings::immutable::visible::width::exclude_ansi_colors::utf8(bytes); - Ok(()) - } -} - impl<'a> TablePrinter<'a> { - /// Compute how much horizontal space a `JSValue` will take when printed. - fn get_width_for_value(&mut self, value: JSValue) -> JsResult { - let mut width: usize = 0; - // PERF: writes straight into the counter with no buffering between - // the generic writer adapter and the counter. Profile if hot. - let mut counter = VisibleCharacterCounter { width: &mut width }; + /// Format a cell value once, returning both the rendered bytes and the + /// visible terminal width. + fn format_cell( + &mut self, + value: JSValue, + ) -> JsResult { + let mut text: Vec = Vec::new(); let mut value_formatter = self.value_formatter.shallow_clone(); let tag = formatter::Tag::get(value, self.global_object)?; @@ -705,41 +712,58 @@ impl<'a> TablePrinter<'a> { tag.tag, TagPayload::String | TagPayload::StringPossiblyFormatted )); - let _ = value_formatter.format::(tag, &mut counter, value, self.global_object); - // VisibleCharacterCounter write cannot fail. - let _ = bun_io::Write::flush(&mut counter); + let _ = + value_formatter.format::(tag, &mut text, value, self.global_object); - Ok(width as u32) + let width = strings::immutable::visible::width::exclude_ansi_colors::utf8(&text) as u32; + Ok(FormattedCell { text, width }) } - /// Update the sizes of the columns for the values of a given row, and - /// create any additional columns as needed. - fn update_columns_for_row( + fn format_row_key(row_key: &RowKey) -> FormattedCell { + match row_key { + RowKey::Str(value) => { + let width = + u32::try_from(value.visible_width_exclude_ansi_colors(false)).expect("int cast"); + FormattedCell { + text: format!("{value}").into_bytes(), + width, + } + } + RowKey::Num(value) => FormattedCell { + text: format!("{value}").into_bytes(), + width: bun_core::fmt::digit_count(*value) as u32, + }, + } + } + + /// Read the row's cell values once, format them, update column widths + /// (creating columns on demand), and return the collected row. + fn collect_row( &mut self, columns: &mut Vec, row_key: &RowKey, row_value: JSValue, - ) -> JsResult<()> { - // update size of "(index)" column - let row_key_len: u32 = match &row_key { - RowKey::Str(value) => { - u32::try_from(value.visible_width_exclude_ansi_colors(false)).expect("int cast") - } - RowKey::Num(value) => bun_core::fmt::digit_count(*value) as u32, + ) -> JsResult { + let key = Self::format_row_key(row_key); + columns[0].width = columns[0].width.max(key.width); + + let mut row = FormattedRow { + key, + cells: Vec::new(), + values_cell: None, }; - columns[0].width = columns[0].width.max(row_key_len); // special handling for Map: column with idx=1 is "Keys" if self.jstype.is_map() { let entry_key = row_value.get_index(self.global_object, 0)?; let entry_value = row_value.get_index(self.global_object, 1)?; - columns[1].width = columns[1].width.max(self.get_width_for_value(entry_key)?); - self.values_col_width = Some( - self.values_col_width - .unwrap_or(0) - .max(self.get_width_for_value(entry_value)?), - ); - return Ok(()); + let key_cell = self.format_cell::(entry_key)?; + let value_cell = self.format_cell::(entry_value)?; + columns[1].width = columns[1].width.max(key_cell.width); + self.values_col_width = Some(self.values_col_width.unwrap_or(0).max(value_cell.width)); + row.cells.push(Some(key_cell)); + row.values_cell = Some(value_cell); + return Ok(row); } if let Some(obj) = row_value.get_object() { @@ -751,7 +775,11 @@ impl<'a> TablePrinter<'a> { if !self.properties.is_undefined() { for column in columns[1..].iter_mut() { if let Some(value) = row_value.get_own(self.global_object, &column.name)? { - column.width = column.width.max(self.get_width_for_value(value)?); + let cell = self.format_cell::(value)?; + column.width = column.width.max(cell.width); + row.cells.push(Some(cell)); + } else { + row.cells.push(None); } } } else { @@ -768,14 +796,14 @@ impl<'a> TablePrinter<'a> { let value = cols_iter.value; // find or create the column for the property - let column: &mut Column = 'brk: { + let col_idx: usize = 'brk: { let col_str = BunString::init(col_key); // reshaped for borrowck — split find/append. if let Some(idx) = columns[1..].iter().position(|col| col.name.eql(&col_str)) { - break 'brk &mut columns[1 + idx]; + break 'brk 1 + idx; } // Need to ref this string because JSPropertyIterator @@ -787,22 +815,25 @@ impl<'a> TablePrinter<'a> { name: col_str, width: 1, }); - let last = columns.len() - 1; - break 'brk &mut columns[last]; + break 'brk columns.len() - 1; }; - column.width = column.width.max(self.get_width_for_value(value)?); + let cell = self.format_cell::(value)?; + columns[col_idx].width = columns[col_idx].width.max(cell.width); + let slot = col_idx - 1; + if row.cells.len() <= slot { + row.cells.resize_with(slot + 1, || None); + } + row.cells[slot] = Some(cell); } } } else if self.properties.is_undefined() { // not object -> the value will go to the special "Values" column - self.values_col_width = Some( - self.values_col_width - .unwrap_or(1) - .max(self.get_width_for_value(row_value)?), - ); + let cell = self.format_cell::(row_value)?; + self.values_col_width = Some(self.values_col_width.unwrap_or(1).max(cell.width)); + row.values_cell = Some(cell); } - Ok(()) + Ok(row) } fn write_string_n_times( @@ -819,33 +850,15 @@ impl<'a> TablePrinter<'a> { Ok(()) } - fn print_row( - &mut self, - writer: &mut dyn bun_io::Write, - columns: &mut [Column], - row_key: &RowKey, - row_value: JSValue, - ) -> JsResult<()> { + fn print_row(&self, writer: &mut dyn bun_io::Write, columns: &[Column], row: &FormattedRow) { writer.write_all("│".as_bytes()).ok(); { - let len: u32 = match &row_key { - RowKey::Str(value) => value.visible_width_exclude_ansi_colors(false) as u32, - RowKey::Num(value) => bun_core::fmt::digit_count(*value) as u32, - }; - let needed = columns[0].width.saturating_sub(len); - + let needed = columns[0].width.saturating_sub(row.key.width); // Right-align the number column writer .splat_byte_all(b' ', (needed + PADDING) as usize) .ok(); - match &row_key { - RowKey::Str(value) => { - write!(writer, "{value}").ok(); - } - RowKey::Num(value) => { - write!(writer, "{value}").ok(); - } - } + writer.write_all(&row.key.text).ok(); writer.splat_byte_all(b' ', PADDING as usize).ok(); } @@ -854,74 +867,29 @@ impl<'a> TablePrinter<'a> { writer.write_all("│".as_bytes()).ok(); - let mut value = JSValue::ZERO; - if col_idx == 1 && self.jstype.is_map() { - // is the "Keys" column, when iterating a Map? - value = row_value.get_index(self.global_object, 0)?; - } else if col_idx == self.values_col_idx { - // is the "Values" column? - if self.jstype.is_map() { - value = row_value.get_index(self.global_object, 1)?; - } else if !row_value.is_object() { - value = row_value; - } - } else if row_value.is_object() { - value = row_value - .get_own(self.global_object, &col.name)? - .unwrap_or(JSValue::ZERO); - } - - if value.is_empty() { - writer - .splat_byte_all(b' ', (col.width + PADDING * 2) as usize) - .ok(); + let cell = if col_idx == self.values_col_idx { + row.values_cell.as_ref() } else { - let len: u32 = self.get_width_for_value(value)?; - let needed = col.width.saturating_sub(len); - writer.splat_byte_all(b' ', PADDING as usize).ok(); - let tag = formatter::Tag::get(value, self.global_object)?; - let mut value_formatter = self.value_formatter.shallow_clone(); - - value_formatter.quote_strings = !(matches!( - tag.tag, - TagPayload::String | TagPayload::StringPossiblyFormatted - )); + row.cells.get(col_idx - 1).and_then(Option::as_ref) + }; - // Release pooled visit map after formatting. - // `shallow_clone()` guarantees the source's `map_node` is - // `None`, so only the local clone needs draining. - // `Formatter::Drop` does the same release, so a plain scope - // is sufficient. - { - let result = value_formatter.format::( - tag, - writer, - value, - self.global_object, - ); - if let Some(mut node) = value_formatter.map_node.take() { - self.value_formatter.map_node = None; - let data = formatter::visited::node_data_mut(&mut node); - if data.capacity() > 512 { - data.deinit(); - } else { - data.clear(); - } - // SAFETY: `node` was obtained from `Pool::get_node()` and is - // exclusively owned by this formatter; `Map::INIT` is `Some`, - // so `data` is initialized. Ownership returns to the pool. - unsafe { formatter::visited::Pool::release(node.as_mut()) }; - } - result?; + match cell { + None => { + writer + .splat_byte_all(b' ', (col.width + PADDING * 2) as usize) + .ok(); + } + Some(cell) => { + let needed = col.width.saturating_sub(cell.width); + writer.splat_byte_all(b' ', PADDING as usize).ok(); + writer.write_all(&cell.text).ok(); + writer + .splat_byte_all(b' ', (needed + PADDING) as usize) + .ok(); } - - writer - .splat_byte_all(b' ', (needed + PADDING) as usize) - .ok(); } } writer.write_all("│\n".as_bytes()).ok(); - Ok(()) } pub fn print_table( @@ -964,12 +932,17 @@ impl<'a> TablePrinter<'a> { } } - // rows first pass - calculate the column widths + // single pass over the rows: read and format each cell exactly once, + // computing column widths as we go. The rendered cells are kept so the + // render loop below does not re-read properties or re-run the iterator + // (which would re-invoke getters and exhaust one-shot iterables). + let mut rows: Vec = Vec::new(); { if self.is_iterable { struct Ctx<'c, 'a> { this: &'c mut TablePrinter<'a>, columns: &'c mut Vec, + rows: &'c mut Vec, idx: u32, err: bool, } @@ -978,10 +951,11 @@ impl<'a> TablePrinter<'a> { let mut ctx = Ctx { this: self, columns, + rows: &mut rows, idx: 0, err: false, }; - extern "C" fn callback( + extern "C" fn callback( _: *mut jsc::VM, _: &JSGlobalObject, ctx: *mut c_void, @@ -989,19 +963,19 @@ impl<'a> TablePrinter<'a> { ) { // SAFETY: ctx points to the stack `Ctx` above. let ctx = unsafe { bun_ptr::callback_ctx::>(ctx) }; - if ctx + match ctx .this - .update_columns_for_row(ctx.columns, &RowKey::Num(ctx.idx), value) - .is_err() + .collect_row::(ctx.columns, &RowKey::Num(ctx.idx), value) { - ctx.err = true; + Ok(row) => ctx.rows.push(row), + Err(_) => ctx.err = true, } ctx.idx += 1; } tabular_data.for_each_with_context( global_object, (&raw mut ctx).cast::(), - callback, + callback::, )?; if ctx.err { return Err(jsc::JsError::Thrown); @@ -1018,11 +992,12 @@ impl<'a> TablePrinter<'a> { )?; while let Some(row_key) = rows_iter.next()? { - self.update_columns_for_row( + let row = self.collect_row::( columns, &RowKey::Str(BunString::init(row_key)), rows_iter.value, )?; + rows.push(row); } } } @@ -1093,77 +1068,9 @@ impl<'a> TablePrinter<'a> { writer.write_all("┤\n".as_bytes()).ok(); } - // rows second pass - print the actual table rows - { - if self.is_iterable { - struct Ctx<'c, 'a> { - this: &'c mut TablePrinter<'a>, - columns: &'c mut Vec, - writer: &'c mut dyn bun_io::Write, - idx: u32, - err: bool, - } - // Capture before constructing `ctx` (which mutably borrows `*self`). - let tabular_data = self.tabular_data; - let mut ctx = Ctx { - this: self, - columns, - writer, - idx: 0, - err: false, - }; - extern "C" fn callback( - _: *mut jsc::VM, - _: &JSGlobalObject, - ctx: *mut c_void, - value: JSValue, - ) { - // SAFETY: ctx points to the stack `Ctx` above. - let ctx = unsafe { bun_ptr::callback_ctx::>(ctx) }; - if ctx - .this - .print_row::(ctx.writer, ctx.columns, &RowKey::Num(ctx.idx), value) - .is_err() - { - ctx.err = true; - } - ctx.idx += 1; - } - tabular_data.for_each_with_context( - global_object, - (&raw mut ctx).cast::(), - callback::, - )?; - if ctx.err { - return Err(jsc::JsError::Thrown); - } - } else { - let Some(cell) = self.tabular_data.to_cell() else { - return Err(global_object.throw_type_error(format_args!( - "tabular_data must be an object or array" - ))); - }; - // `JSCell` is an `opaque_ffi!` ZST handle; `opaque_ref` is the - // centralised non-null deref proof (live JSC heap cell). - let row_obj = jsc::JSCell::opaque_ref(cell).to_object(global_object); - let mut rows_iter = jsc::JSPropertyIterator::init( - global_object, - row_obj, - jsc::PropertyIteratorOptions { - skip_empty_name: false, - include_value: true, - }, - )?; - - while let Some(row_key) = rows_iter.next()? { - self.print_row::( - writer, - columns, - &RowKey::Str(BunString::init(row_key)), - rows_iter.value, - )?; - } - } + // render the rows collected above + for row in &rows { + self.print_row(writer, columns, row); } // print the table bottom border diff --git a/test/js/bun/console/console-table.test.ts b/test/js/bun/console/console-table.test.ts index 653994bc14a5..c6a1f0131daf 100644 --- a/test/js/bun/console/console-table.test.ts +++ b/test/js/bun/console/console-table.test.ts @@ -217,3 +217,75 @@ test("console.table repeat 50", () => { expect(renderTable([{ n: 8 }])).toBe(expected); } }); + +// Every cell must be read exactly once, matching Node. The table is built in +// two logical passes (column sizing, then rendering); re-reading in the second +// pass doubles getter side effects and renders the second call's value. +describe("console.table reads each cell once", () => { + const box = (v: string) => `┌───┬───┐\n│ │ x │\n├───┼───┤\n│ 0 │ ${v} │\n└───┴───┘\n`; + + test("enumerable getter on an array row", () => { + let calls = 0; + const row = {}; + Object.defineProperty(row, "x", { get: () => ++calls, enumerable: true }); + const out = Bun.inspect.table([row]); + expect({ calls, out }).toEqual({ calls: 1, out: box("1") }); + }); + + test("enumerable getter with an explicit properties list", () => { + let calls = 0; + const row = {}; + Object.defineProperty(row, "x", { get: () => ++calls, enumerable: true }); + const out = Bun.inspect.table([row], ["x"]); + expect({ calls, out }).toEqual({ calls: 1, out: box("1") }); + }); + + test("getter on a plain-object row key", () => { + let calls = 0; + const data = {}; + Object.defineProperty(data, "r", { + get() { + calls++; + return { a: calls }; + }, + enumerable: true, + }); + const out = Bun.inspect.table(data); + expect({ calls, out }).toEqual({ + calls: 1, + out: `┌───┬───┐\n│ │ a │\n├───┼───┤\n│ r │ 1 │\n└───┴───┘\n`, + }); + }); + + test("a generator is not consumed twice", () => { + function* rows() { + yield { a: 1 }; + yield { a: 2 }; + } + expect(Bun.inspect.table(rows())).toBe( + `┌───┬───┐\n│ │ a │\n├───┼───┤\n│ 0 │ 1 │\n│ 1 │ 2 │\n└───┴───┘\n`, + ); + }); + + test("console.table", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + `let calls = 0; +const row = {}; +Object.defineProperty(row, "x", { get: () => ++calls, enumerable: true }); +console.table([row]); +console.log("calls=" + calls);`, + ], + env: bunEnv, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([ + proc.stdout.text(), + proc.stderr.text(), + proc.exited, + ]); + expect({ stdout, exitCode }).toEqual({ stdout: box("1") + "calls=1\n", exitCode: 0 }); + }); +}); From edf83d11a319c85514a431d062b1d7bd8a1827da Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Sat, 27 Jun 2026 23:42:43 +0000 Subject: [PATCH 02/15] [autofix.ci] apply automated fixes --- src/jsc/ConsoleObject.rs | 4 ++-- test/js/bun/console/console-table.test.ts | 10 ++-------- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/src/jsc/ConsoleObject.rs b/src/jsc/ConsoleObject.rs index 38caf4dccb59..641598a35009 100644 --- a/src/jsc/ConsoleObject.rs +++ b/src/jsc/ConsoleObject.rs @@ -722,8 +722,8 @@ impl<'a> TablePrinter<'a> { fn format_row_key(row_key: &RowKey) -> FormattedCell { match row_key { RowKey::Str(value) => { - let width = - u32::try_from(value.visible_width_exclude_ansi_colors(false)).expect("int cast"); + let width = u32::try_from(value.visible_width_exclude_ansi_colors(false)) + .expect("int cast"); FormattedCell { text: format!("{value}").into_bytes(), width, diff --git a/test/js/bun/console/console-table.test.ts b/test/js/bun/console/console-table.test.ts index c6a1f0131daf..7cbbdf2b78ee 100644 --- a/test/js/bun/console/console-table.test.ts +++ b/test/js/bun/console/console-table.test.ts @@ -262,9 +262,7 @@ describe("console.table reads each cell once", () => { yield { a: 1 }; yield { a: 2 }; } - expect(Bun.inspect.table(rows())).toBe( - `┌───┬───┐\n│ │ a │\n├───┼───┤\n│ 0 │ 1 │\n│ 1 │ 2 │\n└───┴───┘\n`, - ); + expect(Bun.inspect.table(rows())).toBe(`┌───┬───┐\n│ │ a │\n├───┼───┤\n│ 0 │ 1 │\n│ 1 │ 2 │\n└───┴───┘\n`); }); test("console.table", async () => { @@ -281,11 +279,7 @@ console.log("calls=" + calls);`, env: bunEnv, stderr: "pipe", }); - const [stdout, stderr, exitCode] = await Promise.all([ - proc.stdout.text(), - proc.stderr.text(), - proc.exited, - ]); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); expect({ stdout, exitCode }).toEqual({ stdout: box("1") + "calls=1\n", exitCode: 0 }); }); }); From ec514b518560ca2d5c6e30f382d8eec403124394 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 28 Jun 2026 01:49:04 +0000 Subject: [PATCH 03/15] ci: retrigger From 07b50f066dad099e368115024b4a5ea20cac9ace Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 28 Jun 2026 02:18:55 +0000 Subject: [PATCH 04/15] console.table: propagate cell formatter exceptions format_cell dropped the JsResult from Formatter::format, but the render loop that used to propagate a throwing custom inspect no longer reads JS values, so the exception was left pending across the next host call. Propagate it with ? and cover the previously untested primitive Values column getter path. --- src/jsc/ConsoleObject.rs | 6 ++++-- test/js/bun/console/console-table.test.ts | 26 +++++++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/jsc/ConsoleObject.rs b/src/jsc/ConsoleObject.rs index 641598a35009..cbb04e3677ba 100644 --- a/src/jsc/ConsoleObject.rs +++ b/src/jsc/ConsoleObject.rs @@ -712,8 +712,10 @@ impl<'a> TablePrinter<'a> { tag.tag, TagPayload::String | TagPayload::StringPossiblyFormatted )); - let _ = - value_formatter.format::(tag, &mut text, value, self.global_object); + // A throwing custom inspect (or stack overflow) must surface to the + // caller; the render loop that used to propagate it no longer reads + // JS values. + value_formatter.format::(tag, &mut text, value, self.global_object)?; let width = strings::immutable::visible::width::exclude_ansi_colors::utf8(&text) as u32; Ok(FormattedCell { text, width }) diff --git a/test/js/bun/console/console-table.test.ts b/test/js/bun/console/console-table.test.ts index 7cbbdf2b78ee..691be3edb4c2 100644 --- a/test/js/bun/console/console-table.test.ts +++ b/test/js/bun/console/console-table.test.ts @@ -265,6 +265,32 @@ describe("console.table reads each cell once", () => { expect(Bun.inspect.table(rows())).toBe(`┌───┬───┐\n│ │ a │\n├───┼───┤\n│ 0 │ 1 │\n│ 1 │ 2 │\n└───┴───┘\n`); }); + test("getter on a primitive routed to the Values column", () => { + let calls = 0; + const data = {}; + Object.defineProperty(data, "a", { get: () => ++calls, enumerable: true }); + const out = Bun.inspect.table(data); + expect({ calls, out }).toEqual({ + calls: 1, + out: `┌───┬────────┐\n│ │ Values │\n├───┼────────┤\n│ a │ 1 │\n└───┴────────┘\n`, + }); + }); + + test("a throwing custom inspect in a cell still propagates", () => { + const boom = new Error("boom"); + expect(() => + Bun.inspect.table([ + { + x: { + [Bun.inspect.custom]() { + throw boom; + }, + }, + }, + ]), + ).toThrow(boom); + }); + test("console.table", async () => { await using proc = Bun.spawn({ cmd: [ From 10fdae561896d57409978fdd1d0b8d704713ec41 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 28 Jun 2026 05:57:59 +0000 Subject: [PATCH 05/15] console.table: hold cell JSValues in a MarkedArgumentBuffer, not rendered text Buffering every cell's rendered bytes for the whole table roughly doubled peak memory for the call (the cached text plus the final output buffer). Keep the two-pass shape instead: the width pass reads each property exactly once, appends the cell JSValue to a MarkedArgumentBuffer scoped across the whole print, and records its visible width. The render pass formats those rooted values. Per cell this costs a 16-byte CellRef inline in the row (no per-cell heap allocation) plus one buffer slot; JSC cells do not move, so the inline copies stay valid while the buffer roots them. Also includes stderr in the spawned test's combined assertion. --- src/jsc/ConsoleObject.rs | 467 ++++++++++++---------- test/js/bun/console/console-table.test.ts | 2 +- 2 files changed, 260 insertions(+), 209 deletions(-) diff --git a/src/jsc/ConsoleObject.rs b/src/jsc/ConsoleObject.rs index cbb04e3677ba..31a0f5ab7e29 100644 --- a/src/jsc/ConsoleObject.rs +++ b/src/jsc/ConsoleObject.rs @@ -643,26 +643,53 @@ impl Default for Column { } enum RowKey { - Str(BunString), + /// Pre-rendered property-name bytes + visible width (plain-object + /// tabular data). Owned, so no WTF refcount outlives the iterator. + Str { text: Vec, width: u32 }, + /// Row index (array / iterable tabular data). Rendered on demand. Num(u32), } -/// A cell value formatted to bytes, with its visible width cached. -struct FormattedCell { - text: Vec, +impl RowKey { + fn str(name: &BunString) -> Self { + Self::Str { + width: u32::try_from(name.visible_width_exclude_ansi_colors(false)).expect("int cast"), + text: format!("{name}").into_bytes(), + } + } + + fn width(&self) -> u32 { + match self { + RowKey::Str { width, .. } => *width, + RowKey::Num(value) => bun_core::fmt::digit_count(*value) as u32, + } + } +} + +/// A cell read during the width pass: its value (kept alive by the +/// `MarkedArgumentBuffer` that roots every captured cell for the whole +/// `print_table` call) and its pre-computed visible width. +#[derive(Clone, Copy)] +struct CellRef { + value: JSValue, width: u32, } -/// One row's pre-formatted cells, collected while computing column widths so -/// the render loop does not re-read properties (which would re-invoke getters). -struct FormattedRow { - key: FormattedCell, - /// Indexed by `column_index - 1`. `None` means the row has no value for - /// that column. May be shorter than `columns.len() - 1` when later rows - /// added columns this row lacks. - cells: Vec>, - /// Cell for the trailing "Values" column (primitives / Map values). - values_cell: Option, +/// An absent cell: the row has no value for that column. +const EMPTY_CELL: CellRef = CellRef { + value: JSValue::ZERO, + width: 0, +}; + +/// One table row, collected during the width pass so the render pass never +/// re-reads a property (which would re-invoke its getter). +struct CollectedRow { + key: RowKey, + /// Indexed by `column_index - 1`. May be shorter than + /// `columns.len() - 1` when later rows added columns this row lacks. + cells: Vec, + /// Cell routed to the trailing "Values" column (primitives, Map values). + values_cell: CellRef, } const PADDING: u32 = 1; @@ -697,74 +724,83 @@ impl<'a> TablePrinter<'a> { } } +/// A `Write` sink that counts visible terminal columns instead of bytes. +struct VisibleCharacterCounter<'a> { + width: &'a mut usize, +} + +impl bun_io::Write for VisibleCharacterCounter<'_> { + fn write_all(&mut self, bytes: &[u8]) -> bun_io::Result<()> { + *self.width += strings::immutable::visible::width::exclude_ansi_colors::utf8(bytes); + Ok(()) + } +} + impl<'a> TablePrinter<'a> { - /// Format a cell value once, returning both the rendered bytes and the - /// visible terminal width. - fn format_cell( - &mut self, + /// Format one cell value to `writer`: bare for strings, quoted otherwise. + fn format_value_to( + &self, value: JSValue, - ) -> JsResult { - let mut text: Vec = Vec::new(); + writer: &mut dyn bun_io::Write, + ) -> JsResult<()> { let mut value_formatter = self.value_formatter.shallow_clone(); - let tag = formatter::Tag::get(value, self.global_object)?; value_formatter.quote_strings = !(matches!( tag.tag, TagPayload::String | TagPayload::StringPossiblyFormatted )); - // A throwing custom inspect (or stack overflow) must surface to the - // caller; the render loop that used to propagate it no longer reads - // JS values. - value_formatter.format::(tag, &mut text, value, self.global_object)?; + value_formatter.format::(tag, writer, value, self.global_object) + } - let width = strings::immutable::visible::width::exclude_ansi_colors::utf8(&text) as u32; - Ok(FormattedCell { text, width }) + /// Compute how much horizontal space a `JSValue` will take when printed. + fn get_width_for_value(&self, value: JSValue) -> JsResult { + let mut width: usize = 0; + // PERF: writes straight into the counter with no buffering between + // the generic writer adapter and the counter. Profile if hot. + let mut counter = VisibleCharacterCounter { width: &mut width }; + self.format_value_to::(value, &mut counter)?; + Ok(width as u32) } - fn format_row_key(row_key: &RowKey) -> FormattedCell { - match row_key { - RowKey::Str(value) => { - let width = u32::try_from(value.visible_width_exclude_ansi_colors(false)) - .expect("int cast"); - FormattedCell { - text: format!("{value}").into_bytes(), - width, - } - } - RowKey::Num(value) => FormattedCell { - text: format!("{value}").into_bytes(), - width: bun_core::fmt::digit_count(*value) as u32, - }, - } + /// Root `value` for the render pass and compute its visible width. + fn capture_cell( + &self, + roots: &mut jsc::MarkedArgumentBuffer, + value: JSValue, + ) -> JsResult { + roots.append(value); + Ok(CellRef { + value, + width: self.get_width_for_value(value)?, + }) } - /// Read the row's cell values once, format them, update column widths - /// (creating columns on demand), and return the collected row. - fn collect_row( + /// Read the row's cell values exactly once, size the columns (creating + /// them on demand), and return the collected row for the render pass. + fn collect_row( &mut self, + roots: &mut jsc::MarkedArgumentBuffer, columns: &mut Vec, - row_key: &RowKey, + row_key: RowKey, row_value: JSValue, - ) -> JsResult { - let key = Self::format_row_key(row_key); - columns[0].width = columns[0].width.max(key.width); + ) -> JsResult { + columns[0].width = columns[0].width.max(row_key.width()); - let mut row = FormattedRow { - key, + let mut row = CollectedRow { + key: row_key, cells: Vec::new(), - values_cell: None, + values_cell: EMPTY_CELL, }; // special handling for Map: column with idx=1 is "Keys" if self.jstype.is_map() { - let entry_key = row_value.get_index(self.global_object, 0)?; - let entry_value = row_value.get_index(self.global_object, 1)?; - let key_cell = self.format_cell::(entry_key)?; - let value_cell = self.format_cell::(entry_value)?; + let key_cell = self.capture_cell(roots, row_value.get_index(self.global_object, 0)?)?; + let value_cell = + self.capture_cell(roots, row_value.get_index(self.global_object, 1)?)?; columns[1].width = columns[1].width.max(key_cell.width); self.values_col_width = Some(self.values_col_width.unwrap_or(0).max(value_cell.width)); - row.cells.push(Some(key_cell)); - row.values_cell = Some(value_cell); + row.cells.push(key_cell); + row.values_cell = value_cell; return Ok(row); } @@ -777,11 +813,11 @@ impl<'a> TablePrinter<'a> { if !self.properties.is_undefined() { for column in columns[1..].iter_mut() { if let Some(value) = row_value.get_own(self.global_object, &column.name)? { - let cell = self.format_cell::(value)?; + let cell = self.capture_cell(roots, value)?; column.width = column.width.max(cell.width); - row.cells.push(Some(cell)); + row.cells.push(cell); } else { - row.cells.push(None); + row.cells.push(EMPTY_CELL); } } } else { @@ -820,20 +856,20 @@ impl<'a> TablePrinter<'a> { break 'brk columns.len() - 1; }; - let cell = self.format_cell::(value)?; + let cell = self.capture_cell(roots, value)?; columns[col_idx].width = columns[col_idx].width.max(cell.width); let slot = col_idx - 1; if row.cells.len() <= slot { - row.cells.resize_with(slot + 1, || None); + row.cells.resize(slot + 1, EMPTY_CELL); } - row.cells[slot] = Some(cell); + row.cells[slot] = cell; } } } else if self.properties.is_undefined() { // not object -> the value will go to the special "Values" column - let cell = self.format_cell::(row_value)?; + let cell = self.capture_cell(roots, row_value)?; self.values_col_width = Some(self.values_col_width.unwrap_or(1).max(cell.width)); - row.values_cell = Some(cell); + row.values_cell = cell; } Ok(row) } @@ -852,15 +888,28 @@ impl<'a> TablePrinter<'a> { Ok(()) } - fn print_row(&self, writer: &mut dyn bun_io::Write, columns: &[Column], row: &FormattedRow) { + fn print_row( + &self, + writer: &mut dyn bun_io::Write, + columns: &[Column], + row: &CollectedRow, + ) -> JsResult<()> { writer.write_all("│".as_bytes()).ok(); { - let needed = columns[0].width.saturating_sub(row.key.width); + let needed = columns[0].width.saturating_sub(row.key.width()); + // Right-align the number column writer .splat_byte_all(b' ', (needed + PADDING) as usize) .ok(); - writer.write_all(&row.key.text).ok(); + match &row.key { + RowKey::Str { text, .. } => { + writer.write_all(text).ok(); + } + RowKey::Num(value) => { + write!(writer, "{value}").ok(); + } + } writer.splat_byte_all(b' ', PADDING as usize).ok(); } @@ -870,28 +919,26 @@ impl<'a> TablePrinter<'a> { writer.write_all("│".as_bytes()).ok(); let cell = if col_idx == self.values_col_idx { - row.values_cell.as_ref() + row.values_cell } else { - row.cells.get(col_idx - 1).and_then(Option::as_ref) + row.cells.get(col_idx - 1).copied().unwrap_or(EMPTY_CELL) }; - match cell { - None => { - writer - .splat_byte_all(b' ', (col.width + PADDING * 2) as usize) - .ok(); - } - Some(cell) => { - let needed = col.width.saturating_sub(cell.width); - writer.splat_byte_all(b' ', PADDING as usize).ok(); - writer.write_all(&cell.text).ok(); - writer - .splat_byte_all(b' ', (needed + PADDING) as usize) - .ok(); - } + if cell.value.is_empty() { + writer + .splat_byte_all(b' ', (col.width + PADDING * 2) as usize) + .ok(); + } else { + let needed = col.width.saturating_sub(cell.width); + writer.splat_byte_all(b' ', PADDING as usize).ok(); + self.format_value_to::(cell.value, writer)?; + writer + .splat_byte_all(b' ', (needed + PADDING) as usize) + .ok(); } } writer.write_all("│\n".as_bytes()).ok(); + Ok(()) } pub fn print_table( @@ -934,146 +981,150 @@ impl<'a> TablePrinter<'a> { } } - // single pass over the rows: read and format each cell exactly once, - // computing column widths as we go. The rendered cells are kept so the - // render loop below does not re-read properties or re-run the iterator + // `roots` keeps every cell value read by the width pass alive so the + // render loop can format the same values instead of re-reading them // (which would re-invoke getters and exhaust one-shot iterables). - let mut rows: Vec = Vec::new(); - { - if self.is_iterable { - struct Ctx<'c, 'a> { - this: &'c mut TablePrinter<'a>, - columns: &'c mut Vec, - rows: &'c mut Vec, - idx: u32, - err: bool, - } - // Capture before constructing `ctx` (which mutably borrows `*self`). - let tabular_data = self.tabular_data; - let mut ctx = Ctx { - this: self, - columns, - rows: &mut rows, - idx: 0, - err: false, - }; - extern "C" fn callback( - _: *mut jsc::VM, - _: &JSGlobalObject, - ctx: *mut c_void, - value: JSValue, - ) { - // SAFETY: ctx points to the stack `Ctx` above. - let ctx = unsafe { bun_ptr::callback_ctx::>(ctx) }; - match ctx - .this - .collect_row::(ctx.columns, &RowKey::Num(ctx.idx), value) - { - Ok(row) => ctx.rows.push(row), - Err(_) => ctx.err = true, + jsc::MarkedArgumentBuffer::new(|roots| -> JsResult<()> { + // width pass: read each cell exactly once, sizing columns as we go + let mut rows: Vec = Vec::new(); + { + if self.is_iterable { + struct Ctx<'c, 'a> { + this: &'c mut TablePrinter<'a>, + roots: &'c mut jsc::MarkedArgumentBuffer, + columns: &'c mut Vec, + rows: &'c mut Vec, + idx: u32, + err: bool, } - ctx.idx += 1; - } - tabular_data.for_each_with_context( - global_object, - (&raw mut ctx).cast::(), - callback::, - )?; - if ctx.err { - return Err(jsc::JsError::Thrown); - } - } else { - let tabular_obj = self.tabular_data.to_object(global_object)?; - let mut rows_iter = jsc::JSPropertyIterator::init( - global_object, - tabular_obj, - jsc::PropertyIteratorOptions { - skip_empty_name: false, - include_value: true, - }, - )?; - - while let Some(row_key) = rows_iter.next()? { - let row = self.collect_row::( + // Capture before constructing `ctx` (which mutably borrows `*self`). + let tabular_data = self.tabular_data; + let mut ctx = Ctx { + this: self, + roots, columns, - &RowKey::Str(BunString::init(row_key)), - rows_iter.value, + rows: &mut rows, + idx: 0, + err: false, + }; + extern "C" fn callback( + _: *mut jsc::VM, + _: &JSGlobalObject, + ctx: *mut c_void, + value: JSValue, + ) { + // SAFETY: ctx points to the stack `Ctx` above. + let ctx = unsafe { bun_ptr::callback_ctx::>(ctx) }; + match ctx.this.collect_row( + ctx.roots, + ctx.columns, + RowKey::Num(ctx.idx), + value, + ) { + Ok(row) => ctx.rows.push(row), + Err(_) => ctx.err = true, + } + ctx.idx += 1; + } + tabular_data.for_each_with_context( + global_object, + (&raw mut ctx).cast::(), + callback, + )?; + if ctx.err { + return Err(jsc::JsError::Thrown); + } + } else { + let tabular_obj = self.tabular_data.to_object(global_object)?; + let mut rows_iter = jsc::JSPropertyIterator::init( + global_object, + tabular_obj, + jsc::PropertyIteratorOptions { + skip_empty_name: false, + include_value: true, + }, )?; - rows.push(row); + + while let Some(row_key) = rows_iter.next()? { + let key = RowKey::str(&BunString::init(row_key)); + let row = self.collect_row(roots, columns, key, rows_iter.value)?; + rows.push(row); + } } } - } - - // append the special "Values" column as the last one, if it is present - if let Some(width) = self.values_col_width { - self.values_col_idx = columns.len(); - columns.push(Column { - name: BunString::static_("Values"), - width, - }); - } - // print the table header (border line + column names line + border line) - { - for col in columns.iter_mut() { - // also update the col width with the length of the column name itself - col.width = col.width.max( - u32::try_from(col.name.visible_width_exclude_ansi_colors(false)) - .expect("int cast"), - ); + // append the special "Values" column as the last one, if it is present + if let Some(width) = self.values_col_width { + self.values_col_idx = columns.len(); + columns.push(Column { + name: BunString::static_("Values"), + width, + }); } - writer.write_all("┌".as_bytes()).ok(); - for (i, col) in columns.iter().enumerate() { - if i > 0 { - writer.write_all("┬".as_bytes()).ok(); + // print the table header (border line + column names line + border line) + { + for col in columns.iter_mut() { + // also update the col width with the length of the column name itself + col.width = col.width.max( + u32::try_from(col.name.visible_width_exclude_ansi_colors(false)) + .expect("int cast"), + ); } - Self::write_string_n_times( - writer, - "─".as_bytes(), - (col.width + PADDING * 2) as usize, - ) - .ok(); - } - - writer.write_all("┐\n│".as_bytes()).ok(); - for (i, col) in columns.iter().enumerate() { - if i > 0 { - writer.write_all("│".as_bytes()).ok(); - } - let len = col.name.visible_width_exclude_ansi_colors(false); - let needed = (col.width as usize).saturating_sub(len); - writer.splat_byte_all(b' ', 1).ok(); - if ENABLE_ANSI_COLORS { - writer.write_all(pfmt!("", true).as_bytes()).ok(); + writer.write_all("┌".as_bytes()).ok(); + for (i, col) in columns.iter().enumerate() { + if i > 0 { + writer.write_all("┬".as_bytes()).ok(); + } + Self::write_string_n_times( + writer, + "─".as_bytes(), + (col.width + PADDING * 2) as usize, + ) + .ok(); } - write!(writer, "{}", col.name).ok(); - if ENABLE_ANSI_COLORS { - writer.write_all(pfmt!("", true).as_bytes()).ok(); + + writer.write_all("┐\n│".as_bytes()).ok(); + + for (i, col) in columns.iter().enumerate() { + if i > 0 { + writer.write_all("│".as_bytes()).ok(); + } + let len = col.name.visible_width_exclude_ansi_colors(false); + let needed = (col.width as usize).saturating_sub(len); + writer.splat_byte_all(b' ', 1).ok(); + if ENABLE_ANSI_COLORS { + writer.write_all(pfmt!("", true).as_bytes()).ok(); + } + write!(writer, "{}", col.name).ok(); + if ENABLE_ANSI_COLORS { + writer.write_all(pfmt!("", true).as_bytes()).ok(); + } + writer.splat_byte_all(b' ', needed + PADDING as usize).ok(); } - writer.splat_byte_all(b' ', needed + PADDING as usize).ok(); - } - writer.write_all("│\n├".as_bytes()).ok(); - for (i, col) in columns.iter().enumerate() { - if i > 0 { - writer.write_all("┼".as_bytes()).ok(); + writer.write_all("│\n├".as_bytes()).ok(); + for (i, col) in columns.iter().enumerate() { + if i > 0 { + writer.write_all("┼".as_bytes()).ok(); + } + Self::write_string_n_times( + writer, + "─".as_bytes(), + (col.width + PADDING * 2) as usize, + ) + .ok(); } - Self::write_string_n_times( - writer, - "─".as_bytes(), - (col.width + PADDING * 2) as usize, - ) - .ok(); + writer.write_all("┤\n".as_bytes()).ok(); } - writer.write_all("┤\n".as_bytes()).ok(); - } - // render the rows collected above - for row in &rows { - self.print_row(writer, columns, row); - } + // render pass: format the captured cell values + for row in rows.iter() { + self.print_row::(writer, columns, row)?; + } + Ok(()) + })?; // print the table bottom border { diff --git a/test/js/bun/console/console-table.test.ts b/test/js/bun/console/console-table.test.ts index 691be3edb4c2..47abb25996c8 100644 --- a/test/js/bun/console/console-table.test.ts +++ b/test/js/bun/console/console-table.test.ts @@ -306,6 +306,6 @@ console.log("calls=" + calls);`, stderr: "pipe", }); const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - expect({ stdout, exitCode }).toEqual({ stdout: box("1") + "calls=1\n", exitCode: 0 }); + expect({ stdout, stderr, exitCode }).toEqual({ stdout: box("1") + "calls=1\n", stderr: "", exitCode: 0 }); }); }); From 11cf6d94d115ef09ba197bcf2904ed890c0e10b9 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 28 Jun 2026 12:36:14 +0000 Subject: [PATCH 06/15] MarkedArgumentBuffer: crash on overflow instead of silently dropping the value Under JSC's RecordOverflow policy, append() is a no-op when the spill-to-heap allocation fails, leaving the value unrooted while the Rust caller (console.table's CellRef cache) keeps using it. No Rust caller can recover from an unrooted value, so route allocation failure to a loud OOM crash via appendWithCrashOnOverflow. --- src/jsc/MarkedArgumentBuffer.rs | 3 +++ src/jsc/bindings/MarkedArgumentBufferBinding.cpp | 7 ++++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/jsc/MarkedArgumentBuffer.rs b/src/jsc/MarkedArgumentBuffer.rs index a7520a127f77..8935d46e6dd9 100644 --- a/src/jsc/MarkedArgumentBuffer.rs +++ b/src/jsc/MarkedArgumentBuffer.rs @@ -47,6 +47,9 @@ impl MarkedArgumentBuffer { ctx.r.unwrap() } + /// Append `value` and GC-root it for the buffer's lifetime. Infallible: + /// the C++ side uses `appendWithCrashOnOverflow`, so an allocation + /// failure is a loud OOM crash, never a silently unrooted value. pub fn append(&mut self, value: JSValue) { MarkedArgumentBuffer__append(self, value) } diff --git a/src/jsc/bindings/MarkedArgumentBufferBinding.cpp b/src/jsc/bindings/MarkedArgumentBufferBinding.cpp index 5d9bde10c205..946d2efa29b3 100644 --- a/src/jsc/bindings/MarkedArgumentBufferBinding.cpp +++ b/src/jsc/bindings/MarkedArgumentBufferBinding.cpp @@ -14,7 +14,12 @@ extern "C" SUPPRESS_ASAN void MarkedArgumentBuffer__run( callback(ctx, &args); } +// appendWithCrashOnOverflow, not append: under MarkedArgumentBuffer's +// RecordOverflow policy a failed spill-to-heap allocation makes append() a +// silent no-op, leaving the value unrooted while Rust callers keep using it. +// No Rust caller can recover from that, so turn allocation failure into a +// loud OOM crash instead of a GC use-after-free. extern "C" void MarkedArgumentBuffer__append(void* args, JSC::EncodedJSValue value) { - static_cast(args)->append(JSC::JSValue::decode(value)); + static_cast(args)->appendWithCrashOnOverflow(JSC::JSValue::decode(value)); } From b0354244f6014ce40cc9cb70ddc89e48dce3b905 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 28 Jun 2026 13:03:01 +0000 Subject: [PATCH 07/15] console.table: use the to_utf8_bytes helper for the row key Also shorten the appendWithCrashOnOverflow comment. --- src/jsc/ConsoleObject.rs | 2 +- src/jsc/bindings/MarkedArgumentBufferBinding.cpp | 8 +++----- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/jsc/ConsoleObject.rs b/src/jsc/ConsoleObject.rs index 31a0f5ab7e29..ae9b91f52724 100644 --- a/src/jsc/ConsoleObject.rs +++ b/src/jsc/ConsoleObject.rs @@ -654,7 +654,7 @@ impl RowKey { fn str(name: &BunString) -> Self { Self::Str { width: u32::try_from(name.visible_width_exclude_ansi_colors(false)).expect("int cast"), - text: format!("{name}").into_bytes(), + text: name.to_utf8_bytes(), } } diff --git a/src/jsc/bindings/MarkedArgumentBufferBinding.cpp b/src/jsc/bindings/MarkedArgumentBufferBinding.cpp index 946d2efa29b3..8aa84d20a901 100644 --- a/src/jsc/bindings/MarkedArgumentBufferBinding.cpp +++ b/src/jsc/bindings/MarkedArgumentBufferBinding.cpp @@ -14,11 +14,9 @@ extern "C" SUPPRESS_ASAN void MarkedArgumentBuffer__run( callback(ctx, &args); } -// appendWithCrashOnOverflow, not append: under MarkedArgumentBuffer's -// RecordOverflow policy a failed spill-to-heap allocation makes append() a -// silent no-op, leaving the value unrooted while Rust callers keep using it. -// No Rust caller can recover from that, so turn allocation failure into a -// loud OOM crash instead of a GC use-after-free. +// appendWithCrashOnOverflow: with RecordOverflow, plain append() silently +// drops the value on a failed spill-to-heap, leaving it unrooted while Rust +// keeps using it. No caller can recover, so prefer a loud OOM to a GC UAF. extern "C" void MarkedArgumentBuffer__append(void* args, JSC::EncodedJSValue value) { static_cast(args)->appendWithCrashOnOverflow(JSC::JSValue::decode(value)); From 0cdfd2fa97137b71004c9fa8f4af1a33a66c3159 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 28 Jun 2026 14:23:08 +0000 Subject: [PATCH 08/15] ci: retrigger From 31564bd736696f9a2d74a61b630b64511c2f3609 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 28 Jun 2026 18:18:14 +0000 Subject: [PATCH 09/15] console.table: test GC rooting, cell slot mapping, and Proxy reads The width pass caches each cell's JSValue in a Rust Vec, which JSC's conservative stack scan never visits, so the MarkedArgumentBuffer rooting them is the only thing keeping them alive across the user code that runs between the two passes. With roots.append removed, all 64 cells in the new GC test are collected before they are rendered; nothing else in the suite detects that. 64 rows also spills the buffer past its 8-slot inline storage into the heap-registered regime. Also pins the positional cell-slot mapping for a row whose key order differs from the discovered column order, and the Proxy case: each cell is read once via [[Get]] (matching Node), where the old render pass re-read through [[GetOwnProperty]] and drew the raw target value into a column sized for the trap value. --- test/js/bun/console/console-table.test.ts | 36 +++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/test/js/bun/console/console-table.test.ts b/test/js/bun/console/console-table.test.ts index 47abb25996c8..863cdc8484ae 100644 --- a/test/js/bun/console/console-table.test.ts +++ b/test/js/bun/console/console-table.test.ts @@ -291,6 +291,42 @@ describe("console.table reads each cell once", () => { ).toThrow(boom); }); + // The width pass caches each cell's JSValue in a Rust Vec, which JSC's + // conservative scan never visits; only the MarkedArgumentBuffer roots them. + // 64 rows spills it past the 8-slot inline (stack-scanned) buffer. + test("cell values survive a full GC between the width and render passes", () => { + const N = 64; + const rows = Array.from({ length: N }, (_, i) => ({ + get x() { + Bun.gc(true); + return { id: i }; + }, + })); + const out = Bun.inspect.table(rows); + const missing: number[] = []; + for (let i = 0; i < N; i++) if (!out.includes(`{ id: ${i} }`)) missing.push(i); + expect(missing).toEqual([]); + }); + + // Cells are keyed by column index in the width pass. A row that revisits an + // already-discovered column after creating a later one must not displace or + // truncate the cells it already captured. + test("a row whose key order differs from the column order", () => { + expect(Bun.inspect.table([{ a: 1 }, { b: 2, a: 3 }])).toBe( + `┌───┬───┬───┐\n│ │ a │ b │\n├───┼───┼───┤\n│ 0 │ 1 │ │\n│ 1 │ 3 │ 2 │\n└───┴───┴───┘\n`, + ); + }); + + // A single read per cell means the column is sized from the same value that + // gets rendered: the [[Get]] result, matching Node. The old render pass + // re-read through [[GetOwnProperty]], which a Proxy can observably diverge. + test("a Proxy row renders the [[Get]] value the width pass saw", () => { + const p = new Proxy({ x: "FROM_TARGET" }, { get: () => "FROM_GET" }); + expect(Bun.inspect.table([p])).toBe( + `┌───┬──────────┐\n│ │ x │\n├───┼──────────┤\n│ 0 │ FROM_GET │\n└───┴──────────┘\n`, + ); + }); + test("console.table", async () => { await using proc = Bun.spawn({ cmd: [ From df3d47d6b6e00a6e5379ce6b094cbf0a7d37c2d4 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 28 Jun 2026 18:28:06 +0000 Subject: [PATCH 10/15] console.table: propagate the typed JsError from the iterable row pass The callback context carried a bool, so every collect_row failure on the iterable path was reported as JsError::Thrown, discarding OutOfMemory and Terminated. The non-iterable path already preserved the variant via ?. Carry the JsError itself and stop reading further cells once one has failed, so no more user code runs with an exception pending. --- src/jsc/ConsoleObject.rs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/jsc/ConsoleObject.rs b/src/jsc/ConsoleObject.rs index ae9b91f52724..751239a4372a 100644 --- a/src/jsc/ConsoleObject.rs +++ b/src/jsc/ConsoleObject.rs @@ -995,7 +995,7 @@ impl<'a> TablePrinter<'a> { columns: &'c mut Vec, rows: &'c mut Vec, idx: u32, - err: bool, + err: Option, } // Capture before constructing `ctx` (which mutably borrows `*self`). let tabular_data = self.tabular_data; @@ -1005,7 +1005,7 @@ impl<'a> TablePrinter<'a> { columns, rows: &mut rows, idx: 0, - err: false, + err: None, }; extern "C" fn callback( _: *mut jsc::VM, @@ -1015,6 +1015,12 @@ impl<'a> TablePrinter<'a> { ) { // SAFETY: ctx points to the stack `Ctx` above. let ctx = unsafe { bun_ptr::callback_ctx::>(ctx) }; + // Once a cell failed, a JS exception may be pending (or + // the VM terminating); don't re-enter user code for the + // remaining elements. + if ctx.err.is_some() { + return; + } match ctx.this.collect_row( ctx.roots, ctx.columns, @@ -1022,7 +1028,7 @@ impl<'a> TablePrinter<'a> { value, ) { Ok(row) => ctx.rows.push(row), - Err(_) => ctx.err = true, + Err(err) => ctx.err = Some(err), } ctx.idx += 1; } @@ -1031,8 +1037,8 @@ impl<'a> TablePrinter<'a> { (&raw mut ctx).cast::(), callback, )?; - if ctx.err { - return Err(jsc::JsError::Thrown); + if let Some(err) = ctx.err { + return Err(err); } } else { let tabular_obj = self.tabular_data.to_object(global_object)?; From 37294328aeead69120b195b14f2987175bf03f46 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 28 Jun 2026 20:27:00 +0000 Subject: [PATCH 11/15] console.table: store the row key as a ZigStringSlice instead of cloned bytes --- src/jsc/ConsoleObject.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/jsc/ConsoleObject.rs b/src/jsc/ConsoleObject.rs index 751239a4372a..aabfca1a6340 100644 --- a/src/jsc/ConsoleObject.rs +++ b/src/jsc/ConsoleObject.rs @@ -643,9 +643,13 @@ impl Default for Column { } enum RowKey { - /// Pre-rendered property-name bytes + visible width (plain-object - /// tabular data). Owned, so no WTF refcount outlives the iterator. - Str { text: Vec, width: u32 }, + /// Property-name UTF-8 slice + visible width (plain-object tabular data). + /// `to_utf8` refs the WTF impl (or owns a transcoded copy) and Drop + /// releases it, so the slice is safe to keep past the property iterator. + Str { + text: bun_core::ZigStringSlice, + width: u32, + }, /// Row index (array / iterable tabular data). Rendered on demand. Num(u32), } @@ -654,7 +658,7 @@ impl RowKey { fn str(name: &BunString) -> Self { Self::Str { width: u32::try_from(name.visible_width_exclude_ansi_colors(false)).expect("int cast"), - text: name.to_utf8_bytes(), + text: name.to_utf8(), } } @@ -904,7 +908,7 @@ impl<'a> TablePrinter<'a> { .ok(); match &row.key { RowKey::Str { text, .. } => { - writer.write_all(text).ok(); + writer.write_all(text.slice()).ok(); } RowKey::Num(value) => { write!(writer, "{value}").ok(); From f9f81ebf6834caab79230fa0bcde9b5239d34f77 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 28 Jun 2026 21:15:44 +0000 Subject: [PATCH 12/15] ci: retrigger From 1b2dbeb30479fcc492b1bc9638b49996ef3a45bc Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 28 Jun 2026 22:37:35 +0000 Subject: [PATCH 13/15] console.table: format each cell once into a shared buffer A table is built in two passes: column sizing, then rendering. Reading a property in both passes invoked its getter twice and rendered the second call's value; formatting a value in both passes ran its custom inspect hook twice, with the same symptom one level down. Buffering every cell's rendered bytes in its own Vec fixed the reads but roughly doubled peak memory; rooting the JSValues in a MarkedArgumentBuffer instead fixed the memory but kept the double format. Format each cell exactly once during the width pass, appending the rendered bytes to one shared scratch buffer, and record its byte range plus visible width. The render pass replays those ranges. Each property is read once, each value is string-ified once, and the only per-cell cost is the offset/len/width record. MarkedArgumentBuffer is back to main since nothing here uses it anymore. --- src/jsc/ConsoleObject.rs | 430 +++++++++--------- src/jsc/MarkedArgumentBuffer.rs | 3 - .../bindings/MarkedArgumentBufferBinding.cpp | 5 +- test/js/bun/console/console-table.test.ts | 24 +- 4 files changed, 233 insertions(+), 229 deletions(-) diff --git a/src/jsc/ConsoleObject.rs b/src/jsc/ConsoleObject.rs index aabfca1a6340..0af86221ba43 100644 --- a/src/jsc/ConsoleObject.rs +++ b/src/jsc/ConsoleObject.rs @@ -670,30 +670,35 @@ impl RowKey { } } -/// A cell read during the width pass: its value (kept alive by the -/// `MarkedArgumentBuffer` that roots every captured cell for the whole -/// `print_table` call) and its pre-computed visible width. +/// One formatted cell: the byte range of its rendered text in the shared +/// `cell_text` scratch buffer, plus that text's visible terminal width. #[derive(Clone, Copy)] struct CellRef { - value: JSValue, + /// Byte offset into `cell_text`. + offset: usize, + /// Byte length in `cell_text`. + len: u32, + /// Visible width of the text, excluding ANSI escape sequences. width: u32, } -/// An absent cell: the row has no value for that column. -const EMPTY_CELL: CellRef = CellRef { - value: JSValue::ZERO, - width: 0, -}; +impl CellRef { + fn text<'t>(&self, cell_text: &'t [u8]) -> &'t [u8] { + &cell_text[self.offset..self.offset + self.len as usize] + } +} /// One table row, collected during the width pass so the render pass never -/// re-reads a property (which would re-invoke its getter). +/// re-reads a property (which would re-invoke its getter) and never +/// re-formats a value (which would re-run a custom inspect hook). struct CollectedRow { key: RowKey, - /// Indexed by `column_index - 1`. May be shorter than - /// `columns.len() - 1` when later rows added columns this row lacks. - cells: Vec, + /// Indexed by `column_index - 1`. `None` is an absent cell. May be + /// shorter than `columns.len() - 1` when later rows added columns this + /// row lacks. + cells: Vec>, /// Cell routed to the trailing "Values" column (primitives, Map values). - values_cell: CellRef, + values_cell: Option, } const PADDING: u32 = 1; @@ -726,64 +731,38 @@ impl<'a> TablePrinter<'a> { values_col_idx: usize::MAX, }) } -} - -/// A `Write` sink that counts visible terminal columns instead of bytes. -struct VisibleCharacterCounter<'a> { - width: &'a mut usize, -} - -impl bun_io::Write for VisibleCharacterCounter<'_> { - fn write_all(&mut self, bytes: &[u8]) -> bun_io::Result<()> { - *self.width += strings::immutable::visible::width::exclude_ansi_colors::utf8(bytes); - Ok(()) - } -} -impl<'a> TablePrinter<'a> { - /// Format one cell value to `writer`: bare for strings, quoted otherwise. - fn format_value_to( + /// Format `value` exactly once (bare for strings, quoted otherwise), + /// appending its rendered bytes to the shared `cell_text` scratch, and + /// return the recorded byte range plus its visible width. + fn format_cell( &self, + cell_text: &mut Vec, value: JSValue, - writer: &mut dyn bun_io::Write, - ) -> JsResult<()> { + ) -> JsResult { + let offset = cell_text.len(); let mut value_formatter = self.value_formatter.shallow_clone(); let tag = formatter::Tag::get(value, self.global_object)?; value_formatter.quote_strings = !(matches!( tag.tag, TagPayload::String | TagPayload::StringPossiblyFormatted )); - value_formatter.format::(tag, writer, value, self.global_object) - } - - /// Compute how much horizontal space a `JSValue` will take when printed. - fn get_width_for_value(&self, value: JSValue) -> JsResult { - let mut width: usize = 0; - // PERF: writes straight into the counter with no buffering between - // the generic writer adapter and the counter. Profile if hot. - let mut counter = VisibleCharacterCounter { width: &mut width }; - self.format_value_to::(value, &mut counter)?; - Ok(width as u32) - } + value_formatter.format::(tag, cell_text, value, self.global_object)?; - /// Root `value` for the render pass and compute its visible width. - fn capture_cell( - &self, - roots: &mut jsc::MarkedArgumentBuffer, - value: JSValue, - ) -> JsResult { - roots.append(value); + let text = &cell_text[offset..]; Ok(CellRef { - value, - width: self.get_width_for_value(value)?, + offset, + len: u32::try_from(text.len()).expect("int cast"), + width: strings::immutable::visible::width::exclude_ansi_colors::utf8(text) as u32, }) } - /// Read the row's cell values exactly once, size the columns (creating - /// them on demand), and return the collected row for the render pass. - fn collect_row( + /// Read and format the row's cell values exactly once, size the columns + /// (creating them on demand), and return the collected row for the + /// render pass. + fn collect_row( &mut self, - roots: &mut jsc::MarkedArgumentBuffer, + cell_text: &mut Vec, columns: &mut Vec, row_key: RowKey, row_value: JSValue, @@ -793,18 +772,23 @@ impl<'a> TablePrinter<'a> { let mut row = CollectedRow { key: row_key, cells: Vec::new(), - values_cell: EMPTY_CELL, + values_cell: None, }; // special handling for Map: column with idx=1 is "Keys" if self.jstype.is_map() { - let key_cell = self.capture_cell(roots, row_value.get_index(self.global_object, 0)?)?; - let value_cell = - self.capture_cell(roots, row_value.get_index(self.global_object, 1)?)?; + let key_cell = self.format_cell::( + cell_text, + row_value.get_index(self.global_object, 0)?, + )?; + let value_cell = self.format_cell::( + cell_text, + row_value.get_index(self.global_object, 1)?, + )?; columns[1].width = columns[1].width.max(key_cell.width); self.values_col_width = Some(self.values_col_width.unwrap_or(0).max(value_cell.width)); - row.cells.push(key_cell); - row.values_cell = value_cell; + row.cells.push(Some(key_cell)); + row.values_cell = Some(value_cell); return Ok(row); } @@ -817,11 +801,11 @@ impl<'a> TablePrinter<'a> { if !self.properties.is_undefined() { for column in columns[1..].iter_mut() { if let Some(value) = row_value.get_own(self.global_object, &column.name)? { - let cell = self.capture_cell(roots, value)?; + let cell = self.format_cell::(cell_text, value)?; column.width = column.width.max(cell.width); - row.cells.push(cell); + row.cells.push(Some(cell)); } else { - row.cells.push(EMPTY_CELL); + row.cells.push(None); } } } else { @@ -860,20 +844,20 @@ impl<'a> TablePrinter<'a> { break 'brk columns.len() - 1; }; - let cell = self.capture_cell(roots, value)?; + let cell = self.format_cell::(cell_text, value)?; columns[col_idx].width = columns[col_idx].width.max(cell.width); let slot = col_idx - 1; if row.cells.len() <= slot { - row.cells.resize(slot + 1, EMPTY_CELL); + row.cells.resize(slot + 1, None); } - row.cells[slot] = cell; + row.cells[slot] = Some(cell); } } } else if self.properties.is_undefined() { // not object -> the value will go to the special "Values" column - let cell = self.capture_cell(roots, row_value)?; + let cell = self.format_cell::(cell_text, row_value)?; self.values_col_width = Some(self.values_col_width.unwrap_or(1).max(cell.width)); - row.values_cell = cell; + row.values_cell = Some(cell); } Ok(row) } @@ -892,12 +876,13 @@ impl<'a> TablePrinter<'a> { Ok(()) } - fn print_row( + fn print_row( &self, writer: &mut dyn bun_io::Write, columns: &[Column], row: &CollectedRow, - ) -> JsResult<()> { + cell_text: &[u8], + ) { writer.write_all("│".as_bytes()).ok(); { let needed = columns[0].width.saturating_sub(row.key.width()); @@ -925,24 +910,26 @@ impl<'a> TablePrinter<'a> { let cell = if col_idx == self.values_col_idx { row.values_cell } else { - row.cells.get(col_idx - 1).copied().unwrap_or(EMPTY_CELL) + row.cells.get(col_idx - 1).copied().flatten() }; - if cell.value.is_empty() { - writer - .splat_byte_all(b' ', (col.width + PADDING * 2) as usize) - .ok(); - } else { - let needed = col.width.saturating_sub(cell.width); - writer.splat_byte_all(b' ', PADDING as usize).ok(); - self.format_value_to::(cell.value, writer)?; - writer - .splat_byte_all(b' ', (needed + PADDING) as usize) - .ok(); + match cell { + None => { + writer + .splat_byte_all(b' ', (col.width + PADDING * 2) as usize) + .ok(); + } + Some(cell) => { + let needed = col.width.saturating_sub(cell.width); + writer.splat_byte_all(b' ', PADDING as usize).ok(); + writer.write_all(cell.text(cell_text)).ok(); + writer + .splat_byte_all(b' ', (needed + PADDING) as usize) + .ok(); + } } } writer.write_all("│\n".as_bytes()).ok(); - Ok(()) } pub fn print_table( @@ -985,156 +972,161 @@ impl<'a> TablePrinter<'a> { } } - // `roots` keeps every cell value read by the width pass alive so the - // render loop can format the same values instead of re-reading them - // (which would re-invoke getters and exhaust one-shot iterables). - jsc::MarkedArgumentBuffer::new(|roots| -> JsResult<()> { - // width pass: read each cell exactly once, sizing columns as we go - let mut rows: Vec = Vec::new(); - { - if self.is_iterable { - struct Ctx<'c, 'a> { - this: &'c mut TablePrinter<'a>, - roots: &'c mut jsc::MarkedArgumentBuffer, - columns: &'c mut Vec, - rows: &'c mut Vec, - idx: u32, - err: Option, + // Width pass: read and format each cell exactly once, appending its + // rendered bytes to `cell_text` and sizing columns as we go. The + // render pass replays those byte ranges, so no property is re-read + // (which would re-invoke its getter and exhaust one-shot iterables) + // and no value is re-formatted (which would re-run a custom inspect + // hook). + let mut cell_text: Vec = Vec::new(); + let mut rows: Vec = Vec::new(); + { + if self.is_iterable { + struct Ctx<'c, 'a> { + this: &'c mut TablePrinter<'a>, + cell_text: &'c mut Vec, + columns: &'c mut Vec, + rows: &'c mut Vec, + idx: u32, + err: Option, + } + // Capture before constructing `ctx` (which mutably borrows `*self`). + let tabular_data = self.tabular_data; + let mut ctx = Ctx { + this: self, + cell_text: &mut cell_text, + columns, + rows: &mut rows, + idx: 0, + err: None, + }; + extern "C" fn callback( + _: *mut jsc::VM, + _: &JSGlobalObject, + ctx: *mut c_void, + value: JSValue, + ) { + // SAFETY: ctx points to the stack `Ctx` above. + let ctx = unsafe { bun_ptr::callback_ctx::>(ctx) }; + // Once a cell failed, a JS exception may be pending (or + // the VM terminating); don't re-enter user code for the + // remaining elements. + if ctx.err.is_some() { + return; } - // Capture before constructing `ctx` (which mutably borrows `*self`). - let tabular_data = self.tabular_data; - let mut ctx = Ctx { - this: self, - roots, - columns, - rows: &mut rows, - idx: 0, - err: None, - }; - extern "C" fn callback( - _: *mut jsc::VM, - _: &JSGlobalObject, - ctx: *mut c_void, - value: JSValue, + match ctx.this.collect_row::( + ctx.cell_text, + ctx.columns, + RowKey::Num(ctx.idx), + value, ) { - // SAFETY: ctx points to the stack `Ctx` above. - let ctx = unsafe { bun_ptr::callback_ctx::>(ctx) }; - // Once a cell failed, a JS exception may be pending (or - // the VM terminating); don't re-enter user code for the - // remaining elements. - if ctx.err.is_some() { - return; - } - match ctx.this.collect_row( - ctx.roots, - ctx.columns, - RowKey::Num(ctx.idx), - value, - ) { - Ok(row) => ctx.rows.push(row), - Err(err) => ctx.err = Some(err), - } - ctx.idx += 1; + Ok(row) => ctx.rows.push(row), + Err(err) => ctx.err = Some(err), } - tabular_data.for_each_with_context( - global_object, - (&raw mut ctx).cast::(), - callback, - )?; - if let Some(err) = ctx.err { - return Err(err); - } - } else { - let tabular_obj = self.tabular_data.to_object(global_object)?; - let mut rows_iter = jsc::JSPropertyIterator::init( - global_object, - tabular_obj, - jsc::PropertyIteratorOptions { - skip_empty_name: false, - include_value: true, - }, - )?; + ctx.idx += 1; + } + tabular_data.for_each_with_context( + global_object, + (&raw mut ctx).cast::(), + callback::, + )?; + if let Some(err) = ctx.err { + return Err(err); + } + } else { + let tabular_obj = self.tabular_data.to_object(global_object)?; + let mut rows_iter = jsc::JSPropertyIterator::init( + global_object, + tabular_obj, + jsc::PropertyIteratorOptions { + skip_empty_name: false, + include_value: true, + }, + )?; - while let Some(row_key) = rows_iter.next()? { - let key = RowKey::str(&BunString::init(row_key)); - let row = self.collect_row(roots, columns, key, rows_iter.value)?; - rows.push(row); - } + while let Some(row_key) = rows_iter.next()? { + let key = RowKey::str(&BunString::init(row_key)); + let row = self.collect_row::( + &mut cell_text, + columns, + key, + rows_iter.value, + )?; + rows.push(row); } } + } - // append the special "Values" column as the last one, if it is present - if let Some(width) = self.values_col_width { - self.values_col_idx = columns.len(); - columns.push(Column { - name: BunString::static_("Values"), - width, - }); - } + // append the special "Values" column as the last one, if it is present + if let Some(width) = self.values_col_width { + self.values_col_idx = columns.len(); + columns.push(Column { + name: BunString::static_("Values"), + width, + }); + } - // print the table header (border line + column names line + border line) - { - for col in columns.iter_mut() { - // also update the col width with the length of the column name itself - col.width = col.width.max( - u32::try_from(col.name.visible_width_exclude_ansi_colors(false)) - .expect("int cast"), - ); - } + // print the table header (border line + column names line + border line) + { + for col in columns.iter_mut() { + // also update the col width with the length of the column name itself + col.width = col.width.max( + u32::try_from(col.name.visible_width_exclude_ansi_colors(false)) + .expect("int cast"), + ); + } - writer.write_all("┌".as_bytes()).ok(); - for (i, col) in columns.iter().enumerate() { - if i > 0 { - writer.write_all("┬".as_bytes()).ok(); - } - Self::write_string_n_times( - writer, - "─".as_bytes(), - (col.width + PADDING * 2) as usize, - ) - .ok(); + writer.write_all("┌".as_bytes()).ok(); + for (i, col) in columns.iter().enumerate() { + if i > 0 { + writer.write_all("┬".as_bytes()).ok(); } + Self::write_string_n_times( + writer, + "─".as_bytes(), + (col.width + PADDING * 2) as usize, + ) + .ok(); + } - writer.write_all("┐\n│".as_bytes()).ok(); + writer.write_all("┐\n│".as_bytes()).ok(); - for (i, col) in columns.iter().enumerate() { - if i > 0 { - writer.write_all("│".as_bytes()).ok(); - } - let len = col.name.visible_width_exclude_ansi_colors(false); - let needed = (col.width as usize).saturating_sub(len); - writer.splat_byte_all(b' ', 1).ok(); - if ENABLE_ANSI_COLORS { - writer.write_all(pfmt!("", true).as_bytes()).ok(); - } - write!(writer, "{}", col.name).ok(); - if ENABLE_ANSI_COLORS { - writer.write_all(pfmt!("", true).as_bytes()).ok(); - } - writer.splat_byte_all(b' ', needed + PADDING as usize).ok(); + for (i, col) in columns.iter().enumerate() { + if i > 0 { + writer.write_all("│".as_bytes()).ok(); } - - writer.write_all("│\n├".as_bytes()).ok(); - for (i, col) in columns.iter().enumerate() { - if i > 0 { - writer.write_all("┼".as_bytes()).ok(); - } - Self::write_string_n_times( - writer, - "─".as_bytes(), - (col.width + PADDING * 2) as usize, - ) - .ok(); + let len = col.name.visible_width_exclude_ansi_colors(false); + let needed = (col.width as usize).saturating_sub(len); + writer.splat_byte_all(b' ', 1).ok(); + if ENABLE_ANSI_COLORS { + writer.write_all(pfmt!("", true).as_bytes()).ok(); } - writer.write_all("┤\n".as_bytes()).ok(); + write!(writer, "{}", col.name).ok(); + if ENABLE_ANSI_COLORS { + writer.write_all(pfmt!("", true).as_bytes()).ok(); + } + writer.splat_byte_all(b' ', needed + PADDING as usize).ok(); } - // render pass: format the captured cell values - for row in rows.iter() { - self.print_row::(writer, columns, row)?; + writer.write_all("│\n├".as_bytes()).ok(); + for (i, col) in columns.iter().enumerate() { + if i > 0 { + writer.write_all("┼".as_bytes()).ok(); + } + Self::write_string_n_times( + writer, + "─".as_bytes(), + (col.width + PADDING * 2) as usize, + ) + .ok(); } - Ok(()) - })?; + writer.write_all("┤\n".as_bytes()).ok(); + } + + // render pass: replay each row's pre-formatted cell bytes + for row in rows.iter() { + self.print_row(writer, columns, row, &cell_text); + } // print the table bottom border { diff --git a/src/jsc/MarkedArgumentBuffer.rs b/src/jsc/MarkedArgumentBuffer.rs index 8935d46e6dd9..a7520a127f77 100644 --- a/src/jsc/MarkedArgumentBuffer.rs +++ b/src/jsc/MarkedArgumentBuffer.rs @@ -47,9 +47,6 @@ impl MarkedArgumentBuffer { ctx.r.unwrap() } - /// Append `value` and GC-root it for the buffer's lifetime. Infallible: - /// the C++ side uses `appendWithCrashOnOverflow`, so an allocation - /// failure is a loud OOM crash, never a silently unrooted value. pub fn append(&mut self, value: JSValue) { MarkedArgumentBuffer__append(self, value) } diff --git a/src/jsc/bindings/MarkedArgumentBufferBinding.cpp b/src/jsc/bindings/MarkedArgumentBufferBinding.cpp index 8aa84d20a901..5d9bde10c205 100644 --- a/src/jsc/bindings/MarkedArgumentBufferBinding.cpp +++ b/src/jsc/bindings/MarkedArgumentBufferBinding.cpp @@ -14,10 +14,7 @@ extern "C" SUPPRESS_ASAN void MarkedArgumentBuffer__run( callback(ctx, &args); } -// appendWithCrashOnOverflow: with RecordOverflow, plain append() silently -// drops the value on a failed spill-to-heap, leaving it unrooted while Rust -// keeps using it. No caller can recover, so prefer a loud OOM to a GC UAF. extern "C" void MarkedArgumentBuffer__append(void* args, JSC::EncodedJSValue value) { - static_cast(args)->appendWithCrashOnOverflow(JSC::JSValue::decode(value)); + static_cast(args)->append(JSC::JSValue::decode(value)); } diff --git a/test/js/bun/console/console-table.test.ts b/test/js/bun/console/console-table.test.ts index 863cdc8484ae..32fee910127d 100644 --- a/test/js/bun/console/console-table.test.ts +++ b/test/js/bun/console/console-table.test.ts @@ -276,6 +276,25 @@ describe("console.table reads each cell once", () => { }); }); + // String-ifying a cell runs user code. It must run exactly once per cell, + // and the table must show that single call's result, not a later one's. + test("a custom inspect on a cell value is invoked exactly once", () => { + let calls = 0; + const out = Bun.inspect.table([ + { + x: { + [Bun.inspect.custom]() { + return "C" + ++calls; + }, + }, + }, + ]); + expect({ calls, out }).toEqual({ + calls: 1, + out: `┌───┬────┐\n│ │ x │\n├───┼────┤\n│ 0 │ C1 │\n└───┴────┘\n`, + }); + }); + test("a throwing custom inspect in a cell still propagates", () => { const boom = new Error("boom"); expect(() => @@ -291,9 +310,8 @@ describe("console.table reads each cell once", () => { ).toThrow(boom); }); - // The width pass caches each cell's JSValue in a Rust Vec, which JSC's - // conservative scan never visits; only the MarkedArgumentBuffer roots them. - // 64 rows spills it past the 8-slot inline (stack-scanned) buffer. + // Each getter runs arbitrary user code, including a full GC. The cell must + // still render the value that its single read returned. test("cell values survive a full GC between the width and render passes", () => { const N = 64; const rows = Array.from({ length: N }, (_, i) => ({ From 2d04225fecc52449f3a174faa43576fc407bf5af Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 28 Jun 2026 23:05:55 +0000 Subject: [PATCH 14/15] console.table: store the cell byte length as usize The formatted byte length of a cell is user-controlled (a single JS string plus escape expansion can exceed u32::MAX bytes as UTF-8), so narrowing it through u32::try_from(...).expect() could panic the process on large input. CellRef only uses the length to slice cell_text, so keep it as usize end to end. --- src/jsc/ConsoleObject.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/jsc/ConsoleObject.rs b/src/jsc/ConsoleObject.rs index 0af86221ba43..4d91d1130578 100644 --- a/src/jsc/ConsoleObject.rs +++ b/src/jsc/ConsoleObject.rs @@ -677,14 +677,14 @@ struct CellRef { /// Byte offset into `cell_text`. offset: usize, /// Byte length in `cell_text`. - len: u32, + len: usize, /// Visible width of the text, excluding ANSI escape sequences. width: u32, } impl CellRef { fn text<'t>(&self, cell_text: &'t [u8]) -> &'t [u8] { - &cell_text[self.offset..self.offset + self.len as usize] + &cell_text[self.offset..self.offset + self.len] } } @@ -752,7 +752,7 @@ impl<'a> TablePrinter<'a> { let text = &cell_text[offset..]; Ok(CellRef { offset, - len: u32::try_from(text.len()).expect("int cast"), + len: text.len(), width: strings::immutable::visible::width::exclude_ansi_colors::utf8(text) as u32, }) } From 0a36c5e5f9a2d30fa135cc0753280be1ebd24619 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 28 Jun 2026 23:23:36 +0000 Subject: [PATCH 15/15] console.table: shorten the width-pass comment to three lines --- src/jsc/ConsoleObject.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/jsc/ConsoleObject.rs b/src/jsc/ConsoleObject.rs index 4d91d1130578..9e2a44e80e53 100644 --- a/src/jsc/ConsoleObject.rs +++ b/src/jsc/ConsoleObject.rs @@ -972,12 +972,9 @@ impl<'a> TablePrinter<'a> { } } - // Width pass: read and format each cell exactly once, appending its - // rendered bytes to `cell_text` and sizing columns as we go. The - // render pass replays those byte ranges, so no property is re-read - // (which would re-invoke its getter and exhaust one-shot iterables) - // and no value is re-formatted (which would re-run a custom inspect - // hook). + // Width pass: format each cell exactly once, appending its bytes to + // `cell_text` and sizing columns. The render pass replays those byte + // ranges, so no property is re-read and no value is re-formatted. let mut cell_text: Vec = Vec::new(); let mut rows: Vec = Vec::new(); {