Temporal: a month difference past the calendar's range raises RangeError instead of spinning - #3452
Merged
lahma merged 1 commit intoAug 27, 2026
Conversation
…ead of spinning
`NonIsoCalendars.CalendarDateUntil` walks one month at a time towards its
target and stops when a step passes it. When the conversion back to ISO
saturated it answered with `cal.MaxSupportedDateTime` — the calendar's
*maximum* whichever end had been overrun — so every further step landed on
that same date, no step ever passed the target, and the loop had no other
exit.
Temporal.PlainDate.from('1910-01-01').withCalendar('chinese')
.until(Temporal.PlainDate.from('1900-01-03').withCalendar('chinese'),
{ largestUnit: 'year' });
// never returned
`ChineseLunisolarCalendar` spans ISO 1901-02-19 to 2101-01-28, so the target
is outside it. It is a CLR loop inside one interpreter step, so it crosses no
statement boundary: `LimitStatements` never counted, `LimitExecutionTime`
never got a check and a `CancellationToken` was never observed. A host that
bounded untrusted script the documented way still lost the thread.
Out-of-range calendar arithmetic now reports itself instead of clamping.
`NonISODateAdd` is implementation-defined and may throw, and `CalendarDateAdd`
raises a `RangeError` for a result it cannot represent, which is what the two
`TemporalHelpers` entry points now do with it. The same clamp was answering
`add`/`subtract` too: subtracting a century from a 1950 Chinese date moved it
forward to 2101-01-28 and reported success.
Two sibling no-progress fallbacks go with it — `IndianCalendarDateAdd` and
`IslamicTabularCalendarDateAdd` answered with their *input* date when the
conversion failed — and the walk keeps a no-progress guard of its own, so a
step that does not move the date can never again mean a loop that does not end.
`DedicatedThread.Run` starts a body at normal priority and demotes the thread
to `Lowest` only once its join has timed out, rather than starting every body
there. A `Lowest` thread is the last thing a saturated runner schedules: on a
two-core affinity mask with two busy threads a 0.1 ms body took 2.5 s at
`Lowest` against 0.14 s at `Normal`, which is how one of the tests below missed
a 15-second budget doing ten microseconds of work on a four-core CI runner
running four test assemblies at once. The demotion still keeps a runaway off
the cores the rest of the run needs, which is all it was ever for.
Fixes sebastienros#3428
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014W5mbjGhyvgAS4pivXoc4S
lahma
force-pushed
the
issue-3428-noniso-month-difference
branch
from
August 27, 2026 02:16
911a8ac to
1e2b48d
Compare
lahma
added a commit
to lahma/jint
that referenced
this pull request
Aug 27, 2026
sebastienros#3453's Windows leg was red with every assembly reporting `Passed!`: Passed! - Failed: 0, Passed: 10784 ... Jint.Tests.dll (net8.0) Passed! - Failed: 0, Passed: 10784 ... Jint.Tests.dll (net10.0) Passed! - Failed: 0, Passed: 102495, Skipped: 189 ... Test262 ##[error]Process completed with exit code 1. `Jint.Tests.dll (net472)` is absent from that list, because its run never started: vstest.console process failed to connect to testhost process after 90 seconds. This may occur due to machine slowness, please set environment variable VSTEST_CONNECTION_TIMEOUT to increase timeout. Test Run Aborted. No test failed. vstest gives a testhost 90 seconds to connect back and aborts the whole run when it does not, and these jobs run four test assemblies at once on a four-core runner. The net472 host is the one that loses that race: slowest start, and it goes last. The cost is not the retry, it is that the failure carries no information. A red leg whose log says every assembly passed sends whoever reads it looking for a test that does not exist -- twice today, on changes that could not have caused it. So the three workflows set `VSTEST_CONNECTION_TIMEOUT: 300`. Five minutes is startup contention rather than a hang; anything genuinely stuck is caught by the suite's own per-test budgets, which is where a hang should be reported and where it says which test hung. Related, same oversubscription: sebastienros#3452 found every test body ran at `ThreadPriority.Lowest`, worth 2.5 s against 0.14 s for a 0.1 ms body with only two competitors. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014W5mbjGhyvgAS4pivXoc4S
lahma
added a commit
that referenced
this pull request
Aug 27, 2026
#3453's Windows leg was red with every assembly reporting `Passed!`: Passed! - Failed: 0, Passed: 10784 ... Jint.Tests.dll (net8.0) Passed! - Failed: 0, Passed: 10784 ... Jint.Tests.dll (net10.0) Passed! - Failed: 0, Passed: 102495, Skipped: 189 ... Test262 ##[error]Process completed with exit code 1. `Jint.Tests.dll (net472)` is absent from that list, because its run never started: vstest.console process failed to connect to testhost process after 90 seconds. This may occur due to machine slowness, please set environment variable VSTEST_CONNECTION_TIMEOUT to increase timeout. Test Run Aborted. No test failed. vstest gives a testhost 90 seconds to connect back and aborts the whole run when it does not, and these jobs run four test assemblies at once on a four-core runner. The net472 host is the one that loses that race: slowest start, and it goes last. The cost is not the retry, it is that the failure carries no information. A red leg whose log says every assembly passed sends whoever reads it looking for a test that does not exist -- twice today, on changes that could not have caused it. So the three workflows set `VSTEST_CONNECTION_TIMEOUT: 300`. Five minutes is startup contention rather than a hang; anything genuinely stuck is caught by the suite's own per-test budgets, which is where a hang should be reported and where it says which test hung. Related, same oversubscription: #3452 found every test body ran at `ThreadPriority.Lowest`, worth 2.5 s against 0.14 s for a 0.1 ms body with only two competitors. Claude-Session: https://claude.ai/code/session_014W5mbjGhyvgAS4pivXoc4S Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
lahma
added a commit
to lahma/jint
that referenced
this pull request
Aug 27, 2026
…calendar `ICalendarProvider` supplies two conversions, and the engine consulted them for the field accessors, `from`, `with`, `toString` and the `PlainYearMonth` / `PlainMonthDay` conversions — but not for `add`, `subtract`, `until` or `since`, which were written per calendar against the BCL. So a host that *corrected* a calendar corrected half of it, and the two halves disagreed about the same date; a host that *added* one got `RangeError: Calendar arithmetic is not implemented for 'mayan'`. For a calendar the configured provider answers for, the year-and-month walk is now expressed in the two conversions themselves: the same monthCode placed in the target year, ordinal months stepped across year boundaries by the month count the conversion reports, and the day clamped to the month length it reports. Every one of those conversions may decline, and each declining ends in the "reject" signal an out-of-range date already raises, so no null is dereferenced and no CLR exception leaves the engine. `CalendarDateUntil` needed only the engine threading through: it was already written in calendar-field terms, over `IsoToCalendarDate` and `CalendarDateAdd`. The dispatch is the same identity-then-membership test the two conversions already make — not the default singleton, and the provider claims this calendar — so a date's arithmetic and its field accessors are never answered by two different reckonings. An unconfigured engine reaches the per-calendar implementation it always did: 774,158 `CalendarDateAdd` results over the eleven built-in calendars, before and after, are byte-identical. Three things came out of writing it down. The whole-year month estimate in `CalendarDateUntil` was a hardcoded 12 or 13 and is now the count the conversion reports, which needed a back-off loop because the forward walk only ever steps one way and an estimate that already passed the target used to come back as a negative day count. `DifferenceISODateTime` reached `CalendarDateUntil` with no realm, so its refusal escaped `Engine.Evaluate` as a CLR exception. And the month walk never terminated when a conversion saturated. That last one is sebastienros#3428, which sebastienros#3452 has since fixed for the eleven calendars the engine reckons itself, by taking away the clamp that answered with a boundary date at all. The walk keeps the no-progress guard both changes arrived at, and it raises `CalendarRangeException` there rather than answering with a degraded difference: for a built-in calendar it is now unreachable, which is the shape provider that clamps and a walk that turns forever, since the walk is written in whichever two conversions answer for the calendar. `NonIsoCalendarRangeTests` pins the built-in half and still passes with the guard removed; the provider half is the new case in `TemporalCalendarArithmeticTests`, which does not return at all without it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014W5mbjGhyvgAS4pivXoc4S
lahma
added a commit
to lahma/jint
that referenced
this pull request
Aug 27, 2026
…calendar `ICalendarProvider` supplies two conversions, and the engine consulted them for the field accessors, `from`, `with`, `toString` and the `PlainYearMonth` / `PlainMonthDay` conversions — but not for `add`, `subtract`, `until` or `since`, which were written per calendar against the BCL. So a host that *corrected* a calendar corrected half of it, and the two halves disagreed about the same date; a host that *added* one got `RangeError: Calendar arithmetic is not implemented for 'mayan'`. For a calendar the configured provider answers for, the year-and-month walk is now expressed in the two conversions themselves: the same monthCode placed in the target year, ordinal months stepped across year boundaries by the month count the conversion reports, and the day clamped to the month length it reports. Every one of those conversions may decline, and each declining ends in the "reject" signal an out-of-range date already raises, so no null is dereferenced and no CLR exception leaves the engine. `CalendarDateUntil` needed only the engine threading through: it was already written in calendar-field terms, over `IsoToCalendarDate` and `CalendarDateAdd`. The dispatch is the same identity-then-membership test the two conversions already make — not the default singleton, and the provider claims this calendar — so a date's arithmetic and its field accessors are never answered by two different reckonings. An unconfigured engine reaches the per-calendar implementation it always did: 774,158 `CalendarDateAdd` results over the eleven built-in calendars, before and after, are byte-identical. Three things came out of writing it down. The whole-year month estimate in `CalendarDateUntil` was a hardcoded 12 or 13 and is now the count the conversion reports, which needed a back-off loop because the forward walk only ever steps one way and an estimate that already passed the target used to come back as a negative day count. `DifferenceISODateTime` reached `CalendarDateUntil` with no realm, so its refusal escaped `Engine.Evaluate` as a CLR exception. And the month walk never terminated when a conversion saturated. That last one is sebastienros#3428, which sebastienros#3452 has since fixed for the eleven calendars the engine reckons itself, by taking away the clamp that answered with a boundary date at all. The walk keeps the no-progress guard both changes arrived at, and it raises `CalendarRangeException` there rather than answering with a degraded difference: for a built-in calendar it is now unreachable, which is the shape provider that clamps and a walk that turns forever, since the walk is written in whichever two conversions answer for the calendar. `NonIsoCalendarRangeTests` pins the built-in half and still passes with the guard removed; the provider half is the new case in `TemporalCalendarArithmeticTests`, which does not return at all without it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014W5mbjGhyvgAS4pivXoc4S
lahma
added a commit
to lahma/jint
that referenced
this pull request
Aug 27, 2026
…calendar `ICalendarProvider` supplies two conversions, and the engine consulted them for the field accessors, `from`, `with`, `toString` and the `PlainYearMonth` / `PlainMonthDay` conversions — but not for `add`, `subtract`, `until` or `since`, which were written per calendar against the BCL. So a host that *corrected* a calendar corrected half of it, and the two halves disagreed about the same date; a host that *added* one got `RangeError: Calendar arithmetic is not implemented for 'mayan'`. For a calendar the configured provider answers for, the year-and-month walk is now expressed in the two conversions themselves: the same monthCode placed in the target year, ordinal months stepped across year boundaries by the month count the conversion reports, and the day clamped to the month length it reports. Every one of those conversions may decline, and each declining ends in the "reject" signal an out-of-range date already raises, so no null is dereferenced and no CLR exception leaves the engine. `CalendarDateUntil` needed only the engine threading through: it was already written in calendar-field terms, over `IsoToCalendarDate` and `CalendarDateAdd`. The dispatch is the same identity-then-membership test the two conversions already make — not the default singleton, and the provider claims this calendar — so a date's arithmetic and its field accessors are never answered by two different reckonings. An unconfigured engine reaches the per-calendar implementation it always did: 774,158 `CalendarDateAdd` results over the eleven built-in calendars, before and after, are byte-identical. Three things came out of writing it down. The whole-year month estimate in `CalendarDateUntil` was a hardcoded 12 or 13 and is now the count the conversion reports, which needed a back-off loop because the forward walk only ever steps one way and an estimate that already passed the target used to come back as a negative day count. `DifferenceISODateTime` reached `CalendarDateUntil` with no realm, so its refusal escaped `Engine.Evaluate` as a CLR exception. And the month walk never terminated when a conversion saturated. That last one is sebastienros#3428, which sebastienros#3452 has since fixed for the eleven calendars the engine reckons itself, by taking away the clamp that answered with a boundary date at all. The walk keeps the no-progress guard both changes arrived at, and it raises `CalendarRangeException` there rather than answering with a degraded difference: for a built-in calendar it is now unreachable, which is the shape provider that clamps and a walk that turns forever, since the walk is written in whichever two conversions answer for the calendar. `NonIsoCalendarRangeTests` pins the built-in half and still passes with the guard removed; the provider half is the new case in `TemporalCalendarArithmeticTests`, which does not return at all without it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014W5mbjGhyvgAS4pivXoc4S
lahma
added a commit
to lahma/jint
that referenced
this pull request
Aug 27, 2026
…calendar `ICalendarProvider` supplies two conversions, and the engine consulted them for the field accessors, `from`, `with`, `toString` and the `PlainYearMonth` / `PlainMonthDay` conversions — but not for `add`, `subtract`, `until` or `since`, which were written per calendar against the BCL. So a host that *corrected* a calendar corrected half of it, and the two halves disagreed about the same date; a host that *added* one got `RangeError: Calendar arithmetic is not implemented for 'mayan'`. For a calendar the configured provider answers for, the year-and-month walk is now expressed in the two conversions themselves: the same monthCode placed in the target year, ordinal months stepped across year boundaries by the month count the conversion reports, and the day clamped to the month length it reports. Every one of those conversions may decline, and each declining ends in the "reject" signal an out-of-range date already raises, so no null is dereferenced and no CLR exception leaves the engine. `CalendarDateUntil` needed only the engine threading through: it was already written in calendar-field terms, over `IsoToCalendarDate` and `CalendarDateAdd`. The dispatch is the same identity-then-membership test the two conversions already make — not the default singleton, and the provider claims this calendar — so a date's arithmetic and its field accessors are never answered by two different reckonings. An unconfigured engine reaches the per-calendar implementation it always did: 774,158 `CalendarDateAdd` results over the eleven built-in calendars, before and after, are byte-identical. Three things came out of writing it down. The whole-year month estimate in `CalendarDateUntil` was a hardcoded 12 or 13 and is now the count the conversion reports, which needed a back-off loop because the forward walk only ever steps one way and an estimate that already passed the target used to come back as a negative day count. `DifferenceISODateTime` reached `CalendarDateUntil` with no realm, so its refusal escaped `Engine.Evaluate` as a CLR exception. And the month walk never terminated when a conversion saturated. That last one is sebastienros#3428, which sebastienros#3452 has since fixed for the eleven calendars the engine reckons itself, by taking away the clamp that answered with a boundary date at all. The walk keeps the no-progress guard both changes arrived at, and it raises `CalendarRangeException` there rather than answering with a degraded difference: for a built-in calendar it is now unreachable, which is the shape provider that clamps and a walk that turns forever, since the walk is written in whichever two conversions answer for the calendar. `NonIsoCalendarRangeTests` pins the built-in half and still passes with the guard removed; the provider half is the new case in `TemporalCalendarArithmeticTests`, which does not return at all without it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014W5mbjGhyvgAS4pivXoc4S
lahma
added a commit
that referenced
this pull request
Aug 27, 2026
…calendar (#3429) `ICalendarProvider` supplies two conversions, and the engine consulted them for the field accessors, `from`, `with`, `toString` and the `PlainYearMonth` / `PlainMonthDay` conversions — but not for `add`, `subtract`, `until` or `since`, which were written per calendar against the BCL. So a host that *corrected* a calendar corrected half of it, and the two halves disagreed about the same date; a host that *added* one got `RangeError: Calendar arithmetic is not implemented for 'mayan'`. For a calendar the configured provider answers for, the year-and-month walk is now expressed in the two conversions themselves: the same monthCode placed in the target year, ordinal months stepped across year boundaries by the month count the conversion reports, and the day clamped to the month length it reports. Every one of those conversions may decline, and each declining ends in the "reject" signal an out-of-range date already raises, so no null is dereferenced and no CLR exception leaves the engine. `CalendarDateUntil` needed only the engine threading through: it was already written in calendar-field terms, over `IsoToCalendarDate` and `CalendarDateAdd`. The dispatch is the same identity-then-membership test the two conversions already make — not the default singleton, and the provider claims this calendar — so a date's arithmetic and its field accessors are never answered by two different reckonings. An unconfigured engine reaches the per-calendar implementation it always did: 774,158 `CalendarDateAdd` results over the eleven built-in calendars, before and after, are byte-identical. Three things came out of writing it down. The whole-year month estimate in `CalendarDateUntil` was a hardcoded 12 or 13 and is now the count the conversion reports, which needed a back-off loop because the forward walk only ever steps one way and an estimate that already passed the target used to come back as a negative day count. `DifferenceISODateTime` reached `CalendarDateUntil` with no realm, so its refusal escaped `Engine.Evaluate` as a CLR exception. And the month walk never terminated when a conversion saturated. That last one is #3428, which #3452 has since fixed for the eleven calendars the engine reckons itself, by taking away the clamp that answered with a boundary date at all. The walk keeps the no-progress guard both changes arrived at, and it raises `CalendarRangeException` there rather than answering with a degraded difference: for a built-in calendar it is now unreachable, which is the shape provider that clamps and a walk that turns forever, since the walk is written in whichever two conversions answer for the calendar. `NonIsoCalendarRangeTests` pins the built-in half and still passes with the guard removed; the provider half is the new case in `TemporalCalendarArithmeticTests`, which does not return at all without it. Claude-Session: https://claude.ai/code/session_014W5mbjGhyvgAS4pivXoc4S Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
lahma
added a commit
to lahma/jint
that referenced
this pull request
Aug 27, 2026
…ot because the machine was quick enough WebApiTimerTests.ATimerOutlivingTheUnwrapTimeoutTimesOutByDesign failed once on windows/net8.0 with "Expected: <PromiseRejectedException> But was: null" - a 200 ms unwrap returning without throwing against a five-second timer. A 25x margin does not close through runner load, so it looked like a race in the unwrap-timeout path. It is not. It is the same wall-clock assumption sebastienros#3369 and sebastienros#3388 spent a campaign removing, wearing a margin big enough to look safe. The five seconds do not bound the unwrap. They bound the gap between the setTimeout call inside Evaluate and the unwrap starting on the next line, both measured against the system clock. Spend five seconds in that gap and the timer is already due when the unwrap begins - and the first thing DrainEventLoopUntil does is run the continuations it finds, so the timer fires, the promise FULFILS, and nothing is thrown. Sleeping 5.1 s between those two statements reproduces the reported failure exactly, message for message. A runner whose test bodies all run at ThreadPriority.Lowest (sebastienros#3452) under three concurrent legs has spent longer than that on less; the same suite had a 200 ms budget measured at 47 s in sebastienros#3406. The fix is the one this file already uses four tests above: a ManualClock the test owns and never advances, so the timer can never be due and the unwrap's own bound is the only thing that can end the wait. No margin, no clock to lose a race against. With the 5.1 s widening still in place the test now passes; twenty consecutive runs of the fixture are green without it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014W5mbjGhyvgAS4pivXoc4S
lahma
added a commit
to lahma/jint
that referenced
this pull request
Aug 27, 2026
…ot because the machine was quick enough WebApiTimerTests.ATimerOutlivingTheUnwrapTimeoutTimesOutByDesign failed once on windows/net8.0 with "Expected: <PromiseRejectedException> But was: null" - a 200 ms unwrap returning without throwing against a five-second timer. A 25x margin does not close through runner load, so it looked like a race in the unwrap-timeout path. It is not. It is the same wall-clock assumption sebastienros#3369 and sebastienros#3388 spent a campaign removing, wearing a margin big enough to look safe. The five seconds do not bound the unwrap. They bound the gap between the setTimeout call inside Evaluate and the unwrap starting on the next line, both measured against the system clock. Spend five seconds in that gap and the timer is already due when the unwrap begins - and the first thing DrainEventLoopUntil does is run the continuations it finds, so the timer fires, the promise FULFILS, and nothing is thrown. Sleeping 5.1 s between those two statements reproduces the reported failure exactly, message for message. A runner whose test bodies all run at ThreadPriority.Lowest (sebastienros#3452) under three concurrent legs has spent longer than that on less; the same suite had a 200 ms budget measured at 47 s in sebastienros#3406. The fix is the one this file already uses four tests above: a ManualClock the test owns and never advances, so the timer can never be due and the unwrap's own bound is the only thing that can end the wait. No margin, no clock to lose a race against. With the 5.1 s widening still in place the test now passes; twenty consecutive runs of the fixture are green without it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014W5mbjGhyvgAS4pivXoc4S
lahma
added a commit
that referenced
this pull request
Aug 27, 2026
…ot because the machine was quick enough (#3477) WebApiTimerTests.ATimerOutlivingTheUnwrapTimeoutTimesOutByDesign failed once on windows/net8.0 with "Expected: <PromiseRejectedException> But was: null" - a 200 ms unwrap returning without throwing against a five-second timer. A 25x margin does not close through runner load, so it looked like a race in the unwrap-timeout path. It is not. It is the same wall-clock assumption #3369 and #3388 spent a campaign removing, wearing a margin big enough to look safe. The five seconds do not bound the unwrap. They bound the gap between the setTimeout call inside Evaluate and the unwrap starting on the next line, both measured against the system clock. Spend five seconds in that gap and the timer is already due when the unwrap begins - and the first thing DrainEventLoopUntil does is run the continuations it finds, so the timer fires, the promise FULFILS, and nothing is thrown. Sleeping 5.1 s between those two statements reproduces the reported failure exactly, message for message. A runner whose test bodies all run at ThreadPriority.Lowest (#3452) under three concurrent legs has spent longer than that on less; the same suite had a 200 ms budget measured at 47 s in #3406. The fix is the one this file already uses four tests above: a ManualClock the test owns and never advances, so the timer can never be due and the unwrap's own bound is the only thing that can end the wait. No margin, no clock to lose a race against. With the 5.1 s widening still in place the test now passes; twenty consecutive runs of the fixture are green without it. Claude-Session: https://claude.ai/code/session_014W5mbjGhyvgAS4pivXoc4S Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 30, 2026
lahma
added a commit
that referenced
this pull request
Sep 1, 2026
…ead of spinning (#3555) Backport of #3452 (b942c6a) to 4.x. Fixes #3428. NonIsoCalendars.CalendarDateUntil measures a difference by walking one month at a time towards the target, and its only exit is "this step passed the target". CalendarDateAdd takes those steps, and when the conversion back to ISO left the range of the backing System.Globalization.Calendar it answered with ClampToCalendarRange, which ignored its year/month/day arguments and returned cal.MaxSupportedDateTime -- the calendar's maximum, whichever end had been overrun. Every further step of a backwards walk landed on that same date, so no step ever passed the target and the loop had no other exit. It is a CLR loop inside one interpreter step, so it crosses no statement boundary: no execution constraint can interrupt it and a CancellationToken is never observed. Out-of-range calendar arithmetic now reports itself instead of clamping. The Indian and Islamic-tabular arms answered `result ?? isoDate`, handing back their input date, which is the same no-progress answer by a different route; both now report too. CalendarDateUntil keeps a no-progress guard of its own so a clamp reintroduced below it cannot become a hang again. TemporalHelpers maps CalendarRangeException to Throw.RangeError, and realm is threaded through DifferenceISODateTime and is required on CalendarDateUntil so no call site can turn a script-visible RangeError into a CLR exception escaping Engine.Evaluate. Claude-Session: https://claude.ai/code/session_014W5mbjGhyvgAS4pivXoc4S Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3428.
What was wrong
NonIsoCalendars.CalendarDateUntilmeasures a difference by walking one month at a time from the sourcetowards the target, and its only exit is "this step passed the target".
NonIsoCalendars.CalendarDateAddtakes those steps, and when the conversion back to ISO left the range of the backing
System.Globalization.Calendarit did not report that — it answered withClampToCalendarRange, whichignored its year, month and day arguments entirely and returned
cal.MaxSupportedDateTime:The calendar's maximum, whichever end had been overrun. So every further step of a backwards walk landed
on that same date, no step ever passed the target, and the loop had no other exit.
ChineseLunisolarCalendarspans ISO 1901-02-19 to 2101-01-28, so the target is outside it.What an embedder saw
The thread, and nothing else. It is a CLR loop inside one interpreter step, so it crosses no statement
boundary:
LimitStatementsnever counted,LimitExecutionTimenever got a check, and aCancellationTokenwas never observed.Jint.Constraints.OperationDeadlineConstraintdoes not helpeither, for the same reason. A host that bounded untrusted script the documented way still lost the
thread, and the only way back was to end the process.
Reachable from any script that can name a
chinese,dangi,hebreworpersiandate, throughuntil,sinceandtotalonPlainDate,PlainDateTime,PlainYearMonth,ZonedDateTimeandDuration.Present in every released version; nothing recent introduced it.
The same clamp was also answering, rather than hanging, on the addition side — with the wrong date, and
with no signal that it was wrong:
What changed
Out-of-range calendar arithmetic reports itself instead of clamping.
NonISODateAddisimplementation-defined and is declared as returning "either a normal completion containing an ISO Date
Record or a throw completion"
(spec), and
CalendarDateAddraises aRangeErrorfor a result it cannot represent(spec), so that is what the two
TemporalHelpersentry points now do with it. Bothoverflowmodes report it: leaving the calendar'srange is not the month-or-day overflow
constrainis allowed to clamp, because there is no nearby validdate to clamp to — only the calendar's own boundary.
NonIsoCalendars.CalendarDateAdd—ClampToCalendarRangeis gone; theArgumentOutOfRangeExceptionfrom
cal.ToDateTimebecomes aCalendarRangeException.NonIsoCalendars.IndianCalendarDateAddandIslamicTabularCalendarDateAdd— both answeredresult ?? isoDate, handing back their input date when the conversion failed. That is the sameno-progress answer by a different route:
addreports that adding a year changed nothing, and the walktakes the same step forever. Both now report instead.
NonIsoCalendars.CalendarDateUntil— the walk keeps a no-progress guard of its own. A step that doesnot move the date can never pass the target however many more are taken, so it is a
RangeErrortoo.With the three fixes above it is unreachable today; it stays as the structural guarantee that this loop
terminates, so a clamp reintroduced anywhere below it cannot become a hang again.
TemporalHelpers.CalendarDateAdd/CalendarDateUntilmap it toThrow.RangeError.realmis nowthreaded through
DifferenceISODateTime(all three of its callers already had one) and is no longeroptional on
CalendarDateUntil, so no call site can lose it and turn a script-visibleRangeErrorintoa CLR exception escaping
Engine.Evaluate.All internal; no public API change.
Siblings checked and found sound
CalendarDateUntil's other units.week,dayand smaller are closed-form ISO epoch-dayarithmetic with no loop; the year count is arithmetic plus a single overshoot check. Only the month walk
loops, and with
largestUnit: 'year'it takes at most ~13 steps whatever the span.FixedEpochCalendarDateAdd(coptic/ethiopic/ethioaa) is pure epoch-day arithmetic with no backingBCL calendar and no clamp, so it always progresses. Same for
IndianToCalendarDate,IslamicCivilToCalendarDateand theIsoToCalendarDatearms behind them. Those six calendars stillmeasure across the whole Temporal range, which a test now pins.
*CalendarDateAddvariants (while (newMonth > 12/13),while (newMonth < 1)) are bounded by a months-in-year that every arm documents as never below 12.HebrewAlgorithmicFromIso,PersianToCalendarDate,EpochDaysToFixedEpoch— correct a bounded estimate against a monotone year-start function.MaxDaysForChineseLeapMonth,FindFixedEpochReferenceYear,FindIndianReferenceYear, …) are boundedforloops over fixed windows.TemporalHelpers.CalendarDateUntil's Gregorian-based path counts years and months by directarithmetic and has no unbounded loop.
Failing-test evidence
Jint.Tests/Runtime/NonIsoCalendarRangeTests.csruns every case on a dedicated thread with a 15-secondjoin, so a regression fails the run rather than wedging it. Against unfixed
main, the same file:Nine of those fifteen are the hang; six are the boundary date answered as if it were the result. With the
fix: 22 passed / 0 failed on
net472,net8.0andnet10.0.Follow-up: the
net472red leg on the first push, diagnosedThe first push's Windows leg failed one of the new tests on
net472only:It is not the walk. Measured on
net472, calling the walk directly:Ten microseconds, with the right answer. It could not be an exception-type or range divergence in the
netfx BCL either:
coptic,ethiopicandethioaahave noSystem.Globalization.Calendarbehind themat all —
CalendarDateAddroutes them toFixedEpochCalendarDateAdd, which is pure epoch-day arithmetic,so no
catchand no BCL range is involved on any target framework.The 15 seconds was thread scheduling.
DedicatedThread.Runstarted every body atThreadPriority.Lowest. Pinning the test process to a two-core affinity mask and running twonormal-priority busy threads — a small CI runner reproduced without saturating a 32-core box — the same
warm 0.1 ms evaluation:
A 17× penalty from two competitors. The CI job is far harsher: its log shows four test assemblies in
flight at once on a four-core
windows-latestrunner —— each with
[assembly: Parallelizable(ParallelScope.Fixtures)]andLevelOfParallelismdefaulting tothe core count, so roughly sixteen normal-priority workers over four cores. The
net472assembly alonetook 1 m 28 s there against 19 s uncontended locally, a 4.6× inflation at normal priority; a
Lowestthread inside that fares far worse. And the body was cold: the first Temporal script in a
net472processpays the intrinsic graph plus JIT, measured at 547 ms of CPU against 0.1 ms for every one after it — and
ACalendarReckonedByArithmeticStillMeasuresAcrossTheWholeTemporalRangesorts first among the fixture'smethods, so it is the body that absorbs that warmup, on the starved thread.
The fix, and it is not a bigger budget
Jint.Tests/DedicatedThread.csnow starts a body at normal priority and demotes the thread toLowestonly once its join has timed out — at which point it is known to be a runaway that must be kept off the
cores the rest of the run needs. The helper's comment already said what the demotion is for; only its
timing moves. The priority a runaway deserves is not the priority a body that is going to finish deserves,
and every caller of the helper was paying the former.
Verified by reproducing the CI failure locally and then removing it — same machine, same two-core affinity
mask, same full
net472assembly, one line of difference:DedicatedThreadnet472resultLowest(main's)ACalendarReckoned…("coptic"),did not finish within 00:00:15[20 s], 7426 passedThe 15-second budget is untouched. Under the two-core reproduction the body completes in ~0.14 s against
it, a 100× margin.
Test suites
Rebased onto
c39396fd4. Onedotnet test -c Releaserun, everything green:test262: 102,495 / 0 / 189 — the control number.
dotnet build -c Release: 0 errors, no new warnings.Two siblings found but not fixed here
Both are wrong answers rather than hangs, with their own blast radius, so they are filed separately:
Temporal.Duration.prototype.roundreads the calendar off aPlainDaterelativeToand thenreckons in
iso8601. It is why theroundhalf of the report never hung: it was not reaching the walkthat did.
calendar's name (
Temporal.PlainDate.from('1800-01-01').withCalendar('chinese').monthCodeis'M01').Now that the arithmetic refuses at the boundary, the field accessors are the remaining place where the
engine answers something it does not know.
🤖 Generated with Claude Code
https://claude.ai/code/session_014W5mbjGhyvgAS4pivXoc4S