From fa17193c52e28d04b9f23d94c498bd358445a9a3 Mon Sep 17 00:00:00 2001 From: Elberte Plinio Date: Mon, 20 Jul 2026 04:16:30 -0300 Subject: [PATCH 1/4] refactor: centralize bounded subprocess supervision --- app/lib/agent/droid_agent_harness.dart | 199 +------ app/lib/agent/minimal_agent_harness.dart | 91 +--- app/lib/core/patch_capture.dart | 240 +-------- app/lib/evaluators/evaluator_process.dart | 295 ++--------- app/lib/providers/droid_exec_provider.dart | 198 +------ app/lib/runner/bounded_subprocess.dart | 501 ++++++++++++++++++ app/lib/runner/workdir_manager.dart | 345 ++---------- .../evaluators/evaluator_process_test.dart | 29 + app/test/runner/bounded_subprocess_test.dart | 199 +++++++ 9 files changed, 894 insertions(+), 1203 deletions(-) create mode 100644 app/lib/runner/bounded_subprocess.dart create mode 100644 app/test/runner/bounded_subprocess_test.dart diff --git a/app/lib/agent/droid_agent_harness.dart b/app/lib/agent/droid_agent_harness.dart index d329090..7641a70 100644 --- a/app/lib/agent/droid_agent_harness.dart +++ b/app/lib/agent/droid_agent_harness.dart @@ -4,6 +4,7 @@ import 'dart:io'; import 'package:dart_arena/agent/agent_harness.dart'; import 'package:dart_arena/agent/agent_run_result.dart'; import 'package:dart_arena/providers/factory_custom_model_environment.dart'; +import 'package:dart_arena/runner/bounded_subprocess.dart'; import 'package:dart_arena/runner/generated_code_sandbox.dart'; import 'package:dart_arena/runner/subprocess_environment.dart'; import 'package:path/path.dart' as p; @@ -175,7 +176,6 @@ $instruction if (!Platform.isWindows) { environment['PWD'] = workingDirectory.path; } - late final Process process; try { final processStart = generatedCodeSandbox == null ? SandboxedProcessStart( @@ -193,82 +193,22 @@ $instruction resourceLimits: null, extraReadOnlyPaths: extraReadOnlyPaths, ); - process = await Process.start( - processStart.executable, - processStart.arguments, + final processResult = await runBoundedSubprocess( + executable: processStart.executable, + arguments: processStart.arguments, workingDirectory: processStart.workingDirectory, - runInShell: false, environment: processStart.environment, - includeParentEnvironment: false, + maxOutputBytes: maxProcessOutputChars, + timeout: timeout, + capture: BoundedSubprocessCapture.trailing, ); - } on Object { - await cwdProxy?.dispose(); - rethrow; - } - try { - final stdoutBuffer = _BoundedTailTextCollector(maxProcessOutputChars); - final stderrBuffer = _BoundedTailTextCollector(maxProcessOutputChars); - final outputLimitExceeded = Completer(); - void markOutputLimitExceeded() { - if (!outputLimitExceeded.isCompleted) { - outputLimitExceeded.complete(); - } - } - - final stdoutDone = process.stdout - .transform(systemEncoding.decoder) - .listen((chunk) { - stdoutBuffer.write(chunk); - if (stdoutBuffer.exceeded) markOutputLimitExceeded(); - }) - .asFuture(); - final stderrDone = process.stderr - .transform(systemEncoding.decoder) - .listen((chunk) { - stderrBuffer.write(chunk); - if (stderrBuffer.exceeded) markOutputLimitExceeded(); - }) - .asFuture(); - - var timedOut = false; - var outputLimitHit = false; - int exitCode; - final timeoutExceeded = Completer(); - final timeoutTimer = Timer(timeout, () { - if (!timeoutExceeded.isCompleted) timeoutExceeded.complete(); - }); - try { - final signal = await Future.any([ - process.exitCode, - timeoutExceeded.future.then( - (_) => const _AgentProcessTimeoutExceeded(), - ), - outputLimitExceeded.future.then( - (_) => const _AgentProcessOutputLimitExceeded(), - ), - ]); - if (signal is int) { - exitCode = signal; - } else { - timedOut = signal is _AgentProcessTimeoutExceeded; - outputLimitHit = signal is _AgentProcessOutputLimitExceeded; - await terminateProcessTree(process.pid, ProcessSignal.sigterm); - exitCode = await process.exitCode.timeout( - const Duration(seconds: 2), - onTimeout: () async { - await terminateProcessTree(process.pid, ProcessSignal.sigkill); - return -1; - }, - ); - } - } finally { - timeoutTimer.cancel(); - } - await Future.wait([stdoutDone, stderrDone]); await cwdProxy?.syncBack(); sw.stop(); - final stdoutPreview = _trimPreview(stdoutBuffer.text, 16 * 1024); - final rawStderrPreview = _trimPreview(stderrBuffer.text, 16 * 1024); + final timedOut = + processResult.termination == BoundedSubprocessTermination.timedOut; + final outputLimitHit = processResult.outputLimitExceeded; + final stdoutPreview = _trimPreview(processResult.stdout, 16 * 1024); + final rawStderrPreview = _trimPreview(processResult.stderr, 16 * 1024); final stderrPreview = outputLimitHit && rawStderrPreview.isEmpty ? 'agent harness output exceeded $maxProcessOutputChars characters' : rawStderrPreview; @@ -276,12 +216,14 @@ $instruction return AgentRunResult( status: timedOut ? AgentRunStatus.timeout - : exitCode == 0 && !outputLimitHit + : processResult.exitCode == 0 && !outputLimitHit ? AgentRunStatus.success : AgentRunStatus.failure, stdoutPreview: stdoutPreview, stderrPreview: stderrPreview, - exitCode: timedOut && exitCode == -1 ? null : exitCode, + exitCode: timedOut && processResult.exitCode == -1 + ? null + : processResult.exitCode, latency: sw.elapsed, metadata: { 'executable': exe, @@ -363,85 +305,6 @@ $instruction if (value.length <= maxChars) return value; return value.substring(value.length - maxChars); } - - static Future terminateProcessTree( - int pid, - ProcessSignal signal, - ) async { - if (Platform.isWindows) { - final args = ['/PID', '$pid', '/T']; - if (signal == ProcessSignal.sigkill) args.add('/F'); - await _tryRunProcess('taskkill', args); - return; - } - - final descendants = await _descendantPids(pid); - for (final childPid in descendants.reversed) { - _killPid(childPid, signal); - } - _killPid(pid, signal); - } - - static Future> _descendantPids(int pid) async { - final descendants = []; - for (final childPid in await _childPids(pid)) { - descendants.add(childPid); - descendants.addAll(await _descendantPids(childPid)); - } - return descendants; - } - - static Future> _childPids(int pid) async { - final pgrep = await _tryRunProcess('pgrep', ['-P', '$pid']); - if (pgrep?.exitCode == 0) { - return _parsePids(pgrep!.stdout.toString()); - } - - final ps = await _tryRunProcess('ps', ['-o', 'pid=', '--ppid', '$pid']); - if (ps?.exitCode == 0) { - return _parsePids(ps!.stdout.toString()); - } - return const []; - } - - static Future _tryRunProcess( - String executable, - List arguments, - ) async { - try { - return await Process.run( - executable, - arguments, - runInShell: false, - environment: benchmarkSubprocessEnvironment(), - includeParentEnvironment: false, - ); - } on Object { - return null; - } - } - - static List _parsePids(String output) => output - .split(RegExp(r'\s+')) - .map((s) => int.tryParse(s.trim())) - .whereType() - .toList(growable: false); - - static void _killPid(int pid, ProcessSignal signal) { - if (!_tryKillPid(pid, signal)) { - _tryKillPid(pid, null); - } - } - - static bool _tryKillPid(int pid, ProcessSignal? signal) { - try { - return signal == null - ? Process.killPid(pid) - : Process.killPid(pid, signal); - } on Object { - return false; - } - } } class _WorkingDirectoryProxy { @@ -463,33 +326,3 @@ class _WorkingDirectoryProxy { if (await root.exists()) await root.delete(recursive: true); } } - -class _AgentProcessTimeoutExceeded { - const _AgentProcessTimeoutExceeded(); -} - -class _AgentProcessOutputLimitExceeded { - const _AgentProcessOutputLimitExceeded(); -} - -class _BoundedTailTextCollector { - _BoundedTailTextCollector(this.maxChars); - - final int maxChars; - final _buffer = StringBuffer(); - var exceeded = false; - - void write(String chunk) { - if (chunk.isEmpty) return; - _buffer.write(chunk); - if (_buffer.length > maxChars) { - exceeded = true; - final value = _buffer.toString(); - _buffer - ..clear() - ..write(value.substring(value.length - maxChars)); - } - } - - String get text => _buffer.toString(); -} diff --git a/app/lib/agent/minimal_agent_harness.dart b/app/lib/agent/minimal_agent_harness.dart index ec2fbcf..ed9d5ba 100644 --- a/app/lib/agent/minimal_agent_harness.dart +++ b/app/lib/agent/minimal_agent_harness.dart @@ -3,9 +3,9 @@ import 'dart:io'; import 'package:dart_arena/agent/agent_harness.dart'; import 'package:dart_arena/agent/agent_run_result.dart'; -import 'package:dart_arena/agent/droid_agent_harness.dart'; import 'package:dart_arena/providers/model_provider.dart'; import 'package:dart_arena/providers/model_stream_event.dart'; +import 'package:dart_arena/runner/bounded_subprocess.dart'; import 'package:dart_arena/runner/generated_code_sandbox.dart'; import 'package:dart_arena/runner/subprocess_environment.dart'; @@ -229,79 +229,26 @@ class MinimalAgentHarness implements AgentHarness, AgentHarnessProvenance { environment: environment, allowInternet: allowInternet, ); - final process = await Process.start( - processStart.executable, - processStart.arguments, + final result = await runBoundedSubprocess( + executable: processStart.executable, + arguments: processStart.arguments, workingDirectory: processStart.workingDirectory, - runInShell: false, environment: processStart.environment, - includeParentEnvironment: false, + maxOutputBytes: maxOutputChars, + timeout: timeout, + capture: BoundedSubprocessCapture.trailing, ); - final stdout = _BoundedTailText(maxOutputChars); - final stderr = _BoundedTailText(maxOutputChars); - final outputLimitExceeded = Completer(); - void markOutputLimitExceeded() { - if (!outputLimitExceeded.isCompleted) outputLimitExceeded.complete(); + if (result.termination == BoundedSubprocessTermination.timedOut) { + throw TimeoutException('bash command timed out'); } - - final stdoutDone = process.stdout.transform(systemEncoding.decoder).listen(( - chunk, - ) { - stdout.write(chunk); - if (stdout.exceeded) markOutputLimitExceeded(); - }).asFuture(); - final stderrDone = process.stderr.transform(systemEncoding.decoder).listen(( - chunk, - ) { - stderr.write(chunk); - if (stderr.exceeded) markOutputLimitExceeded(); - }).asFuture(); - final timeoutExceeded = Completer(); - final timeoutTimer = Timer(timeout, () { - if (!timeoutExceeded.isCompleted) timeoutExceeded.complete(); - }); - var outputLimitHit = false; - var timedOut = false; - int exitCode; - try { - final signal = await Future.any([ - process.exitCode, - timeoutExceeded.future.then((_) => const _CommandTimedOut()), - outputLimitExceeded.future.then((_) => const _CommandOutputLimited()), - ]); - if (signal is int) { - exitCode = signal; - } else { - timedOut = signal is _CommandTimedOut; - outputLimitHit = signal is _CommandOutputLimited; - await DroidAgentHarness.terminateProcessTree( - process.pid, - ProcessSignal.sigterm, - ); - exitCode = await process.exitCode.timeout( - const Duration(seconds: 2), - onTimeout: () async { - await DroidAgentHarness.terminateProcessTree( - process.pid, - ProcessSignal.sigkill, - ); - return -1; - }, - ); - } - } finally { - timeoutTimer.cancel(); - } - await Future.wait([stdoutDone, stderrDone]); - if (timedOut) throw TimeoutException('bash command timed out'); - if (outputLimitHit) { - throw _CommandOutputLimitExceeded(stdout.text, stderr.text); + if (result.outputLimitExceeded) { + throw _CommandOutputLimitExceeded(result.stdout, result.stderr); } return _CommandResult( - exitCode: exitCode, - stdout: stdout.text, - stderr: stderr.text, - outputTruncated: stdout.exceeded || stderr.exceeded, + exitCode: result.exitCode, + stdout: result.stdout, + stderr: result.stderr, + outputTruncated: result.stdoutLimitExceeded || result.stderrLimitExceeded, ); } @@ -399,14 +346,6 @@ class _CommandResult { final bool outputTruncated; } -class _CommandTimedOut { - const _CommandTimedOut(); -} - -class _CommandOutputLimited { - const _CommandOutputLimited(); -} - class _CommandOutputLimitExceeded implements Exception { const _CommandOutputLimitExceeded(this.stdout, this.stderr); diff --git a/app/lib/core/patch_capture.dart b/app/lib/core/patch_capture.dart index ae2ff7e..2f50fbf 100644 --- a/app/lib/core/patch_capture.dart +++ b/app/lib/core/patch_capture.dart @@ -3,6 +3,7 @@ import 'dart:convert'; import 'dart:io'; import 'package:crypto/crypto.dart'; +import 'package:dart_arena/runner/bounded_subprocess.dart'; import 'package:dart_arena/runner/subprocess_environment.dart'; const patchBaselineRef = 'arena_baseline'; @@ -57,91 +58,29 @@ class PatchCapture { ); } - Future<_GitProcessResult> _runGit( + Future _runGit( Directory workspace, List args, ) async { final processEnvironment = _gitEnvironment(); - final process = await Process.start( - gitExecutable, - args, + return runBoundedSubprocess( + executable: gitExecutable, + arguments: args, workingDirectory: workspace.path, - runInShell: false, environment: processEnvironment, - includeParentEnvironment: false, - ); - final stdoutBuffer = _BoundedTextCollector(maxOutputChars); - final stderrBuffer = _BoundedTextCollector(maxOutputChars); - final outputLimitExceeded = Completer(); - void markOutputLimitExceeded() { - if (!outputLimitExceeded.isCompleted) outputLimitExceeded.complete(); - } - - final stdoutDone = process.stdout.transform(systemEncoding.decoder).listen(( - chunk, - ) { - stdoutBuffer.write(chunk); - if (stdoutBuffer.exceeded) markOutputLimitExceeded(); - }).asFuture(); - final stderrDone = process.stderr.transform(systemEncoding.decoder).listen(( - chunk, - ) { - stderrBuffer.write(chunk); - if (stderrBuffer.exceeded) markOutputLimitExceeded(); - }).asFuture(); - - final timeoutExceeded = Completer(); - final timeoutTimer = Timer(timeout, () { - if (!timeoutExceeded.isCompleted) timeoutExceeded.complete(); - }); - late final Object signal; - try { - signal = await Future.any([ - process.exitCode, - timeoutExceeded.future.then((_) => const _GitProcessTimedOut()), - outputLimitExceeded.future.then( - (_) => const _GitProcessOutputLimitExceeded(), - ), - ]); - } finally { - timeoutTimer.cancel(); - } - - if (signal is int) { - await Future.wait([stdoutDone, stderrDone]); - return _GitProcessResult( - stdout: stdoutBuffer.text, - stderr: stderrBuffer.text, - exitCode: signal, - timedOut: false, - outputLimitExceeded: stdoutBuffer.exceeded || stderrBuffer.exceeded, - ); - } - - await _terminateProcessTree( - process.pid, - ProcessSignal.sigterm, - environment: processEnvironment, - ); - final exitCode = await _awaitProcessExit( - process, - environment: processEnvironment, - ); - await Future.wait([stdoutDone, stderrDone]); - return _GitProcessResult( - stdout: stdoutBuffer.text, - stderr: stderrBuffer.text, - exitCode: exitCode, - timedOut: signal is _GitProcessTimedOut, - outputLimitExceeded: signal is _GitProcessOutputLimitExceeded, + maxOutputBytes: maxOutputChars, + timeout: timeout, + helperEnvironment: processEnvironment, ); } - void _checkGitResult(_GitProcessResult result, List args) { - if (result.timedOut) { + void _checkGitResult(BoundedSubprocessResult result, List args) { + if (result.termination == BoundedSubprocessTermination.timedOut) { throw TimeoutException('patch capture git command timed out', timeout); } - if (result.outputLimitExceeded) { + if (result.outputLimitExceeded || + result.stdoutLimitExceeded || + result.stderrLimitExceeded) { throw ProcessException( gitExecutable, args, @@ -160,23 +99,6 @@ class PatchCapture { } } - Future _awaitProcessExit( - Process process, { - required Map environment, - }) { - return process.exitCode.timeout( - const Duration(seconds: 2), - onTimeout: () async { - await _terminateProcessTree( - process.pid, - ProcessSignal.sigkill, - environment: environment, - ); - return -1; - }, - ); - } - Map _gitEnvironment() { return benchmarkSubprocessEnvironment( baseEnvironment: baseEnvironment, @@ -189,139 +111,3 @@ class PatchCapture { )..addAll(const {'GIT_CONFIG_NOSYSTEM': '1', 'GIT_TERMINAL_PROMPT': '0'}); } } - -Future _terminateProcessTree( - int pid, - ProcessSignal signal, { - required Map environment, -}) async { - if (Platform.isWindows) { - final args = ['/PID', '$pid', '/T']; - if (signal == ProcessSignal.sigkill) args.add('/F'); - await _tryRunProcess('taskkill', args, environment: environment); - return; - } - - final descendants = await _descendantPids(pid, environment: environment); - for (final childPid in descendants.reversed) { - _killPid(childPid, signal); - } - _killPid(pid, signal); -} - -Future> _descendantPids( - int pid, { - required Map environment, -}) async { - final descendants = []; - for (final childPid in await _childPids(pid, environment: environment)) { - descendants.add(childPid); - descendants.addAll( - await _descendantPids(childPid, environment: environment), - ); - } - return descendants; -} - -Future> _childPids( - int pid, { - required Map environment, -}) async { - final pgrep = await _tryRunProcess('pgrep', [ - '-P', - '$pid', - ], environment: environment); - if (pgrep?.exitCode == 0) return _parsePids(pgrep!.stdout.toString()); - - final ps = await _tryRunProcess('ps', [ - '-o', - 'pid=', - '--ppid', - '$pid', - ], environment: environment); - if (ps?.exitCode == 0) return _parsePids(ps!.stdout.toString()); - return const []; -} - -Future _tryRunProcess( - String executable, - List arguments, { - required Map environment, -}) async { - try { - return await Process.run( - executable, - arguments, - runInShell: false, - environment: environment, - includeParentEnvironment: false, - ); - } on Object { - return null; - } -} - -List _parsePids(String output) => output - .split(RegExp(r'\s+')) - .map((s) => int.tryParse(s.trim())) - .whereType() - .toList(growable: false); - -void _killPid(int pid, ProcessSignal signal) { - if (!_tryKillPid(pid, signal)) _tryKillPid(pid, null); -} - -bool _tryKillPid(int pid, ProcessSignal? signal) { - try { - return signal == null ? Process.killPid(pid) : Process.killPid(pid, signal); - } on Object { - return false; - } -} - -class _GitProcessResult { - const _GitProcessResult({ - required this.stdout, - required this.stderr, - required this.exitCode, - required this.timedOut, - required this.outputLimitExceeded, - }); - - final String stdout; - final String stderr; - final int exitCode; - final bool timedOut; - final bool outputLimitExceeded; -} - -class _BoundedTextCollector { - _BoundedTextCollector(this.maxChars); - - final int maxChars; - final _buffer = StringBuffer(); - bool exceeded = false; - - String get text => _buffer.toString(); - - void write(String chunk) { - if (exceeded) return; - final remaining = maxChars - _buffer.length; - if (chunk.length <= remaining) { - _buffer.write(chunk); - return; - } - if (remaining > 0) { - _buffer.write(chunk.substring(0, remaining)); - } - exceeded = true; - } -} - -class _GitProcessTimedOut { - const _GitProcessTimedOut(); -} - -class _GitProcessOutputLimitExceeded { - const _GitProcessOutputLimitExceeded(); -} diff --git a/app/lib/evaluators/evaluator_process.dart b/app/lib/evaluators/evaluator_process.dart index 89c9b77..6a205da 100644 --- a/app/lib/evaluators/evaluator_process.dart +++ b/app/lib/evaluators/evaluator_process.dart @@ -1,7 +1,6 @@ import 'dart:async'; import 'dart:io'; -import 'dart:typed_data'; - +import 'package:dart_arena/runner/bounded_subprocess.dart'; import 'package:dart_arena/runner/generated_code_sandbox.dart'; import 'package:dart_arena/runner/subprocess_environment.dart'; @@ -73,208 +72,90 @@ Future runEvaluatorProcess( ), extraReadOnlyPaths: extraReadOnlyPaths, ); - final process = await Process.start( - processStart.executable, - processStart.arguments, - workingDirectory: processStart.workingDirectory, - runInShell: false, - environment: processStart.environment, - includeParentEnvironment: generatedCodeSandbox == null - ? includeParentEnvironment - : false, - ); - final stdoutBuffer = _BoundedTextCollector(maxOutputChars); - final stderrBuffer = _BoundedTextCollector(maxOutputChars); - final outputLimitExceeded = Completer<_EvaluatorProcessSignal>(); - final processLimitExceeded = Completer<_EvaluatorProcessSignal>(); - final memoryLimitExceeded = Completer<_EvaluatorProcessSignal>(); - int? observedProcessCount; - int? observedMemoryMb; - void markOutputLimitExceeded() { - if (!outputLimitExceeded.isCompleted) { - outputLimitExceeded.complete( - const _EvaluatorProcessOutputLimitExceeded(), - ); - } - } - - void markProcessLimitExceeded(int count) { - observedProcessCount = count; - if (!processLimitExceeded.isCompleted) { - processLimitExceeded.complete( - _EvaluatorProcessLimitExceeded(processCount: count), - ); - } - } - - void markMemoryLimitExceeded(int memoryMb) { - observedMemoryMb = memoryMb; - if (!memoryLimitExceeded.isCompleted) { - memoryLimitExceeded.complete( - _EvaluatorMemoryLimitExceeded(memoryMb: memoryMb), - ); - } - } - - final stdoutDone = process.stdout.listen((chunk) { - stdoutBuffer.writeBytes(chunk); - if (stdoutBuffer.exceeded) markOutputLimitExceeded(); - }).asFuture(); - final stderrDone = process.stderr.listen((chunk) { - stderrBuffer.writeBytes(chunk); - if (stderrBuffer.exceeded) markOutputLimitExceeded(); - }).asFuture(); - - if (timeout != null && timeout.compareTo(Duration.zero) <= 0) { - await _terminateProcessTree( - process.pid, - ProcessSignal.sigterm, - environment: helperEnvironment, - ); - await _awaitProcessExit(process, environment: helperEnvironment); - await Future.wait([stdoutDone, stderrDone]); - return EvaluatorProcessResult( - stdout: stdoutBuffer.text, - stderr: stderrBuffer.text, - exitCode: -1, - timedOut: true, - outputLimitExceeded: false, - processLimitExceeded: false, - memoryLimitExceeded: false, - observedProcessCount: null, - observedMemoryMb: null, - ); - } - - Timer? resourceLimitTimer; - var resourceCheckRunning = false; - if (maxProcesses != null || maxMemoryMb != null) { - resourceLimitTimer = Timer.periodic(const Duration(milliseconds: 100), ( - _, - ) async { + BoundedSubprocessMonitor resourceMonitor(int pid) { + final limitExceeded = Completer(); + Object? observedLimit; + var resourceCheckRunning = false; + final timer = Timer.periodic(const Duration(milliseconds: 100), (_) async { if (resourceCheckRunning) return; resourceCheckRunning = true; try { final descendants = await _descendantPids( - process.pid, + pid, environment: helperEnvironment, ); final processCount = 1 + descendants.length; if (maxProcesses != null && processCount > maxProcesses) { - markProcessLimitExceeded(processCount); + observedLimit = _EvaluatorProcessLimitExceeded( + processCount: processCount, + ); + if (!limitExceeded.isCompleted) { + limitExceeded.complete(observedLimit!); + } + return; } if (maxMemoryMb != null) { final memoryMb = await _processTreeMemoryMb( - process.pid, + pid, descendants, environment: helperEnvironment, ); if (memoryMb != null && memoryMb > maxMemoryMb) { - markMemoryLimitExceeded(memoryMb); + observedLimit = _EvaluatorMemoryLimitExceeded(memoryMb: memoryMb); + if (!limitExceeded.isCompleted) { + limitExceeded.complete(observedLimit!); + } } } } finally { resourceCheckRunning = false; } }); - } - final timeoutExceeded = Completer<_EvaluatorProcessSignal>(); - final timeoutTimer = timeout == null - ? null - : Timer(timeout, () { - if (!timeoutExceeded.isCompleted) { - timeoutExceeded.complete(const _EvaluatorProcessTimedOut()); - } - }); - late final _EvaluatorProcessSignal signal; - try { - signal = await Future.any<_EvaluatorProcessSignal>([ - process.exitCode.then(_EvaluatorProcessExit.new), - if (timeout != null) timeoutExceeded.future, - outputLimitExceeded.future, - if (maxProcesses != null) processLimitExceeded.future, - if (maxMemoryMb != null) memoryLimitExceeded.future, - ]); - } finally { - timeoutTimer?.cancel(); - resourceLimitTimer?.cancel(); - } - - if (signal is _EvaluatorProcessExit) { - await Future.wait([stdoutDone, stderrDone]); - return EvaluatorProcessResult( - stdout: stdoutBuffer.text, - stderr: stderrBuffer.text, - exitCode: signal.exitCode, - timedOut: false, - outputLimitExceeded: stdoutBuffer.exceeded || stderrBuffer.exceeded, - processLimitExceeded: observedProcessCount != null, - memoryLimitExceeded: observedMemoryMb != null, - observedProcessCount: observedProcessCount, - observedMemoryMb: observedMemoryMb, + return BoundedSubprocessMonitor( + signal: limitExceeded.future, + observed: () => observedLimit, + dispose: timer.cancel, ); } - await _terminateProcessTree( - process.pid, - ProcessSignal.sigterm, - environment: helperEnvironment, + final result = await runBoundedSubprocess( + executable: processStart.executable, + arguments: processStart.arguments, + workingDirectory: processStart.workingDirectory, + environment: processStart.environment, + includeParentEnvironment: generatedCodeSandbox == null + ? includeParentEnvironment + : false, + timeout: timeout, + maxOutputBytes: maxOutputChars, + helperEnvironment: helperEnvironment, + monitor: maxProcesses != null || maxMemoryMb != null + ? resourceMonitor + : null, ); - await _awaitProcessExit(process, environment: helperEnvironment); - await Future.wait([stdoutDone, stderrDone]); + final externalLimit = result.externalLimit; return EvaluatorProcessResult( - stdout: stdoutBuffer.text, - stderr: stderrBuffer.text, - exitCode: -1, - timedOut: signal is _EvaluatorProcessTimedOut, - outputLimitExceeded: signal is _EvaluatorProcessOutputLimitExceeded, - processLimitExceeded: signal is _EvaluatorProcessLimitExceeded, - memoryLimitExceeded: signal is _EvaluatorMemoryLimitExceeded, - observedProcessCount: signal is _EvaluatorProcessLimitExceeded - ? signal.processCount + stdout: result.stdout, + stderr: result.stderr, + exitCode: result.termination == BoundedSubprocessTermination.exited + ? result.exitCode + : -1, + timedOut: result.termination == BoundedSubprocessTermination.timedOut, + outputLimitExceeded: + result.outputLimitExceeded || + (result.termination == BoundedSubprocessTermination.exited && + (result.stdoutLimitExceeded || result.stderrLimitExceeded)), + processLimitExceeded: externalLimit is _EvaluatorProcessLimitExceeded, + memoryLimitExceeded: externalLimit is _EvaluatorMemoryLimitExceeded, + observedProcessCount: externalLimit is _EvaluatorProcessLimitExceeded + ? externalLimit.processCount : null, - observedMemoryMb: signal is _EvaluatorMemoryLimitExceeded - ? signal.memoryMb + observedMemoryMb: externalLimit is _EvaluatorMemoryLimitExceeded + ? externalLimit.memoryMb : null, ); } -Future _awaitProcessExit( - Process process, { - Map? environment, -}) async { - await process.exitCode.timeout( - const Duration(seconds: 2), - onTimeout: () async { - await _terminateProcessTree( - process.pid, - ProcessSignal.sigkill, - environment: environment, - ); - return -1; - }, - ); -} - -Future _terminateProcessTree( - int pid, - ProcessSignal signal, { - Map? environment, -}) async { - if (Platform.isWindows) { - final args = ['/PID', '$pid', '/T']; - if (signal == ProcessSignal.sigkill) args.add('/F'); - await _tryRunProcess('taskkill', args, environment: environment); - return; - } - - final descendants = await _descendantPids(pid, environment: environment); - for (final childPid in descendants.reversed) { - _killPid(childPid, signal); - } - _killPid(pid, signal); -} - Future> _descendantPids( int pid, { Map? environment, @@ -355,77 +236,13 @@ List _parsePids(String output) => output .whereType() .toList(growable: false); -void _killPid(int pid, ProcessSignal signal) { - if (!_tryKillPid(pid, signal)) _tryKillPid(pid, null); -} - -bool _tryKillPid(int pid, ProcessSignal? signal) { - try { - return signal == null ? Process.killPid(pid) : Process.killPid(pid, signal); - } on Object { - return false; - } -} - -/// Collects raw process output, enforcing the limit on undecoded bytes so -/// multibyte encodings cannot exceed the byte contract before decode. -class _BoundedTextCollector { - _BoundedTextCollector(this.maxBytes) : assert(maxBytes > 0); - - final int maxBytes; - final _bytes = BytesBuilder(copy: false); - bool exceeded = false; - - String get text { - final bytes = _bytes.toBytes(); - try { - return systemEncoding.decode(bytes); - } on Object { - return String.fromCharCodes(bytes); - } - } - - void writeBytes(List chunk) { - if (exceeded) return; - final remaining = maxBytes - _bytes.length; - if (remaining <= 0) { - exceeded = true; - return; - } - if (chunk.length <= remaining) { - _bytes.add(chunk); - return; - } - _bytes.add(chunk.sublist(0, remaining)); - exceeded = true; - } -} - -sealed class _EvaluatorProcessSignal { - const _EvaluatorProcessSignal(); -} - -class _EvaluatorProcessExit extends _EvaluatorProcessSignal { - const _EvaluatorProcessExit(this.exitCode); - - final int exitCode; -} - -class _EvaluatorProcessTimedOut extends _EvaluatorProcessSignal { - const _EvaluatorProcessTimedOut(); -} - -class _EvaluatorProcessOutputLimitExceeded extends _EvaluatorProcessSignal { - const _EvaluatorProcessOutputLimitExceeded(); -} - -class _EvaluatorProcessLimitExceeded extends _EvaluatorProcessSignal { +class _EvaluatorProcessLimitExceeded { const _EvaluatorProcessLimitExceeded({required this.processCount}); final int processCount; } -class _EvaluatorMemoryLimitExceeded extends _EvaluatorProcessSignal { +class _EvaluatorMemoryLimitExceeded { const _EvaluatorMemoryLimitExceeded({required this.memoryMb}); final int memoryMb; diff --git a/app/lib/providers/droid_exec_provider.dart b/app/lib/providers/droid_exec_provider.dart index a74675f..22298c6 100644 --- a/app/lib/providers/droid_exec_provider.dart +++ b/app/lib/providers/droid_exec_provider.dart @@ -4,6 +4,7 @@ import 'dart:io'; import 'package:dart_arena/core/model_response.dart'; import 'package:dart_arena/providers/factory_custom_model_environment.dart'; import 'package:dart_arena/providers/model_provider.dart'; +import 'package:dart_arena/runner/bounded_subprocess.dart'; import 'package:dart_arena/runner/subprocess_environment.dart'; class DroidProcessResult { @@ -62,91 +63,33 @@ class DroidExecProvider implements ModelProvider, ModelRuntimeMetadataProvider { Iterable allowedSensitiveEnvironmentKeys, int maxProcessOutputChars, ) async { - final process = await Process.start( - exe, - args, - runInShell: false, + final processResult = await runBoundedSubprocess( + executable: exe, + arguments: args, + workingDirectory: Directory.current.path, environment: benchmarkSubprocessEnvironment( additionalDeniedKeys: deniedEnvironmentKeys, allowedSensitiveKeys: allowedSensitiveEnvironmentKeys, ), - includeParentEnvironment: false, + maxOutputBytes: maxProcessOutputChars, + timeout: timeout, + capture: BoundedSubprocessCapture.trailing, ); - final stdoutBuffer = _BoundedTailTextCollector(maxProcessOutputChars); - final stderrBuffer = _BoundedTailTextCollector(maxProcessOutputChars); - final outputLimitExceeded = Completer(); - void markOutputLimitExceeded() { - if (!outputLimitExceeded.isCompleted) { - outputLimitExceeded.complete(); - } - } - - final stdoutDone = process.stdout.transform(systemEncoding.decoder).listen(( - chunk, - ) { - stdoutBuffer.write(chunk); - if (stdoutBuffer.exceeded) markOutputLimitExceeded(); - }).asFuture(); - final stderrDone = process.stderr.transform(systemEncoding.decoder).listen(( - chunk, - ) { - stderrBuffer.write(chunk); - if (stderrBuffer.exceeded) markOutputLimitExceeded(); - }).asFuture(); - - var timedOut = false; - var outputLimitHit = false; - int exitCode; - Timer? timeoutTimer; - final timeoutExceeded = Completer(); - if (timeout != null) { - timeoutTimer = Timer(timeout, () { - if (!timeoutExceeded.isCompleted) timeoutExceeded.complete(); - }); - } - try { - final signal = await Future.any([ - process.exitCode, - if (timeout != null) - timeoutExceeded.future.then( - (_) => const _DroidProcessTimeoutExceeded(), - ), - outputLimitExceeded.future.then( - (_) => const _DroidProcessOutputLimitExceeded(), - ), - ]); - if (signal is int) { - exitCode = signal; - } else { - timedOut = signal is _DroidProcessTimeoutExceeded; - outputLimitHit = signal is _DroidProcessOutputLimitExceeded; - await _terminateProcessTree(process.pid, ProcessSignal.sigterm); - exitCode = await process.exitCode.timeout( - const Duration(seconds: 2), - onTimeout: () async { - await _terminateProcessTree(process.pid, ProcessSignal.sigkill); - return -1; - }, - ); - } - } finally { - timeoutTimer?.cancel(); - } - await Future.wait([stdoutDone, stderrDone]); - if (timedOut) { + if (processResult.termination == BoundedSubprocessTermination.timedOut) { throw TimeoutException('droid exec timed out', timeout); } - final stderr = stderrBuffer.text; - final stderrWithLimit = outputLimitHit + final stderrWithLimit = processResult.outputLimitExceeded ? [ 'droid exec output exceeded $maxProcessOutputChars characters', - if (stderr.isNotEmpty) stderr, + if (processResult.stderr.isNotEmpty) processResult.stderr, ].join('\n') - : stderr; + : processResult.stderr; return DroidProcessResult( - stdout: stdoutBuffer.text, + stdout: processResult.stdout, stderr: stderrWithLimit, - exitCode: outputLimitHit && exitCode == 0 ? -1 : exitCode, + exitCode: processResult.outputLimitExceeded && processResult.exitCode == 0 + ? -1 + : processResult.exitCode, ); } @@ -304,113 +247,4 @@ class DroidExecProvider implements ModelProvider, ModelRuntimeMetadataProvider { latency: sw.elapsed, ); } - - static Future _terminateProcessTree( - int pid, - ProcessSignal signal, - ) async { - if (Platform.isWindows) { - final args = ['/PID', '$pid', '/T']; - if (signal == ProcessSignal.sigkill) args.add('/F'); - await _tryRunProcess('taskkill', args); - return; - } - - final descendants = await _descendantPids(pid); - for (final childPid in descendants.reversed) { - _killPid(childPid, signal); - } - _killPid(pid, signal); - } - - static Future> _descendantPids(int pid) async { - final descendants = []; - for (final childPid in await _childPids(pid)) { - descendants.add(childPid); - descendants.addAll(await _descendantPids(childPid)); - } - return descendants; - } - - static Future> _childPids(int pid) async { - final pgrep = await _tryRunProcess('pgrep', ['-P', '$pid']); - if (pgrep?.exitCode == 0) { - return _parsePids(pgrep!.stdout.toString()); - } - - final ps = await _tryRunProcess('ps', ['-o', 'pid=', '--ppid', '$pid']); - if (ps?.exitCode == 0) { - return _parsePids(ps!.stdout.toString()); - } - return const []; - } - - static Future _tryRunProcess( - String executable, - List arguments, - ) async { - try { - return await Process.run( - executable, - arguments, - runInShell: false, - environment: benchmarkSubprocessEnvironment(), - includeParentEnvironment: false, - ); - } on Object { - return null; - } - } - - static List _parsePids(String output) => output - .split(RegExp(r'\s+')) - .map((s) => int.tryParse(s.trim())) - .whereType() - .toList(growable: false); - - static void _killPid(int pid, ProcessSignal signal) { - if (!_tryKillPid(pid, signal)) { - _tryKillPid(pid, null); - } - } - - static bool _tryKillPid(int pid, ProcessSignal? signal) { - try { - return signal == null - ? Process.killPid(pid) - : Process.killPid(pid, signal); - } on Object { - return false; - } - } -} - -class _DroidProcessTimeoutExceeded { - const _DroidProcessTimeoutExceeded(); -} - -class _DroidProcessOutputLimitExceeded { - const _DroidProcessOutputLimitExceeded(); -} - -class _BoundedTailTextCollector { - _BoundedTailTextCollector(this.maxChars); - - final int maxChars; - final _buffer = StringBuffer(); - var exceeded = false; - - void write(String chunk) { - if (chunk.isEmpty) return; - _buffer.write(chunk); - if (_buffer.length > maxChars) { - exceeded = true; - final value = _buffer.toString(); - _buffer - ..clear() - ..write(value.substring(value.length - maxChars)); - } - } - - String get text => _buffer.toString(); } diff --git a/app/lib/runner/bounded_subprocess.dart b/app/lib/runner/bounded_subprocess.dart new file mode 100644 index 0000000..c9b7164 --- /dev/null +++ b/app/lib/runner/bounded_subprocess.dart @@ -0,0 +1,501 @@ +import 'dart:async'; +import 'dart:convert'; +import 'dart:io'; +import 'dart:typed_data'; + +import 'package:dart_arena/runner/subprocess_environment.dart'; + +enum BoundedSubprocessTermination { + exited, + timedOut, + cancelled, + outputLimitExceeded, + externalLimitExceeded, +} + +enum BoundedSubprocessCapture { leading, trailing } + +class BoundedSubprocessResult { + const BoundedSubprocessResult({ + required this.stdout, + required this.stderr, + required this.exitCode, + required this.termination, + required this.stdoutLimitExceeded, + required this.stderrLimitExceeded, + this.externalLimit, + }); + + final String stdout; + final String stderr; + final int exitCode; + final BoundedSubprocessTermination termination; + final bool stdoutLimitExceeded; + final bool stderrLimitExceeded; + final Object? externalLimit; + + bool get outputLimitExceeded => + termination == BoundedSubprocessTermination.outputLimitExceeded; +} + +class BoundedSubprocessMonitor { + const BoundedSubprocessMonitor({ + required this.signal, + required this.observed, + required this.dispose, + }); + + final Future signal; + final Object? Function() observed; + final void Function() dispose; +} + +typedef BoundedSubprocessMonitorFactory = + BoundedSubprocessMonitor Function(int pid); + +Future runBoundedSubprocess({ + required String executable, + required List arguments, + required String workingDirectory, + required Map environment, + required int maxOutputBytes, + bool includeParentEnvironment = false, + Duration? timeout, + Future? cancellationSignal, + BoundedSubprocessCapture capture = BoundedSubprocessCapture.leading, + List? stdinBytes, + Map? helperEnvironment, + BoundedSubprocessMonitorFactory? monitor, +}) async { + assert(maxOutputBytes > 0); + final startsProcessGroup = + Platform.isLinux && File('/usr/bin/setsid').existsSync(); + final process = await Process.start( + startsProcessGroup ? '/usr/bin/setsid' : executable, + startsProcessGroup ? ['--wait', executable, ...arguments] : arguments, + workingDirectory: workingDirectory, + runInShell: false, + environment: environment, + includeParentEnvironment: includeParentEnvironment, + ); + final stdout = _BoundedByteCollector(maxOutputBytes, capture); + final stderr = _BoundedByteCollector(maxOutputBytes, capture); + final outputLimitExceeded = Completer<_SubprocessSignal>(); + + void collect(_BoundedByteCollector collector, List chunk) { + collector.add(chunk); + if (collector.exceeded && !outputLimitExceeded.isCompleted) { + outputLimitExceeded.complete(const _OutputLimitExceeded()); + } + } + + final stdoutSubscription = process.stdout.listen( + (chunk) => collect(stdout, chunk), + ); + final stderrSubscription = process.stderr.listen( + (chunk) => collect(stderr, chunk), + ); + final streams = _ProcessStreams(stdoutSubscription, stderrSubscription); + final stdinDone = _writeStdin(process, stdinBytes); + final timeoutExceeded = Completer<_SubprocessSignal>(); + final timeoutAlreadyExceeded = + timeout != null && timeout.compareTo(Duration.zero) <= 0; + final timeoutTimer = timeout == null || timeoutAlreadyExceeded + ? null + : Timer(timeout, () { + if (!timeoutExceeded.isCompleted) { + timeoutExceeded.complete(const _TimedOut()); + } + }); + + BoundedSubprocessMonitor? activeMonitor; + try { + activeMonitor = monitor?.call(process.pid); + } on Object { + timeoutTimer?.cancel(); + await _stopAndDrain( + process, + streams, + stdinDone, + helperEnvironment: helperEnvironment, + processGroup: startsProcessGroup, + ); + rethrow; + } + + late final _SubprocessSignal signal; + try { + signal = timeoutAlreadyExceeded + ? const _TimedOut() + : await Future.any<_SubprocessSignal>([ + process.exitCode.then(_Exited.new), + if (timeout != null) timeoutExceeded.future, + if (cancellationSignal != null) + cancellationSignal.then((_) => const _Cancelled()), + outputLimitExceeded.future, + if (activeMonitor != null) + activeMonitor.signal.then(_ExternalLimitExceeded.new), + ]); + } finally { + timeoutTimer?.cancel(); + activeMonitor?.dispose(); + } + + final exitCode = signal is _Exited + ? signal.exitCode + : await _stopAndDrain( + process, + streams, + stdinDone, + helperEnvironment: helperEnvironment, + processGroup: startsProcessGroup, + ); + if (signal is _Exited) { + await _drainAfterExit( + process, + streams, + stdinDone, + helperEnvironment: helperEnvironment, + processGroup: startsProcessGroup, + ); + } + + final termination = switch (signal) { + _Exited() => BoundedSubprocessTermination.exited, + _TimedOut() => BoundedSubprocessTermination.timedOut, + _Cancelled() => BoundedSubprocessTermination.cancelled, + _OutputLimitExceeded() => BoundedSubprocessTermination.outputLimitExceeded, + _ExternalLimitExceeded() => + BoundedSubprocessTermination.externalLimitExceeded, + }; + final externalLimit = switch (signal) { + _ExternalLimitExceeded() => signal.value, + _Exited() => activeMonitor?.observed(), + _ => null, + }; + return BoundedSubprocessResult( + stdout: stdout.text, + stderr: stderr.text, + exitCode: exitCode, + termination: termination, + stdoutLimitExceeded: stdout.exceeded, + stderrLimitExceeded: stderr.exceeded, + externalLimit: externalLimit, + ); +} + +Future _writeStdin(Process process, List? bytes) async { + try { + if (bytes != null && bytes.isNotEmpty) process.stdin.add(bytes); + await process.stdin.close(); + } on Object { + // Process termination can close stdin while supervision is still draining. + } +} + +Future _drainAfterExit( + Process process, + _ProcessStreams streams, + Future stdinDone, { + Map? helperEnvironment, + required bool processGroup, +}) async { + if (streams.isDone) { + await stdinDone; + return; + } + await _terminateProcessTree( + process.pid, + ProcessSignal.sigterm, + environment: helperEnvironment, + processGroup: processGroup, + ); + await _awaitDrainWithEscalation( + process, + streams, + stdinDone, + helperEnvironment: helperEnvironment, + processGroup: processGroup, + ); +} + +Future _stopAndDrain( + Process process, + _ProcessStreams streams, + Future stdinDone, { + Map? helperEnvironment, + required bool processGroup, +}) async { + await _terminateProcessTree( + process.pid, + ProcessSignal.sigterm, + environment: helperEnvironment, + processGroup: processGroup, + ); + return _awaitDrainWithEscalation( + process, + streams, + stdinDone, + helperEnvironment: helperEnvironment, + processGroup: processGroup, + ); +} + +Future _awaitDrainWithEscalation( + Process process, + _ProcessStreams streams, + Future stdinDone, { + Map? helperEnvironment, + required bool processGroup, +}) async { + final completed = await _waitForExitAndDrain(process, streams, stdinDone) + .then((value) => value) + .timeout(const Duration(seconds: 2), onTimeout: () => null); + if (completed != null) return completed; + + await _terminateProcessTree( + process.pid, + ProcessSignal.sigkill, + environment: helperEnvironment, + processGroup: processGroup, + ); + final killed = await _waitForExitAndDrain(process, streams, stdinDone) + .then((value) => value) + .timeout(const Duration(seconds: 2), onTimeout: () => null); + if (killed != null) return killed; + + await streams.cancel(); + return process.exitCode.timeout( + const Duration(seconds: 1), + onTimeout: () => -1, + ); +} + +Future _waitForExitAndDrain( + Process process, + _ProcessStreams streams, + Future stdinDone, +) async { + final exitCode = await process.exitCode; + await Future.wait([streams.done, stdinDone]); + return exitCode; +} + +Future _terminateProcessTree( + int pid, + ProcessSignal signal, { + Map? environment, + required bool processGroup, +}) async { + if (Platform.isWindows) { + final args = ['/PID', '$pid', '/T']; + if (signal == ProcessSignal.sigkill) args.add('/F'); + await _tryRunProcess('taskkill', args, environment: environment); + return; + } + + if (processGroup) { + _tryKillPid(-pid, signal); + _killPid(pid, signal); + } + final descendants = await _descendantPids(pid, environment: environment); + for (final childPid in descendants.reversed) { + _killPid(childPid, signal); + } + if (!processGroup) _killPid(pid, signal); +} + +Future> _descendantPids( + int pid, { + Map? environment, +}) async { + final ps = await _tryRunProcess(_systemTool('ps'), const [ + '-eo', + 'pid=,ppid=', + ], environment: environment); + if (ps?.exitCode != 0) return const []; + final childrenByParent = >{}; + for (final line in ps!.stdout.toString().split('\n')) { + final values = _parsePids(line); + if (values.length != 2) continue; + childrenByParent.putIfAbsent(values[1], () => []).add(values[0]); + } + final descendants = []; + final pending = [pid]; + final seen = {pid}; + while (pending.isNotEmpty) { + final parent = pending.removeLast(); + for (final child in childrenByParent[parent] ?? const []) { + if (!seen.add(child)) continue; + descendants.add(child); + pending.add(child); + } + } + return descendants; +} + +String _systemTool(String name) { + for (final root in const ['/usr/bin', '/bin']) { + final path = '$root/$name'; + if (File(path).existsSync()) return path; + } + return name; +} + +Future _tryRunProcess( + String executable, + List arguments, { + Map? environment, +}) async { + try { + return await Process.run( + executable, + arguments, + runInShell: false, + environment: environment ?? benchmarkSubprocessEnvironment(), + includeParentEnvironment: false, + ).timeout(const Duration(milliseconds: 500)); + } on Object { + return null; + } +} + +List _parsePids(String output) => output + .split(RegExp(r'\s+')) + .map((value) => int.tryParse(value.trim())) + .whereType() + .toList(growable: false); + +void _killPid(int pid, ProcessSignal signal) { + if (!_tryKillPid(pid, signal)) _tryKillPid(pid, null); +} + +bool _tryKillPid(int pid, ProcessSignal? signal) { + try { + return signal == null ? Process.killPid(pid) : Process.killPid(pid, signal); + } on Object { + return false; + } +} + +class _ProcessStreams { + _ProcessStreams(this._stdout, this._stderr) { + done = Future.wait([ + _stdout.asFuture(), + _stderr.asFuture(), + ]).whenComplete(() => isDone = true); + } + + final StreamSubscription> _stdout; + final StreamSubscription> _stderr; + late final Future done; + bool isDone = false; + + Future cancel() => + Future.wait([_stdout.cancel(), _stderr.cancel()]); +} + +class _BoundedByteCollector { + _BoundedByteCollector(this.maxBytes, this.capture) + : _tail = capture == BoundedSubprocessCapture.trailing + ? Uint8List(maxBytes) + : null; + + final int maxBytes; + final BoundedSubprocessCapture capture; + final BytesBuilder _head = BytesBuilder(copy: false); + final Uint8List? _tail; + var _tailStart = 0; + var _tailLength = 0; + var _seenBytes = 0; + + bool get exceeded => _seenBytes > maxBytes; + + void add(List chunk) { + if (chunk.isEmpty) return; + _seenBytes += chunk.length; + if (capture == BoundedSubprocessCapture.leading) { + final remaining = maxBytes - _head.length; + if (remaining > 0) { + _head.add( + chunk.length <= remaining ? chunk : chunk.sublist(0, remaining), + ); + } + return; + } + _addTrailing(chunk); + } + + void _addTrailing(List chunk) { + final tail = _tail!; + if (chunk.length >= maxBytes) { + tail.setRange(0, maxBytes, chunk, chunk.length - maxBytes); + _tailStart = 0; + _tailLength = maxBytes; + return; + } + final overflow = (_tailLength + chunk.length - maxBytes).clamp(0, maxBytes); + _tailStart = (_tailStart + overflow) % maxBytes; + _tailLength = (_tailLength + chunk.length).clamp(0, maxBytes); + final writeStart = (_tailStart + _tailLength - chunk.length) % maxBytes; + final firstLength = (maxBytes - writeStart).clamp(0, chunk.length); + tail.setRange(writeStart, writeStart + firstLength, chunk); + if (firstLength < chunk.length) { + tail.setRange(0, chunk.length - firstLength, chunk, firstLength); + } + } + + String get text => _decode(bytes); + + List get bytes { + if (capture == BoundedSubprocessCapture.leading) return _head.toBytes(); + if (_tailLength == 0) return const []; + final tail = _tail!; + if (_tailStart + _tailLength <= maxBytes) { + return tail.sublist(_tailStart, _tailStart + _tailLength); + } + return [ + ...tail.sublist(_tailStart), + ...tail.sublist(0, (_tailStart + _tailLength) % maxBytes), + ]; + } + + String _decode(List bytes) { + try { + if (!Platform.isWindows) { + return utf8.decode(bytes, allowMalformed: true); + } + return systemEncoding.decode(bytes); + } on Object { + return String.fromCharCodes(bytes); + } + } +} + +sealed class _SubprocessSignal { + const _SubprocessSignal(); +} + +class _Exited extends _SubprocessSignal { + const _Exited(this.exitCode); + + final int exitCode; +} + +class _TimedOut extends _SubprocessSignal { + const _TimedOut(); +} + +class _Cancelled extends _SubprocessSignal { + const _Cancelled(); +} + +class _OutputLimitExceeded extends _SubprocessSignal { + const _OutputLimitExceeded(); +} + +class _ExternalLimitExceeded extends _SubprocessSignal { + const _ExternalLimitExceeded(this.value); + + final Object value; +} diff --git a/app/lib/runner/workdir_manager.dart b/app/lib/runner/workdir_manager.dart index 56ec9cc..42a05cf 100644 --- a/app/lib/runner/workdir_manager.dart +++ b/app/lib/runner/workdir_manager.dart @@ -7,6 +7,7 @@ import 'package:dart_arena/core/patch_capture.dart'; import 'package:dart_arena/core/path_safety.dart'; import 'package:dart_arena/core/task_workspace.dart'; import 'package:dart_arena/core/workspace_path.dart'; +import 'package:dart_arena/runner/bounded_subprocess.dart'; import 'package:dart_arena/runner/generated_code_sandbox.dart'; import 'package:dart_arena/runner/subprocess_environment.dart'; import 'package:path/path.dart' as p; @@ -421,88 +422,37 @@ class WorkdirManager { allowInternet: sandboxAllowInternet, resourceLimits: SandboxResourceLimits(cpuCores: maxCpuCores), ); - final process = await Process.start( - processStart.executable, - processStart.arguments, + final timeout = remainingTimeout?.call(); + final result = await runBoundedSubprocess( + executable: processStart.executable, + arguments: processStart.arguments, workingDirectory: processStart.workingDirectory, - runInShell: false, environment: processStart.environment, - includeParentEnvironment: false, + maxOutputBytes: prepareMaxOutputChars, + timeout: timeout, + cancellationSignal: cancellationSignal, + helperEnvironment: benchmarkSubprocessEnvironment( + additionalDeniedKeys: deniedEnvironmentKeys, + ), ); - final stdoutBuffer = _BoundedTextCollector(prepareMaxOutputChars); - final stderrBuffer = _BoundedTextCollector(prepareMaxOutputChars); - final outputLimitExceeded = Completer<_PrepareProcessSignal>(); - void markOutputLimitExceeded() { - if (!outputLimitExceeded.isCompleted) { - outputLimitExceeded.complete( - const _PrepareProcessOutputLimitExceeded(), - ); - } - } - - final stdoutText = process.stdout.transform(systemEncoding.decoder); - final stderrText = process.stderr.transform(systemEncoding.decoder); - final stdoutDone = stdoutText.listen((chunk) { - stdoutBuffer.write(chunk); - if (stdoutBuffer.exceeded) markOutputLimitExceeded(); - }).asFuture(); - final stderrDone = stderrText.listen((chunk) { - stderrBuffer.write(chunk); - if (stderrBuffer.exceeded) markOutputLimitExceeded(); - }).asFuture(); - - final timeout = remainingTimeout?.call(); - if (timeout != null && timeout.compareTo(Duration.zero) <= 0) { - await _terminateProcessTree(process.pid, ProcessSignal.sigterm); - await _awaitProcessExit(process); - await Future.wait([stdoutDone, stderrDone]); - throw TimeoutException('prepare timed out', timeout); - } - - final timeoutExceeded = Completer<_PrepareProcessSignal>(); - final timeoutTimer = timeout == null - ? null - : Timer(timeout, () { - if (!timeoutExceeded.isCompleted) { - timeoutExceeded.complete(_PrepareProcessTimedOut(timeout)); - } - }); - late final _PrepareProcessSignal signal; - try { - signal = await Future.any<_PrepareProcessSignal>([ - process.exitCode.then(_PrepareProcessExit.new), - if (timeout != null) timeoutExceeded.future, - outputLimitExceeded.future, - if (cancellationSignal != null) - cancellationSignal.then((_) => const _PrepareProcessCancelled()), - ]); - } finally { - timeoutTimer?.cancel(); - } - - if (signal is _PrepareProcessExit) { - await Future.wait([stdoutDone, stderrDone]); + if (result.termination == BoundedSubprocessTermination.exited) { cancellationCheck?.call(); return _PrepareProcessResult( - stdout: stdoutBuffer.text, - stderr: stderrBuffer.text, - exitCode: signal.exitCode, + stdout: result.stdout, + stderr: result.stderr, + exitCode: result.exitCode, outputLimitExceeded: false, ); } - - await _terminateProcessTree(process.pid, ProcessSignal.sigterm); - await _awaitProcessExit(process); - await Future.wait([stdoutDone, stderrDone]); - if (signal is _PrepareProcessTimedOut) { - throw TimeoutException('prepare timed out', signal.timeout); + if (result.termination == BoundedSubprocessTermination.timedOut) { + throw TimeoutException('prepare timed out', timeout); } - if (signal is _PrepareProcessOutputLimitExceeded) { + if (result.outputLimitExceeded) { return _PrepareProcessResult( - stdout: stdoutBuffer.text, + stdout: result.stdout, stderr: 'prepare output exceeded $prepareMaxOutputChars characters\n' - '${stdoutBuffer.text}\n${stderrBuffer.text}', + '${result.stdout}\n${result.stderr}', exitCode: -1, outputLimitExceeded: true, ); @@ -510,94 +460,6 @@ class WorkdirManager { throw TimeoutException('prepare cancelled'); } - Future _awaitProcessExit(Process process) async { - await process.exitCode.timeout( - const Duration(seconds: 2), - onTimeout: () async { - await _terminateProcessTree(process.pid, ProcessSignal.sigkill); - return -1; - }, - ); - } - - Future _terminateProcessTree(int pid, ProcessSignal signal) async { - if (Platform.isWindows) { - final args = ['/PID', '$pid', '/T']; - if (signal == ProcessSignal.sigkill) args.add('/F'); - await _tryRunProcess('taskkill', args); - return; - } - - final descendants = await _descendantPids(pid); - for (final childPid in descendants.reversed) { - _killPid(childPid, signal); - } - _killPid(pid, signal); - } - - Future> _descendantPids(int pid) async { - final descendants = []; - for (final childPid in await _childPids(pid)) { - descendants.add(childPid); - descendants.addAll(await _descendantPids(childPid)); - } - return descendants; - } - - Future> _childPids(int pid) async { - final pgrep = await _tryRunProcess('pgrep', ['-P', '$pid']); - if (pgrep?.exitCode == 0) { - return _parsePids(pgrep!.stdout.toString()); - } - - final ps = await _tryRunProcess('ps', ['-o', 'pid=', '--ppid', '$pid']); - if (ps?.exitCode == 0) { - return _parsePids(ps!.stdout.toString()); - } - return const []; - } - - Future _tryRunProcess( - String executable, - List arguments, - ) async { - try { - return await Process.run( - executable, - arguments, - runInShell: false, - environment: benchmarkSubprocessEnvironment( - additionalDeniedKeys: deniedEnvironmentKeys, - ), - includeParentEnvironment: false, - ); - } on Object { - return null; - } - } - - List _parsePids(String output) => output - .split(RegExp(r'\s+')) - .map((s) => int.tryParse(s.trim())) - .whereType() - .toList(growable: false); - - void _killPid(int pid, ProcessSignal signal) { - if (!_tryKillPid(pid, signal)) { - _tryKillPid(pid, null); - } - } - - bool _tryKillPid(int pid, ProcessSignal? signal) { - try { - return signal == null - ? Process.killPid(pid) - : Process.killPid(pid, signal); - } on Object { - return false; - } - } - Future _copyFixtureRoot(Directory sourceRoot, Directory target) async { if (!await sourceRoot.exists()) { throw ArgumentError.value( @@ -782,83 +644,41 @@ class WorkdirManager { ); } - final process = await Process.start( - gitExecutable, - args, + final environment = _baselineGitEnvironment(); + final result = await runBoundedSubprocess( + executable: gitExecutable, + arguments: args, workingDirectory: dir.path, - runInShell: false, - environment: _baselineGitEnvironment(), - includeParentEnvironment: false, + environment: environment, + maxOutputBytes: gitMaxOutputChars, + timeout: gitTimeout, + stdinBytes: stdin == null ? null : utf8.encode(stdin), + helperEnvironment: environment, ); - if (stdin != null) { - process.stdin.add(utf8.encode(stdin)); - await process.stdin.close(); - } - - final stdoutBuffer = _BoundedTextCollector(gitMaxOutputChars); - final stderrBuffer = _BoundedTextCollector(gitMaxOutputChars); - final outputLimitExceeded = Completer<_GitProcessSignal>(); - void markOutputLimitExceeded() { - if (!outputLimitExceeded.isCompleted) { - outputLimitExceeded.complete(const _GitProcessOutputLimitExceeded()); - } - } - - final stdoutText = process.stdout.transform(systemEncoding.decoder); - final stderrText = process.stderr.transform(systemEncoding.decoder); - final stdoutDone = stdoutText.listen((chunk) { - stdoutBuffer.write(chunk); - if (stdoutBuffer.exceeded) markOutputLimitExceeded(); - }).asFuture(); - final stderrDone = stderrText.listen((chunk) { - stderrBuffer.write(chunk); - if (stderrBuffer.exceeded) markOutputLimitExceeded(); - }).asFuture(); - - final timeoutExceeded = Completer<_GitProcessSignal>(); - final timeoutTimer = Timer(gitTimeout, () { - if (!timeoutExceeded.isCompleted) { - timeoutExceeded.complete(_GitProcessTimedOut(gitTimeout)); - } - }); - final signal = await Future.any<_GitProcessSignal>([ - process.exitCode.then(_GitProcessExit.new), - timeoutExceeded.future, - outputLimitExceeded.future, - ]); - timeoutTimer.cancel(); - - if (signal is _GitProcessExit) { - await Future.wait([stdoutDone, stderrDone]); - if (signal.exitCode != 0) { - throw ProcessException( - gitExecutable, - args, - '${stdoutBuffer.text}\n${stderrBuffer.text}', - signal.exitCode, - ); - } - return stdoutBuffer.text; - } - - await _terminateProcessTree(process.pid, ProcessSignal.sigterm); - await _awaitProcessExit(process); - await Future.wait([stdoutDone, stderrDone]); - - if (signal is _GitProcessTimedOut) { + if (result.termination == BoundedSubprocessTermination.timedOut) { throw TimeoutException( 'baseline git initialization timed out', - signal.timeout, + gitTimeout, ); } - - throw ProcessException( - gitExecutable, - args, - 'baseline git output exceeded $gitMaxOutputChars characters\n' - '${stdoutBuffer.text}\n${stderrBuffer.text}', - -1, - ); + if (result.outputLimitExceeded) { + throw ProcessException( + gitExecutable, + args, + 'baseline git output exceeded $gitMaxOutputChars characters\n' + '${result.stdout}\n${result.stderr}', + -1, + ); + } + if (result.exitCode != 0) { + throw ProcessException( + gitExecutable, + args, + '${result.stdout}\n${result.stderr}', + result.exitCode, + ); + } + return result.stdout; } Map _baselineGitEnvironment() { @@ -873,49 +693,6 @@ class WorkdirManager { } } -class _BoundedTextCollector { - _BoundedTextCollector(this.maxChars); - - final int maxChars; - final _buffer = StringBuffer(); - bool exceeded = false; - - String get text => _buffer.toString(); - - void write(String chunk) { - if (exceeded) return; - final remaining = maxChars - _buffer.length; - if (chunk.length <= remaining) { - _buffer.write(chunk); - return; - } - if (remaining > 0) { - _buffer.write(chunk.substring(0, remaining)); - } - exceeded = true; - } -} - -sealed class _GitProcessSignal { - const _GitProcessSignal(); -} - -class _GitProcessExit extends _GitProcessSignal { - const _GitProcessExit(this.exitCode); - - final int exitCode; -} - -class _GitProcessTimedOut extends _GitProcessSignal { - const _GitProcessTimedOut(this.timeout); - - final Duration timeout; -} - -class _GitProcessOutputLimitExceeded extends _GitProcessSignal { - const _GitProcessOutputLimitExceeded(); -} - class _PrepareProcessResult { const _PrepareProcessResult({ required this.stdout, @@ -929,27 +706,3 @@ class _PrepareProcessResult { final int exitCode; final bool outputLimitExceeded; } - -sealed class _PrepareProcessSignal { - const _PrepareProcessSignal(); -} - -class _PrepareProcessExit extends _PrepareProcessSignal { - const _PrepareProcessExit(this.exitCode); - - final int exitCode; -} - -class _PrepareProcessTimedOut extends _PrepareProcessSignal { - const _PrepareProcessTimedOut(this.timeout); - - final Duration timeout; -} - -class _PrepareProcessCancelled extends _PrepareProcessSignal { - const _PrepareProcessCancelled(); -} - -class _PrepareProcessOutputLimitExceeded extends _PrepareProcessSignal { - const _PrepareProcessOutputLimitExceeded(); -} diff --git a/app/test/evaluators/evaluator_process_test.dart b/app/test/evaluators/evaluator_process_test.dart index 6881e0e..0959e78 100644 --- a/app/test/evaluators/evaluator_process_test.dart +++ b/app/test/evaluators/evaluator_process_test.dart @@ -57,6 +57,35 @@ void main() { expect(result.stdout, 'hello-bytes'); }, skip: Platform.isWindows); + test( + 'timeout remains primary when TERM handling floods output', + () async { + final tmp = await Directory.systemTemp.createTemp( + 'dart_arena_eval_timeout_output_', + ); + addTearDown(() async { + if (await tmp.exists()) await tmp.delete(recursive: true); + }); + + final result = await runEvaluatorProcess( + 'sh', + const [ + '-c', + "trap 'head -c 4096 /dev/zero; exit 0' TERM; while :; do sleep 1; done", + ], + workingDirectory: tmp.path, + environment: {'PATH': '/usr/bin:/bin'}, + timeout: const Duration(milliseconds: 100), + maxOutputChars: 128, + ); + + expect(result.timedOut, isTrue); + expect(result.stdout.length, 128); + expect(result.outputLimitExceeded, isFalse); + }, + skip: Platform.isWindows, + ); + test( 'resource probe helpers scrub sensitive environment variables', () async { diff --git a/app/test/runner/bounded_subprocess_test.dart b/app/test/runner/bounded_subprocess_test.dart new file mode 100644 index 0000000..d8d9ce5 --- /dev/null +++ b/app/test/runner/bounded_subprocess_test.dart @@ -0,0 +1,199 @@ +import 'dart:async'; +import 'dart:io'; + +import 'package:dart_arena/runner/bounded_subprocess.dart'; +import 'package:test/test.dart'; + +void main() { + test( + 'drains stdout and stderr concurrently without backpressure hangs', + () async { + final result = await _runShell( + r''' +(head -c 1048576 /dev/zero | tr '\0' o) & +(head -c 1048576 /dev/zero | tr '\0' e >&2) & +wait +''', + maxOutputBytes: 2 * 1024 * 1024, + timeout: const Duration(seconds: 5), + ); + + expect(result.termination, BoundedSubprocessTermination.exited); + expect(result.exitCode, 0); + expect(result.stdout.length, 1024 * 1024); + expect(result.stderr.length, 1024 * 1024); + }, + skip: Platform.isWindows, + ); + + test( + 'bounds finite excess output regardless of exit arbitration', + () async { + final result = await _runShell( + 'head -c 4096 /dev/zero | tr \'\\0\' x', + maxOutputBytes: 128, + timeout: const Duration(seconds: 5), + ); + + expect( + result.termination, + anyOf( + BoundedSubprocessTermination.exited, + BoundedSubprocessTermination.outputLimitExceeded, + ), + ); + expect(result.stdoutLimitExceeded, isTrue); + expect(result.stdout.length, 128); + }, + skip: Platform.isWindows, + ); + + test( + 'keeps readable multibyte text when capture ends mid-sequence', + () async { + final result = await _runShell( + r"printf '\342\230\203\342\230\203'", + maxOutputBytes: 4, + timeout: const Duration(seconds: 5), + ); + + expect(result.stdout, startsWith('☃')); + expect(result.stdout, endsWith('�')); + }, + skip: Platform.isWindows, + ); + + test( + 'retains an observed external limit when process exit wins', + () async { + const observedLimit = 'process-limit'; + final signal = Completer(); + final result = await runBoundedSubprocess( + executable: 'sh', + arguments: const ['-c', 'exit 0'], + workingDirectory: Directory.current.path, + environment: {'PATH': Platform.environment['PATH'] ?? '/usr/bin:/bin'}, + maxOutputBytes: 128, + timeout: const Duration(seconds: 5), + monitor: (_) => BoundedSubprocessMonitor( + signal: signal.future, + observed: () => observedLimit, + dispose: () {}, + ), + ); + + expect(result.termination, BoundedSubprocessTermination.exited); + expect(result.externalLimit, observedLimit); + }, + skip: Platform.isWindows, + ); + + test( + 'an already-expired timeout takes precedence over cancellation', + () async { + final cancellation = Completer()..complete(); + final result = await _runShell( + 'sleep 20', + maxOutputBytes: 128, + timeout: Duration.zero, + cancellationSignal: cancellation.future, + ); + + expect(result.termination, BoundedSubprocessTermination.timedOut); + }, + skip: Platform.isWindows, + ); + + test( + 'cancellation takes precedence over a subsequent output flood', + () async { + final cancellation = Completer()..complete(); + final result = await _runShell( + 'while :; do printf xxxxxxxxxxxxxxxx; done', + maxOutputBytes: 128, + timeout: const Duration(seconds: 5), + cancellationSignal: cancellation.future, + ); + + expect(result.termination, BoundedSubprocessTermination.cancelled); + }, + skip: Platform.isWindows, + ); + + test( + 'cleans up descendants that keep streams open after parent exit', + () async { + final temp = await Directory.systemTemp.createTemp( + 'dart_arena_subprocess_drain_', + ); + addTearDown(() async { + if (await temp.exists()) await temp.delete(recursive: true); + }); + final pidFile = File('${temp.path}/child.pid'); + final stopwatch = Stopwatch()..start(); + + final result = await _runShell( + 'sleep 20 & echo \$! > \'${pidFile.path}\'; exit 0', + maxOutputBytes: 128, + timeout: const Duration(seconds: 5), + ); + stopwatch.stop(); + + expect(result.termination, BoundedSubprocessTermination.exited); + expect(stopwatch.elapsed, lessThan(const Duration(seconds: 3))); + final pid = int.parse((await pidFile.readAsString()).trim()); + await Future.delayed(const Duration(milliseconds: 100)); + expect(await _pidIsRunning(pid), isFalse); + }, + skip: Platform.isWindows, + ); + + test( + 'escalates from TERM to KILL for an unresponsive process group', + () async { + final temp = await Directory.systemTemp.createTemp( + 'dart_arena_subprocess_kill_', + ); + addTearDown(() async { + if (await temp.exists()) await temp.delete(recursive: true); + }); + final pidFile = File('${temp.path}/child.pid'); + + final result = await _runShell( + "trap '' TERM; sleep 20 & echo \$! > '${pidFile.path}'; wait", + maxOutputBytes: 128, + timeout: const Duration(milliseconds: 100), + ); + + expect(result.termination, BoundedSubprocessTermination.timedOut); + final pid = int.parse((await pidFile.readAsString()).trim()); + await Future.delayed(const Duration(milliseconds: 100)); + expect(await _pidIsRunning(pid), isFalse); + }, + skip: Platform.isWindows, + ); +} + +Future _runShell( + String command, { + required int maxOutputBytes, + required Duration timeout, + Future? cancellationSignal, +}) { + return runBoundedSubprocess( + executable: 'sh', + arguments: ['-c', command], + workingDirectory: Directory.current.path, + environment: {'PATH': Platform.environment['PATH'] ?? '/usr/bin:/bin'}, + maxOutputBytes: maxOutputBytes, + timeout: timeout, + cancellationSignal: cancellationSignal, + ); +} + +Future _pidIsRunning(int pid) async { + final result = await Process.run('ps', ['-p', '$pid', '-o', 'stat=']); + if (result.exitCode != 0) return false; + final state = result.stdout.toString().trim(); + return state.isNotEmpty && !state.startsWith('Z'); +} From 969d4aa624532435dda6d36ff7d7ebe55746998a Mon Sep 17 00:00:00 2001 From: Elberte Plinio Date: Mon, 20 Jul 2026 09:57:25 -0300 Subject: [PATCH 2/4] fix: harden bounded subprocess supervision --- .github/workflows/ci.yml | 1 + app/lib/agent/droid_agent_harness.dart | 3 +- app/lib/agent/minimal_agent_harness.dart | 3 +- app/lib/core/patch_capture.dart | 3 +- app/lib/evaluators/analyze_evaluator.dart | 8 +- app/lib/evaluators/compile_evaluator.dart | 8 +- app/lib/evaluators/evaluator_process.dart | 51 ++- app/lib/evaluators/hidden_test_evaluator.dart | 10 +- app/lib/evaluators/test_author_evaluator.dart | 8 +- app/lib/evaluators/test_evaluator.dart | 10 +- app/lib/evaluators/widget_tree_evaluator.dart | 8 +- app/lib/providers/droid_exec_provider.dart | 3 +- app/lib/runner/agentic_run_orchestrator.dart | 2 +- app/lib/runner/bounded_subprocess.dart | 292 +++++++++++++++--- app/lib/runner/evaluator_resource_limits.dart | 58 ++-- app/lib/runner/task_qa_runner.dart | 8 +- app/lib/runner/workdir_manager.dart | 6 +- .../evaluators/compile_evaluator_test.dart | 4 +- .../evaluators/evaluator_process_test.dart | 69 ++++- .../hidden_test_evaluator_test.dart | 4 +- app/test/evaluators/test_evaluator_test.dart | 8 +- .../runner/agentic_run_orchestrator_test.dart | 4 +- app/test/runner/bounded_subprocess_test.dart | 178 ++++++++++- .../codegen_task_executor_blocking_test.dart | 2 +- .../evaluator_resource_limits_test.dart | 22 +- .../runner/generated_code_sandbox_test.dart | 2 +- 26 files changed, 612 insertions(+), 163 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index aabd94f..074fa7c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -89,6 +89,7 @@ jobs: test/storage/settings_readme_test.dart \ test/storage/settings_test.dart dart test --concurrency=1 \ + test/runner/bounded_subprocess_test.dart \ test/runner/codegen_task_executor_blocking_test.dart \ test/runner/corpus_task_qa_test.dart \ test/runner/evaluator_resource_limits_test.dart \ diff --git a/app/lib/agent/droid_agent_harness.dart b/app/lib/agent/droid_agent_harness.dart index 7641a70..1ab205c 100644 --- a/app/lib/agent/droid_agent_harness.dart +++ b/app/lib/agent/droid_agent_harness.dart @@ -198,7 +198,8 @@ $instruction arguments: processStart.arguments, workingDirectory: processStart.workingDirectory, environment: processStart.environment, - maxOutputBytes: maxProcessOutputChars, + maxOutputBytes: maxEncodedOutputBytes(maxProcessOutputChars), + maxOutputCharacters: maxProcessOutputChars, timeout: timeout, capture: BoundedSubprocessCapture.trailing, ); diff --git a/app/lib/agent/minimal_agent_harness.dart b/app/lib/agent/minimal_agent_harness.dart index ed9d5ba..37f2b11 100644 --- a/app/lib/agent/minimal_agent_harness.dart +++ b/app/lib/agent/minimal_agent_harness.dart @@ -234,7 +234,8 @@ class MinimalAgentHarness implements AgentHarness, AgentHarnessProvenance { arguments: processStart.arguments, workingDirectory: processStart.workingDirectory, environment: processStart.environment, - maxOutputBytes: maxOutputChars, + maxOutputBytes: maxEncodedOutputBytes(maxOutputChars), + maxOutputCharacters: maxOutputChars, timeout: timeout, capture: BoundedSubprocessCapture.trailing, ); diff --git a/app/lib/core/patch_capture.dart b/app/lib/core/patch_capture.dart index 2f50fbf..0d99745 100644 --- a/app/lib/core/patch_capture.dart +++ b/app/lib/core/patch_capture.dart @@ -68,7 +68,8 @@ class PatchCapture { arguments: args, workingDirectory: workspace.path, environment: processEnvironment, - maxOutputBytes: maxOutputChars, + maxOutputBytes: maxEncodedOutputBytes(maxOutputChars), + maxOutputCharacters: maxOutputChars, timeout: timeout, helperEnvironment: processEnvironment, ); diff --git a/app/lib/evaluators/analyze_evaluator.dart b/app/lib/evaluators/analyze_evaluator.dart index ab7767a..07d8b7c 100644 --- a/app/lib/evaluators/analyze_evaluator.dart +++ b/app/lib/evaluators/analyze_evaluator.dart @@ -9,7 +9,7 @@ import 'package:dart_arena/runner/subprocess_environment.dart'; class AnalyzeEvaluator implements Evaluator { const AnalyzeEvaluator({ this.timeout = defaultEvaluatorProcessTimeout, - this.maxOutputChars = defaultEvaluatorMaxOutputChars, + this.maxOutputBytes = defaultEvaluatorMaxOutputBytes, this.maxProcesses, this.maxMemoryMb, this.dartExecutable = 'dart', @@ -17,7 +17,7 @@ class AnalyzeEvaluator implements Evaluator { }); final Duration? timeout; - final int maxOutputChars; + final int maxOutputBytes; final int? maxProcesses; final int? maxMemoryMb; final String dartExecutable; @@ -41,7 +41,7 @@ class AnalyzeEvaluator implements Evaluator { ), includeParentEnvironment: false, timeout: timeout, - maxOutputChars: maxOutputChars, + maxOutputBytes: maxOutputBytes, maxCpuCores: ctx.task.effectiveResourceLimits.cpus, maxProcesses: maxProcesses, maxMemoryMb: maxMemoryMb, @@ -77,7 +77,7 @@ class AnalyzeEvaluator implements Evaluator { if (res.timedOut && timeout != null) 'timeout_ms': timeout!.inMilliseconds, if (res.outputLimitExceeded) 'output_limit_exceeded': true, - if (res.outputLimitExceeded) 'max_output_chars': maxOutputChars, + if (res.outputLimitExceeded) 'max_output_bytes': maxOutputBytes, if (res.processLimitExceeded) 'process_limit_exceeded': true, if (res.processLimitExceeded && maxProcesses != null) 'max_processes': maxProcesses, diff --git a/app/lib/evaluators/compile_evaluator.dart b/app/lib/evaluators/compile_evaluator.dart index 769fef4..713ee22 100644 --- a/app/lib/evaluators/compile_evaluator.dart +++ b/app/lib/evaluators/compile_evaluator.dart @@ -7,7 +7,7 @@ import 'package:dart_arena/runner/subprocess_environment.dart'; class CompileEvaluator implements Evaluator { const CompileEvaluator({ this.timeout = defaultEvaluatorProcessTimeout, - this.maxOutputChars = defaultEvaluatorMaxOutputChars, + this.maxOutputBytes = defaultEvaluatorMaxOutputBytes, this.maxProcesses, this.maxMemoryMb, this.dartExecutable = 'dart', @@ -15,7 +15,7 @@ class CompileEvaluator implements Evaluator { }); final Duration? timeout; - final int maxOutputChars; + final int maxOutputBytes; final int? maxProcesses; final int? maxMemoryMb; final String dartExecutable; @@ -39,7 +39,7 @@ class CompileEvaluator implements Evaluator { ), includeParentEnvironment: false, timeout: timeout, - maxOutputChars: maxOutputChars, + maxOutputBytes: maxOutputBytes, maxCpuCores: ctx.task.effectiveResourceLimits.cpus, maxProcesses: maxProcesses, maxMemoryMb: maxMemoryMb, @@ -76,7 +76,7 @@ class CompileEvaluator implements Evaluator { if (analyze.timedOut && timeout != null) 'timeout_ms': timeout!.inMilliseconds, if (analyze.outputLimitExceeded) 'output_limit_exceeded': true, - if (analyze.outputLimitExceeded) 'max_output_chars': maxOutputChars, + if (analyze.outputLimitExceeded) 'max_output_bytes': maxOutputBytes, if (analyze.processLimitExceeded) 'process_limit_exceeded': true, if (analyze.processLimitExceeded && maxProcesses != null) 'max_processes': maxProcesses, diff --git a/app/lib/evaluators/evaluator_process.dart b/app/lib/evaluators/evaluator_process.dart index 6a205da..cd5f32d 100644 --- a/app/lib/evaluators/evaluator_process.dart +++ b/app/lib/evaluators/evaluator_process.dart @@ -5,7 +5,7 @@ import 'package:dart_arena/runner/generated_code_sandbox.dart'; import 'package:dart_arena/runner/subprocess_environment.dart'; const defaultEvaluatorProcessTimeout = Duration(minutes: 5); -const defaultEvaluatorMaxOutputChars = 1024 * 1024; +const defaultEvaluatorMaxOutputBytes = 1024 * 1024; class EvaluatorProcessResult { const EvaluatorProcessResult({ @@ -38,7 +38,7 @@ Future runEvaluatorProcess( required Map environment, bool includeParentEnvironment = false, Duration? timeout = defaultEvaluatorProcessTimeout, - int maxOutputChars = defaultEvaluatorMaxOutputChars, + int maxOutputBytes = defaultEvaluatorMaxOutputBytes, int? maxCpuCores, int? maxProcesses, int? maxMemoryMb, @@ -74,7 +74,7 @@ Future runEvaluatorProcess( ); BoundedSubprocessMonitor resourceMonitor(int pid) { final limitExceeded = Completer(); - Object? observedLimit; + final observedLimits = _EvaluatorObservedLimits(); var resourceCheckRunning = false; final timer = Timer.periodic(const Duration(milliseconds: 100), (_) async { if (resourceCheckRunning) return; @@ -86,13 +86,7 @@ Future runEvaluatorProcess( ); final processCount = 1 + descendants.length; if (maxProcesses != null && processCount > maxProcesses) { - observedLimit = _EvaluatorProcessLimitExceeded( - processCount: processCount, - ); - if (!limitExceeded.isCompleted) { - limitExceeded.complete(observedLimit!); - } - return; + observedLimits.processCount = processCount; } if (maxMemoryMb != null) { final memoryMb = await _processTreeMemoryMb( @@ -101,19 +95,19 @@ Future runEvaluatorProcess( environment: helperEnvironment, ); if (memoryMb != null && memoryMb > maxMemoryMb) { - observedLimit = _EvaluatorMemoryLimitExceeded(memoryMb: memoryMb); - if (!limitExceeded.isCompleted) { - limitExceeded.complete(observedLimit!); - } + observedLimits.memoryMb = memoryMb; } } + if (observedLimits.exceeded && !limitExceeded.isCompleted) { + limitExceeded.complete(observedLimits); + } } finally { resourceCheckRunning = false; } }); return BoundedSubprocessMonitor( signal: limitExceeded.future, - observed: () => observedLimit, + observed: () => observedLimits.exceeded ? observedLimits : null, dispose: timer.cancel, ); } @@ -127,7 +121,7 @@ Future runEvaluatorProcess( ? includeParentEnvironment : false, timeout: timeout, - maxOutputBytes: maxOutputChars, + maxOutputBytes: maxOutputBytes, helperEnvironment: helperEnvironment, monitor: maxProcesses != null || maxMemoryMb != null ? resourceMonitor @@ -145,12 +139,16 @@ Future runEvaluatorProcess( result.outputLimitExceeded || (result.termination == BoundedSubprocessTermination.exited && (result.stdoutLimitExceeded || result.stderrLimitExceeded)), - processLimitExceeded: externalLimit is _EvaluatorProcessLimitExceeded, - memoryLimitExceeded: externalLimit is _EvaluatorMemoryLimitExceeded, - observedProcessCount: externalLimit is _EvaluatorProcessLimitExceeded + processLimitExceeded: + externalLimit is _EvaluatorObservedLimits && + externalLimit.processCount != null, + memoryLimitExceeded: + externalLimit is _EvaluatorObservedLimits && + externalLimit.memoryMb != null, + observedProcessCount: externalLimit is _EvaluatorObservedLimits ? externalLimit.processCount : null, - observedMemoryMb: externalLimit is _EvaluatorMemoryLimitExceeded + observedMemoryMb: externalLimit is _EvaluatorObservedLimits ? externalLimit.memoryMb : null, ); @@ -236,14 +234,9 @@ List _parsePids(String output) => output .whereType() .toList(growable: false); -class _EvaluatorProcessLimitExceeded { - const _EvaluatorProcessLimitExceeded({required this.processCount}); - - final int processCount; -} - -class _EvaluatorMemoryLimitExceeded { - const _EvaluatorMemoryLimitExceeded({required this.memoryMb}); +class _EvaluatorObservedLimits { + int? processCount; + int? memoryMb; - final int memoryMb; + bool get exceeded => processCount != null || memoryMb != null; } diff --git a/app/lib/evaluators/hidden_test_evaluator.dart b/app/lib/evaluators/hidden_test_evaluator.dart index bce80da..1fa45d7 100644 --- a/app/lib/evaluators/hidden_test_evaluator.dart +++ b/app/lib/evaluators/hidden_test_evaluator.dart @@ -12,14 +12,14 @@ class HiddenTestEvaluator implements Evaluator { HiddenTestEvaluator( this.verifier, { this.timeout, - this.maxOutputChars = TestEvaluator.defaultMaxOutputChars, + this.maxOutputBytes = TestEvaluator.defaultMaxOutputBytes, this.maxProcesses, this.maxMemoryMb, }); final VerifierFixture verifier; final Duration? timeout; - final int maxOutputChars; + final int maxOutputBytes; final int? maxProcesses; final int? maxMemoryMb; @@ -63,7 +63,7 @@ class HiddenTestEvaluator implements Evaluator { final result = await TestEvaluator( testPath: stagedTestPath, timeout: timeout, - maxOutputChars: maxOutputChars, + maxOutputBytes: maxOutputBytes, maxProcesses: maxProcesses, maxMemoryMb: maxMemoryMb, readOnlyPaths: [stagingRoot.path], @@ -129,8 +129,8 @@ class HiddenTestEvaluator implements Evaluator { if (result.details['timeout_ms'] != null) 'timeout_ms': result.details['timeout_ms'], if (outputLimitExceeded) 'output_limit_exceeded': true, - if (outputLimitExceeded && result.details['max_output_chars'] != null) - 'max_output_chars': result.details['max_output_chars'], + if (outputLimitExceeded && result.details['max_output_bytes'] != null) + 'max_output_bytes': result.details['max_output_bytes'], if (processLimitExceeded) 'process_limit_exceeded': true, if (processLimitExceeded && result.details['max_processes'] != null) 'max_processes': result.details['max_processes'], diff --git a/app/lib/evaluators/test_author_evaluator.dart b/app/lib/evaluators/test_author_evaluator.dart index b36957f..fb09f27 100644 --- a/app/lib/evaluators/test_author_evaluator.dart +++ b/app/lib/evaluators/test_author_evaluator.dart @@ -28,7 +28,7 @@ class TestAuthorEvaluator implements Evaluator { required this.testPath, required this.mutants, this.timeout = defaultEvaluatorProcessTimeout, - this.maxOutputChars = defaultEvaluatorMaxOutputChars, + this.maxOutputBytes = defaultEvaluatorMaxOutputBytes, this.maxProcesses, this.maxMemoryMb, this.dartExecutable = 'dart', @@ -38,7 +38,7 @@ class TestAuthorEvaluator implements Evaluator { final String testPath; final List mutants; final Duration? timeout; - final int maxOutputChars; + final int maxOutputBytes; final int? maxProcesses; final int? maxMemoryMb; final String dartExecutable; @@ -96,7 +96,7 @@ class TestAuthorEvaluator implements Evaluator { 'timeout_ms': timeout!.inMilliseconds, if (originalRun.outputLimitExceeded) 'output_limit_exceeded': true, if (originalRun.outputLimitExceeded) - 'max_output_chars': maxOutputChars, + 'max_output_bytes': maxOutputBytes, if (originalRun.processLimitExceeded) 'process_limit_exceeded': true, if (originalRun.processLimitExceeded && maxProcesses != null) 'max_processes': maxProcesses, @@ -197,7 +197,7 @@ class TestAuthorEvaluator implements Evaluator { ), includeParentEnvironment: false, timeout: timeout, - maxOutputChars: maxOutputChars, + maxOutputBytes: maxOutputBytes, maxCpuCores: maxCpuCores, maxProcesses: maxProcesses, maxMemoryMb: maxMemoryMb, diff --git a/app/lib/evaluators/test_evaluator.dart b/app/lib/evaluators/test_evaluator.dart index 9147115..52b244c 100644 --- a/app/lib/evaluators/test_evaluator.dart +++ b/app/lib/evaluators/test_evaluator.dart @@ -9,7 +9,7 @@ class TestEvaluator implements Evaluator { TestEvaluator({ this.testPath, this.timeout, - this.maxOutputChars = defaultMaxOutputChars, + this.maxOutputBytes = defaultMaxOutputBytes, this.maxProcesses, this.maxMemoryMb, this.dartExecutable = 'dart', @@ -17,13 +17,13 @@ class TestEvaluator implements Evaluator { this.readOnlyPaths = const [], }); - static const defaultMaxOutputChars = defaultEvaluatorMaxOutputChars; + static const defaultMaxOutputBytes = defaultEvaluatorMaxOutputBytes; /// If provided, only this path is passed to ` test`. When null, the /// runner runs the default test set (the entire `test/` directory). final String? testPath; final Duration? timeout; - final int maxOutputChars; + final int maxOutputBytes; final int? maxProcesses; final int? maxMemoryMb; final String dartExecutable; @@ -53,7 +53,7 @@ class TestEvaluator implements Evaluator { ), includeParentEnvironment: false, timeout: timeout, - maxOutputChars: maxOutputChars, + maxOutputBytes: maxOutputBytes, maxCpuCores: ctx.task.effectiveResourceLimits.cpus, maxProcesses: maxProcesses, maxMemoryMb: maxMemoryMb, @@ -101,7 +101,7 @@ class TestEvaluator implements Evaluator { if (res.timedOut && timeout != null) 'timeout_ms': timeout!.inMilliseconds, if (res.outputLimitExceeded) 'output_limit_exceeded': true, - if (res.outputLimitExceeded) 'max_output_chars': maxOutputChars, + if (res.outputLimitExceeded) 'max_output_bytes': maxOutputBytes, if (res.processLimitExceeded) 'process_limit_exceeded': true, if (res.processLimitExceeded && maxProcesses != null) 'max_processes': maxProcesses, diff --git a/app/lib/evaluators/widget_tree_evaluator.dart b/app/lib/evaluators/widget_tree_evaluator.dart index 956904d..2979dda 100644 --- a/app/lib/evaluators/widget_tree_evaluator.dart +++ b/app/lib/evaluators/widget_tree_evaluator.dart @@ -9,7 +9,7 @@ class WidgetTreeEvaluator implements Evaluator { WidgetTreeEvaluator({ this.testDir = 'test/widget', this.timeout = defaultEvaluatorProcessTimeout, - this.maxOutputChars = defaultEvaluatorMaxOutputChars, + this.maxOutputBytes = defaultEvaluatorMaxOutputBytes, this.maxProcesses, this.maxMemoryMb, this.flutterExecutable = 'flutter', @@ -17,7 +17,7 @@ class WidgetTreeEvaluator implements Evaluator { final String testDir; final Duration? timeout; - final int maxOutputChars; + final int maxOutputBytes; final int? maxProcesses; final int? maxMemoryMb; final String flutterExecutable; @@ -39,7 +39,7 @@ class WidgetTreeEvaluator implements Evaluator { ), includeParentEnvironment: false, timeout: timeout, - maxOutputChars: maxOutputChars, + maxOutputBytes: maxOutputBytes, maxCpuCores: ctx.task.effectiveResourceLimits.cpus, maxProcesses: maxProcesses, maxMemoryMb: maxMemoryMb, @@ -87,7 +87,7 @@ class WidgetTreeEvaluator implements Evaluator { if (res.timedOut && timeout != null) 'timeout_ms': timeout!.inMilliseconds, if (res.outputLimitExceeded) 'output_limit_exceeded': true, - if (res.outputLimitExceeded) 'max_output_chars': maxOutputChars, + if (res.outputLimitExceeded) 'max_output_bytes': maxOutputBytes, if (res.processLimitExceeded) 'process_limit_exceeded': true, if (res.processLimitExceeded && maxProcesses != null) 'max_processes': maxProcesses, diff --git a/app/lib/providers/droid_exec_provider.dart b/app/lib/providers/droid_exec_provider.dart index 22298c6..49b7d95 100644 --- a/app/lib/providers/droid_exec_provider.dart +++ b/app/lib/providers/droid_exec_provider.dart @@ -71,7 +71,8 @@ class DroidExecProvider implements ModelProvider, ModelRuntimeMetadataProvider { additionalDeniedKeys: deniedEnvironmentKeys, allowedSensitiveKeys: allowedSensitiveEnvironmentKeys, ), - maxOutputBytes: maxProcessOutputChars, + maxOutputBytes: maxEncodedOutputBytes(maxProcessOutputChars), + maxOutputCharacters: maxProcessOutputChars, timeout: timeout, capture: BoundedSubprocessCapture.trailing, ); diff --git a/app/lib/runner/agentic_run_orchestrator.dart b/app/lib/runner/agentic_run_orchestrator.dart index 96ccbf8..43e27fe 100644 --- a/app/lib/runner/agentic_run_orchestrator.dart +++ b/app/lib/runner/agentic_run_orchestrator.dart @@ -724,7 +724,7 @@ class AgenticRunOrchestrator { 'metadata', 'metadata_redacted_count', 'output_limit_exceeded', - 'max_output_chars', + 'max_output_bytes', 'stepcount', 'peakcontext', 'peakcontexttokens', diff --git a/app/lib/runner/bounded_subprocess.dart b/app/lib/runner/bounded_subprocess.dart index c9b7164..e46508f 100644 --- a/app/lib/runner/bounded_subprocess.dart +++ b/app/lib/runner/bounded_subprocess.dart @@ -4,6 +4,7 @@ import 'dart:io'; import 'dart:typed_data'; import 'package:dart_arena/runner/subprocess_environment.dart'; +import 'package:path/path.dart' as p; enum BoundedSubprocessTermination { exited, @@ -52,6 +53,10 @@ class BoundedSubprocessMonitor { typedef BoundedSubprocessMonitorFactory = BoundedSubprocessMonitor Function(int pid); +typedef BoundedSubprocessSignalSender = + bool Function(int pid, ProcessSignal? signal); + +int maxEncodedOutputBytes(int maxCharacters) => maxCharacters * 4; Future runBoundedSubprocess({ required String executable, @@ -59,6 +64,7 @@ Future runBoundedSubprocess({ required String workingDirectory, required Map environment, required int maxOutputBytes, + int? maxOutputCharacters, bool includeParentEnvironment = false, Duration? timeout, Future? cancellationSignal, @@ -66,20 +72,42 @@ Future runBoundedSubprocess({ List? stdinBytes, Map? helperEnvironment, BoundedSubprocessMonitorFactory? monitor, + BoundedSubprocessSignalSender signalSender = _tryKillPid, }) async { assert(maxOutputBytes > 0); - final startsProcessGroup = - Platform.isLinux && File('/usr/bin/setsid').existsSync(); + assert(maxOutputCharacters == null || maxOutputCharacters > 0); + final setsid = Platform.isLinux ? _installedSystemTool('setsid') : null; + final startsProcessGroup = setsid != null; + final targetExecutable = startsProcessGroup + ? _resolveExecutable( + executable, + arguments, + workingDirectory: workingDirectory, + environment: environment, + includeParentEnvironment: includeParentEnvironment, + ) + : executable; final process = await Process.start( - startsProcessGroup ? '/usr/bin/setsid' : executable, - startsProcessGroup ? ['--wait', executable, ...arguments] : arguments, + setsid ?? targetExecutable, + startsProcessGroup + ? ['--wait', '--', targetExecutable, ...arguments] + : arguments, workingDirectory: workingDirectory, runInShell: false, environment: environment, includeParentEnvironment: includeParentEnvironment, ); - final stdout = _BoundedByteCollector(maxOutputBytes, capture); - final stderr = _BoundedByteCollector(maxOutputBytes, capture); + final processState = _ProcessState(process); + final stdout = _BoundedByteCollector( + maxOutputBytes, + capture, + maxCharacters: maxOutputCharacters, + ); + final stderr = _BoundedByteCollector( + maxOutputBytes, + capture, + maxCharacters: maxOutputCharacters, + ); final outputLimitExceeded = Completer<_SubprocessSignal>(); void collect(_BoundedByteCollector collector, List chunk) { @@ -95,7 +123,12 @@ Future runBoundedSubprocess({ final stderrSubscription = process.stderr.listen( (chunk) => collect(stderr, chunk), ); - final streams = _ProcessStreams(stdoutSubscription, stderrSubscription); + final streams = _ProcessStreams( + stdoutSubscription, + stderrSubscription, + closeStdout: stdout.close, + closeStderr: stderr.close, + ); final stdinDone = _writeStdin(process, stdinBytes); final timeoutExceeded = Completer<_SubprocessSignal>(); final timeoutAlreadyExceeded = @@ -114,11 +147,12 @@ Future runBoundedSubprocess({ } on Object { timeoutTimer?.cancel(); await _stopAndDrain( - process, + processState, streams, stdinDone, helperEnvironment: helperEnvironment, processGroup: startsProcessGroup, + signalSender: signalSender, ); rethrow; } @@ -128,7 +162,7 @@ Future runBoundedSubprocess({ signal = timeoutAlreadyExceeded ? const _TimedOut() : await Future.any<_SubprocessSignal>([ - process.exitCode.then(_Exited.new), + processState.exitCode.then(_Exited.new), if (timeout != null) timeoutExceeded.future, if (cancellationSignal != null) cancellationSignal.then((_) => const _Cancelled()), @@ -144,19 +178,21 @@ Future runBoundedSubprocess({ final exitCode = signal is _Exited ? signal.exitCode : await _stopAndDrain( - process, + processState, streams, stdinDone, helperEnvironment: helperEnvironment, processGroup: startsProcessGroup, + signalSender: signalSender, ); if (signal is _Exited) { await _drainAfterExit( - process, + processState, streams, stdinDone, helperEnvironment: helperEnvironment, processGroup: startsProcessGroup, + signalSender: signalSender, ); } @@ -194,59 +230,75 @@ Future _writeStdin(Process process, List? bytes) async { } Future _drainAfterExit( - Process process, + _ProcessState process, _ProcessStreams streams, Future stdinDone, { Map? helperEnvironment, required bool processGroup, + required BoundedSubprocessSignalSender signalSender, }) async { if (streams.isDone) { await stdinDone; return; } + final retainedDescendants = {}; await _terminateProcessTree( process.pid, ProcessSignal.sigterm, + retainedDescendants: retainedDescendants, + rootExited: () => true, environment: helperEnvironment, processGroup: processGroup, + signalSender: signalSender, ); await _awaitDrainWithEscalation( process, streams, stdinDone, + retainedDescendants: retainedDescendants, helperEnvironment: helperEnvironment, processGroup: processGroup, + signalSender: signalSender, ); } Future _stopAndDrain( - Process process, + _ProcessState process, _ProcessStreams streams, Future stdinDone, { Map? helperEnvironment, required bool processGroup, + required BoundedSubprocessSignalSender signalSender, }) async { + final retainedDescendants = {}; await _terminateProcessTree( process.pid, ProcessSignal.sigterm, + retainedDescendants: retainedDescendants, + rootExited: () => process.exited, environment: helperEnvironment, processGroup: processGroup, + signalSender: signalSender, ); return _awaitDrainWithEscalation( process, streams, stdinDone, + retainedDescendants: retainedDescendants, helperEnvironment: helperEnvironment, processGroup: processGroup, + signalSender: signalSender, ); } Future _awaitDrainWithEscalation( - Process process, + _ProcessState process, _ProcessStreams streams, Future stdinDone, { + required Set retainedDescendants, Map? helperEnvironment, required bool processGroup, + required BoundedSubprocessSignalSender signalSender, }) async { final completed = await _waitForExitAndDrain(process, streams, stdinDone) .then((value) => value) @@ -256,8 +308,11 @@ Future _awaitDrainWithEscalation( await _terminateProcessTree( process.pid, ProcessSignal.sigkill, + retainedDescendants: retainedDescendants, + rootExited: () => process.exited, environment: helperEnvironment, processGroup: processGroup, + signalSender: signalSender, ); final killed = await _waitForExitAndDrain(process, streams, stdinDone) .then((value) => value) @@ -272,7 +327,7 @@ Future _awaitDrainWithEscalation( } Future _waitForExitAndDrain( - Process process, + _ProcessState process, _ProcessStreams streams, Future stdinDone, ) async { @@ -284,29 +339,36 @@ Future _waitForExitAndDrain( Future _terminateProcessTree( int pid, ProcessSignal signal, { + required Set retainedDescendants, + required bool Function() rootExited, Map? environment, required bool processGroup, + required BoundedSubprocessSignalSender signalSender, }) async { if (Platform.isWindows) { + if (rootExited()) return; final args = ['/PID', '$pid', '/T']; if (signal == ProcessSignal.sigkill) args.add('/F'); await _tryRunProcess('taskkill', args, environment: environment); return; } - if (processGroup) { - _tryKillPid(-pid, signal); - _killPid(pid, signal); + final descendantRoots = {...retainedDescendants}; + if (!rootExited()) descendantRoots.add(pid); + if (descendantRoots.isNotEmpty) { + retainedDescendants.addAll( + await _descendantPids(descendantRoots, environment: environment), + ); } - final descendants = await _descendantPids(pid, environment: environment); - for (final childPid in descendants.reversed) { - _killPid(childPid, signal); + if (processGroup) signalSender(-pid, signal); + for (final childPid in retainedDescendants.toList().reversed) { + _killPid(childPid, signal, signalSender); } - if (!processGroup) _killPid(pid, signal); + if (!rootExited()) _killPid(pid, signal, signalSender); } Future> _descendantPids( - int pid, { + Set roots, { Map? environment, }) async { final ps = await _tryRunProcess(_systemTool('ps'), const [ @@ -321,8 +383,8 @@ Future> _descendantPids( childrenByParent.putIfAbsent(values[1], () => []).add(values[0]); } final descendants = []; - final pending = [pid]; - final seen = {pid}; + final pending = roots.toList(); + final seen = {...roots}; while (pending.isNotEmpty) { final parent = pending.removeLast(); for (final child in childrenByParent[parent] ?? const []) { @@ -334,12 +396,62 @@ Future> _descendantPids( return descendants; } -String _systemTool(String name) { +String _systemTool(String name) => _installedSystemTool(name) ?? name; + +String? _installedSystemTool(String name) { for (final root in const ['/usr/bin', '/bin']) { final path = '$root/$name'; if (File(path).existsSync()) return path; } - return name; + return null; +} + +String _resolveExecutable( + String executable, + List arguments, { + required String workingDirectory, + required Map environment, + required bool includeParentEnvironment, +}) { + final candidates = []; + if (p.isAbsolute(executable) || executable.contains('/')) { + candidates.add( + p.isAbsolute(executable) + ? executable + : p.join(workingDirectory, executable), + ); + } else { + final path = + environment['PATH'] ?? + (includeParentEnvironment ? Platform.environment['PATH'] : null) ?? + Platform.environment['PATH'] ?? + '/usr/bin:/bin'; + for (final root in path.split(':')) { + final searchRoot = root.isEmpty || !p.isAbsolute(root) + ? p.join(workingDirectory, root) + : root; + candidates.add(p.join(searchRoot, executable)); + } + } + + var foundNonExecutable = false; + for (final candidate in candidates) { + if (FileSystemEntity.typeSync(candidate, followLinks: true) != + FileSystemEntityType.file) { + continue; + } + if (File(candidate).statSync().mode & 0x49 == 0) { + foundNonExecutable = true; + continue; + } + return p.normalize(p.absolute(candidate)); + } + throw ProcessException( + executable, + arguments, + foundNonExecutable ? 'Permission denied' : 'No such file or directory', + foundNonExecutable ? 13 : 2, + ); } Future _tryRunProcess( @@ -366,8 +478,12 @@ List _parsePids(String output) => output .whereType() .toList(growable: false); -void _killPid(int pid, ProcessSignal signal) { - if (!_tryKillPid(pid, signal)) _tryKillPid(pid, null); +void _killPid( + int pid, + ProcessSignal signal, + BoundedSubprocessSignalSender signalSender, +) { + if (!signalSender(pid, signal)) signalSender(pid, null); } bool _tryKillPid(int pid, ProcessSignal? signal) { @@ -378,42 +494,73 @@ bool _tryKillPid(int pid, ProcessSignal? signal) { } } +class _ProcessState { + _ProcessState(this.process) { + exitCode = process.exitCode.whenComplete(() => exited = true); + } + + final Process process; + late final Future exitCode; + bool exited = false; + + int get pid => process.pid; +} + class _ProcessStreams { - _ProcessStreams(this._stdout, this._stderr) { + _ProcessStreams( + this._stdout, + this._stderr, { + required void Function() closeStdout, + required void Function() closeStderr, + }) : _closeStdout = closeStdout, + _closeStderr = closeStderr { done = Future.wait([ - _stdout.asFuture(), - _stderr.asFuture(), + _stdout.asFuture().whenComplete(_closeStdout), + _stderr.asFuture().whenComplete(_closeStderr), ]).whenComplete(() => isDone = true); } final StreamSubscription> _stdout; final StreamSubscription> _stderr; + final void Function() _closeStdout; + final void Function() _closeStderr; late final Future done; bool isDone = false; - Future cancel() => - Future.wait([_stdout.cancel(), _stderr.cancel()]); + Future cancel() async { + await Future.wait([_stdout.cancel(), _stderr.cancel()]); + _closeStdout(); + _closeStderr(); + } } class _BoundedByteCollector { - _BoundedByteCollector(this.maxBytes, this.capture) - : _tail = capture == BoundedSubprocessCapture.trailing + _BoundedByteCollector(this.maxBytes, this.capture, {int? maxCharacters}) + : _maxCharacters = maxCharacters, + _characterLimit = maxCharacters == null + ? null + : _CharacterLimitTracker(maxCharacters), + _tail = capture == BoundedSubprocessCapture.trailing ? Uint8List(maxBytes) : null; final int maxBytes; final BoundedSubprocessCapture capture; + final int? _maxCharacters; + final _CharacterLimitTracker? _characterLimit; final BytesBuilder _head = BytesBuilder(copy: false); final Uint8List? _tail; var _tailStart = 0; var _tailLength = 0; var _seenBytes = 0; - bool get exceeded => _seenBytes > maxBytes; + bool get exceeded => + _seenBytes > maxBytes || (_characterLimit?.exceeded ?? false); void add(List chunk) { if (chunk.isEmpty) return; _seenBytes += chunk.length; + _characterLimit?.add(chunk); if (capture == BoundedSubprocessCapture.leading) { final remaining = maxBytes - _head.length; if (remaining > 0) { @@ -445,7 +592,18 @@ class _BoundedByteCollector { } } - String get text => _decode(bytes); + void close() => _characterLimit?.close(); + + String get text { + final decoded = _decode(bytes); + final maxCharacters = _maxCharacters; + if (maxCharacters == null || decoded.length <= maxCharacters) { + return decoded; + } + return capture == BoundedSubprocessCapture.leading + ? decoded.substring(0, maxCharacters) + : decoded.substring(decoded.length - maxCharacters); + } List get bytes { if (capture == BoundedSubprocessCapture.leading) return _head.toBytes(); @@ -472,6 +630,64 @@ class _BoundedByteCollector { } } +class _CharacterLimitTracker { + _CharacterLimitTracker(int maxCharacters) + : _counter = _CharacterCountingSink(maxCharacters) { + final decoder = Platform.isWindows + ? systemEncoding.decoder + : const Utf8Decoder(allowMalformed: true); + _sink = decoder.startChunkedConversion( + StringConversionSink.fromStringSink(_counter), + ); + } + + final _CharacterCountingSink _counter; + late final Sink> _sink; + bool _closed = false; + + bool get exceeded => _counter.exceeded; + + void add(List bytes) { + if (!_closed) _sink.add(bytes); + } + + void close() { + if (_closed) return; + _closed = true; + _sink.close(); + } +} + +class _CharacterCountingSink implements StringSink { + _CharacterCountingSink(this.maxCharacters); + + final int maxCharacters; + var _length = 0; + + bool get exceeded => _length > maxCharacters; + + @override + void write(Object? object) { + _length += object.toString().length; + } + + @override + void writeAll(Iterable objects, [String separator = '']) { + write(objects.join(separator)); + } + + @override + void writeCharCode(int charCode) { + _length += String.fromCharCode(charCode).length; + } + + @override + void writeln([Object? object = '']) { + write(object); + _length++; + } +} + sealed class _SubprocessSignal { const _SubprocessSignal(); } diff --git a/app/lib/runner/evaluator_resource_limits.dart b/app/lib/runner/evaluator_resource_limits.dart index f211396..16f7fe4 100644 --- a/app/lib/runner/evaluator_resource_limits.dart +++ b/app/lib/runner/evaluator_resource_limits.dart @@ -23,10 +23,12 @@ Evaluator applyResourceLimitsToEvaluator( Evaluator evaluator, TaskResourceLimits limits, ) { - final maxOutputBytes = limits.maxOutputBytes; + final taskMaxOutputBytes = limits.maxOutputBytes; final maxProcesses = limits.maxProcesses; final maxMemoryMb = limits.memoryMb; - if (maxOutputBytes == null && maxProcesses == null && maxMemoryMb == null) { + if (taskMaxOutputBytes == null && + maxProcesses == null && + maxMemoryMb == null) { return evaluator; } @@ -34,7 +36,7 @@ Evaluator applyResourceLimitsToEvaluator( TestEvaluator( testPath: final testPath, timeout: final timeout, - maxOutputChars: final maxOutputChars, + maxOutputBytes: final evaluatorMaxOutputBytes, maxProcesses: final evaluatorMaxProcesses, maxMemoryMb: final evaluatorMaxMemoryMb, dartExecutable: final dartExecutable, @@ -44,9 +46,9 @@ Evaluator applyResourceLimitsToEvaluator( TestEvaluator( testPath: testPath, timeout: timeout, - maxOutputChars: _effectiveMaxOutputChars( - maxOutputChars, - maxOutputBytes, + maxOutputBytes: _effectiveMaxOutputBytes( + evaluatorMaxOutputBytes, + taskMaxOutputBytes, ), maxProcesses: _effectiveMaxProcesses( evaluatorMaxProcesses, @@ -60,16 +62,16 @@ Evaluator applyResourceLimitsToEvaluator( HiddenTestEvaluator( verifier: final verifier, timeout: final timeout, - maxOutputChars: final maxOutputChars, + maxOutputBytes: final evaluatorMaxOutputBytes, maxProcesses: final evaluatorMaxProcesses, maxMemoryMb: final evaluatorMaxMemoryMb, ) => HiddenTestEvaluator( verifier, timeout: timeout, - maxOutputChars: _effectiveMaxOutputChars( - maxOutputChars, - maxOutputBytes, + maxOutputBytes: _effectiveMaxOutputBytes( + evaluatorMaxOutputBytes, + taskMaxOutputBytes, ), maxProcesses: _effectiveMaxProcesses( evaluatorMaxProcesses, @@ -79,7 +81,7 @@ Evaluator applyResourceLimitsToEvaluator( ), AnalyzeEvaluator( timeout: final timeout, - maxOutputChars: final maxOutputChars, + maxOutputBytes: final evaluatorMaxOutputBytes, maxProcesses: final evaluatorMaxProcesses, maxMemoryMb: final evaluatorMaxMemoryMb, dartExecutable: final dartExecutable, @@ -87,9 +89,9 @@ Evaluator applyResourceLimitsToEvaluator( ) => AnalyzeEvaluator( timeout: timeout, - maxOutputChars: _effectiveMaxOutputChars( - maxOutputChars, - maxOutputBytes, + maxOutputBytes: _effectiveMaxOutputBytes( + evaluatorMaxOutputBytes, + taskMaxOutputBytes, ), maxProcesses: _effectiveMaxProcesses( evaluatorMaxProcesses, @@ -101,7 +103,7 @@ Evaluator applyResourceLimitsToEvaluator( ), CompileEvaluator( timeout: final timeout, - maxOutputChars: final maxOutputChars, + maxOutputBytes: final evaluatorMaxOutputBytes, maxProcesses: final evaluatorMaxProcesses, maxMemoryMb: final evaluatorMaxMemoryMb, dartExecutable: final dartExecutable, @@ -109,9 +111,9 @@ Evaluator applyResourceLimitsToEvaluator( ) => CompileEvaluator( timeout: timeout, - maxOutputChars: _effectiveMaxOutputChars( - maxOutputChars, - maxOutputBytes, + maxOutputBytes: _effectiveMaxOutputBytes( + evaluatorMaxOutputBytes, + taskMaxOutputBytes, ), maxProcesses: _effectiveMaxProcesses( evaluatorMaxProcesses, @@ -124,7 +126,7 @@ Evaluator applyResourceLimitsToEvaluator( WidgetTreeEvaluator( testDir: final testDir, timeout: final timeout, - maxOutputChars: final maxOutputChars, + maxOutputBytes: final evaluatorMaxOutputBytes, maxProcesses: final evaluatorMaxProcesses, maxMemoryMb: final evaluatorMaxMemoryMb, flutterExecutable: final flutterExecutable, @@ -132,9 +134,9 @@ Evaluator applyResourceLimitsToEvaluator( WidgetTreeEvaluator( testDir: testDir, timeout: timeout, - maxOutputChars: _effectiveMaxOutputChars( - maxOutputChars, - maxOutputBytes, + maxOutputBytes: _effectiveMaxOutputBytes( + evaluatorMaxOutputBytes, + taskMaxOutputBytes, ), maxProcesses: _effectiveMaxProcesses( evaluatorMaxProcesses, @@ -147,7 +149,7 @@ Evaluator applyResourceLimitsToEvaluator( testPath: final testPath, mutants: final mutants, timeout: final timeout, - maxOutputChars: final maxOutputChars, + maxOutputBytes: final evaluatorMaxOutputBytes, maxProcesses: final evaluatorMaxProcesses, maxMemoryMb: final evaluatorMaxMemoryMb, dartExecutable: final dartExecutable, @@ -157,9 +159,9 @@ Evaluator applyResourceLimitsToEvaluator( testPath: testPath, mutants: mutants, timeout: timeout, - maxOutputChars: _effectiveMaxOutputChars( - maxOutputChars, - maxOutputBytes, + maxOutputBytes: _effectiveMaxOutputBytes( + evaluatorMaxOutputBytes, + taskMaxOutputBytes, ), maxProcesses: _effectiveMaxProcesses( evaluatorMaxProcesses, @@ -173,9 +175,9 @@ Evaluator applyResourceLimitsToEvaluator( }; } -int _effectiveMaxOutputChars(int evaluatorMax, int? taskMax) { +int _effectiveMaxOutputBytes(int evaluatorMax, int? taskMax) { if (taskMax == null) return evaluatorMax; - if (evaluatorMax == TestEvaluator.defaultMaxOutputChars) return taskMax; + if (evaluatorMax == TestEvaluator.defaultMaxOutputBytes) return taskMax; return math.min(evaluatorMax, taskMax); } diff --git a/app/lib/runner/task_qa_runner.dart b/app/lib/runner/task_qa_runner.dart index b3875b8..401af76 100644 --- a/app/lib/runner/task_qa_runner.dart +++ b/app/lib/runner/task_qa_runner.dart @@ -842,7 +842,7 @@ class TaskQaRunner { final qaEvaluator = switch (evaluator) { TestEvaluator( :final testPath, - :final maxOutputChars, + :final maxOutputBytes, :final maxProcesses, :final maxMemoryMb, :final dartExecutable, @@ -851,7 +851,7 @@ class TaskQaRunner { TestEvaluator( testPath: testPath, timeout: evaluatorTimeout, - maxOutputChars: maxOutputChars, + maxOutputBytes: maxOutputBytes, maxProcesses: maxProcesses, maxMemoryMb: maxMemoryMb, dartExecutable: dartExecutable, @@ -859,14 +859,14 @@ class TaskQaRunner { ), HiddenTestEvaluator( :final verifier, - :final maxOutputChars, + :final maxOutputBytes, :final maxProcesses, :final maxMemoryMb, ) => HiddenTestEvaluator( verifier, timeout: evaluatorTimeout, - maxOutputChars: maxOutputChars, + maxOutputBytes: maxOutputBytes, maxProcesses: maxProcesses, maxMemoryMb: maxMemoryMb, ), diff --git a/app/lib/runner/workdir_manager.dart b/app/lib/runner/workdir_manager.dart index 42a05cf..eea5d95 100644 --- a/app/lib/runner/workdir_manager.dart +++ b/app/lib/runner/workdir_manager.dart @@ -428,7 +428,8 @@ class WorkdirManager { arguments: processStart.arguments, workingDirectory: processStart.workingDirectory, environment: processStart.environment, - maxOutputBytes: prepareMaxOutputChars, + maxOutputBytes: maxEncodedOutputBytes(prepareMaxOutputChars), + maxOutputCharacters: prepareMaxOutputChars, timeout: timeout, cancellationSignal: cancellationSignal, helperEnvironment: benchmarkSubprocessEnvironment( @@ -650,7 +651,8 @@ class WorkdirManager { arguments: args, workingDirectory: dir.path, environment: environment, - maxOutputBytes: gitMaxOutputChars, + maxOutputBytes: maxEncodedOutputBytes(gitMaxOutputChars), + maxOutputCharacters: gitMaxOutputChars, timeout: gitTimeout, stdinBytes: stdin == null ? null : utf8.encode(stdin), helperEnvironment: environment, diff --git a/app/test/evaluators/compile_evaluator_test.dart b/app/test/evaluators/compile_evaluator_test.dart index f3195aa..8008a22 100644 --- a/app/test/evaluators/compile_evaluator_test.dart +++ b/app/test/evaluators/compile_evaluator_test.dart @@ -104,14 +104,14 @@ environment: final result = await CompileEvaluator( dartExecutable: fakeDart.path, timeout: const Duration(seconds: 5), - maxOutputChars: 32, + maxOutputBytes: 32, ).evaluate(_ctx(dir)); expect(result.passed, isFalse); expect(result.score, 0.0); expect(result.rationale, 'analysis output limit exceeded'); expect(result.details['output_limit_exceeded'], isTrue); - expect(result.details['max_output_chars'], 32); + expect(result.details['max_output_bytes'], 32); expect(result.details['exitCode'], -1); }, skip: Platform.isWindows ? 'POSIX shell script test' : false, diff --git a/app/test/evaluators/evaluator_process_test.dart b/app/test/evaluators/evaluator_process_test.dart index 0959e78..cbee30d 100644 --- a/app/test/evaluators/evaluator_process_test.dart +++ b/app/test/evaluators/evaluator_process_test.dart @@ -5,6 +5,37 @@ import 'package:test/test.dart'; import 'package:path/path.dart' as p; void main() { + test( + 'missing executable preserves infrastructure spawn failure', + () async { + final tmp = await Directory.systemTemp.createTemp( + 'dart_arena_eval_missing_', + ); + addTearDown(() async { + if (await tmp.exists()) await tmp.delete(recursive: true); + }); + final executable = p.join(tmp.path, 'missing-evaluator'); + + await expectLater( + runEvaluatorProcess( + executable, + const [], + workingDirectory: tmp.path, + environment: {'PATH': '/usr/bin:/bin'}, + timeout: const Duration(seconds: 2), + ), + throwsA( + isA().having( + (error) => error.executable, + 'executable', + executable, + ), + ), + ); + }, + skip: Platform.isWindows, + ); + test( 'output limit counts raw bytes before decode, not decoded chars', () async { @@ -15,8 +46,8 @@ void main() { if (await tmp.exists()) await tmp.delete(recursive: true); }); - // 600 snowman characters are 600 decoded chars but 1800 UTF-8 bytes: - // under a 1024-char limit, over the 1024-byte contract. + // 600 snowman characters are 1800 UTF-8 bytes, over the + // 1024-byte contract. final result = await runEvaluatorProcess( 'sh', const [ @@ -26,7 +57,7 @@ void main() { workingDirectory: tmp.path, environment: {'PATH': '/usr/bin:/bin'}, timeout: const Duration(seconds: 10), - maxOutputChars: 1024, + maxOutputBytes: 1024, ); expect(result.outputLimitExceeded, isTrue); @@ -49,7 +80,7 @@ void main() { workingDirectory: tmp.path, environment: {'PATH': '/usr/bin:/bin'}, timeout: const Duration(seconds: 10), - maxOutputChars: 1024, + maxOutputBytes: 1024, ); expect(result.exitCode, 0); @@ -76,7 +107,7 @@ void main() { workingDirectory: tmp.path, environment: {'PATH': '/usr/bin:/bin'}, timeout: const Duration(milliseconds: 100), - maxOutputChars: 128, + maxOutputBytes: 128, ); expect(result.timedOut, isTrue); @@ -86,6 +117,34 @@ void main() { skip: Platform.isWindows, ); + test( + 'preserves simultaneous process and memory limit observations', + () async { + final tmp = await Directory.systemTemp.createTemp( + 'dart_arena_eval_limits_', + ); + addTearDown(() async { + if (await tmp.exists()) await tmp.delete(recursive: true); + }); + + final result = await runEvaluatorProcess( + 'sh', + const ['-c', 'sleep 20 & wait'], + workingDirectory: tmp.path, + environment: {'PATH': '/usr/bin:/bin'}, + timeout: const Duration(seconds: 5), + maxProcesses: 1, + maxMemoryMb: 0, + ); + + expect(result.processLimitExceeded, isTrue); + expect(result.memoryLimitExceeded, isTrue); + expect(result.observedProcessCount, greaterThan(1)); + expect(result.observedMemoryMb, greaterThan(0)); + }, + skip: Platform.isWindows, + ); + test( 'resource probe helpers scrub sensitive environment variables', () async { diff --git a/app/test/evaluators/hidden_test_evaluator_test.dart b/app/test/evaluators/hidden_test_evaluator_test.dart index af95278..257499b 100644 --- a/app/test/evaluators/hidden_test_evaluator_test.dart +++ b/app/test/evaluators/hidden_test_evaluator_test.dart @@ -390,14 +390,14 @@ void main() { }, testPath: 'test/_hidden/answer_hidden_test.dart', ), - maxOutputChars: 1024, + maxOutputBytes: 1024, ).evaluate(_ctx(dir)); expect(result.passed, isFalse); expect(result.rationale, 'hidden verifier output limit exceeded'); expect(result.score, 0.0); expect(result.details['output_limit_exceeded'], isTrue); - expect(result.details['max_output_chars'], 1024); + expect(result.details['max_output_bytes'], 1024); expect(result.details['injected_files'], [ 'test/_hidden/answer_hidden_test.dart', ]); diff --git a/app/test/evaluators/test_evaluator_test.dart b/app/test/evaluators/test_evaluator_test.dart index 522bd2e..fbc6f7a 100644 --- a/app/test/evaluators/test_evaluator_test.dart +++ b/app/test/evaluators/test_evaluator_test.dart @@ -160,14 +160,14 @@ void main() { ); final result = await TestEvaluator( timeout: const Duration(seconds: 60), - maxOutputChars: 2048, + maxOutputBytes: 2048, ).evaluate(_ctx(dir)); expect(result.passed, isFalse); expect(result.score, 0.0); expect(result.rationale, 'test process output limit exceeded'); expect(result.details['output_limit_exceeded'], isTrue); - expect(result.details['max_output_chars'], 2048); + expect(result.details['max_output_bytes'], 2048); expect(result.details['timed_out'], isNull); expect(result.details['exit_code'], -1); }, @@ -189,14 +189,14 @@ void main() { final result = await TestEvaluator( dartExecutable: fakeDart.path, timeout: const Duration(seconds: 5), - maxOutputChars: 128, + maxOutputBytes: 128, ).evaluate(_ctx(dir)); expect(result.passed, isFalse); expect(result.score, 0.0); expect(result.rationale, 'test process output limit exceeded'); expect(result.details['output_limit_exceeded'], isTrue); - expect(result.details['max_output_chars'], 128); + expect(result.details['max_output_bytes'], 128); expect(result.details['timed_out'], isNull); }, skip: Platform.isWindows ? 'POSIX shell script test' : false, diff --git a/app/test/runner/agentic_run_orchestrator_test.dart b/app/test/runner/agentic_run_orchestrator_test.dart index 3cbeb1e..c6fe375 100644 --- a/app/test/runner/agentic_run_orchestrator_test.dart +++ b/app/test/runner/agentic_run_orchestrator_test.dart @@ -671,7 +671,7 @@ void main() { 'stepCount': 6, 'peakContextTokens': 12000, 'output_limit_exceeded': true, - 'max_output_chars': 128, + 'max_output_bytes': 128, 'runtimeBoundary': {'enforced': true, 'backend': 'bubblewrap'}, 'metadata': {'workspacePath': privateWorkspace, 'stepCount': 7}, 'exception': 'StateError', @@ -693,7 +693,7 @@ void main() { expect(details, containsPair('stepCount', 6)); expect(details, containsPair('peakContextTokens', 12000)); expect(details, containsPair('output_limit_exceeded', true)); - expect(details, containsPair('max_output_chars', 128)); + expect(details, containsPair('max_output_bytes', 128)); expect(details['runtimeBoundary'], { 'enforced': true, 'backend': 'bubblewrap', diff --git a/app/test/runner/bounded_subprocess_test.dart b/app/test/runner/bounded_subprocess_test.dart index d8d9ce5..14b93f1 100644 --- a/app/test/runner/bounded_subprocess_test.dart +++ b/app/test/runner/bounded_subprocess_test.dart @@ -1,10 +1,45 @@ import 'dart:async'; +import 'dart:convert'; import 'dart:io'; import 'package:dart_arena/runner/bounded_subprocess.dart'; import 'package:test/test.dart'; void main() { + test( + 'propagates missing absolute and PATH executable spawn failures', + () async { + final temp = await Directory.systemTemp.createTemp( + 'dart_arena_subprocess_missing_', + ); + addTearDown(() async { + if (await temp.exists()) await temp.delete(recursive: true); + }); + final missingAbsolute = '${temp.path}/missing-command'; + + for (final executable in [missingAbsolute, 'missing-path-command']) { + await expectLater( + runBoundedSubprocess( + executable: executable, + arguments: const ['argument'], + workingDirectory: temp.path, + environment: {'PATH': temp.path}, + maxOutputBytes: 128, + timeout: const Duration(seconds: 2), + ), + throwsA( + isA().having( + (error) => error.executable, + 'executable', + executable, + ), + ), + ); + } + }, + skip: Platform.isWindows, + ); + test( 'drains stdout and stderr concurrently without backpressure hangs', () async { @@ -48,6 +83,29 @@ wait skip: Platform.isWindows, ); + test( + 'preserves character-based adapter limits within a raw-byte bound', + () async { + final result = await runBoundedSubprocess( + executable: 'sh', + arguments: const [ + '-c', + r"printf '\342\230\203\342\230\203\342\230\203\342\230\203\342\230\203'", + ], + workingDirectory: Directory.current.path, + environment: {'PATH': Platform.environment['PATH'] ?? '/usr/bin:/bin'}, + maxOutputBytes: maxEncodedOutputBytes(4), + maxOutputCharacters: 4, + timeout: const Duration(seconds: 5), + ); + + expect(result.stdoutLimitExceeded, isTrue); + expect(result.stdout, '☃☃☃☃'); + expect(utf8.encode(result.stdout).length, 12); + }, + skip: Platform.isWindows, + ); + test( 'keeps readable multibyte text when capture ends mid-sequence', () async { @@ -149,7 +207,58 @@ wait ); test( - 'escalates from TERM to KILL for an unresponsive process group', + 'does not direct-signal a root whose exit code already completed', + () async { + final temp = await Directory.systemTemp.createTemp( + 'dart_arena_subprocess_reaped_root_', + ); + addTearDown(() async { + if (await temp.exists()) await temp.delete(recursive: true); + }); + final pidFile = File('${temp.path}/child.pid'); + final monitorSignal = Completer(); + final sentSignals = <({int pid, ProcessSignal? signal})>[]; + int? rootPid; + + final result = await _runShell( + "sleep 20 & echo \$! > '${pidFile.path}'; exit 0", + maxOutputBytes: 128, + timeout: const Duration(seconds: 5), + monitor: (pid) { + rootPid = pid; + return BoundedSubprocessMonitor( + signal: monitorSignal.future, + observed: () => null, + dispose: () {}, + ); + }, + signalSender: (pid, signal) { + sentSignals.add((pid: pid, signal: signal)); + try { + return signal == null + ? Process.killPid(pid) + : Process.killPid(pid, signal); + } on Object { + return false; + } + }, + ); + + expect(result.termination, BoundedSubprocessTermination.exited); + expect(rootPid, isNotNull); + expect( + sentSignals, + contains((pid: -rootPid!, signal: ProcessSignal.sigterm)), + ); + expect(sentSignals.where((sent) => sent.pid > 0), isEmpty); + final childPid = int.parse((await pidFile.readAsString()).trim()); + expect(await _pidIsRunning(childPid), isFalse); + }, + skip: Platform.isWindows, + ); + + test( + 'escalates to KILL when both root and child ignore TERM', () async { final temp = await Directory.systemTemp.createTemp( 'dart_arena_subprocess_kill_', @@ -158,19 +267,63 @@ wait if (await temp.exists()) await temp.delete(recursive: true); }); final pidFile = File('${temp.path}/child.pid'); + final sentSignals = <({int pid, ProcessSignal? signal})>[]; final result = await _runShell( - "trap '' TERM; sleep 20 & echo \$! > '${pidFile.path}'; wait", + "trap '' TERM; sh -c 'trap \"\" TERM; echo \$\$ > \"${pidFile.path}\"; while :; do sleep 1; done' & while [ ! -s '${pidFile.path}' ]; do sleep 0.01; done; wait", maxOutputBytes: 128, - timeout: const Duration(milliseconds: 100), + timeout: const Duration(milliseconds: 200), + signalSender: (pid, signal) { + sentSignals.add((pid: pid, signal: signal)); + return _sendSignal(pid, signal); + }, ); expect(result.termination, BoundedSubprocessTermination.timedOut); + expect( + sentSignals.any((sent) => sent.signal == ProcessSignal.sigkill), + isTrue, + ); final pid = int.parse((await pidFile.readAsString()).trim()); await Future.delayed(const Duration(milliseconds: 100)); expect(await _pidIsRunning(pid), isFalse); }, skip: Platform.isWindows, + timeout: const Timeout(Duration(seconds: 8)), + ); + + test( + 'kills a TERM-ignoring descendant that escapes the root session', + () async { + final setsid = _setsidPath(); + if (setsid == null) return; + final temp = await Directory.systemTemp.createTemp( + 'dart_arena_subprocess_escaped_', + ); + addTearDown(() async { + if (await temp.exists()) await temp.delete(recursive: true); + }); + final pidFile = File('${temp.path}/child.pid'); + final sentSignals = <({int pid, ProcessSignal? signal})>[]; + + final result = await _runShell( + "trap '' TERM; '$setsid' sh -c 'trap \"\" TERM; echo \$\$ > \"${pidFile.path}\"; while :; do sleep 1; done' & while [ ! -s '${pidFile.path}' ]; do sleep 0.01; done; wait", + maxOutputBytes: 128, + timeout: const Duration(milliseconds: 200), + signalSender: (pid, signal) { + sentSignals.add((pid: pid, signal: signal)); + return _sendSignal(pid, signal); + }, + ); + + expect(result.termination, BoundedSubprocessTermination.timedOut); + final pid = int.parse((await pidFile.readAsString()).trim()); + expect(sentSignals, contains((pid: pid, signal: ProcessSignal.sigkill))); + await Future.delayed(const Duration(milliseconds: 100)); + expect(await _pidIsRunning(pid), isFalse); + }, + skip: Platform.isWindows, + timeout: const Timeout(Duration(seconds: 8)), ); } @@ -179,6 +332,8 @@ Future _runShell( required int maxOutputBytes, required Duration timeout, Future? cancellationSignal, + BoundedSubprocessMonitorFactory? monitor, + BoundedSubprocessSignalSender? signalSender, }) { return runBoundedSubprocess( executable: 'sh', @@ -188,9 +343,26 @@ Future _runShell( maxOutputBytes: maxOutputBytes, timeout: timeout, cancellationSignal: cancellationSignal, + monitor: monitor, + signalSender: signalSender ?? _sendSignal, ); } +bool _sendSignal(int pid, ProcessSignal? signal) { + try { + return signal == null ? Process.killPid(pid) : Process.killPid(pid, signal); + } on Object { + return false; + } +} + +String? _setsidPath() { + for (final path in const ['/usr/bin/setsid', '/bin/setsid']) { + if (File(path).existsSync()) return path; + } + return null; +} + Future _pidIsRunning(int pid) async { final result = await Process.run('ps', ['-p', '$pid', '-o', 'stat=']); if (result.exitCode != 0) return false; diff --git a/app/test/runner/codegen_task_executor_blocking_test.dart b/app/test/runner/codegen_task_executor_blocking_test.dart index c78b3a6..f0e9cb6 100644 --- a/app/test/runner/codegen_task_executor_blocking_test.dart +++ b/app/test/runner/codegen_task_executor_blocking_test.dart @@ -195,7 +195,7 @@ void main() { expect(evaluation.evaluatorId, 'test'); expect(evaluation.rationale, 'test process output limit exceeded'); expect(evaluation.details['output_limit_exceeded'], isTrue); - expect(evaluation.details['max_output_chars'], 1024); + expect(evaluation.details['max_output_bytes'], 1024); }, timeout: const Timeout(Duration(minutes: 2)), ); diff --git a/app/test/runner/evaluator_resource_limits_test.dart b/app/test/runner/evaluator_resource_limits_test.dart index e4306ff..c04d506 100644 --- a/app/test/runner/evaluator_resource_limits_test.dart +++ b/app/test/runner/evaluator_resource_limits_test.dart @@ -20,13 +20,13 @@ void main() { ); expect(evaluator, isA()); - expect((evaluator as TestEvaluator).maxOutputChars, 4096); + expect((evaluator as TestEvaluator).maxOutputBytes, 4096); }); test('task output limit caps custom public test limit', () { final evaluator = applyResourceLimitsToEvaluator( TestEvaluator( - maxOutputChars: 2048, + maxOutputBytes: 2048, dartExecutable: '/fake/dart', flutterExecutable: '/fake/flutter', ), @@ -34,18 +34,18 @@ void main() { ); final testEvaluator = evaluator as TestEvaluator; - expect(testEvaluator.maxOutputChars, 1024); + expect(testEvaluator.maxOutputBytes, 1024); expect(testEvaluator.dartExecutable, '/fake/dart'); expect(testEvaluator.flutterExecutable, '/fake/flutter'); }); test('stricter custom public test limit is preserved', () { final evaluator = applyResourceLimitsToEvaluator( - TestEvaluator(maxOutputChars: 512), + TestEvaluator(maxOutputBytes: 512), const TaskResourceLimits(maxOutputBytes: 1024), ); - expect((evaluator as TestEvaluator).maxOutputChars, 512); + expect((evaluator as TestEvaluator).maxOutputBytes, 512); }); test('task output limit applies to hidden test evaluator', () { @@ -55,7 +55,7 @@ void main() { ); expect(evaluator, isA()); - expect((evaluator as HiddenTestEvaluator).maxOutputChars, 256); + expect((evaluator as HiddenTestEvaluator).maxOutputBytes, 256); }); test('task output limit applies to analyze evaluator', () { @@ -65,7 +65,7 @@ void main() { ); expect(evaluator, isA()); - expect((evaluator as AnalyzeEvaluator).maxOutputChars, 768); + expect((evaluator as AnalyzeEvaluator).maxOutputBytes, 768); }); test('task output limit applies to compile evaluator', () { @@ -75,7 +75,7 @@ void main() { ); expect(evaluator, isA()); - expect((evaluator as CompileEvaluator).maxOutputChars, 768); + expect((evaluator as CompileEvaluator).maxOutputBytes, 768); }); test('task output limit applies to widget tree evaluator', () { @@ -85,7 +85,7 @@ void main() { ); expect(evaluator, isA()); - expect((evaluator as WidgetTreeEvaluator).maxOutputChars, 768); + expect((evaluator as WidgetTreeEvaluator).maxOutputBytes, 768); }); test('task output limit applies to test author evaluator', () { @@ -98,7 +98,7 @@ void main() { ); expect(evaluator, isA()); - expect((evaluator as TestAuthorEvaluator).maxOutputChars, 768); + expect((evaluator as TestAuthorEvaluator).maxOutputBytes, 768); }); test('task process limit applies to bounded evaluators', () { @@ -205,7 +205,7 @@ void main() { ], _PartialLimitTask()); final evaluator = evaluators.single as TestEvaluator; - expect(evaluator.maxOutputChars, 2048); + expect(evaluator.maxOutputBytes, 2048); expect(evaluator.maxProcesses, 64); expect(evaluator.maxMemoryMb, 8192); }); diff --git a/app/test/runner/generated_code_sandbox_test.dart b/app/test/runner/generated_code_sandbox_test.dart index 62dc067..f4525d8 100644 --- a/app/test/runner/generated_code_sandbox_test.dart +++ b/app/test/runner/generated_code_sandbox_test.dart @@ -622,7 +622,7 @@ except OSError: generatedCodeSandbox: const BubblewrapGeneratedCodeSandbox(), allowInternet: false, timeout: const Duration(seconds: 5), - maxOutputChars: 128, + maxOutputBytes: 128, ); expect(result.exitCode, -1); From cb903f1ddafcf56b91091f42a672ca693a21619f Mon Sep 17 00:00:00 2001 From: Elberte Plinio Date: Mon, 20 Jul 2026 15:32:33 -0300 Subject: [PATCH 3/4] fix: propagate setsid exec failures --- app/lib/runner/bounded_subprocess.dart | 444 +++++++++++++++--- .../evaluators/evaluator_process_test.dart | 55 +++ app/test/runner/bounded_subprocess_test.dart | 156 ++++++ 3 files changed, 593 insertions(+), 62 deletions(-) diff --git a/app/lib/runner/bounded_subprocess.dart b/app/lib/runner/bounded_subprocess.dart index e46508f..4ff9b23 100644 --- a/app/lib/runner/bounded_subprocess.dart +++ b/app/lib/runner/bounded_subprocess.dart @@ -4,7 +4,6 @@ import 'dart:io'; import 'dart:typed_data'; import 'package:dart_arena/runner/subprocess_environment.dart'; -import 'package:path/path.dart' as p; enum BoundedSubprocessTermination { exited, @@ -58,6 +57,13 @@ typedef BoundedSubprocessSignalSender = int maxEncodedOutputBytes(int maxCharacters) => maxCharacters * 4; +const _linuxHandshakeRoot = '/tmp'; +const _execHandshakeTimeout = Duration(seconds: 5); +const _maxExecHandshakeBytes = 4096; +const _solSocket = 1; +const _soPeerCredentials = 17; +const _peerCredentialsSize = 12; + Future runBoundedSubprocess({ required String executable, required List arguments, @@ -78,25 +84,23 @@ Future runBoundedSubprocess({ assert(maxOutputCharacters == null || maxOutputCharacters > 0); final setsid = Platform.isLinux ? _installedSystemTool('setsid') : null; final startsProcessGroup = setsid != null; - final targetExecutable = startsProcessGroup - ? _resolveExecutable( + final process = startsProcessGroup + ? await _startLinuxProcessGroup( + setsid, executable, arguments, workingDirectory: workingDirectory, environment: environment, includeParentEnvironment: includeParentEnvironment, ) - : executable; - final process = await Process.start( - setsid ?? targetExecutable, - startsProcessGroup - ? ['--wait', '--', targetExecutable, ...arguments] - : arguments, - workingDirectory: workingDirectory, - runInShell: false, - environment: environment, - includeParentEnvironment: includeParentEnvironment, - ); + : await Process.start( + executable, + arguments, + workingDirectory: workingDirectory, + runInShell: false, + environment: environment, + includeParentEnvironment: includeParentEnvironment, + ); final processState = _ProcessState(process); final stdout = _BoundedByteCollector( maxOutputBytes, @@ -220,6 +224,370 @@ Future runBoundedSubprocess({ ); } +const _pythonLinuxExecLauncher = r''' +import json +import os +import signal +import socket +import struct +import sys + +handshake = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) +handshake.set_inheritable(False) +handshake.connect(sys.argv[1]) + +def read_exact(length): + chunks = [] + remaining = length + while remaining: + chunk = handshake.recv(remaining) + if not chunk: + raise RuntimeError('Incomplete target environment') + chunks.append(chunk) + remaining -= len(chunk) + return b''.join(chunks) + +environment_length = struct.unpack('!I', read_exact(4))[0] +environment = json.loads(read_exact(environment_length)) +executable = sys.argv[2] +target_arguments = sys.argv[2:] +for signal_name in ('SIGPIPE', 'SIGXFSZ'): + signal_number = getattr(signal, signal_name, None) + if signal_number is not None: + signal.signal(signal_number, signal.SIG_DFL) +try: + if '/' in executable: + os.execve(executable, target_arguments, environment) + else: + os.execvpe(executable, target_arguments, environment) +except OSError as error: + message = (error.strerror or str(error)).encode('utf-8', 'replace') + handshake.sendall(struct.pack('!I', error.errno or 0) + message) + handshake.close() + raise SystemExit(127 if error.errno == 2 else 126) +'''; + +const _dartLinuxExecLauncher = r''' +import 'dart:async'; +import 'dart:convert'; +import 'dart:ffi'; +import 'dart:io'; +import 'dart:typed_data'; + +typedef _ExecNative = Int32 Function( + Pointer, + Pointer>, + Pointer>, +); +typedef _ExecDart = int Function( + Pointer, + Pointer>, + Pointer>, +); +typedef _MallocNative = Pointer Function(IntPtr); +typedef _MallocDart = Pointer Function(int); +typedef _ErrnoNative = Pointer Function(); +typedef _ErrnoDart = Pointer Function(); +typedef _StrerrorNative = Pointer Function(Int32); +typedef _StrerrorDart = Pointer Function(int); +typedef _SignalNative = Pointer Function(Int32, Pointer); +typedef _SignalDart = Pointer Function(int, Pointer); + +final _libc = DynamicLibrary.process(); +final _malloc = _libc.lookupFunction<_MallocNative, _MallocDart>('malloc'); +final _execve = _libc.lookupFunction<_ExecNative, _ExecDart>('execve'); +final _execvpe = _libc.lookupFunction<_ExecNative, _ExecDart>('execvpe'); +final _errno = _libc.lookupFunction<_ErrnoNative, _ErrnoDart>( + '__errno_location', +); +final _strerror = _libc.lookupFunction<_StrerrorNative, _StrerrorDart>( + 'strerror', +); +final _signal = _libc.lookupFunction<_SignalNative, _SignalDart>('signal'); + +Pointer _nativeString(String value) { + final bytes = [...utf8.encode(value), 0]; + final pointer = _malloc(bytes.length).cast(); + pointer.asTypedList(bytes.length).setAll(0, bytes); + return pointer; +} + +Pointer> _nativeVector(List values) { + final pointer = _malloc( + (values.length + 1) * sizeOf>(), + ).cast>(); + for (var index = 0; index < values.length; index++) { + pointer[index] = _nativeString(values[index]); + } + pointer[values.length] = nullptr; + return pointer; +} + +String _errorMessage(int errorCode) { + final pointer = _strerror(errorCode); + final bytes = []; + for (var index = 0; pointer[index] != 0; index++) { + bytes.add(pointer[index]); + } + return utf8.decode(bytes, allowMalformed: true); +} + +Future _readMessage(Socket socket) { + final completer = Completer(); + final bytes = BytesBuilder(copy: false); + late final StreamSubscription subscription; + subscription = socket.listen( + (chunk) { + if (completer.isCompleted) return; + bytes.add(chunk); + final payload = bytes.toBytes(); + if (payload.length < 4) return; + final length = ByteData.sublistView(payload).getUint32(0); + if (payload.length < 4 + length) return; + subscription.pause(); + completer.complete(Uint8List.sublistView(payload, 4, 4 + length)); + }, + onError: completer.completeError, + onDone: () { + if (!completer.isCompleted) { + completer.completeError( + const FormatException('Incomplete target environment'), + ); + } + }, + ); + return completer.future; +} + +Future main(List arguments) async { + final handshake = await Socket.connect( + InternetAddress(arguments[0], type: InternetAddressType.unix), + 0, + ); + final environment = (jsonDecode( + utf8.decode(await _readMessage(handshake)), + ) as Map).cast(); + final executable = arguments[1]; + final targetArguments = arguments.sublist(1); + final environmentEntries = environment.entries + .map((entry) => '${entry.key}=${entry.value}') + .toList(growable: false); + + const signalPipe = 13; + const signalFileSizeExceeded = 25; + _signal(signalPipe, nullptr); + _signal(signalFileSizeExceeded, nullptr); + final executablePointer = _nativeString(executable); + final argumentPointers = _nativeVector(targetArguments); + final environmentPointers = _nativeVector(environmentEntries); + if (executable.contains('/')) { + _execve(executablePointer, argumentPointers, environmentPointers); + } else { + _execvpe(executablePointer, argumentPointers, environmentPointers); + } + + final errorCode = _errno().value; + final message = utf8.encode(_errorMessage(errorCode)); + final response = Uint8List(4 + message.length); + ByteData.sublistView(response).setUint32(0, errorCode); + response.setRange(4, response.length, message); + handshake.add(response); + await handshake.flush(); + await handshake.close(); + exit(errorCode == 2 ? 127 : 126); +} +'''; + +Future _startLinuxProcessGroup( + String setsid, + String targetExecutable, + List arguments, { + required String workingDirectory, + required Map environment, + required bool includeParentEnvironment, +}) async { + final targetEnvironment = { + if (includeParentEnvironment) ...Platform.environment, + ...environment, + }; + final handshakeDirectory = await Directory( + _linuxHandshakeRoot, + ).createTemp('dart_arena_exec_'); + final launcher = File('${handshakeDirectory.path}/launcher.dart'); + final socketPath = '${handshakeDirectory.path}/handshake.sock'; + ServerSocket? server; + Process? process; + try { + final python = _installedSystemTool('python3'); + if (python == null) { + await launcher.writeAsString(_dartLinuxExecLauncher); + } + server = await ServerSocket.bind( + InternetAddress(socketPath, type: InternetAddressType.unix), + 0, + ); + process = await Process.start( + setsid, + [ + '--wait', + '--', + if (python != null) ...[ + python, + '-I', + '-S', + '-c', + _pythonLinuxExecLauncher, + ] else ...[ + Platform.resolvedExecutable, + launcher.path, + ], + socketPath, + targetExecutable, + ...arguments, + ], + workingDirectory: workingDirectory, + runInShell: false, + environment: { + 'PATH': + targetEnvironment['PATH'] ?? + Platform.environment['PATH'] ?? + '/usr/bin:/bin', + }, + includeParentEnvironment: false, + ); + + late final _ExecHandshake result; + try { + result = await _receiveExecHandshake( + server, + process.pid, + targetEnvironment, + cleanupHandshake: () async { + if (await handshakeDirectory.exists()) { + await handshakeDirectory.delete(recursive: true); + } + }, + ).timeout(_execHandshakeTimeout); + } on TimeoutException { + await _drainFailedLauncher(process, terminate: true); + throw ProcessException( + targetExecutable, + arguments, + 'Subprocess launcher did not complete its exec handshake', + ); + } on Object catch (error) { + await _drainFailedLauncher(process, terminate: true); + throw ProcessException( + targetExecutable, + arguments, + 'Subprocess launcher failed: $error', + ); + } + + final errorCode = result.errorCode; + if (errorCode != null) { + final exception = ProcessException( + targetExecutable, + arguments, + result.message, + errorCode, + ); + await _drainFailedLauncher(process); + throw exception; + } + return process; + } finally { + await server?.close(); + if (await handshakeDirectory.exists()) { + await handshakeDirectory.delete(recursive: true); + } + } +} + +Future<_ExecHandshake> _receiveExecHandshake( + ServerSocket server, + int expectedPid, + Map targetEnvironment, { + required Future Function() cleanupHandshake, +}) async { + await for (final socket in server) { + final credentials = socket.getRawOption( + RawSocketOption( + _solSocket, + _soPeerCredentials, + Uint8List(_peerCredentialsSize), + ), + ); + final peerPid = ByteData.sublistView(credentials).getInt32(0, Endian.host); + if (peerPid != expectedPid) { + socket.destroy(); + continue; + } + + await cleanupHandshake(); + final bytes = BytesBuilder(copy: false); + try { + final environment = utf8.encode(jsonEncode(targetEnvironment)); + final header = ByteData(4)..setUint32(0, environment.length); + socket.add(header.buffer.asUint8List()); + socket.add(environment); + await socket.flush(); + await for (final chunk in socket) { + bytes.add(chunk); + if (bytes.length > _maxExecHandshakeBytes) break; + } + } finally { + socket.destroy(); + } + final payload = bytes.takeBytes(); + if (payload.isEmpty) return const _ExecHandshake.success(); + if (payload.length < 4) { + throw const FormatException('Incomplete exec failure handshake'); + } + final errorCode = ByteData.sublistView(payload).getUint32(0); + final message = utf8.decode(payload.sublist(4), allowMalformed: true); + return _ExecHandshake.failure(errorCode, message); + } + throw const SocketException('Exec handshake socket closed unexpectedly'); +} + +Future _drainFailedLauncher( + Process process, { + bool terminate = false, +}) async { + final drains = Future.wait([ + process.stdout.drain(), + process.stderr.drain(), + ]); + try { + await process.stdin.close(); + } on Object { + // The failed launcher may already have closed stdin. + } + if (terminate) { + _tryKillPid(-process.pid, ProcessSignal.sigkill); + _tryKillPid(process.pid, ProcessSignal.sigkill); + } + try { + await Future.wait([ + drains, + process.exitCode.then((_) {}), + ]).timeout(const Duration(seconds: 2)); + } on TimeoutException { + _tryKillPid(-process.pid, ProcessSignal.sigkill); + _tryKillPid(process.pid, ProcessSignal.sigkill); + } +} + +class _ExecHandshake { + const _ExecHandshake.success() : errorCode = null, message = ''; + + const _ExecHandshake.failure(this.errorCode, this.message); + + final int? errorCode; + final String message; +} + Future _writeStdin(Process process, List? bytes) async { try { if (bytes != null && bytes.isNotEmpty) process.stdin.add(bytes); @@ -406,54 +774,6 @@ String? _installedSystemTool(String name) { return null; } -String _resolveExecutable( - String executable, - List arguments, { - required String workingDirectory, - required Map environment, - required bool includeParentEnvironment, -}) { - final candidates = []; - if (p.isAbsolute(executable) || executable.contains('/')) { - candidates.add( - p.isAbsolute(executable) - ? executable - : p.join(workingDirectory, executable), - ); - } else { - final path = - environment['PATH'] ?? - (includeParentEnvironment ? Platform.environment['PATH'] : null) ?? - Platform.environment['PATH'] ?? - '/usr/bin:/bin'; - for (final root in path.split(':')) { - final searchRoot = root.isEmpty || !p.isAbsolute(root) - ? p.join(workingDirectory, root) - : root; - candidates.add(p.join(searchRoot, executable)); - } - } - - var foundNonExecutable = false; - for (final candidate in candidates) { - if (FileSystemEntity.typeSync(candidate, followLinks: true) != - FileSystemEntityType.file) { - continue; - } - if (File(candidate).statSync().mode & 0x49 == 0) { - foundNonExecutable = true; - continue; - } - return p.normalize(p.absolute(candidate)); - } - throw ProcessException( - executable, - arguments, - foundNonExecutable ? 'Permission denied' : 'No such file or directory', - foundNonExecutable ? 13 : 2, - ); -} - Future _tryRunProcess( String executable, List arguments, { diff --git a/app/test/evaluators/evaluator_process_test.dart b/app/test/evaluators/evaluator_process_test.dart index cbee30d..a56ac67 100644 --- a/app/test/evaluators/evaluator_process_test.dart +++ b/app/test/evaluators/evaluator_process_test.dart @@ -36,6 +36,61 @@ void main() { skip: Platform.isWindows, ); + test( + 'missing shebang interpreter preserves infrastructure spawn failure', + () async { + final tmp = await Directory.systemTemp.createTemp( + 'dart_arena_eval_shebang_', + ); + addTearDown(() async { + if (await tmp.exists()) await tmp.delete(recursive: true); + }); + final executable = await _writeExecutable( + tmp, + 'missing-interpreter', + '#!/definitely/missing/interpreter\nexit 0\n', + ); + + await expectLater( + runEvaluatorProcess( + executable.path, + const [], + workingDirectory: tmp.path, + environment: const {'PATH': '/usr/bin:/bin'}, + timeout: const Duration(seconds: 2), + ), + throwsA( + isA() + .having( + (error) => error.executable, + 'executable', + executable.path, + ) + .having((error) => error.errorCode, 'errorCode', 2), + ), + ); + }, + skip: !Platform.isLinux, + ); + + test('legitimate exit 127 remains an evaluator result', () async { + final tmp = await Directory.systemTemp.createTemp('dart_arena_eval_127_'); + addTearDown(() async { + if (await tmp.exists()) await tmp.delete(recursive: true); + }); + + final result = await runEvaluatorProcess( + 'sh', + const ['-c', 'exit 127'], + workingDirectory: tmp.path, + environment: const {'PATH': '/usr/bin:/bin'}, + timeout: const Duration(seconds: 2), + ); + + expect(result.exitCode, 127); + expect(result.timedOut, isFalse); + }, skip: !Platform.isLinux); + test( 'output limit counts raw bytes before decode, not decoded chars', () async { diff --git a/app/test/runner/bounded_subprocess_test.dart b/app/test/runner/bounded_subprocess_test.dart index 14b93f1..e84bb5f 100644 --- a/app/test/runner/bounded_subprocess_test.dart +++ b/app/test/runner/bounded_subprocess_test.dart @@ -40,6 +40,147 @@ void main() { skip: Platform.isWindows, ); + test( + 'propagates a missing shebang interpreter as an exec failure', + () async { + final temp = await Directory.systemTemp.createTemp( + 'dart_arena_subprocess_shebang_', + ); + addTearDown(() async { + if (await temp.exists()) await temp.delete(recursive: true); + }); + final script = await _writeFile( + temp, + 'missing-interpreter', + '#!/definitely/missing/interpreter\nexit 0\n', + executable: true, + ); + + await expectLater( + runBoundedSubprocess( + executable: script.path, + arguments: const [], + workingDirectory: temp.path, + environment: const {'PATH': '/usr/bin:/bin'}, + maxOutputBytes: 128, + timeout: const Duration(seconds: 2), + ), + throwsA( + isA() + .having((error) => error.executable, 'executable', script.path) + .having((error) => error.errorCode, 'errorCode', 2), + ), + ); + }, + skip: !Platform.isLinux, + ); + + test( + 'propagates a non-executable target as an exec failure', + () async { + final temp = await Directory.systemTemp.createTemp( + 'dart_arena_subprocess_permissions_', + ); + addTearDown(() async { + if (await temp.exists()) await temp.delete(recursive: true); + }); + final script = await _writeFile( + temp, + 'non-executable', + '#!/bin/sh\nexit 0\n', + ); + + await expectLater( + runBoundedSubprocess( + executable: script.path, + arguments: const [], + workingDirectory: temp.path, + environment: const {'PATH': '/usr/bin:/bin'}, + maxOutputBytes: 128, + timeout: const Duration(seconds: 2), + ), + throwsA( + isA() + .having((error) => error.executable, 'executable', script.path) + .having((error) => error.errorCode, 'errorCode', 13), + ), + ); + }, + skip: !Platform.isLinux, + ); + + test( + 'uses execvp permission precedence across PATH candidates', + () async { + final temp = await Directory.systemTemp.createTemp( + 'dart_arena_subprocess_path_', + ); + addTearDown(() async { + if (await temp.exists()) await temp.delete(recursive: true); + }); + final blocked = Directory('${temp.path}/blocked')..createSync(); + final runnable = Directory('${temp.path}/runnable')..createSync(); + await _writeFile(blocked, 'candidate', '#!/bin/sh\nexit 1\n'); + await _writeFile( + runnable, + 'candidate', + '#!/bin/sh\nprintf fallback\n', + executable: true, + ); + + final result = await runBoundedSubprocess( + executable: 'candidate', + arguments: const [], + workingDirectory: temp.path, + environment: {'PATH': '${blocked.path}:${runnable.path}'}, + maxOutputBytes: 128, + timeout: const Duration(seconds: 2), + ); + + expect(result.termination, BoundedSubprocessTermination.exited); + expect(result.exitCode, 0); + expect(result.stdout, 'fallback'); + }, + skip: !Platform.isLinux, + ); + + test( + 'preserves the exact target environment across the launcher', + () async { + final result = await runBoundedSubprocess( + executable: '/usr/bin/env', + arguments: const [], + workingDirectory: Directory.current.path, + environment: const {'ONLY_TARGET_VALUE': 'preserved'}, + includeParentEnvironment: false, + maxOutputBytes: 128, + timeout: const Duration(seconds: 2), + ); + + expect(result.termination, BoundedSubprocessTermination.exited); + expect(result.exitCode, 0); + expect(result.stdout, 'ONLY_TARGET_VALUE=preserved\n'); + }, + skip: !Platform.isLinux, + ); + + test( + 'keeps legitimate target exits 126 and 127 as ordinary exits', + () async { + for (final exitCode in const [126, 127]) { + final result = await _runShell( + 'exit $exitCode', + maxOutputBytes: 128, + timeout: const Duration(seconds: 2), + ); + + expect(result.termination, BoundedSubprocessTermination.exited); + expect(result.exitCode, exitCode); + } + }, + skip: !Platform.isLinux, + ); + test( 'drains stdout and stderr concurrently without backpressure hangs', () async { @@ -327,6 +468,21 @@ wait ); } +Future _writeFile( + Directory directory, + String name, + String contents, { + bool executable = false, +}) async { + final file = File('${directory.path}/$name'); + await file.writeAsString(contents); + if (executable) { + final chmod = await Process.run('chmod', ['+x', file.path]); + expect(chmod.exitCode, 0, reason: chmod.stderr.toString()); + } + return file; +} + Future _runShell( String command, { required int maxOutputBytes, From 910782ca3ce6185a5a3d435414fad0d64197331c Mon Sep 17 00:00:00 2001 From: Elberte Plinio Date: Mon, 20 Jul 2026 17:17:31 -0300 Subject: [PATCH 4/4] fix: simplify Linux subprocess launcher --- app/lib/runner/bounded_subprocess.dart | 177 +++---------------- app/test/runner/bounded_subprocess_test.dart | 48 ++++- 2 files changed, 71 insertions(+), 154 deletions(-) diff --git a/app/lib/runner/bounded_subprocess.dart b/app/lib/runner/bounded_subprocess.dart index 4ff9b23..0fe7a90 100644 --- a/app/lib/runner/bounded_subprocess.dart +++ b/app/lib/runner/bounded_subprocess.dart @@ -82,11 +82,15 @@ Future runBoundedSubprocess({ }) async { assert(maxOutputBytes > 0); assert(maxOutputCharacters == null || maxOutputCharacters > 0); - final setsid = Platform.isLinux ? _installedSystemTool('setsid') : null; - final startsProcessGroup = setsid != null; + final launcher = selectLinuxExecLauncherForTesting( + setsid: Platform.isLinux ? _installedSystemTool('setsid') : null, + python3: Platform.isLinux ? _installedSystemTool('python3') : null, + ); + final startsProcessGroup = launcher != null; final process = startsProcessGroup ? await _startLinuxProcessGroup( - setsid, + launcher.$1, + launcher.$2, executable, arguments, workingDirectory: workingDirectory, @@ -225,6 +229,7 @@ Future runBoundedSubprocess({ } const _pythonLinuxExecLauncher = r''' +import errno import json import os import signal @@ -260,146 +265,24 @@ try: os.execve(executable, target_arguments, environment) else: os.execvpe(executable, target_arguments, environment) -except OSError as error: - message = (error.strerror or str(error)).encode('utf-8', 'replace') - handshake.sendall(struct.pack('!I', error.errno or 0) + message) +except (OSError, ValueError) as error: + error_number = error.errno if isinstance(error, OSError) else errno.EINVAL + message = (getattr(error, 'strerror', None) or str(error)).encode( + 'utf-8', 'replace' + ) + handshake.sendall(struct.pack('!I', error_number or 0) + message) handshake.close() - raise SystemExit(127 if error.errno == 2 else 126) + raise SystemExit(127 if error_number == errno.ENOENT else 126) '''; -const _dartLinuxExecLauncher = r''' -import 'dart:async'; -import 'dart:convert'; -import 'dart:ffi'; -import 'dart:io'; -import 'dart:typed_data'; - -typedef _ExecNative = Int32 Function( - Pointer, - Pointer>, - Pointer>, -); -typedef _ExecDart = int Function( - Pointer, - Pointer>, - Pointer>, -); -typedef _MallocNative = Pointer Function(IntPtr); -typedef _MallocDart = Pointer Function(int); -typedef _ErrnoNative = Pointer Function(); -typedef _ErrnoDart = Pointer Function(); -typedef _StrerrorNative = Pointer Function(Int32); -typedef _StrerrorDart = Pointer Function(int); -typedef _SignalNative = Pointer Function(Int32, Pointer); -typedef _SignalDart = Pointer Function(int, Pointer); - -final _libc = DynamicLibrary.process(); -final _malloc = _libc.lookupFunction<_MallocNative, _MallocDart>('malloc'); -final _execve = _libc.lookupFunction<_ExecNative, _ExecDart>('execve'); -final _execvpe = _libc.lookupFunction<_ExecNative, _ExecDart>('execvpe'); -final _errno = _libc.lookupFunction<_ErrnoNative, _ErrnoDart>( - '__errno_location', -); -final _strerror = _libc.lookupFunction<_StrerrorNative, _StrerrorDart>( - 'strerror', -); -final _signal = _libc.lookupFunction<_SignalNative, _SignalDart>('signal'); - -Pointer _nativeString(String value) { - final bytes = [...utf8.encode(value), 0]; - final pointer = _malloc(bytes.length).cast(); - pointer.asTypedList(bytes.length).setAll(0, bytes); - return pointer; -} - -Pointer> _nativeVector(List values) { - final pointer = _malloc( - (values.length + 1) * sizeOf>(), - ).cast>(); - for (var index = 0; index < values.length; index++) { - pointer[index] = _nativeString(values[index]); - } - pointer[values.length] = nullptr; - return pointer; -} - -String _errorMessage(int errorCode) { - final pointer = _strerror(errorCode); - final bytes = []; - for (var index = 0; pointer[index] != 0; index++) { - bytes.add(pointer[index]); - } - return utf8.decode(bytes, allowMalformed: true); -} - -Future _readMessage(Socket socket) { - final completer = Completer(); - final bytes = BytesBuilder(copy: false); - late final StreamSubscription subscription; - subscription = socket.listen( - (chunk) { - if (completer.isCompleted) return; - bytes.add(chunk); - final payload = bytes.toBytes(); - if (payload.length < 4) return; - final length = ByteData.sublistView(payload).getUint32(0); - if (payload.length < 4 + length) return; - subscription.pause(); - completer.complete(Uint8List.sublistView(payload, 4, 4 + length)); - }, - onError: completer.completeError, - onDone: () { - if (!completer.isCompleted) { - completer.completeError( - const FormatException('Incomplete target environment'), - ); - } - }, - ); - return completer.future; -} - -Future main(List arguments) async { - final handshake = await Socket.connect( - InternetAddress(arguments[0], type: InternetAddressType.unix), - 0, - ); - final environment = (jsonDecode( - utf8.decode(await _readMessage(handshake)), - ) as Map).cast(); - final executable = arguments[1]; - final targetArguments = arguments.sublist(1); - final environmentEntries = environment.entries - .map((entry) => '${entry.key}=${entry.value}') - .toList(growable: false); - - const signalPipe = 13; - const signalFileSizeExceeded = 25; - _signal(signalPipe, nullptr); - _signal(signalFileSizeExceeded, nullptr); - final executablePointer = _nativeString(executable); - final argumentPointers = _nativeVector(targetArguments); - final environmentPointers = _nativeVector(environmentEntries); - if (executable.contains('/')) { - _execve(executablePointer, argumentPointers, environmentPointers); - } else { - _execvpe(executablePointer, argumentPointers, environmentPointers); - } - - final errorCode = _errno().value; - final message = utf8.encode(_errorMessage(errorCode)); - final response = Uint8List(4 + message.length); - ByteData.sublistView(response).setUint32(0, errorCode); - response.setRange(4, response.length, message); - handshake.add(response); - await handshake.flush(); - await handshake.close(); - exit(errorCode == 2 ? 127 : 126); -} -'''; +(String, String)? selectLinuxExecLauncherForTesting({ + required String? setsid, + required String? python3, +}) => setsid != null && python3 != null ? (setsid, python3) : null; Future _startLinuxProcessGroup( String setsid, + String python3, String targetExecutable, List arguments, { required String workingDirectory, @@ -413,15 +296,10 @@ Future _startLinuxProcessGroup( final handshakeDirectory = await Directory( _linuxHandshakeRoot, ).createTemp('dart_arena_exec_'); - final launcher = File('${handshakeDirectory.path}/launcher.dart'); final socketPath = '${handshakeDirectory.path}/handshake.sock'; ServerSocket? server; Process? process; try { - final python = _installedSystemTool('python3'); - if (python == null) { - await launcher.writeAsString(_dartLinuxExecLauncher); - } server = await ServerSocket.bind( InternetAddress(socketPath, type: InternetAddressType.unix), 0, @@ -431,16 +309,11 @@ Future _startLinuxProcessGroup( [ '--wait', '--', - if (python != null) ...[ - python, - '-I', - '-S', - '-c', - _pythonLinuxExecLauncher, - ] else ...[ - Platform.resolvedExecutable, - launcher.path, - ], + python3, + '-I', + '-S', + '-c', + _pythonLinuxExecLauncher, socketPath, targetExecutable, ...arguments, diff --git a/app/test/runner/bounded_subprocess_test.dart b/app/test/runner/bounded_subprocess_test.dart index e84bb5f..2e9abc2 100644 --- a/app/test/runner/bounded_subprocess_test.dart +++ b/app/test/runner/bounded_subprocess_test.dart @@ -6,6 +6,16 @@ import 'package:dart_arena/runner/bounded_subprocess.dart'; import 'package:test/test.dart'; void main() { + test('selects direct mode when trusted python is unavailable', () { + expect( + selectLinuxExecLauncherForTesting( + setsid: '/usr/bin/setsid', + python3: null, + ), + isNull, + ); + }); + test( 'propagates missing absolute and PATH executable spawn failures', () async { @@ -151,7 +161,7 @@ void main() { executable: '/usr/bin/env', arguments: const [], workingDirectory: Directory.current.path, - environment: const {'ONLY_TARGET_VALUE': 'preserved'}, + environment: const {'ONLY_TARGET_VALUE': 'preserved=value'}, includeParentEnvironment: false, maxOutputBytes: 128, timeout: const Duration(seconds: 2), @@ -159,7 +169,41 @@ void main() { expect(result.termination, BoundedSubprocessTermination.exited); expect(result.exitCode, 0); - expect(result.stdout, 'ONLY_TARGET_VALUE=preserved\n'); + expect(result.stdout, 'ONLY_TARGET_VALUE=preserved=value\n'); + }, + skip: !Platform.isLinux, + ); + + test( + 'reports malformed target environments as spawn failures', + () async { + const malformedEnvironments = >[ + {'': 'value'}, + {'BAD=KEY': 'value'}, + {'BAD\u0000KEY': 'value'}, + {'BAD_VALUE': 'bad\u0000value'}, + ]; + + for (final environment in malformedEnvironments) { + await expectLater( + runBoundedSubprocess( + executable: '/usr/bin/env', + arguments: const [], + workingDirectory: Directory.current.path, + environment: environment, + includeParentEnvironment: false, + maxOutputBytes: 128, + timeout: const Duration(seconds: 2), + ), + throwsA( + isA().having( + (error) => error.errorCode, + 'errorCode', + 22, + ), + ), + ); + } }, skip: !Platform.isLinux, );