Skip to content

Say that LimitMemory bounds what a script allocates, not what a host copies out of its result - #4182

Merged
lahma merged 1 commit into
sebastienros:mainfrom
lahma:docs/4175-limitmemory-result-scope
Sep 25, 2026
Merged

lahma merged 1 commit into
sebastienros:mainfrom
lahma:docs/4175-limitmemory-result-scope

Conversation

@lahma

@lahma lahma commented Sep 25, 2026

Copy link
Copy Markdown
Collaborator

#4173 charges a deferred + when it is built, so no single value a script hands back can cost more than its budget to read. #4175 is what that leaves open: many values that share characters. This PR does the first thing the issue lists. The documentation now says that LimitMemory bounds what the script allocates while it runs, not what a host copies out of the result afterwards, and it names the read that is bounded.

What the docs now say

docs/guide/constraints.md. The LimitMemory paragraph now states the scope and links to a new section, Bound what the host copies out of a result. The section says:

  • A long slice can be a view of the string it was cut from, and a long a + b refers to its operands. Each is charged only for what it adds.
  • ToString() and ToObject() copy every value in full, after the operation has ended and outside every constraint. The section gives the measured numbers.
  • The bounded read is engine.ConvertResult(value, ResultLimits.Conservative). A compiled sample (docs:guide-bounded-result in Jint.Documentation.Samples) shows it beside ToObject().
  • What each limit checks:
    • MaxStringLength is checked against a string's length before its characters are copied.
    • MaxOutputCharacters is counted after each copy.
    • Together they bound a conversion: at most 3,000,000 characters under Conservative.
  • A refusal throws ResultLimitExceededException, and its Limit names the bound that was reached.
  • Called without limits, ConvertResult uses Options.ResultLimits. That is unlimited by default, and UntrustedCodeLimits.ResultLimits under ForUntrustedCode. An unlimited conversion still runs under LimitMemory, but it is charged only after the copy is made.

The #4173 paragraph that mixed both points keeps only the build-time charge.

XML remarks. No paragraph was added past the four docs/xml-doc-style.md allows.

  • MemoryLimitConstraint: the fourth paragraph is reworded, still four lines. The AllocatedBytes sentence it displaced moves to that property's own <remarks>.
  • Engine.ConvertResult: a third paragraph says which two limits to set and what each checks.
  • JsValue.ToObject: a two-line <remarks> says nothing bounds what it copies, and points to ConvertResult.

Elsewhere.

  • untrusted-code.md gets one host-responsibility bullet.
  • THREAT_MODEL.md gets a residual-mitigation bullet in TM-06 (memory) and one in TM-17 (result conversion). The TM-17 bullet records the MaxOutputCharacters ordering.
  • Jint/Constraints/AGENTS.md is unchanged, at 32,582 bytes. Its Charge a deferred string concatenation to LimitMemory when it is built #4173 bullet already says "values sharing one operand stay open; ResultLimits bounds those".

Why: the two shapes from #4175, measured on this branch

The numbers come from a probe console outside the repository. They are GC.GetAllocatedBytesForCurrentThread deltas and MemoryLimitConstraint.AllocatedBytes, never timings. The engine has LimitMemory(16 MiB), and each variant ran in its own process.

shape charged while the script ran host ToObject() ConvertResult(…, Conservative) ConvertResult, only MaxOutputCharacters = 2M ConvertResult, default ResultLimits
64 × s + 'q' over one 1M-char rope 2.2 MB 134 MB refused on StringLength, 0.0 MB 4.2 MB copied, then refused 134 MB copied, then MemoryLimitExceededException
64 slice views of one 1M-char string 2.2 MB 132 MB refused on StringLength, 0.0 MB not run 132 MB copied, then MemoryLimitExceededException
64 × 900,000-char slices (under MaxStringLength) 2.2 MB not run 2.7M chars observed, 5.4 MB copied, refused on OutputCharacters 5.4 MB, same not run

The last two columns are why the docs say "set both":

  • With only MaxOutputCharacters, a conversion copies the string that crosses the limit before it refuses.
  • With neither, LimitMemory sees the copy only after it has been made.

The issue body says MaxOutputCharacters "checks the logical length before anything is flattened". That is not what the code does: ResultConverter counts output characters after ToString(). The #4162 measurement it quotes was refused by MaxStringLength.

Tests

HostDeferredStringMemoryLimitTests.ConvertResultRefusesValuesSharingOneStringBeforeCopyingThem has two cases, slice views and concatenations over one operand. Each asserts that ConvertResult(value, ResultLimits.Conservative) throws ResultLimitExceededException with Limit == StringLength. Each also asserts that the conversion entry's AllocatedBytes is under 1 MB; copying one value would allocate 2 MB. The test uses only the public API. Neither case flattens its subject.

