replace forkpty() with posix_openpt() + posix_spawn() (T-96)
`forkpty` calls `fork()` underneath. `fork()` in a multithreaded process is unsafe: only the calling thread survives in the child, but libc locks held by other threads remain "locked forever." With the multi-threaded Dart VM as parent, ~5% of spawns deadlocked in the child before `execve` (forensic probe: child stuck in S state with comm=`DartWorker`, master fd never sees POLLIN). `posix_spawn` uses `vfork` on glibc/musl/macOS, keeping the parent suspended until execve completes — no Dart code runs in the child. Pty pair built via the POSIX-standard `posix_openpt` / `grantpt` / `unlockpt` / `ptsname` sequence. Probed: zero hangs in 300 sequential spawns vs ~5% before. Behavior change: missing executable / missing workingDirectory now surface as a `PtyException` thrown by `NativePty.start` rather than a diagnostic written from the child to the slave PTY. Cleaner error path for callers. Side benefit: drops the `libutil.so.1` dynamic-library dependency. PTY now resolves entirely against libc via `DynamicLibrary.process()`. Splits the library-level `@Tags(['forkpty'])` on session_test.dart into a per-test tag, so the now-runnable-under-flutter-test cases contribute to coverage. `dart_test.yaml` declares the tag so the exclude-tags filters honor it. Drops the `retry: 2` workaround from the formerly-flaky registry test. D-5 amended. Trims session-introduced CHANGELOG entries that were over-verbose for the Keep-a-Changelog format. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -44,20 +44,19 @@ void main() {
|
||||
test('output events base64-encode the child bytes', tags: ['forkpty'], () async {
|
||||
await registry.spawn(
|
||||
kind: PaneKind.terminal,
|
||||
argv: const ['/bin/echo', 'hello-panes'],
|
||||
// Child writes then lingers so the reader's poll has a wide
|
||||
// window to see POLLIN before HUP.
|
||||
argv: const ['/bin/sh', '-c', 'printf hello-panes; sleep 0.25'],
|
||||
);
|
||||
|
||||
// /bin/echo closes its pty quickly. Wait briefly for output +
|
||||
// the resulting pane.exit event to settle.
|
||||
for (var i = 0; i < 30; i++) {
|
||||
if (sink.ofKind('pane.output').isNotEmpty && sink.ofKind('pane.exit').isNotEmpty) break;
|
||||
await Future<void>.delayed(const Duration(milliseconds: 100));
|
||||
final deadline = DateTime.now().add(const Duration(seconds: 2));
|
||||
String decoded() => sink.ofKind('pane.output').map((e) => utf8.decode(base64Decode(e.data['bytes_b64']! as String))).join();
|
||||
while (!decoded().contains('hello-panes') && DateTime.now().isBefore(deadline)) {
|
||||
await Future<void>.delayed(const Duration(milliseconds: 25));
|
||||
}
|
||||
|
||||
final out = sink.ofKind('pane.output').toList();
|
||||
expect(out, isNotEmpty);
|
||||
final decoded = out.map((e) => utf8.decode(base64Decode(e.data['bytes_b64']! as String))).join();
|
||||
expect(decoded, contains('hello-panes'));
|
||||
expect(sink.ofKind('pane.output'), isNotEmpty);
|
||||
expect(decoded(), contains('hello-panes'));
|
||||
});
|
||||
|
||||
test('write + resize emit no spurious events, update state', () async {
|
||||
|
||||
+43
-52
@@ -1,19 +1,21 @@
|
||||
/// NativePty smoke tests.
|
||||
///
|
||||
/// Exercises forkpty() end-to-end: spawn → child output through the
|
||||
/// Exercises posix_spawn() end-to-end: spawn → child output through the
|
||||
/// reader isolate. Linux + macOS only; skipped elsewhere.
|
||||
///
|
||||
/// Tagged `forkpty` — must run via `dart test`, not `flutter test`.
|
||||
/// forkpty() forks the Flutter engine's multi-threaded process; the
|
||||
/// child exec's fine but the master fd never produces readable output
|
||||
/// inside the flutter test runner.
|
||||
@Tags(['forkpty'])
|
||||
/// Per-test `tags: ['forkpty']` marks the tests that need `dart test`
|
||||
/// rather than the flutter test runner — currently just the
|
||||
/// write/read-back bidirectional test (writes to the master fd never
|
||||
/// reach the child under the flutter test runner; reads work fine).
|
||||
/// Everything else runs under `flutter test` and contributes to
|
||||
/// coverage.
|
||||
library;
|
||||
|
||||
import 'dart:async';
|
||||
import 'dart:convert';
|
||||
import 'dart:io';
|
||||
|
||||
import 'package:clide/src/pty/errors.dart';
|
||||
import 'package:clide/src/pty/native_pty.dart';
|
||||
import 'package:test/test.dart';
|
||||
|
||||
@@ -48,7 +50,7 @@ void main() {
|
||||
expect(buf.toString(), contains('hello-pty'));
|
||||
});
|
||||
|
||||
test('write sends keystrokes to child', () async {
|
||||
test('write sends keystrokes to child', tags: ['forkpty'], () async {
|
||||
final s = NativePty.start(
|
||||
executable: '/bin/sh',
|
||||
arguments: [],
|
||||
@@ -122,55 +124,44 @@ void main() {
|
||||
expect(buf.toString(), contains('path-resolution-ok'));
|
||||
});
|
||||
|
||||
test('non-existent workingDirectory produces the chdir-failed diagnostic', () async {
|
||||
// chdir() fails in the child → writes diagnostic + _exit(1).
|
||||
final s = NativePty.start(
|
||||
executable: '/bin/sh',
|
||||
arguments: ['-c', 'echo should-not-run'],
|
||||
columns: 80,
|
||||
rows: 24,
|
||||
workingDirectory: '/tmp/clide-no-such-dir-${DateTime.now().microsecondsSinceEpoch}',
|
||||
environment: {
|
||||
...Platform.environment,
|
||||
'TERM': 'xterm-256color',
|
||||
},
|
||||
test('non-existent workingDirectory surfaces a PtyException at spawn time', () {
|
||||
// posix_spawn returns ENOENT (errno 2) when the file_actions chdir
|
||||
// step finds the directory missing — propagates as a thrown
|
||||
// PtyException, not a child-side diagnostic on the pty.
|
||||
expect(
|
||||
() => NativePty.start(
|
||||
executable: '/bin/sh',
|
||||
arguments: ['-c', 'echo should-not-run'],
|
||||
columns: 80,
|
||||
rows: 24,
|
||||
workingDirectory: '/tmp/clide-no-such-dir-${DateTime.now().microsecondsSinceEpoch}',
|
||||
environment: {
|
||||
...Platform.environment,
|
||||
'TERM': 'xterm-256color',
|
||||
},
|
||||
),
|
||||
throwsA(isA<PtyException>().having((e) => e.errno, 'errno', 2)),
|
||||
);
|
||||
addTearDown(s.close);
|
||||
final buf = StringBuffer();
|
||||
final done = Completer<void>();
|
||||
s.output.listen(
|
||||
(b) => buf.write(utf8.decode(b, allowMalformed: true)),
|
||||
onDone: () {
|
||||
if (!done.isCompleted) done.complete();
|
||||
},
|
||||
);
|
||||
await done.future.timeout(const Duration(seconds: 5), onTimeout: () {});
|
||||
expect(buf.toString(), contains('chdir failed'));
|
||||
});
|
||||
|
||||
test('non-existent executable produces the exec-failed diagnostic', () async {
|
||||
final s = NativePty.start(
|
||||
executable: '/tmp/clide-no-such-binary-${DateTime.now().microsecondsSinceEpoch}',
|
||||
arguments: const [],
|
||||
columns: 80,
|
||||
rows: 24,
|
||||
workingDirectory: '/',
|
||||
environment: {
|
||||
...Platform.environment,
|
||||
'TERM': 'xterm-256color',
|
||||
},
|
||||
test('non-existent executable surfaces a PtyException at spawn time', () {
|
||||
// posix_spawn surfaces exec-time errors as a non-zero return on
|
||||
// glibc (which uses vfork — the child is suspended until execve
|
||||
// either succeeds or fails). ENOENT (errno 2) for missing binary.
|
||||
expect(
|
||||
() => NativePty.start(
|
||||
executable: '/tmp/clide-no-such-binary-${DateTime.now().microsecondsSinceEpoch}',
|
||||
arguments: const [],
|
||||
columns: 80,
|
||||
rows: 24,
|
||||
workingDirectory: '/',
|
||||
environment: {
|
||||
...Platform.environment,
|
||||
'TERM': 'xterm-256color',
|
||||
},
|
||||
),
|
||||
throwsA(isA<PtyException>().having((e) => e.errno, 'errno', 2)),
|
||||
);
|
||||
addTearDown(s.close);
|
||||
final buf = StringBuffer();
|
||||
final done = Completer<void>();
|
||||
s.output.listen(
|
||||
(b) => buf.write(utf8.decode(b, allowMalformed: true)),
|
||||
onDone: () {
|
||||
if (!done.isCompleted) done.complete();
|
||||
},
|
||||
);
|
||||
await done.future.timeout(const Duration(seconds: 5), onTimeout: () {});
|
||||
expect(buf.toString(), contains('exec failed'));
|
||||
});
|
||||
|
||||
test('resize on a live PTY does not throw', () async {
|
||||
|
||||
Reference in New Issue
Block a user