diff --git a/crates/js/src/builtins/object.rs b/crates/js/src/builtins/object.rs index 969a9c6094..af41bb7782 100644 --- a/crates/js/src/builtins/object.rs +++ b/crates/js/src/builtins/object.rs @@ -166,10 +166,11 @@ fn create(interp: &mut Interp, _: Value, args: &[Value], _: bool) -> JsResult JsResult { let o = require_object(interp, &arg(args, 0), "defineProperty")?; - let k = interp.to_key(&arg(args, 1))?; - let desc = to_desc(interp, &arg(args, 2))?; - let roots = desc_values(&desc); - interp.rooted(&roots, |interp| interp.define_own_property(o, k, &desc, true))?; + interp.with_key(&arg(args, 1), |interp, k| { + let desc = to_desc(interp, &arg(args, 2))?; + let roots = desc_values(&desc); + interp.rooted(&roots, |interp| interp.define_own_property(o, k, &desc, true)) + })?; Ok(Value::Object(o)) } diff --git a/crates/js/src/interp.rs b/crates/js/src/interp.rs index edc5282fe5..aa4a4c5c78 100644 --- a/crates/js/src/interp.rs +++ b/crates/js/src/interp.rs @@ -1779,8 +1779,7 @@ impl Interp { return self.replace(2, v); } self.check_coercible(&base, &[])?; - let name = self.to_key(&k)?; - let v = self.get_value(&base, &name)?; + let v = self.with_key(&k, |i, name| i.get_value(&base, &name))?; self.replace(2, v)?; if with_this { self.push(base) } else { Ok(()) } } @@ -1852,8 +1851,7 @@ impl Interp { return self.replace(2, Value::Number(if prefix { new } else { old })); } self.check_coercible(&base, &[])?; - let name = self.to_key(&k)?; - let r = self.inc_property(&base, name, prefix, dec)?; + let r = self.with_key(&k, |i, name| i.inc_property(&base, name, prefix, dec))?; self.replace(2, r) } @@ -2670,6 +2668,18 @@ impl Interp { } } + /// Runs `f` with the property name [`Self::to_key`] makes from `v`. A name converted from a + /// value that is not a string is kept reachable while `f` runs, so a collection that a getter + /// or setter starts still counts its units. + pub(crate) fn with_key(&mut self, v: &Value, f: impl FnOnce(&mut Self, Key) -> JsResult) -> JsResult { + let name = self.to_key(v)?; + if matches!(v, Value::String(_)) { + return f(self, name); + } + let root = Value::String(Str::from_rc(name.clone())); + self.rooted(&[root], |i| f(i, name)) + } + /// The code units of `s`. Flattening a rope makes a copy of its units, so for a rope that /// copy's bytes are reserved and added to the heap first, without taking fuel. The rope stays /// rooted while the copy is reserved; everything else the caller still needs must be diff --git a/crates/js/src/text_tests.rs b/crates/js/src/text_tests.rs index 01fc613cc3..62145b6117 100644 --- a/crates/js/src/text_tests.rs +++ b/crates/js/src/text_tests.rs @@ -434,6 +434,32 @@ fn a_call_keeps_its_operands_while_a_string_receiver_is_boxed() { assert_eq!(realm.run(src), Ok(JsValue::Number(1.0))); } +#[test] +fn a_computed_key_stays_counted_while_its_getter_or_setter_runs() { + // `o` has an accessor named by a flat string of 131,073 units. `key`'s toString returns a new + // rope with the same units, so using `key` as a name makes a second flat copy, which must stay + // counted while an accessor makes a third. The heap has room for one more copy, not two. + let build = "var s = 'x'; for (var i = 0; i < 17; i++) s = s + s; function f(c) { return s + c; } \ + function copy() { var g = f('g'); g[0]; return 1; } var o = {}, name = f('k'); name[0]; \ + Object.defineProperty(o, name, { get: copy, set: copy }); \ + var key = { toString: function () { return f('k'); } }; 0"; + let realm = || { + let mut realm = Realm::with_options(Options::default(), Limits::default()); + assert_eq!(realm.run(build), Ok(JsValue::Number(0.0))); + let interp = realm.interp(); + interp.collect_garbage(); + let copy = crate::heap::string_bytes(131_073); + interp.limits.heap_bytes = interp.heap.bytes + copy + copy / 2; + interp.reset_gc_threshold(); + realm + }; + assert_eq!(realm().run("o[name]"), Ok(JsValue::Number(1.0)), "the accessor's own copy fits"); + for read in ["o[key]", "o[key] = 1", "o[key]++", "Object.defineProperty({}, key, { get value() { return copy(); } })"] { + let result = realm().run(read); + assert!(matches!(result, Err(ref e) if e.kind == RunErrorKind::Limit(LimitKind::Heap)), "{read}: {result:?}"); + } +} + #[test] fn compiling_a_pattern_counts_against_the_heap() { let src = "var s = '[\\\\s\\\\S]a*'; for (var i = 0; i < 14; i++) s = s + s; new RegExp(s).source.length";