Broken on purpose. I moved CountStringLength after value.ToString() in ResultConverter.ConvertPrimitive. Both cases failed with Expected constraint.AllocatedBytes to be less than 1048576L … but found 2100032L (and 2100720L). I then restored the file.

All runs are Release.

  • Jint.Tests.PublicInterface, whole project, net10.0, no --timeout: 3,819 total, 0 failed, 21 skipped (the existing skips).
  • HostDeferredStringMemoryLimitTests, HostResultLimitsTests and PublicApiDocumentationTest on net8.0 and net472: 42 total each, 0 failed.
  • MigrationGuideTests, AgentInstructionFileTests and SpecCitationTests (net10.0): 11 of 11 passed, before and after the rebase.
  • PublicApiTest is unchanged. The change is XML docs only, with no signatures touched.
  • Jint builds with 0 warnings on all five TFMs, and every new cref resolves in each TFM's Jint.xml. Jint.Documentation.Samples builds with 0 warnings.
  • Docs checks:
    • npm run samples:check: 10 snippets current.
    • npm run lint: 0 issues in 97 files.
    • DOCS_BASE=/jint/ npm run build and npm run check-links: no dead internal links, assets or anchors. That includes the new #bound-what-the-host-copies-out-of-a-result.
    • npm run test:tools: 6 of 7. The failing one, check-links.test.mjs "rejects missing generated targets…", fails on Windows only. It expects guide/start.html and gets guide\start.html. This PR does not touch docs/scripts.

No benchmark was run: the engine has no code change.

Base: 7d9cdcc8fa1179b8ea92db17f617f6154dc56b7e (upstream main).

Part of #4175. The issue's engine-side options are still open: charging a view or rope root when it leaves through the result path, or capping the aggregate logical length reachable from the completion value. This PR only documents the bounded read that exists today.

🤖 Generated with Claude Code

…copies out of its result

A long slice is a view of the string it was cut from and a long a + b refers
to its operands, so many values sharing one string's characters are each
charged only for what they add. JsValue.ToString() and ToObject() then copy
every one of them in full, after the operation and outside every constraint:
64 slices of one 1,048,576-character string cost the script about 2 MB under
a 16 MB limit and cost the host's ToObject() more than 130 MB.

docs/guide/constraints.md gains a section saying so, with a compiled sample
contrasting ToObject() with Engine.ConvertResult under ResultLimits, and
states what each result limit checks: MaxStringLength refuses a string by
its length before copying it, MaxOutputCharacters is counted after each
copy, so the two together bound a conversion. The same scope is stated in
the XML remarks of MemoryLimitConstraint, Engine.ConvertResult and
JsValue.ToObject, in untrusted-code.md's host responsibilities, and as
residual mitigations in THREAT_MODEL.md TM-06 and TM-17.

HostDeferredStringMemoryLimitTests pins the documented bounded read for both
shapes: ConvertResult under ResultLimits.Conservative refuses the first
value on StringLength while the conversion entry allocates less than one
copy. Both cases fail when the length check is moved after the copy.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@lahma
lahma merged commit 77d77cb into sebastienros:main Sep 25, 2026
11 checks passed
@lahma
lahma deleted the docs/4175-limitmemory-result-scope branch September 25, 2026 18:52
lahma added a commit that referenced this pull request Oct 10, 2026
… makes it (#4256)

Engine.ConvertResult runs as an engine entry, so LimitMemory bounds it even
with ResultLimits.Unlimited, the default. But the converter checks
constraints once per Engine.ConstraintCheckInterval values (10,000), not per
character, and a slice view or a deferred a + b costs the conversion a copy
of its whole length. Under LimitMemory(16 MB), 64 slices of one
1,048,576-character string cost the script 2.1 MB and cost ConvertResult
134 MB of copies before the entry's closing check refused it; 10,000 such
values would all be copied before the first check.

ResultConverter.ConvertString now checks a slice view or deferred
concatenation that nothing has read yet against what is left of the budget,
by its length, before copying it. It goes through the new internal
MemoryLimitConstraint.CheckBeforeCopying, which charges and refuses a copy
that would not fit, the way ChargeDeferredConcatenation refuses an
over-long node. The same conversion now refuses with 14.7 MB copied. Flat
text is handed back without a copy and is never checked, so a host string
larger than the whole budget still converts. A host LazyJsString is the
host's own allocation and stays on the existing cadence.

JsValue.ToObject() and ToString() stay unbounded, as documented since
#4182. They also amplify shared structure that contains no strings: twenty
nested [x, x] arrays become 100 MB on ToObject().

The constraints and untrusted-code guides, the ConvertResult and
MemoryLimitConstraint remarks, THREAT_MODEL TM-06/TM-17, migration guide
4.27 and Jint/Constraints/AGENTS.md now say so.

Fixes #4175


Claude-Session: https://claude.ai/code/session_01YVwU18SgMKKzYrrCh4TMpT

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant