Skip to content

Commit 5885900

Browse files
committed
fixed #293
1 parent 133c931 commit 5885900

File tree

10 files changed

+203
-49
lines changed

10 files changed

+203
-49
lines changed

CHANGELOG.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,14 @@
11
### [Unreleased]
22

3+
- **`timeLimit` now reliably interrupts long-running evaluations**: `Factorial`,
4+
`Sum`, `Product`, `Loop`, and `Reduce` all respect the `timeLimit` property
5+
and throw `CancellationError` when the deadline is exceeded. Previously,
6+
generators yielded too infrequently (every 1,000–50,000 iterations), allowing
7+
a single `gen.next()` call to block for longer than the timeout. All generators
8+
now yield every iteration. The `Factorial` handler no longer silently swallows
9+
`CancellationError`, and `withDeadline`/`withDeadlineAsync` now use
10+
`try/finally` to always reset the engine deadline.
11+
312
- **Fixed GPU compilation of `Sum`, `Product`, `Loop`, and `Function`**:
413
These constructs no longer leak JavaScript-specific syntax (IIFEs, `let`,
514
`while`, arrow functions, `{ re, im }` objects) into GLSL/WGSL output.

src/compute-engine/boxed-expression/boxed-function.ts

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1521,11 +1521,11 @@ function withDeadline<T>(engine: ComputeEngine, fn: () => T): () => T {
15211521
if (engine._deadline === undefined) {
15221522
engine._deadline = Date.now() + engine.timeLimit;
15231523

1524-
const result: T = fn();
1525-
1526-
engine._deadline = undefined;
1527-
1528-
return result;
1524+
try {
1525+
return fn();
1526+
} finally {
1527+
engine._deadline = undefined;
1528+
}
15291529
}
15301530

15311531
return fn();
@@ -1540,11 +1540,11 @@ function withDeadlineAsync<T>(
15401540
if (engine._deadline === undefined) {
15411541
engine._deadline = Date.now() + engine.timeLimit;
15421542

1543-
const result: T = await fn();
1544-
1545-
engine._deadline = undefined;
1546-
1547-
return result;
1543+
try {
1544+
return await fn();
1545+
} finally {
1546+
engine._deadline = undefined;
1547+
}
15481548
}
15491549

15501550
return fn();

src/compute-engine/library/arithmetic.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ import {
8585
quantityPower,
8686
} from './quantity-arithmetic';
8787
import { range, rangeLast } from './collections';
88-
import { run, runAsync } from '../../common/interruptible';
88+
import { run, runAsync, CancellationError } from '../../common/interruptible';
8989
import type {
9090
Expression,
9191
IComputeEngine as ComputeEngine,
@@ -443,7 +443,8 @@ export const ARITHMETIC_LIBRARY: SymbolDefinitions[] = [
443443
ce._timeRemaining
444444
)
445445
);
446-
} catch {
446+
} catch (e) {
447+
if (e instanceof CancellationError) throw e;
447448
// We can get here if the factorial is too large
448449
return undefined;
449450
}
@@ -472,7 +473,8 @@ export const ARITHMETIC_LIBRARY: SymbolDefinitions[] = [
472473
signal
473474
)
474475
);
475-
} catch {
476+
} catch (e) {
477+
if (e instanceof CancellationError) throw e;
476478
// We can get here if the factorial is too large
477479
return undefined;
478480
}
@@ -502,7 +504,7 @@ export const ARITHMETIC_LIBRARY: SymbolDefinitions[] = [
502504
if (n === null) return undefined;
503505
const ce = x.engine;
504506
if (bignumPreferred(ce))
505-
return ce.number(bigFactorial2(ce, ce.bignum(n)));
507+
return ce.number(run(bigFactorial2(ce, ce.bignum(n)), ce._timeRemaining));
506508

507509
return ce.number(factorial2(n));
508510
},

src/compute-engine/library/collections.ts

Lines changed: 24 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import {
1818
widen,
1919
} from '../../common/type/utils';
2020
import { interval } from '../numerics/interval';
21-
import { CancellationError } from '../../common/interruptible';
21+
import { CancellationError, run } from '../../common/interruptible';
2222
import type {
2323
Expression,
2424
OperatorDefinition,
@@ -764,27 +764,32 @@ export const COLLECTIONS_LIBRARY: SymbolDefinitions = {
764764
const compiled = ce._compile(fn);
765765
if (compiled.calling !== 'lambda' || !compiled.run) return undefined;
766766

767-
let accumulator = initial.re;
768-
let first = true;
769-
for (const item of collection.each()) {
770-
if (first) accumulator = item.re;
771-
else accumulator = compiled.run(accumulator, item.re) as number;
772-
first = false;
773-
}
774-
775-
return ce.box(accumulator);
767+
return run(
768+
(function* () {
769+
let accumulator = initial.re;
770+
let first = true;
771+
for (const item of collection.each()) {
772+
if (first) accumulator = item.re;
773+
else accumulator = compiled.run!(accumulator, item.re) as number;
774+
first = false;
775+
yield;
776+
}
777+
return ce.box(accumulator);
778+
})(),
779+
ce._timeRemaining
780+
);
776781
}
777782
// We don't have a compiled function, so we need to use the
778783
// interpreted version.
779784
const f = applicable(fn);
780-
let accumulator = initial;
781-
let first = true;
782-
for (const item of collection.each()) {
783-
if (first) accumulator = item;
784-
else accumulator = f([accumulator, item]) ?? ce.Nothing;
785-
first = false;
786-
}
787-
return accumulator;
785+
return run(
786+
reduceCollection<Expression>(
787+
collection,
788+
(acc, x) => f([acc, x]) ?? ce.Nothing,
789+
initial
790+
) as Generator<Expression | undefined, Expression | undefined>,
791+
ce._timeRemaining
792+
);
788793
},
789794
},
790795

