Skip to content

Commit 5598fd0

Browse files
jonwalstedtclaude
andcommitted
fix(inference): rely on AbortSignal alone for task timeout, drop pool rebuild
Address review feedback: on task timeout, destroying and recreating the whole Piscina pool rejects all other in-flight detection tasks sharing the pool (up to maxThreads concurrently), not just the timed-out one. Piscina already terminates the specific worker thread when the task's AbortSignal fires, so relying on that signal alone contains the runaway task without collateral impact to sibling requests. Reviewed-at: elastic#288758 (comment) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 066fccb commit 5598fd0

3 files changed

Lines changed: 3 additions & 9 deletions

File tree

x-pack/platform/plugins/shared/inference/server/workflow_anonymization/detection/execute_regex_rules.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ function compileRule(rawPattern: string): CompiledRule {
1818
} catch {
1919
// RE2 does not support lookahead, lookbehind, or backreferences. Fall back to
2020
// native RegExp. ReDoS protection is provided by the Piscina worker timeout and
21-
// pool-rebuild on abort.
21+
// per-task abort via AbortSignal.
2222
return { engine: 'native', pattern: new RegExp(rawPattern, 'g') };
2323
}
2424
}
@@ -80,7 +80,7 @@ function findSpans(
8080
*
8181
* RE2JS is tried first for each pattern. Patterns that contain constructs RE2 does
8282
* not support (lookahead, lookbehind, backreferences) fall back to native RegExp.
83-
* The Piscina worker timeout and pool-rebuild on abort provide ReDoS protection for
83+
* The Piscina worker timeout and per-task AbortSignal provide ReDoS protection for
8484
* native RegExp patterns.
8585
*
8686
* Zero-length matches advance one character and continue scanning; they do not

x-pack/platform/plugins/shared/inference/server/workflow_anonymization/detection/regex_worker_service.test.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -78,12 +78,11 @@ describe('PiiRegexWorkerService', () => {
7878
).rejects.toThrow();
7979
});
8080

81-
it('aborts the task and recreates the pool when taskTimeout elapses', async () => {
81+
it('aborts the timed-out task and throws when taskTimeout elapses', async () => {
8282
service = new PiiRegexWorkerService(
8383
createTestConfig({ taskTimeout: { asMilliseconds: () => 1 } } as any),
8484
logger
8585
);
86-
const workerBefore = (service as any).worker;
8786

8887
// (?=a)(a+)+$ falls back to native RegExp (RE2 rejects the lookahead) and
8988
// backtracks catastrophically on a long all-'a' string — guaranteed timeout.
@@ -93,9 +92,6 @@ describe('PiiRegexWorkerService', () => {
9392
records: [{ content: 'a'.repeat(10_000) + 'b' }],
9493
})
9594
).rejects.toThrow('timed out');
96-
97-
// Pool is rebuilt after abort — the new instance is a different object
98-
expect((service as any).worker).not.toBe(workerBefore);
9995
});
10096

10197
it('returns [] and logs when failureMode is allow_unsafe', async () => {

x-pack/platform/plugins/shared/inference/server/workflow_anonymization/detection/regex_worker_service.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,6 @@ export class PiiRegexWorkerService {
8787
return await this.worker.run(payload, { signal: controller.signal });
8888
} catch (err) {
8989
if (err instanceof Error && err.name === 'AbortError') {
90-
await this.worker.destroy().catch(() => {});
91-
this.worker = this.createWorkerPool();
9290
throw new Error(
9391
`PII regex detection task timed out after ${this.config.taskTimeout.asMilliseconds()}ms`
9492
);

0 commit comments

Comments
 (0)