@@ -2253,12 +2258,10 @@ export function* reduceCollection<T>(
22532258
initial: T
22542259
): Generator<T | undefined> {
22552260
let acc = initial;
2256-
let counter = 0;
22572261
for (const x of collection.each()) {
22582262
const result = fn(acc, x);
22592263
if (result === null) return undefined;
2260-
counter += 1;
2261-
if (counter % 1000 === 0) yield acc;
2264+
yield acc;
22622265
acc = result;
22632266
}
22642267
return acc;

src/compute-engine/library/control-structures.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ function* runLoop(
186186
if (isFunction(result, 'Break')) return result.op1;
187187
if (result.operator === 'Return') return result;
188188
i += 1;
189-
if (i % 1000 === 0) yield result;
189+
yield result;
190190
if (i > ce.iterationLimit)
191191
throw new CancellationError({ cause: 'iteration-limit-exceeded' });
192192
}
@@ -202,7 +202,7 @@ function* runLoop(
202202
if (isFunction(result, 'Break')) return result.op1;
203203
if (result.operator === 'Return') return result;
204204
i += 1;
205-
if (i % 1000 === 0) yield result;
205+
yield result;
206206
if (i > ce.iterationLimit)
207207
throw new CancellationError({ cause: 'iteration-limit-exceeded' });
208208
}

src/compute-engine/library/utils.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -516,12 +516,10 @@ export function* reduceBigOp<T>(
516516
// Iterate over the cartesian product and evaluate the body
517517
//
518518
let result: T | undefined = initial;
519-
let counter = 0;
520519
for (const element of cartesianArray) {
521520
indexingSets.forEach((x, i) => ce.assign(x.index!, element[i]));
522521
result = fn(result, body) ?? undefined;
523-
counter += 1;
524-
if (counter % 1000 === 0) yield result;
522+
yield result;
525523
if (result === undefined) break;
526524
}
527525

@@ -640,7 +638,6 @@ function* reduceElementIndexingSets<T>(
640638
}
641639

642640
let result: T | undefined = initial;
643-
let counter = 0;
644641

645642
while (true) {
646643
// Apply current combination of assignments
@@ -653,8 +650,7 @@ function* reduceElementIndexingSets<T>(
653650

654651
// Evaluate and accumulate
655652
result = fn(result, body) ?? undefined;
656-
counter++;
657-
if (counter % 1000 === 0) yield result;
653+
yield result;
658654
if (result === undefined) break;
659655

660656
// Move to next combination

src/compute-engine/numerics/numeric-bigint.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -60,17 +60,13 @@ export function* factorial(n: bigint): Generator<bigint, bigint> {
6060
let loop = n;
6161
let sum = n;
6262
let val = n;
63-
let counter = 0;
6463

6564
while (loop > 2) {
6665
loop -= BigInt(2); // Process even numbers only
6766
sum += loop; // Accumulate the sum of current and previous values
6867
val *= sum; // Update the factorial product
6968

70-
// Yield periodically for interruptibility
71-
counter += 1;
72-
if (counter % 50000 === 0 || (counter > 10000 && counter % 500 === 0))
73-
yield val;
69+
yield val;
7470
}
7571

7672
return val; // Final factorial result

src/compute-engine/numerics/numeric-bignum.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,18 @@ export function lcm(a: BigNum, b: BigNum): BigNum {
1111
return a.mul(b).div(gcd(a, b));
1212
}
1313

14-
export function factorial2(ce: IBigNum, n: BigNum): BigNum {
14+
export function* factorial2(
15+
ce: IBigNum,
16+
n: BigNum
17+
): Generator<BigNum, BigNum> {
1518
if (!n.isInteger() || n.isNegative()) return ce._BIGNUM_NAN;
1619
if (n.lessThan(1)) return ce._BIGNUM_ONE;
1720

1821
let result = n;
1922
while (n.greaterThan(2)) {
2023
n = n.minus(2);
2124
result = result.mul(n);
25+
yield result;
2226
}
2327

2428
return result;
Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
import { ComputeEngine } from '../../src/compute-engine';
2+
import { CancellationError } from '../../src/common/interruptible';
3+
4+
// Use a dedicated engine with a short timeout for all tests
5+
let ce: ComputeEngine;
6+
7+
beforeEach(() => {
8+
ce = new ComputeEngine();
9+
ce.timeLimit = 200; // 200ms — short enough to catch hangs, long enough for CI
10+
});
11+
12+
describe('TIMEOUT', () => {
13+
describe('Factorial', () => {
14+
it('normal factorial completes within timeout', () => {
15+
const result = ce.parse('700!').evaluate();
16+
expect(result.isFinite).toBe(true);
17+
});
18+
19+
it('nested factorial (700!)! throws CancellationError (sync)', () => {
20+
expect(() => ce.parse('(700!)!').evaluate()).toThrow(CancellationError);
21+
});
22+
23+
it('nested factorial (700!)! throws CancellationError (async)', async () => {
24+
await expect(ce.parse('(700!)!').evaluateAsync()).rejects.toThrow(
25+
CancellationError
26+
);
27+
});
28+
});
29+
30+
describe('Factorial2', () => {
31+
it('normal double factorial completes within timeout', () => {
32+
const result = ce.parse('10!!').evaluate();
33+
expect(result.re).toBe(3840);
34+
});
35+
36+
it('large double factorial in bignum mode throws CancellationError', () => {
37+
ce.precision = 200; // enable bignum path
38+
ce.timeLimit = 5; // very short — 5ms
39+
expect(() => ce.parse('100000!!').evaluate()).toThrow(CancellationError);
40+
});
41+
});
42+
43+
describe('Sum', () => {
44+
it('small sum completes within timeout', () => {
45+
const result = ce
46+
.box(['Sum', 'k', ['Tuple', 'k', 1, 100]])
47+
.evaluate();
48+
expect(result.re).toBe(5050);
49+
});
50+
51+
it('large sum throws CancellationError', () => {
52+
// Sum k^k for k=1..100000 — each iteration evaluates k^k which gets
53+
// expensive, and there are 100K of them
54+
expect(() =>
55+
ce
56+
.box([
57+
'Sum',
58+
['Power', 'k', 'k'],
59+
['Tuple', 'k', 1, 100_000],
60+
])
61+
.evaluate()
62+
).toThrow(CancellationError);
63+
});
64+
});
65+
66+
describe('Product', () => {
67+
it('small product completes within timeout', () => {
68+
const result = ce
69+
.box(['Product', 'k', ['Tuple', 'k', 1, 10]])
70+
.evaluate();
71+
// 10! = 3628800
72+
expect(result.re).toBe(3628800);
73+
});
74+
75+
it('large product throws CancellationError', () => {
76+
expect(() =>
77+
ce
78+
.box([
79+
'Product',
80+
['Power', 'k', 2],
81+
['Tuple', 'k', 1, 100_000],
82+
])
83+
.evaluate()
84+
).toThrow(CancellationError);
85+
});
86+
});
87+
88+
describe('Loop', () => {
89+
it('finite loop completes within timeout', () => {
90+
const list = ce.function(
91+
'List',
92+
Array.from({ length: 5 }, (_, i) => ce.number(i + 1))
93+
);
94+
// Loop applies body (a lambda) to each element, returns the last result
95+
const result = ce
96+
.box(['Loop', ['Function', ['Multiply', 'x', 2], 'x'], list])
97+
.evaluate();
98+
expect(result.re).toBe(10); // last element 5 * 2 = 10
99+
});
100+
101+
it('loop over large collection throws CancellationError', () => {
102+
const list = ce.function(
103+
'List',
104+
Array.from({ length: 50_000 }, (_, i) => ce.number(i))
105+
);
106+
// Body does expensive work per element
107+
expect(() =>
108+
ce
109+
.box([
110+
'Loop',
111+
['Function', ['Power', 'x', 'x'], 'x'],
112+
list,
113+
])
114+
.evaluate()
115+
).toThrow(CancellationError);
116+
});
117+
});
118+
119+
describe('deadline cleanup', () => {
120+
it('deadline is reset after timeout so subsequent evaluations work', () => {
121+
// First: trigger a timeout
122+
expect(() => ce.parse('(700!)!').evaluate()).toThrow(CancellationError);
123+
124+
// Second: a normal evaluation should still work (deadline was cleaned up)
125+
const result = ce.parse('10!').evaluate();
126+
expect(result.re).toBe(3628800);
127+
});
128+
129+
it('deadline is reset after successful evaluation', () => {
130+
ce.parse('100!').evaluate();
131+
132+
// _deadline should be undefined after completion
133+
expect(ce._deadline).toBeUndefined();
134+
});
135+
});
136+
});

0 commit comments

Comments
 (0)