From c1a58233cdff48da9fa4dd51ca718473faad9039 Mon Sep 17 00:00:00 2001 From: AhmedAdelWafdy7 Date: Fri, 24 Jul 2026 16:48:55 +0000 Subject: [PATCH 1/9] add flatpak apps lifecycle to launch and stop apps --- .github/workflows/ci.yml | 2 + example/launch_app.dart | 35 ++++++++ example/list_running.dart | 22 +++++ lib/flatpak_dart.dart | 1 + lib/src/ffi/bindings.dart | 63 +++++++++++++ lib/src/ffi/codec.dart | 15 ++++ lib/src/flatpak_client.dart | 24 +++++ lib/src/installation.dart | 92 +++++++++++++++++++ lib/src/instance.dart | 35 ++++++++ native/include/flatpak_bridge.h | 10 +++ native/include/flatpak_types.h | 22 +++++ native/include/installation_reader.h | 6 ++ native/src/installation_reader.cpp | 128 +++++++++++++++++++++++++++ native/src/transaction_bridge.cpp | 2 +- native/test/test_types.cpp | 27 ++++++ test/instance_test.dart | 54 +++++++++++ 16 files changed, 537 insertions(+), 1 deletion(-) create mode 100644 example/launch_app.dart create mode 100644 example/list_running.dart create mode 100644 lib/src/instance.dart create mode 100644 test/instance_test.dart diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dde0d42..9bfe166 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -190,6 +190,8 @@ jobs: path: build-cov/coverage_html/ retention-days: 14 - uses: romeovs/lcov-reporter-action@v0.3.1 + # never fail the job on the comment step + continue-on-error: true with: lcov-file: build-cov/coverage.info github-token: ${{ secrets.GITHUB_TOKEN }} diff --git a/example/launch_app.dart b/example/launch_app.dart new file mode 100644 index 0000000..ae61c39 --- /dev/null +++ b/example/launch_app.dart @@ -0,0 +1,35 @@ +// launch_app.dart — launch an installed app, list running instances, then stop it. +// Run as: dart run example/launch_app.dart [app_id] +// Defaults to org.gnome.Calculator. + +import 'package:flatpak_dart/flatpak_dart.dart'; + +Future main(List args) async { + final appId = args.isNotEmpty ? args.first : 'org.gnome.Calculator'; + final client = FlatpakClient.user(); + + print('Launching $appId ...'); + await client.launch(appId); + + // Give the sandbox a moment to register its instance. + await Future.delayed(const Duration(seconds: 1)); + + final running = await client.listRunning(); + print('${running.length} running instance(s):'); + for (final inst in running) { + print(' ${inst.appId.padRight(36)} ' + 'instance=${inst.instanceId} pid=${inst.pid} ' + 'running=${inst.isRunning}'); + } + + final isUp = running.any((i) => i.appId == appId && i.isRunning); + if (isUp) { + print('\nStopping $appId ...'); + await client.stop(appId); + print('Stopped.'); + } else { + print('\n$appId did not report a running instance (nothing to stop).'); + } + + await client.close(); +} diff --git a/example/list_running.dart b/example/list_running.dart new file mode 100644 index 0000000..f02bf24 --- /dev/null +++ b/example/list_running.dart @@ -0,0 +1,22 @@ +// list_running.dart — show currently running Flatpak app instances. +// Run as: dart run example/list_running.dart + +import 'package:flatpak_dart/flatpak_dart.dart'; + +Future main() async { + final client = FlatpakClient.user(); + final running = await client.listRunning(); + + if (running.isEmpty) { + print('No running instances.'); + } else { + print('${running.length} running instance(s):'); + for (final i in running) { + print(' ${i.appId.padRight(36)} ' + 'instance=${i.instanceId} pid=${i.pid} ' + 'child=${i.childPid} running=${i.isRunning}'); + } + } + + await client.close(); +} diff --git a/lib/flatpak_dart.dart b/lib/flatpak_dart.dart index fc4aa29..529a129 100644 --- a/lib/flatpak_dart.dart +++ b/lib/flatpak_dart.dart @@ -8,6 +8,7 @@ library; export 'src/application.dart'; export 'src/exceptions.dart'; export 'src/flatpak_client.dart'; +export 'src/instance.dart'; export 'src/known_remotes.dart'; export 'src/permissions.dart'; export 'src/remote.dart'; diff --git a/lib/src/ffi/bindings.dart b/lib/src/ffi/bindings.dart index a3cabc2..4e6c77a 100644 --- a/lib/src/ffi/bindings.dart +++ b/lib/src/ffi/bindings.dart @@ -104,6 +104,35 @@ external void _readerFetchRemoteMetadata( Pointer ref, ); +@Native< + Void Function( + Pointer, + Int64, + Pointer, + Pointer, + Pointer, + Pointer, + ) +>(symbol: 'flatpak_reader_launch') +external void _readerLaunch( + Pointer handle, + int port, + Pointer appId, + Pointer arch, + Pointer branch, + Pointer commit, +); + +@Native, Int64, Pointer)>( + symbol: 'flatpak_reader_stop', +) +external void _readerStop(Pointer handle, int port, Pointer appId); + +@Native, Int64)>( + symbol: 'flatpak_reader_list_running', +) +external void _readerListRunning(Pointer handle, int port); + // ── Worker ────────────────────────────────────────────────────────────────── @Native Function(Pointer)>(symbol: 'flatpak_worker_create') @@ -394,6 +423,40 @@ abstract final class FlatpakBindings { } } + static void readerLaunch( + Pointer handle, + int port, + String appId, + String arch, + String branch, + String commit, + ) { + final id = appId.toNativeUtf8(); + final a = arch.toNativeUtf8(); + final b = branch.toNativeUtf8(); + final c = commit.toNativeUtf8(); + try { + _readerLaunch(handle, port, id, a, b, c); + } finally { + calloc.free(id); + calloc.free(a); + calloc.free(b); + calloc.free(c); + } + } + + static void readerStop(Pointer handle, int port, String appId) { + final id = appId.toNativeUtf8(); + try { + _readerStop(handle, port, id); + } finally { + calloc.free(id); + } + } + + static void readerListRunning(Pointer handle, int port) => + _readerListRunning(handle, port); + // ── Worker ───────────────────────────────────────────────────────────── static Pointer workerCreate(String installation) { _ensureInitialized(); diff --git a/lib/src/ffi/codec.dart b/lib/src/ffi/codec.dart index ad4e2a4..eb9e9be 100644 --- a/lib/src/ffi/codec.dart +++ b/lib/src/ffi/codec.dart @@ -11,6 +11,7 @@ import 'dart:convert'; import 'dart:typed_data'; import '../application.dart'; +import '../instance.dart'; import '../remote.dart'; // ── Binary reader ────────────────────────────────────────────────────────── @@ -147,6 +148,20 @@ abstract final class GlazeCodec { ); } + static FlatpakInstance decodeInstance(Uint8List data, int offset) { + final r = _BinaryReader(data, offset); + return FlatpakInstance( + appId: r.readString(), + instanceId: r.readString(), + arch: r.readString(), + branch: r.readString(), + commit: r.readString(), + pid: r.readInt32(), + childPid: r.readInt32(), + isRunning: r.readBool(), + ); + } + // ── FlatpakRemoteInfo field order: name, url, title, comment, description, // homepage, defaultBranch, subset, collectionId, filter, // remoteType(i32), disabled(bool), gpgVerify(bool), noDeps(bool), diff --git a/lib/src/flatpak_client.dart b/lib/src/flatpak_client.dart index e2679fd..729eb82 100644 --- a/lib/src/flatpak_client.dart +++ b/lib/src/flatpak_client.dart @@ -17,6 +17,7 @@ import 'application.dart'; import 'ffi/bindings.dart'; import 'ffi/codec.dart' show MetadataEntry; import 'installation.dart'; +import 'instance.dart'; import 'permissions.dart'; import 'remote.dart'; import 'remote_manager.dart'; @@ -75,6 +76,29 @@ class FlatpakClient { /// Check which installed applications have updates available. Future> checkForUpdates() => _installation.checkForUpdates(); + // ── App lifecycle (libflatpak launch / instances) ────────────────────── + + /// Launch an installed application in its sandbox ("tap to open"). + /// Pass empty [arch]/[branch]/[commit] to use the installed defaults. + Future launch( + String appId, { + String arch = '', + String branch = '', + String commit = '', + }) => + _installation.launch( + appId, + arch: arch, + branch: branch, + commit: commit, + ); + + /// Stop every running instance of [appId]. + Future stop(String appId) => _installation.stop(appId); + + /// List running sandbox instances across the host. + Future> listRunning() => _installation.listRunning(); + // ── Write operations (libflatpak FlatpakTransaction serial queue) ─────── /// Install an application from a remote. diff --git a/lib/src/installation.dart b/lib/src/installation.dart index 25af09d..980110b 100644 --- a/lib/src/installation.dart +++ b/lib/src/installation.dart @@ -10,6 +10,7 @@ import 'application.dart'; import 'exceptions.dart'; import 'ffi/bindings.dart'; import 'ffi/codec.dart'; +import 'instance.dart'; import 'permissions.dart'; import 'remote.dart'; @@ -277,6 +278,97 @@ class FlatpakInstallation { return completer.future; } + /// Launch an installed application in its sandbox. + /// Completes when the sandbox has been spawned (non-blocking on the app). + Future launch( + String appId, { + String arch = '', + String branch = '', + String commit = '', + }) async { + final port = ReceivePort('flatpak.launch'); + final completer = Completer(); + + port.listen((dynamic msg) { + if (msg is! Uint8List) return; + switch (msg[0]) { + case 0x02: + final err = GlazeCodec.decodeError(msg, 1); + if (!completer.isCompleted) { + completer.completeError(FlatpakNotFoundException(err)); + } + port.close(); + case 0xFF: + if (!completer.isCompleted) completer.complete(); + port.close(); + } + }); + + FlatpakBindings.readerLaunch( + _handle, + port.sendPort.nativePort, + appId, + arch, + branch, + commit, + ); + return completer.future; + } + + /// Terminate every running instance of [appId]. + /// Throws [FlatpakNotFoundException] if no running instance was found. + Future stop(String appId) async { + final port = ReceivePort('flatpak.stop'); + final completer = Completer(); + + port.listen((dynamic msg) { + if (msg is! Uint8List) return; + switch (msg[0]) { + case 0x02: + final err = GlazeCodec.decodeError(msg, 1); + if (!completer.isCompleted) { + completer.completeError(FlatpakNotFoundException(err)); + } + port.close(); + case 0xFF: + if (!completer.isCompleted) completer.complete(); + port.close(); + } + }); + + FlatpakBindings.readerStop(_handle, port.sendPort.nativePort, appId); + return completer.future; + } + + /// List running sandbox instances across the host. + Future> listRunning() async { + final port = ReceivePort('flatpak.listRunning'); + final completer = Completer>(); + final results = []; + + port.listen((dynamic msg) { + if (msg is! Uint8List) return; + switch (msg[0]) { + case 0x01: + if (msg.length > 1) { + results.add(GlazeCodec.decodeInstance(msg, 1)); + } + case 0x02: + final err = GlazeCodec.decodeError(msg, 1); + if (!completer.isCompleted) { + completer.completeError(FlatpakNotFoundException(err)); + } + port.close(); + case 0xFF: + if (!completer.isCompleted) completer.complete(results); + port.close(); + } + }); + + FlatpakBindings.readerListRunning(_handle, port.sendPort.nativePort); + return completer.future; + } + void close() { FlatpakBindings.readerDestroy(_handle); } diff --git a/lib/src/instance.dart b/lib/src/instance.dart new file mode 100644 index 0000000..a5a1fdb --- /dev/null +++ b/lib/src/instance.dart @@ -0,0 +1,35 @@ +class FlatpakInstance { + /// The application id, e.g. `org.gnome.Calculator`. + final String appId; + + /// The unique instance id assigned by flatpak for this run. + final String instanceId; + + final String arch; + final String branch; + final String commit; + + /// The outermost (bubblewrap) process pid. + final int pid; + + /// The application process pid inside the sandbox. + final int childPid; + + /// Whether the instance is still running. + final bool isRunning; + + const FlatpakInstance({ + required this.appId, + required this.instanceId, + this.arch = '', + this.branch = '', + this.commit = '', + this.pid = 0, + this.childPid = 0, + this.isRunning = false, + }); + + @override + String toString() => + 'FlatpakInstance($appId, instance=$instanceId, pid=$pid, running=$isRunning)'; +} diff --git a/native/include/flatpak_bridge.h b/native/include/flatpak_bridge.h index 3f8fd6f..6aeffe1 100644 --- a/native/include/flatpak_bridge.h +++ b/native/include/flatpak_bridge.h @@ -42,6 +42,16 @@ void flatpak_reader_check_updates(void* handle, Dart_Port port); // Posts the raw metadata string as a 0x01 payload, then 0xFF sentinel. void flatpak_reader_fetch_remote_metadata(void* handle, Dart_Port port, const char* remote, const char* ref); +// Launch an installed app via flatpak_installation_launch(). Non-blocking: +// spawns the sandbox and posts 0xFF on success or 0x02 on error. +void flatpak_reader_launch(void* handle, Dart_Port port, const char* app_id, const char* arch, + const char* branch, const char* commit); +// Terminate every running instance matching app_id (SIGTERM to the bwrap pid). +// Posts 0xFF if at least one was signalled, otherwise 0x02. +void flatpak_reader_stop(void* handle, Dart_Port port, const char* app_id); +// List running sandbox instances (FlatpakInstance) via flatpak_instance_get_all(). +// Posts each as a 0x01 FpInstance payload, then the 0xFF sentinel. +void flatpak_reader_list_running(void* handle, Dart_Port port); // Invalidate cached data so next list call returns fresh results. void flatpak_reader_drop_caches(void* handle); diff --git a/native/include/flatpak_types.h b/native/include/flatpak_types.h index 309bdf8..0be7918 100644 --- a/native/include/flatpak_types.h +++ b/native/include/flatpak_types.h @@ -159,3 +159,25 @@ struct glz::meta { glz::field("bytesTotal", &TransactionProgress::bytesTotal), glz::field("status", &TransactionProgress::status)); }; + +// A running sandboxed application instance (FlatpakInstance). +struct FpInstance { + std::string appId; + std::string instanceId; + std::string arch; + std::string branch; + std::string commit; + int32_t pid{}; // outermost (bubblewrap) pid + int32_t childPid{}; // application process pid + bool isRunning{}; +}; + +template <> +struct glz::meta { + static constexpr auto fields = std::make_tuple( + glz::field("appId", &FpInstance::appId), glz::field("instanceId", &FpInstance::instanceId), + glz::field("arch", &FpInstance::arch), glz::field("branch", &FpInstance::branch), + glz::field("commit", &FpInstance::commit), glz::field("pid", &FpInstance::pid), + glz::field("childPid", &FpInstance::childPid), + glz::field("isRunning", &FpInstance::isRunning)); +}; diff --git a/native/include/installation_reader.h b/native/include/installation_reader.h index 92183a8..ed858cf 100644 --- a/native/include/installation_reader.h +++ b/native/include/installation_reader.h @@ -21,6 +21,12 @@ class InstallationReader { void get_permissions(Dart_Port port, const char* app_id); void check_updates(Dart_Port port); void fetch_remote_metadata(Dart_Port port, const char* remote, const char* ref); + // Launch/stop/list are actions + // flatpak_installation_launch() spawns and returns immediately. + void launch(Dart_Port port, const char* app_id, const char* arch, const char* branch, + const char* commit); + void stop(Dart_Port port, const char* app_id); + void list_running(Dart_Port port); void drop_caches(); private: diff --git a/native/src/installation_reader.cpp b/native/src/installation_reader.cpp index 4dca47d..ecca9de 100644 --- a/native/src/installation_reader.cpp +++ b/native/src/installation_reader.cpp @@ -4,6 +4,9 @@ #include "installation_reader.h" +#include +#include + #include #include "flatpak_bridge.h" @@ -302,6 +305,118 @@ void InstallationReader::fetch_remote_metadata(Dart_Port port, const char* remot post_sentinel(port); } +void InstallationReader::launch(Dart_Port port, const char* app_id, const char* arch, + const char* branch, const char* commit) { + const char* use_arch = (arch && *arch) ? arch : nullptr; + const char* use_branch = (branch && *branch) ? branch : nullptr; + const char* use_commit = (commit && *commit) ? commit : nullptr; + + g_autofree char* resolved_arch = nullptr; + g_autofree char* resolved_branch = nullptr; + if (!use_arch || !use_branch) { + g_autoptr(GError) lerr = nullptr; + g_autoptr(GPtrArray) refs = + flatpak_installation_list_installed_refs(installation_, nullptr, &lerr); + if (refs) { + for (guint i = 0; i < refs->len; i++) { + auto* iref = static_cast(refs->pdata[i]); + if (flatpak_ref_get_kind(FLATPAK_REF(iref)) != FLATPAK_REF_KIND_APP) { + continue; + } + if (g_strcmp0(flatpak_ref_get_name(FLATPAK_REF(iref)), app_id) != 0) { + continue; + } + const char* a = flatpak_ref_get_arch(FLATPAK_REF(iref)); + if (use_arch && g_strcmp0(a, use_arch) != 0) { + continue; // honour a caller-supplied arch filter + } + resolved_arch = g_strdup(a); + resolved_branch = g_strdup(flatpak_ref_get_branch(FLATPAK_REF(iref))); + break; + } + } + if (!resolved_branch) { + post_error(port, "app not installed"); + return; + } + if (!use_arch) { + use_arch = resolved_arch; + } + if (!use_branch) { + use_branch = resolved_branch; + } + } + + g_autoptr(GError) err = nullptr; + g_autoptr(FlatpakInstance) instance = nullptr; + + gboolean ok = flatpak_installation_launch_full(installation_, FLATPAK_LAUNCH_FLAGS_DO_NOT_REAP, + app_id, use_arch, use_branch, use_commit, + &instance, nullptr, &err); + if (!ok) { + post_error(port, err ? err->message : "launch failed"); + return; + } + post_sentinel(port); +} + +void InstallationReader::stop(Dart_Port port, const char* app_id) { + g_autoptr(GPtrArray) instances = flatpak_instance_get_all(); + bool found = false; + if (instances) { + for (guint i = 0; i < instances->len; i++) { + auto* inst = static_cast(instances->pdata[i]); + if (g_strcmp0(flatpak_instance_get_app(inst), app_id) != 0) { + continue; + } + int child_pid = flatpak_instance_get_child_pid(inst); + if (child_pid <= 0 || kill(child_pid, 0) != 0) { + continue; // no live app process for this instance + } + found = true; + kill(child_pid, SIGTERM); + bool exited = false; + for (int k = 0; k < 15; k++) { // up to ~1.5s grace period + if (kill(child_pid, 0) != 0) { + exited = true; + break; + } + usleep(100000); // 100ms + } + if (!exited) { + kill(child_pid, SIGKILL); + } + } + } + if (!found) { + post_error(port, "no running instance for app_id"); + return; + } + post_sentinel(port); +} + +void InstallationReader::list_running(Dart_Port port) { + g_autoptr(GPtrArray) instances = flatpak_instance_get_all(); + if (!instances) { + post_sentinel(port); + return; + } + for (guint i = 0; i < instances->len; i++) { + auto* inst = static_cast(instances->pdata[i]); + FpInstance info; + info.appId = safe_str(flatpak_instance_get_app(inst)); + info.instanceId = safe_str(flatpak_instance_get_id(inst)); + info.arch = safe_str(flatpak_instance_get_arch(inst)); + info.branch = safe_str(flatpak_instance_get_branch(inst)); + info.commit = safe_str(flatpak_instance_get_commit(inst)); + info.pid = flatpak_instance_get_pid(inst); + info.childPid = flatpak_instance_get_child_pid(inst); + info.isRunning = flatpak_instance_is_running(inst); + post_glaze(port, 0x01, info); + } + post_sentinel(port); +} + void InstallationReader::drop_caches() { g_autoptr(GError) err = nullptr; flatpak_installation_drop_caches(installation_, nullptr, &err); @@ -372,6 +487,19 @@ void flatpak_reader_fetch_remote_metadata(void* handle, Dart_Port port, const ch static_cast(handle)->fetch_remote_metadata(port, remote, ref); } +void flatpak_reader_launch(void* handle, Dart_Port port, const char* app_id, const char* arch, + const char* branch, const char* commit) { + static_cast(handle)->launch(port, app_id, arch, branch, commit); +} + +void flatpak_reader_stop(void* handle, Dart_Port port, const char* app_id) { + static_cast(handle)->stop(port, app_id); +} + +void flatpak_reader_list_running(void* handle, Dart_Port port) { + static_cast(handle)->list_running(port); +} + void flatpak_reader_drop_caches(void* handle) { static_cast(handle)->drop_caches(); } diff --git a/native/src/transaction_bridge.cpp b/native/src/transaction_bridge.cpp index 09e145e..7fa8d9f 100644 --- a/native/src/transaction_bridge.cpp +++ b/native/src/transaction_bridge.cpp @@ -90,7 +90,7 @@ static void on_new_operation(FlatpakTransaction*, FlatpakTransactionOperation* o post_glaze(ctx->port, 0x10, p); }), ctx, [](gpointer data, GClosure*) { delete static_cast(data); }, - G_CONNECT_DEFAULT); + static_cast(0)); } static void on_operation_done(FlatpakTransaction*, FlatpakTransactionOperation* op, diff --git a/native/test/test_types.cpp b/native/test/test_types.cpp index 87400a8..a0f75f4 100644 --- a/native/test/test_types.cpp +++ b/native/test/test_types.cpp @@ -94,6 +94,33 @@ TEST(TransactionProgress, RoundtripBasic) { EXPECT_EQ(decoded.status, orig.status); } +TEST(FpInstance, RoundtripFull) { + FpInstance orig; + orig.appId = "org.gnome.Calculator"; + orig.instanceId = "42"; + orig.arch = "x86_64"; + orig.branch = "stable"; + orig.commit = "deadbeef"; + orig.pid = 1234; + orig.childPid = 1240; + orig.isRunning = true; + + std::vector buf; + buf = glz::write_binary(orig); + + FpInstance decoded; + glz::read_binary(buf, decoded); + + EXPECT_EQ(decoded.appId, orig.appId); + EXPECT_EQ(decoded.instanceId, orig.instanceId); + EXPECT_EQ(decoded.arch, orig.arch); + EXPECT_EQ(decoded.branch, orig.branch); + EXPECT_EQ(decoded.commit, orig.commit); + EXPECT_EQ(decoded.pid, orig.pid); + EXPECT_EQ(decoded.childPid, orig.childPid); + EXPECT_EQ(decoded.isRunning, orig.isRunning); +} + TEST(TransactionProgress, ZeroBytesTotal) { TransactionProgress orig; orig.op = "install"; diff --git a/test/instance_test.dart b/test/instance_test.dart new file mode 100644 index 0000000..519f223 --- /dev/null +++ b/test/instance_test.dart @@ -0,0 +1,54 @@ +@TestOn('vm') +library instance_test; + +import 'package:flatpak_dart/flatpak_dart.dart'; +import 'package:test/test.dart'; + +void main() { + group('FlatpakInstance model', () { + test('is exported and constructs', () { + const inst = FlatpakInstance( + appId: 'org.gnome.Calculator', + instanceId: '42', + pid: 1234, + childPid: 1240, + isRunning: true, + ); + expect(inst.appId, 'org.gnome.Calculator'); + expect(inst.instanceId, '42'); + expect(inst.pid, 1234); + expect(inst.childPid, 1240); + expect(inst.isRunning, isTrue); + }); + + test('defaults are sensible', () { + const inst = FlatpakInstance(appId: 'a', instanceId: 'b'); + expect(inst.arch, ''); + expect(inst.branch, ''); + expect(inst.commit, ''); + expect(inst.pid, 0); + expect(inst.childPid, 0); + expect(inst.isRunning, isFalse); + }); + + test('toString includes app id and pid', () { + const inst = FlatpakInstance(appId: 'org.x.Y', instanceId: '1', pid: 99); + expect(inst.toString(), contains('org.x.Y')); + expect(inst.toString(), contains('99')); + }); + }); + + group('FlatpakClient lifecycle API surface', () { + // Signature-only checks — do not invoke the native bridge. + test('launch is a method on FlatpakClient', () { + expect(FlatpakClient.system, isA()); + // launch/stop/listRunning are instance methods; verified by tear-off + // type below without constructing a client (which needs libflatpak). + }); + + test('FlatpakInstance list type is usable', () { + const list = []; + expect(list, isA>()); + }); + }); +} From c04c519833479e3c3602ad630ed3698ee39bc07c Mon Sep 17 00:00:00 2001 From: AhmedAdelWafdy7 Date: Thu, 6 Aug 2026 14:40:03 +0000 Subject: [PATCH 2/9] refactor launch_app.dart to simplify app launch process; enhance FlatpakClient and Installation classes with improved error handling and instance management --- .pubignore | 39 +++++++++ example/launch_app.dart | 28 ++---- hook/build.dart | 135 +++++++++++++++-------------- lib/src/exceptions.dart | 4 + lib/src/flatpak_client.dart | 6 +- lib/src/installation.dart | 29 ++++++- native/include/flatpak_bridge.h | 2 + native/src/installation_reader.cpp | 68 ++++++++++++--- scripts/build_release.sh | 11 ++- 9 files changed, 215 insertions(+), 107 deletions(-) create mode 100644 .pubignore diff --git a/.pubignore b/.pubignore new file mode 100644 index 0000000..389d77e --- /dev/null +++ b/.pubignore @@ -0,0 +1,39 @@ +# Files excluded from the pub.dev archive but kept in git. +# When this file is present, pub uses it instead of falling back to +# .gitignore, so every build/dev artifact must be re-listed here. + +# Build artifacts (any out-of-source CMake build dir) +build/ +build-*/ +.cache/ +*.o +*.a +*.so + +# CMake scratch +CMakeCache.txt +CMakeFiles/ +cmake_install.cmake +compile_commands.json + +# Coverage +coverage/ +coverage_html/ +*.gcno +*.gcda +lcov.info + +# Flutter example sub-package — has its own pubspec and hundreds of files +# (linux/ runner, generated plugin registrant); not needed by consumers. +example/flutter_remote_manager/ + +# IDE / editor / OS junk +.idea/ +*.iml +.vscode/ +*.swp +.DS_Store +Thumbs.db + +# Claude Code state +.claude/ diff --git a/example/launch_app.dart b/example/launch_app.dart index ae61c39..1e18c22 100644 --- a/example/launch_app.dart +++ b/example/launch_app.dart @@ -1,4 +1,4 @@ -// launch_app.dart — launch an installed app, list running instances, then stop it. +// launch_app.dart — launch an installed app, then stop it. // Run as: dart run example/launch_app.dart [app_id] // Defaults to org.gnome.Calculator. @@ -9,27 +9,15 @@ Future main(List args) async { final client = FlatpakClient.user(); print('Launching $appId ...'); - await client.launch(appId); + final instance = await client.launch(appId); + print('instance=${instance.instanceId} pid=${instance.pid} ' + 'childPid=${instance.childPid}'); - // Give the sandbox a moment to register its instance. - await Future.delayed(const Duration(seconds: 1)); + print('\nStopping $appId ...'); + await client.stop(appId); + print('Stopped (SIGTERM sent; waiting up to 2 seconds for exit)'); - final running = await client.listRunning(); - print('${running.length} running instance(s):'); - for (final inst in running) { - print(' ${inst.appId.padRight(36)} ' - 'instance=${inst.instanceId} pid=${inst.pid} ' - 'running=${inst.isRunning}'); - } - - final isUp = running.any((i) => i.appId == appId && i.isRunning); - if (isUp) { - print('\nStopping $appId ...'); - await client.stop(appId); - print('Stopped.'); - } else { - print('\n$appId did not report a running instance (nothing to stop).'); - } + await Future.delayed(const Duration(seconds: 2)); await client.close(); } diff --git a/hook/build.dart b/hook/build.dart index 7888d05..c57018d 100644 --- a/hook/build.dart +++ b/hook/build.dart @@ -1,9 +1,3 @@ -// hook/build.dart — Native asset build hook for flatpak_dart. -// -// Automatically builds libflatpak_nc.so via CMake when the package is -// consumed as a dependency. Requires: cmake, clang (or gcc), pkg-config, -// libflatpak-dev, libglib2.0-dev. - import 'dart:io'; import 'package:code_assets/code_assets.dart'; @@ -11,87 +5,94 @@ import 'package:hooks/hooks.dart'; void main(List args) async { await build(args, (input, output) async { - final packageRoot = input.packageRoot; - final nativeDir = packageRoot.resolve('native/'); - final buildDir = input.outputDirectory.resolve('native_build/'); - final buildDirPath = buildDir.toFilePath(); + if (!input.config.buildCodeAssets) return; - // Ensure build directory exists - await Directory(buildDirPath).create(recursive: true); + // Allow an embedder/build system + final skipDefine = input.userDefines['skip_native_build']; + if (skipDefine == true || skipDefine == 'true') { + stderr.writeln( + 'skip_native_build user-define set — skipping native build.', + ); + return; + } - // Find a C++ compiler - final cxx = _findCompiler(['clang++-19', 'clang++', 'g++']); - final cc = _findCompiler(['clang-19', 'clang', 'gcc']); + final nativeDir = input.packageRoot.resolve('native/').toFilePath(); + final buildDir = + input.outputDirectory.resolve('native_build/').toFilePath(); - // Run CMake configure - final configResult = await Process.run('cmake', [ - '-B', - buildDirPath, - '-S', - nativeDir.toFilePath(), - '-DCMAKE_BUILD_TYPE=Release', - if (cxx != null) '-DCMAKE_CXX_COMPILER=$cxx', - if (cc != null) '-DCMAKE_C_COMPILER=$cc', - ]); - if (configResult.exitCode != 0) { - throw Exception('CMake configure failed:\n${configResult.stderr}'); - } + await Directory(buildDir).create(recursive: true); - // Run CMake build - final cpuCount = Platform.numberOfProcessors; - final buildResult = await Process.run('cmake', [ - '--build', - buildDirPath, - '--parallel', - '$cpuCount', - ]); - if (buildResult.exitCode != 0) { - throw Exception('CMake build failed:\n${buildResult.stderr}'); - } + final hasNinja = await _which('ninja'); - // Add source files as dependencies for rebuild detection - output.dependencies.add(nativeDir.resolve('CMakeLists.txt')); - for (final uri in await _globSources(nativeDir.resolve('src/'))) { - output.dependencies.add(uri); + if (!File('${buildDir}CMakeCache.txt').existsSync()) { + await _run('cmake', [ + '-S', + nativeDir, + '-B', + buildDir, + '-DCMAKE_BUILD_TYPE=Release', + if (hasNinja) ...['-G', 'Ninja'], + ]); } - for (final uri in await _globSources(nativeDir.resolve('include/'))) { - output.dependencies.add(uri); + + await _run('cmake', ['--build', buildDir, '--parallel']); + + final libFile = File('${buildDir}libflatpak_nc.so'); + if (!libFile.existsSync()) { + throw StateError('libflatpak_nc.so not found at ${libFile.path}'); } - // Register the built shared library as a code asset - final soPath = buildDir.resolve('libflatpak_nc.so'); output.assets.code.add( CodeAsset( package: input.packageName, name: 'libflatpak_nc.so', - file: soPath, linkMode: DynamicLoadingBundled(), + file: libFile.uri, ), ); + + // Re-run the hook whenever any C/C++ source or CMake file changes. + for (final dir in ['src', 'include']) { + final d = Directory('$nativeDir$dir'); + if (!d.existsSync()) continue; + for (final entity in d.listSync(recursive: true)) { + if (entity is! File) continue; + final p = entity.path; + if (p.endsWith('.cpp') || + p.endsWith('.cc') || + p.endsWith('.c') || + p.endsWith('.hpp') || + p.endsWith('.h')) { + output.dependencies.add(entity.uri); + } + } + } + output.dependencies.add(Uri.file('${nativeDir}CMakeLists.txt')); + + stderr.writeln('libflatpak_nc built: ${libFile.path}'); }); } -String? _findCompiler(List candidates) { - for (final name in candidates) { - final result = Process.runSync('sh', ['-c', 'command -v $name']); - if (result.exitCode == 0) { - return (result.stdout as String).trim(); - } +const _clearedFlagVars = {'CFLAGS': '', 'CXXFLAGS': '', 'LDFLAGS': ''}; + +Future _run(String exe, List args) async { + final p = await Process.start( + exe, + args, + mode: ProcessStartMode.inheritStdio, + environment: _clearedFlagVars, + ); + final code = await p.exitCode; + if (code != 0) { + throw ProcessException(exe, args, 'exit code $code', code); } - return null; } -Future> _globSources(Uri dir) async { - final directory = Directory.fromUri(dir); - if (!await directory.exists()) return []; - final files = []; - await for (final entity in directory.list(recursive: true)) { - if (entity is File) { - final path = entity.path; - if (path.endsWith('.cpp') || path.endsWith('.c') || path.endsWith('.h')) { - files.add(entity.uri); - } - } +Future _which(String exe) async { + try { + final r = await Process.run('sh', ['-c', 'command -v $exe']); + return r.exitCode == 0; + } catch (_) { + return false; } - return files; } diff --git a/lib/src/exceptions.dart b/lib/src/exceptions.dart index 51f57ae..db574d9 100644 --- a/lib/src/exceptions.dart +++ b/lib/src/exceptions.dart @@ -43,3 +43,7 @@ final class FlatpakNotFoundException extends FlatpakException { final class FlatpakRemoteException extends FlatpakException { const FlatpakRemoteException(super.message); } + +final class FlatpakLaunchException extends FlatpakException { + const FlatpakLaunchException(super.message); +} diff --git a/lib/src/flatpak_client.dart b/lib/src/flatpak_client.dart index 729eb82..67b3ef3 100644 --- a/lib/src/flatpak_client.dart +++ b/lib/src/flatpak_client.dart @@ -80,7 +80,8 @@ class FlatpakClient { /// Launch an installed application in its sandbox ("tap to open"). /// Pass empty [arch]/[branch]/[commit] to use the installed defaults. - Future launch( + /// Returns the [FlatpakInstance] libflatpak created for the launch. + Future launch( String appId, { String arch = '', String branch = '', @@ -93,7 +94,8 @@ class FlatpakClient { commit: commit, ); - /// Stop every running instance of [appId]. + /// Stop every running instance of [appId]. Returns once SIGTERM has been + /// sent; grace period + SIGKILL escalation continue in the background. Future stop(String appId) => _installation.stop(appId); /// List running sandbox instances across the host. diff --git a/lib/src/installation.dart b/lib/src/installation.dart index 980110b..a509c25 100644 --- a/lib/src/installation.dart +++ b/lib/src/installation.dart @@ -280,26 +280,48 @@ class FlatpakInstallation { /// Launch an installed application in its sandbox. /// Completes when the sandbox has been spawned (non-blocking on the app). - Future launch( + /// Returns the [FlatpakInstance] libflatpak created for the launch, so + /// callers get the instanceId/pid immediately instead of polling [listRunning]. + Future launch( String appId, { String arch = '', String branch = '', String commit = '', }) async { final port = ReceivePort('flatpak.launch'); - final completer = Completer(); + final completer = Completer(); + FlatpakInstance? result; port.listen((dynamic msg) { if (msg is! Uint8List) return; switch (msg[0]) { + case 0x01: + if (msg.length > 1) { + result = GlazeCodec.decodeInstance(msg, 1); + } case 0x02: final err = GlazeCodec.decodeError(msg, 1); if (!completer.isCompleted) { completer.completeError(FlatpakNotFoundException(err)); } port.close(); + case 0x03: + final err = GlazeCodec.decodeError(msg, 1); + if (!completer.isCompleted) { + completer.completeError(FlatpakLaunchException(err)); + } + port.close(); case 0xFF: - if (!completer.isCompleted) completer.complete(); + if (!completer.isCompleted) { + final instance = result; + if (instance != null) { + completer.complete(instance); + } else { + completer.completeError( + const FlatpakLaunchException('launch produced no instance'), + ); + } + } port.close(); } }); @@ -316,6 +338,7 @@ class FlatpakInstallation { } /// Terminate every running instance of [appId]. + /// Returns as soon as SIGTERM has been sent to every matched instance. /// Throws [FlatpakNotFoundException] if no running instance was found. Future stop(String appId) async { final port = ReceivePort('flatpak.stop'); diff --git a/native/include/flatpak_bridge.h b/native/include/flatpak_bridge.h index 6aeffe1..4521fce 100644 --- a/native/include/flatpak_bridge.h +++ b/native/include/flatpak_bridge.h @@ -6,6 +6,8 @@ // Message discriminator byte at offset 0: // 0x01 = success / list-end sentinel // 0x02 = error (UTF-8, uint32_t length-prefix) +// 0x03 = launch error (UTF-8, uint32_t length-prefix) — a launch_full() +// failure, distinct from the 0x02 "app not installed" pre-check. // 0x10 = TransactionProgress (glaze-encoded, in-flight during tx_run) // 0x11 = UpdateAvailable (FlatpakMonitor inotify signal) // 0xFF = streaming list end sentinel diff --git a/native/src/installation_reader.cpp b/native/src/installation_reader.cpp index ecca9de..2e5f29f 100644 --- a/native/src/installation_reader.cpp +++ b/native/src/installation_reader.cpp @@ -5,6 +5,7 @@ #include "installation_reader.h" #include +#include #include #include @@ -30,10 +31,26 @@ static void post_error(Dart_Port port, const char* msg) { flatpak_nc::post_framed_error(port, 0x02, msg); } +static void post_launch_error(Dart_Port port, const char* msg) { + flatpak_nc::post_framed_error(port, 0x03, msg); +} + static const char* safe_str(const char* s) { return s ? s : ""; } +static gpointer reap_thread(gpointer data) { + auto pid = static_cast(GPOINTER_TO_INT(data)); + int status = 0; + waitpid(pid, &status, 0); + return nullptr; +} + +static void reap_async(GPid pid) { + GThread* t = g_thread_new("flatpak-reap", reap_thread, GINT_TO_POINTER(pid)); + g_thread_unref(t); +} + // ── InstallationReader ────────────────────────────────────────────────────── InstallationReader::InstallationReader(FlatpakInstallation* inst) @@ -354,12 +371,49 @@ void InstallationReader::launch(Dart_Port port, const char* app_id, const char* app_id, use_arch, use_branch, use_commit, &instance, nullptr, &err); if (!ok) { - post_error(port, err ? err->message : "launch failed"); + post_launch_error(port, err ? err->message : "launch failed"); return; } + + auto outer_pid = static_cast(flatpak_instance_get_pid(instance)); + if (outer_pid > 0) { + reap_async(outer_pid); + } + + FpInstance info; + info.appId = safe_str(flatpak_instance_get_app(instance)); + info.instanceId = safe_str(flatpak_instance_get_id(instance)); + info.arch = safe_str(flatpak_instance_get_arch(instance)); + info.branch = safe_str(flatpak_instance_get_branch(instance)); + info.commit = safe_str(flatpak_instance_get_commit(instance)); + info.pid = flatpak_instance_get_pid(instance); + info.childPid = flatpak_instance_get_child_pid(instance); + info.isRunning = flatpak_instance_is_running(instance); + + post_glaze(port, 0x01, info); post_sentinel(port); } +// Grace period + SIGKILL escalation for stop(), on a detached background thread +static gpointer stop_escalate_thread(gpointer data) { + auto pid = static_cast(GPOINTER_TO_INT(data)); + for (int k = 0; k < 15; k++) { // ~1.5s grace period + if (kill(pid, 0) != 0) { + return nullptr; // already exited + } + usleep(100000); // 100ms + } + if (kill(pid, 0) == 0) { + kill(pid, SIGKILL); + } + return nullptr; +} + +static void stop_escalate_async(pid_t pid) { + GThread* t = g_thread_new("flatpak-stop", stop_escalate_thread, GINT_TO_POINTER(pid)); + g_thread_unref(t); +} + void InstallationReader::stop(Dart_Port port, const char* app_id) { g_autoptr(GPtrArray) instances = flatpak_instance_get_all(); bool found = false; @@ -375,17 +429,7 @@ void InstallationReader::stop(Dart_Port port, const char* app_id) { } found = true; kill(child_pid, SIGTERM); - bool exited = false; - for (int k = 0; k < 15; k++) { // up to ~1.5s grace period - if (kill(child_pid, 0) != 0) { - exited = true; - break; - } - usleep(100000); // 100ms - } - if (!exited) { - kill(child_pid, SIGKILL); - } + stop_escalate_async(child_pid); } } if (!found) { diff --git a/scripts/build_release.sh b/scripts/build_release.sh index aceb451..ca2cf9f 100755 --- a/scripts/build_release.sh +++ b/scripts/build_release.sh @@ -1,16 +1,21 @@ #!/usr/bin/env bash # Build the flatpak_nc shared library in Release mode. # Usage: ./scripts/build_release.sh [build-dir] +# +# The compiler is left to CMake's default (the same toolchain the target's +# libflatpak/glib were built against) so the C++ runtimes match at load time. +# Override with CC/CXX if you need a specific toolchain, e.g.: +# CC=clang-19 CXX=clang++-19 ./scripts/build_release.sh set -euo pipefail BUILD_DIR="${1:-build-release}" CPU_COUNT="$(nproc 2>/dev/null || echo 4)" NATIVE_DIR="$(cd "$(dirname "$0")/../native" && pwd)" echo "=== Building Release ===" -cmake -B "$BUILD_DIR" "$NATIVE_DIR" \ +env -u CFLAGS -u CXXFLAGS -u LDFLAGS cmake -B "$BUILD_DIR" "$NATIVE_DIR" \ -DCMAKE_BUILD_TYPE=Release \ - -DCMAKE_CXX_COMPILER=clang++-19 \ - -DCMAKE_C_COMPILER=clang-19 + ${CXX:+-DCMAKE_CXX_COMPILER="$CXX"} \ + ${CC:+-DCMAKE_C_COMPILER="$CC"} cmake --build "$BUILD_DIR" --parallel "$CPU_COUNT" echo "=== Built: $BUILD_DIR/libflatpak_nc.so ===" echo "" From 4b6f7dceb50e09ffdf53059b68b31c623010106d Mon Sep 17 00:00:00 2001 From: AhmedAdelWafdy7 Date: Mon, 10 Aug 2026 23:06:52 +0000 Subject: [PATCH 3/9] enhance InstallationReader with asynchronous launch and improved process management --- .github/workflows/ci.yml | 12 +- .github/workflows/coverage.yml | 2 +- .gitignore | 1 + hook/build.dart | 19 +++- native/include/installation_reader.h | 13 ++- native/src/installation_reader.cpp | 161 +++++++++++++++++++-------- 6 files changed, 149 insertions(+), 59 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9bfe166..e0295e8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -34,7 +34,7 @@ jobs: sudo apt-get install -y --no-install-recommends \ cmake ninja-build \ clang-19 clang-tidy-19 clang-format-19 \ - libflatpak-dev libglib2.0-dev \ + libflatpak-dev libglib2.0-dev libasio-dev \ libgtest-dev gcovr - uses: dart-lang/setup-dart@v1 with: { sdk: stable } @@ -65,7 +65,7 @@ jobs: sudo apt-get update -qq sudo apt-get install -y --no-install-recommends \ cmake ninja-build clang-19 \ - libflatpak-dev libglib2.0-dev libgtest-dev + libflatpak-dev libglib2.0-dev libasio-dev libgtest-dev - uses: dart-lang/setup-dart@v1 with: { sdk: stable } - run: dart pub get @@ -93,7 +93,7 @@ jobs: dnf install -y \ git cmake ninja-build \ clang clang-tools-extra \ - flatpak-devel glib2-devel \ + flatpak-devel glib2-devel asio-devel \ gtest-devel unzip curl - uses: actions/checkout@v4 - name: Install Dart SDK @@ -126,7 +126,7 @@ jobs: dnf install -y \ git cmake ninja-build \ clang clang-tools-extra \ - flatpak-devel glib2-devel \ + flatpak-devel glib2-devel asio-devel \ gtest-devel unzip curl - uses: actions/checkout@v4 - name: Install Dart SDK (arm64) @@ -156,7 +156,7 @@ jobs: sudo apt-get update -qq sudo apt-get install -y --no-install-recommends \ cmake ninja-build clang-19 libclang-rt-19-dev \ - libflatpak-dev libglib2.0-dev libgtest-dev + libflatpak-dev libglib2.0-dev libasio-dev libgtest-dev - run: ./scripts/asan.sh build-asan # ── Coverage (PR only) ─────────────────────────────────────────────── @@ -177,7 +177,7 @@ jobs: sudo apt-get update -qq sudo apt-get install -y --no-install-recommends \ cmake ninja-build gcc g++ \ - libflatpak-dev libglib2.0-dev \ + libflatpak-dev libglib2.0-dev libasio-dev \ libgtest-dev gcovr - uses: dart-lang/setup-dart@v1 with: { sdk: stable } diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index f23e037..c028d43 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -15,7 +15,7 @@ jobs: sudo apt-get update -qq sudo apt-get install -y --no-install-recommends \ cmake ninja-build gcc g++ \ - libflatpak-dev libglib2.0-dev \ + libflatpak-dev libglib2.0-dev libasio-dev \ libgtest-dev gcovr - uses: dart-lang/setup-dart@v1 with: { sdk: stable } diff --git a/.gitignore b/.gitignore index 372a676..742aa40 100644 --- a/.gitignore +++ b/.gitignore @@ -14,6 +14,7 @@ build*/ .idea/ *.iml .vscode/ +.cache/ # CMake CMakeCache.txt diff --git a/hook/build.dart b/hook/build.dart index c57018d..4eea17f 100644 --- a/hook/build.dart +++ b/hook/build.dart @@ -16,6 +16,9 @@ void main(List args) async { return; } + final clearDefine = input.userDefines['clear_ambient_flags']; + final clearAmbientFlags = clearDefine == true || clearDefine == 'true'; + final nativeDir = input.packageRoot.resolve('native/').toFilePath(); final buildDir = input.outputDirectory.resolve('native_build/').toFilePath(); @@ -32,10 +35,14 @@ void main(List args) async { buildDir, '-DCMAKE_BUILD_TYPE=Release', if (hasNinja) ...['-G', 'Ninja'], - ]); + ], clearAmbientFlags: clearAmbientFlags); } - await _run('cmake', ['--build', buildDir, '--parallel']); + await _run('cmake', [ + '--build', + buildDir, + '--parallel', + ], clearAmbientFlags: clearAmbientFlags); final libFile = File('${buildDir}libflatpak_nc.so'); if (!libFile.existsSync()) { @@ -75,12 +82,16 @@ void main(List args) async { const _clearedFlagVars = {'CFLAGS': '', 'CXXFLAGS': '', 'LDFLAGS': ''}; -Future _run(String exe, List args) async { +Future _run( + String exe, + List args, { + required bool clearAmbientFlags, +}) async { final p = await Process.start( exe, args, mode: ProcessStartMode.inheritStdio, - environment: _clearedFlagVars, + environment: clearAmbientFlags ? _clearedFlagVars : null, ); final code = await p.exitCode; if (code != 0) { diff --git a/native/include/installation_reader.h b/native/include/installation_reader.h index ed858cf..19977af 100644 --- a/native/include/installation_reader.h +++ b/native/include/installation_reader.h @@ -1,9 +1,11 @@ // InstallationReader — read-only libflatpak query bridge. -// All operations use flatpak_installation_* and run on the caller's thread. // Results are posted to the Dart_Port passed per-call. #pragma once #include +#include +#include + #include "dart_api_dl.h" #include "flatpak_types.h" @@ -21,8 +23,6 @@ class InstallationReader { void get_permissions(Dart_Port port, const char* app_id); void check_updates(Dart_Port port); void fetch_remote_metadata(Dart_Port port, const char* remote, const char* ref); - // Launch/stop/list are actions - // flatpak_installation_launch() spawns and returns immediately. void launch(Dart_Port port, const char* app_id, const char* arch, const char* branch, const char* commit); void stop(Dart_Port port, const char* app_id); @@ -31,4 +31,11 @@ class InstallationReader { private: FlatpakInstallation* installation_; + + asio::io_context launch_io_; + asio::executor_work_guard launch_work_guard_; + std::thread launch_thread_; + + void launch_impl(Dart_Port port, const char* app_id, const char* arch, const char* branch, + const char* commit); }; diff --git a/native/src/installation_reader.cpp b/native/src/installation_reader.cpp index 2e5f29f..b946296 100644 --- a/native/src/installation_reader.cpp +++ b/native/src/installation_reader.cpp @@ -4,11 +4,14 @@ #include "installation_reader.h" +#include #include +#include #include #include #include +#include #include "flatpak_bridge.h" #include "flatpak_post.h" @@ -51,13 +54,19 @@ static void reap_async(GPid pid) { g_thread_unref(t); } -// ── InstallationReader ────────────────────────────────────────────────────── +// InstallationReader InstallationReader::InstallationReader(FlatpakInstallation* inst) - : installation_(static_cast(g_object_ref(inst))) { + : installation_(static_cast(g_object_ref(inst))), + launch_work_guard_(asio::make_work_guard(launch_io_)), + launch_thread_([this] { launch_io_.run(); }) { } InstallationReader::~InstallationReader() { + launch_io_.stop(); + if (launch_thread_.joinable()) { + launch_thread_.join(); + } g_object_unref(installation_); } @@ -322,45 +331,90 @@ void InstallationReader::fetch_remote_metadata(Dart_Port port, const char* remot post_sentinel(port); } +static bool resolve_launch_target(FlatpakInstallation* installation, const char* app_id, + const char* hint_arch, const char* hint_branch, + std::string* out_arch, std::string* out_branch) { + g_autoptr(GError) cerr = nullptr; + g_autoptr(FlatpakInstalledRef) current = + flatpak_installation_get_current_installed_app(installation, app_id, nullptr, &cerr); + if (current) { + const char* a = flatpak_ref_get_arch(FLATPAK_REF(current)); + const char* b = flatpak_ref_get_branch(FLATPAK_REF(current)); + bool arch_ok = !hint_arch || g_strcmp0(a, hint_arch) == 0; + bool branch_ok = !hint_branch || g_strcmp0(b, hint_branch) == 0; + if (arch_ok && branch_ok) { + *out_arch = a; + *out_branch = b; + return true; + } + } + + const char* default_arch = flatpak_get_default_arch(); + g_autoptr(GError) lerr = nullptr; + g_autoptr(GPtrArray) refs = + flatpak_installation_list_installed_refs(installation, nullptr, &lerr); + if (!refs) { + return false; + } + bool found = false; + for (guint i = 0; i < refs->len; i++) { + auto* iref = static_cast(refs->pdata[i]); + if (flatpak_ref_get_kind(FLATPAK_REF(iref)) != FLATPAK_REF_KIND_APP) { + continue; + } + if (g_strcmp0(flatpak_ref_get_name(FLATPAK_REF(iref)), app_id) != 0) { + continue; + } + const char* a = flatpak_ref_get_arch(FLATPAK_REF(iref)); + const char* b = flatpak_ref_get_branch(FLATPAK_REF(iref)); + if (hint_arch && g_strcmp0(a, hint_arch) != 0) { + continue; + } + if (hint_branch && g_strcmp0(b, hint_branch) != 0) { + continue; + } + if (!found || (!hint_arch && g_strcmp0(a, default_arch) == 0)) { + *out_arch = a; + *out_branch = b; + found = true; + } + if (g_strcmp0(a, default_arch) == 0) { + break; // can't do better than the default arch + } + } + return found; +} + void InstallationReader::launch(Dart_Port port, const char* app_id, const char* arch, const char* branch, const char* commit) { + std::string appIdStr = safe_str(app_id); + std::string archStr = safe_str(arch); + std::string branchStr = safe_str(branch); + std::string commitStr = safe_str(commit); + asio::post(launch_io_, [this, port, appIdStr, archStr, branchStr, commitStr]() { + launch_impl(port, appIdStr.c_str(), archStr.c_str(), branchStr.c_str(), commitStr.c_str()); + }); +} + +void InstallationReader::launch_impl(Dart_Port port, const char* app_id, const char* arch, + const char* branch, const char* commit) { const char* use_arch = (arch && *arch) ? arch : nullptr; const char* use_branch = (branch && *branch) ? branch : nullptr; const char* use_commit = (commit && *commit) ? commit : nullptr; - g_autofree char* resolved_arch = nullptr; - g_autofree char* resolved_branch = nullptr; + std::string resolved_arch; + std::string resolved_branch; if (!use_arch || !use_branch) { - g_autoptr(GError) lerr = nullptr; - g_autoptr(GPtrArray) refs = - flatpak_installation_list_installed_refs(installation_, nullptr, &lerr); - if (refs) { - for (guint i = 0; i < refs->len; i++) { - auto* iref = static_cast(refs->pdata[i]); - if (flatpak_ref_get_kind(FLATPAK_REF(iref)) != FLATPAK_REF_KIND_APP) { - continue; - } - if (g_strcmp0(flatpak_ref_get_name(FLATPAK_REF(iref)), app_id) != 0) { - continue; - } - const char* a = flatpak_ref_get_arch(FLATPAK_REF(iref)); - if (use_arch && g_strcmp0(a, use_arch) != 0) { - continue; // honour a caller-supplied arch filter - } - resolved_arch = g_strdup(a); - resolved_branch = g_strdup(flatpak_ref_get_branch(FLATPAK_REF(iref))); - break; - } - } - if (!resolved_branch) { + if (!resolve_launch_target(installation_, app_id, use_arch, use_branch, &resolved_arch, + &resolved_branch)) { post_error(port, "app not installed"); return; } if (!use_arch) { - use_arch = resolved_arch; + use_arch = resolved_arch.c_str(); } if (!use_branch) { - use_branch = resolved_branch; + use_branch = resolved_branch.c_str(); } } @@ -371,7 +425,11 @@ void InstallationReader::launch(Dart_Port port, const char* app_id, const char* app_id, use_arch, use_branch, use_commit, &instance, nullptr, &err); if (!ok) { - post_launch_error(port, err ? err->message : "launch failed"); + if (err && err->domain == FLATPAK_ERROR && err->code == FLATPAK_ERROR_NOT_INSTALLED) { + post_error(port, err->message); + } else { + post_launch_error(port, err ? err->message : "launch failed"); + } return; } @@ -394,23 +452,32 @@ void InstallationReader::launch(Dart_Port port, const char* app_id, const char* post_sentinel(port); } -// Grace period + SIGKILL escalation for stop(), on a detached background thread +// pidfd-based signalling: a pidfd refers to the exact process instance it was opened for, so we can +// send signals to it even if the PID has been recycled. +static int pidfd_open_compat(pid_t pid) { + return static_cast(syscall(SYS_pidfd_open, pid, 0)); +} + +static int pidfd_send_signal_compat(int pidfd, int sig) { + return static_cast(syscall(SYS_pidfd_send_signal, pidfd, sig, nullptr, 0)); +} + +// Grace period + SIGKILL escalation for stop(), on a detached background thread. We don't want to +// block the Dart thread waiting for the app to exit, and we don't want to leave a stray process if +// it ignores SIGTERM. static gpointer stop_escalate_thread(gpointer data) { - auto pid = static_cast(GPOINTER_TO_INT(data)); - for (int k = 0; k < 15; k++) { // ~1.5s grace period - if (kill(pid, 0) != 0) { - return nullptr; // already exited - } - usleep(100000); // 100ms - } - if (kill(pid, 0) == 0) { - kill(pid, SIGKILL); + auto pidfd = GPOINTER_TO_INT(data); + struct pollfd pfd = {.fd = pidfd, .events = POLLIN, .revents = 0}; + int ret = poll(&pfd, 1, 1500); // ~1.5s grace period + if (ret == 0) { + pidfd_send_signal_compat(pidfd, SIGKILL); } + close(pidfd); return nullptr; } -static void stop_escalate_async(pid_t pid) { - GThread* t = g_thread_new("flatpak-stop", stop_escalate_thread, GINT_TO_POINTER(pid)); +static void stop_escalate_async(int pidfd) { + GThread* t = g_thread_new("flatpak-stop", stop_escalate_thread, GINT_TO_POINTER(pidfd)); g_thread_unref(t); } @@ -424,12 +491,16 @@ void InstallationReader::stop(Dart_Port port, const char* app_id) { continue; } int child_pid = flatpak_instance_get_child_pid(inst); - if (child_pid <= 0 || kill(child_pid, 0) != 0) { - continue; // no live app process for this instance + if (child_pid <= 0) { + continue; + } + int pidfd = pidfd_open_compat(child_pid); + if (pidfd < 0) { + continue; // ESRCH: no live app process for this instance } found = true; - kill(child_pid, SIGTERM); - stop_escalate_async(child_pid); + pidfd_send_signal_compat(pidfd, SIGTERM); + stop_escalate_async(pidfd); // thread takes ownership, closes it } } if (!found) { From ef5623d2d6da8aaa40cc21f0b832bb5ddbe907bd Mon Sep 17 00:00:00 2001 From: AhmedAdelWafdy7 Date: Fri, 14 Aug 2026 11:43:20 +0000 Subject: [PATCH 4/9] refactor InstallationReader to improve resource management and enhance signal handling during app termination --- native/src/installation_reader.cpp | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/native/src/installation_reader.cpp b/native/src/installation_reader.cpp index b946296..89cbf4d 100644 --- a/native/src/installation_reader.cpp +++ b/native/src/installation_reader.cpp @@ -10,6 +10,8 @@ #include #include +#include +#include #include #include @@ -63,7 +65,7 @@ InstallationReader::InstallationReader(FlatpakInstallation* inst) } InstallationReader::~InstallationReader() { - launch_io_.stop(); + launch_work_guard_.reset(); if (launch_thread_.joinable()) { launch_thread_.join(); } @@ -468,7 +470,22 @@ static int pidfd_send_signal_compat(int pidfd, int sig) { static gpointer stop_escalate_thread(gpointer data) { auto pidfd = GPOINTER_TO_INT(data); struct pollfd pfd = {.fd = pidfd, .events = POLLIN, .revents = 0}; - int ret = poll(&pfd, 1, 1500); // ~1.5s grace period + constexpr int kGraceMs = 1500; + auto deadline = std::chrono::steady_clock::now() + std::chrono::milliseconds(kGraceMs); + int ret = 0; + for (;;) { + auto remaining = std::chrono::duration_cast( + deadline - std::chrono::steady_clock::now()); + int timeout_ms = remaining.count() > 0 ? static_cast(remaining.count()) : 0; + pfd.revents = 0; + ret = poll(&pfd, 1, timeout_ms); + if (ret >= 0 || errno != EINTR) { + break; + } + if (timeout_ms == 0) { + break; + } + } if (ret == 0) { pidfd_send_signal_compat(pidfd, SIGKILL); } @@ -496,7 +513,12 @@ void InstallationReader::stop(Dart_Port port, const char* app_id) { } int pidfd = pidfd_open_compat(child_pid); if (pidfd < 0) { - continue; // ESRCH: no live app process for this instance + if (errno != ESRCH) { + if (kill(child_pid, SIGTERM) == 0) { + found = true; + } + } + continue; } found = true; pidfd_send_signal_compat(pidfd, SIGTERM); From 4dbd79627c4f03992cf68992025415e8b42f4dd9 Mon Sep 17 00:00:00 2001 From: AhmedAdelWafdy7 Date: Sat, 15 Aug 2026 00:36:09 +0000 Subject: [PATCH 5/9] Apply dart format to flatpak_client.dart --- lib/src/flatpak_client.dart | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/lib/src/flatpak_client.dart b/lib/src/flatpak_client.dart index 67b3ef3..0b5b2cd 100644 --- a/lib/src/flatpak_client.dart +++ b/lib/src/flatpak_client.dart @@ -86,13 +86,7 @@ class FlatpakClient { String arch = '', String branch = '', String commit = '', - }) => - _installation.launch( - appId, - arch: arch, - branch: branch, - commit: commit, - ); + }) => _installation.launch(appId, arch: arch, branch: branch, commit: commit); /// Stop every running instance of [appId]. Returns once SIGTERM has been /// sent; grace period + SIGKILL escalation continue in the background. From de7ce0af72919f8fd18222ade2f5d475b73bad69 Mon Sep 17 00:00:00 2001 From: Joel Winarske Date: Tue, 25 Aug 2026 07:40:21 -0700 Subject: [PATCH 6/9] Address launcher review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drop asio in favour of the serial-queue pattern already used by TransactionWorker (std::thread + mutex + condvar + queue). asio was pulling in a system dependency for a single one-op queue that had to be satisfied by four CI distros and every consumer sysroot, and it was never wired into native/CMakeLists.txt — configure succeeded and compile failed cold for anyone building from source. Removing it also means no `DEPENDS += asio` is needed in the meta-flutter recipe. Destruction order is preserved: drain the queue, join, then unref, so a launch already queued still runs against a live installation_. Fix header doc drift in flatpak_bridge.h: launch uses launch_full() with DO_NOT_REAP and posts 0x01 FpInstance + 0xFF on success, 0x02 for not-installed and 0x03 for launch failure; stop signals the sandboxed app process (child pid), not the outer bwrap pid. Also: - TODO on reap_async: one parked thread per running app should become a single epoll'd reaper multiplexing pidfds. - Document that stop() is host-wide (flatpak instances are not scoped to an installation) on both FlatpakClient.stop and Installation.stop. - Comment the non-pidfd kill() fallback as intentionally un-escalated. - Replace the no-op "launch is a method" test with real compile-time signature checks, and pin the FpInstance wire format with a golden buffer emitted by the C++ writer and decoded by GlazeCodec. - Correct the grace period in example/launch_app.dart (1.5s, not 2s). - Note the defensive 0x02 branch in listRunning. - Restore the section divider rule style in installation_reader.cpp. Signed-off-by: Joel Winarske --- .github/workflows/ci.yml | 12 ++-- .github/workflows/coverage.yml | 2 +- example/launch_app.dart | 2 +- lib/src/flatpak_client.dart | 9 ++- lib/src/installation.dart | 8 ++- native/include/flatpak_bridge.h | 13 +++- native/include/installation_reader.h | 20 +++++- native/src/installation_reader.cpp | 59 ++++++++++++++--- test/instance_test.dart | 94 +++++++++++++++++++++++++--- 9 files changed, 185 insertions(+), 34 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e0295e8..9bfe166 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -34,7 +34,7 @@ jobs: sudo apt-get install -y --no-install-recommends \ cmake ninja-build \ clang-19 clang-tidy-19 clang-format-19 \ - libflatpak-dev libglib2.0-dev libasio-dev \ + libflatpak-dev libglib2.0-dev \ libgtest-dev gcovr - uses: dart-lang/setup-dart@v1 with: { sdk: stable } @@ -65,7 +65,7 @@ jobs: sudo apt-get update -qq sudo apt-get install -y --no-install-recommends \ cmake ninja-build clang-19 \ - libflatpak-dev libglib2.0-dev libasio-dev libgtest-dev + libflatpak-dev libglib2.0-dev libgtest-dev - uses: dart-lang/setup-dart@v1 with: { sdk: stable } - run: dart pub get @@ -93,7 +93,7 @@ jobs: dnf install -y \ git cmake ninja-build \ clang clang-tools-extra \ - flatpak-devel glib2-devel asio-devel \ + flatpak-devel glib2-devel \ gtest-devel unzip curl - uses: actions/checkout@v4 - name: Install Dart SDK @@ -126,7 +126,7 @@ jobs: dnf install -y \ git cmake ninja-build \ clang clang-tools-extra \ - flatpak-devel glib2-devel asio-devel \ + flatpak-devel glib2-devel \ gtest-devel unzip curl - uses: actions/checkout@v4 - name: Install Dart SDK (arm64) @@ -156,7 +156,7 @@ jobs: sudo apt-get update -qq sudo apt-get install -y --no-install-recommends \ cmake ninja-build clang-19 libclang-rt-19-dev \ - libflatpak-dev libglib2.0-dev libasio-dev libgtest-dev + libflatpak-dev libglib2.0-dev libgtest-dev - run: ./scripts/asan.sh build-asan # ── Coverage (PR only) ─────────────────────────────────────────────── @@ -177,7 +177,7 @@ jobs: sudo apt-get update -qq sudo apt-get install -y --no-install-recommends \ cmake ninja-build gcc g++ \ - libflatpak-dev libglib2.0-dev libasio-dev \ + libflatpak-dev libglib2.0-dev \ libgtest-dev gcovr - uses: dart-lang/setup-dart@v1 with: { sdk: stable } diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index c028d43..f23e037 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -15,7 +15,7 @@ jobs: sudo apt-get update -qq sudo apt-get install -y --no-install-recommends \ cmake ninja-build gcc g++ \ - libflatpak-dev libglib2.0-dev libasio-dev \ + libflatpak-dev libglib2.0-dev \ libgtest-dev gcovr - uses: dart-lang/setup-dart@v1 with: { sdk: stable } diff --git a/example/launch_app.dart b/example/launch_app.dart index 1e18c22..3533541 100644 --- a/example/launch_app.dart +++ b/example/launch_app.dart @@ -15,7 +15,7 @@ Future main(List args) async { print('\nStopping $appId ...'); await client.stop(appId); - print('Stopped (SIGTERM sent; waiting up to 2 seconds for exit)'); + print('Stopped (SIGTERM sent; SIGKILL follows after a 1.5s grace period)'); await Future.delayed(const Duration(seconds: 2)); diff --git a/lib/src/flatpak_client.dart b/lib/src/flatpak_client.dart index 0b5b2cd..27cbe38 100644 --- a/lib/src/flatpak_client.dart +++ b/lib/src/flatpak_client.dart @@ -88,8 +88,13 @@ class FlatpakClient { String commit = '', }) => _installation.launch(appId, arch: arch, branch: branch, commit: commit); - /// Stop every running instance of [appId]. Returns once SIGTERM has been - /// sent; grace period + SIGKILL escalation continue in the background. + /// Stop every running instance of [appId] across the host. Flatpak + /// instances are not scoped to an installation, so this stops matching + /// instances regardless of whether they were launched from the user or + /// system installation. + /// + /// Returns once SIGTERM has been sent; grace period + SIGKILL escalation + /// continue in the background. Future stop(String appId) => _installation.stop(appId); /// List running sandbox instances across the host. diff --git a/lib/src/installation.dart b/lib/src/installation.dart index a509c25..9093a19 100644 --- a/lib/src/installation.dart +++ b/lib/src/installation.dart @@ -337,7 +337,9 @@ class FlatpakInstallation { return completer.future; } - /// Terminate every running instance of [appId]. + /// Terminate every running instance of [appId] across the host — flatpak + /// instances are not scoped to an installation, so instances launched from + /// the other installation are matched too. /// Returns as soon as SIGTERM has been sent to every matched instance. /// Throws [FlatpakNotFoundException] if no running instance was found. Future stop(String appId) async { @@ -364,6 +366,10 @@ class FlatpakInstallation { } /// List running sandbox instances across the host. + /// + /// The native side only ever posts 0x01 payloads and the 0xFF sentinel here; + /// the 0x02 branch below is a defensive guard so an unexpected error frame + /// completes the future instead of leaving the caller hanging. Future> listRunning() async { final port = ReceivePort('flatpak.listRunning'); final completer = Completer>(); diff --git a/native/include/flatpak_bridge.h b/native/include/flatpak_bridge.h index 4521fce..5ef3bd8 100644 --- a/native/include/flatpak_bridge.h +++ b/native/include/flatpak_bridge.h @@ -44,11 +44,18 @@ void flatpak_reader_check_updates(void* handle, Dart_Port port); // Posts the raw metadata string as a 0x01 payload, then 0xFF sentinel. void flatpak_reader_fetch_remote_metadata(void* handle, Dart_Port port, const char* remote, const char* ref); -// Launch an installed app via flatpak_installation_launch(). Non-blocking: -// spawns the sandbox and posts 0xFF on success or 0x02 on error. +// Launch an installed app via flatpak_installation_launch_full() with +// FLATPAK_LAUNCH_FLAGS_DO_NOT_REAP. Returns immediately; the launch runs on the +// reader's serial launch thread. On success posts the resulting FlatpakInstance +// as a 0x01 FpInstance payload followed by the 0xFF sentinel. On failure posts +// 0x02 if the app is not installed, or 0x03 if launch_full() itself failed. void flatpak_reader_launch(void* handle, Dart_Port port, const char* app_id, const char* arch, const char* branch, const char* commit); -// Terminate every running instance matching app_id (SIGTERM to the bwrap pid). +// Terminate every running instance matching app_id, host-wide — flatpak +// instances are not scoped to an installation, so this also matches instances +// launched from the other installation. Signals the sandboxed app process +// (FlatpakInstance child pid), not the outer bwrap pid, so the app sees the +// SIGTERM and bwrap follows it down. // Posts 0xFF if at least one was signalled, otherwise 0x02. void flatpak_reader_stop(void* handle, Dart_Port port, const char* app_id); // List running sandbox instances (FlatpakInstance) via flatpak_instance_get_all(). diff --git a/native/include/installation_reader.h b/native/include/installation_reader.h index 19977af..9c4a0bd 100644 --- a/native/include/installation_reader.h +++ b/native/include/installation_reader.h @@ -3,7 +3,10 @@ #pragma once #include -#include +#include +#include +#include +#include #include #include "dart_api_dl.h" @@ -32,10 +35,21 @@ class InstallationReader { private: FlatpakInstallation* installation_; - asio::io_context launch_io_; - asio::executor_work_guard launch_work_guard_; + // ── Launch queue — one dedicated thread, serial ────────────────────── + // flatpak_installation_launch_full() blocks while bubblewrap sets the + // sandbox up, so it must not run on the Dart thread. Same shape as + // TransactionWorker (transaction_bridge.h), deliberately built on the + // standard library rather than an async runtime: every other thread in + // this bridge is a plain std::thread or GThread, and adding a system + // dependency for one serial queue would have to be satisfied by every + // consumer sysroot as well as CI. std::thread launch_thread_; + std::mutex launch_mu_; + std::condition_variable launch_cv_; + std::queue> launch_queue_; + bool launch_stop_{false}; + void launch_loop(); void launch_impl(Dart_Port port, const char* app_id, const char* arch, const char* branch, const char* commit); }; diff --git a/native/src/installation_reader.cpp b/native/src/installation_reader.cpp index 89cbf4d..a571c99 100644 --- a/native/src/installation_reader.cpp +++ b/native/src/installation_reader.cpp @@ -44,6 +44,16 @@ static const char* safe_str(const char* s) { return s ? s : ""; } +// FLATPAK_LAUNCH_FLAGS_DO_NOT_REAP leaves the bwrap process as our child, so +// something in this process has to waitpid() it or it becomes a zombie for the +// lifetime of the host app. waitpid(-1) would be wrong in a library — it would +// steal exit statuses from whatever else the embedder has spawned — so we wait +// on the specific pid. +// +// TODO: this parks one thread per launched app for that app's entire lifetime. +// Replace with a single reaper thread multiplexing pidfds via epoll, reaping +// with waitpid(pid, &status, WNOHANG) on POLLIN, which scales to N running apps +// with one thread. static gpointer reap_thread(gpointer data) { auto pid = static_cast(GPOINTER_TO_INT(data)); int status = 0; @@ -56,22 +66,43 @@ static void reap_async(GPid pid) { g_thread_unref(t); } -// InstallationReader +// ── InstallationReader ────────────────────────────────────────────────────── InstallationReader::InstallationReader(FlatpakInstallation* inst) - : installation_(static_cast(g_object_ref(inst))), - launch_work_guard_(asio::make_work_guard(launch_io_)), - launch_thread_([this] { launch_io_.run(); }) { + : installation_(static_cast(g_object_ref(inst))) { + launch_thread_ = std::thread(&InstallationReader::launch_loop, this); } InstallationReader::~InstallationReader() { - launch_work_guard_.reset(); + // Drain, then join, then unref: a launch already queued still gets to run + // (and post its reply) against a live installation_ before we let go of it. + { + std::lock_guard lk(launch_mu_); + launch_stop_ = true; + } + launch_cv_.notify_one(); if (launch_thread_.joinable()) { launch_thread_.join(); } g_object_unref(installation_); } +void InstallationReader::launch_loop() { + for (;;) { + std::function job; + { + std::unique_lock lk(launch_mu_); + launch_cv_.wait(lk, [&] { return launch_stop_ || !launch_queue_.empty(); }); + if (launch_queue_.empty()) { + return; // stopping and drained + } + job = std::move(launch_queue_.front()); + launch_queue_.pop(); + } + job(); + } +} + void InstallationReader::list_apps(Dart_Port port, bool include_runtimes) { g_autoptr(GError) err = nullptr; g_autoptr(GPtrArray) refs = @@ -393,9 +424,14 @@ void InstallationReader::launch(Dart_Port port, const char* app_id, const char* std::string archStr = safe_str(arch); std::string branchStr = safe_str(branch); std::string commitStr = safe_str(commit); - asio::post(launch_io_, [this, port, appIdStr, archStr, branchStr, commitStr]() { - launch_impl(port, appIdStr.c_str(), archStr.c_str(), branchStr.c_str(), commitStr.c_str()); - }); + { + std::lock_guard lk(launch_mu_); + launch_queue_.emplace([this, port, appIdStr, archStr, branchStr, commitStr]() { + launch_impl(port, appIdStr.c_str(), archStr.c_str(), branchStr.c_str(), + commitStr.c_str()); + }); + } + launch_cv_.notify_one(); } void InstallationReader::launch_impl(Dart_Port port, const char* app_id, const char* arch, @@ -513,6 +549,13 @@ void InstallationReader::stop(Dart_Port port, const char* app_id) { } int pidfd = pidfd_open_compat(child_pid); if (pidfd < 0) { + // ESRCH means the app already exited — nothing to signal. + // Any other failure (ENOSYS on pre-5.3 kernels, EMFILE, a + // seccomp filter) falls back to a plain SIGTERM. Intentionally + // no SIGKILL escalation here: without a pidfd we cannot wait + // for the exit without racing pid reuse, and the supported + // targets (kernel >= 5.10) always take the pidfd path. An app + // that ignores SIGTERM on such a kernel survives stop(). if (errno != ESRCH) { if (kill(child_pid, SIGTERM) == 0) { found = true; diff --git a/test/instance_test.dart b/test/instance_test.dart index 519f223..1ffe1ab 100644 --- a/test/instance_test.dart +++ b/test/instance_test.dart @@ -1,9 +1,53 @@ @TestOn('vm') -library instance_test; +library; + +import 'dart:typed_data'; import 'package:flatpak_dart/flatpak_dart.dart'; +import 'package:flatpak_dart/src/ffi/codec.dart'; import 'package:test/test.dart'; +// ── Compile-time signature checks ────────────────────────────────────────── +// These closures only type-check if FlatpakClient's lifecycle methods have +// exactly these shapes, so a signature change breaks the build rather than +// silently passing a runtime `isA()` assertion. They are never +// invoked — constructing a FlatpakClient would dlopen libflatpak. +Future _launch(FlatpakClient c, String appId) => + c.launch(appId); + +Future _launchWithRef(FlatpakClient c, String appId) => + c.launch(appId, arch: 'x86_64', branch: 'stable', commit: 'deadbeef'); + +Future _stop(FlatpakClient c, String appId) => c.stop(appId); + +Future> _listRunning(FlatpakClient c) => c.listRunning(); + +// Golden wire-format buffer for FpInstance, emitted by the C++ writer +// (glz::write_binary, native/include/glaze_meta.h) for: +// appId="org.gnome.Calculator" instanceId="42" arch="x86_64" +// branch="stable" commit="deadbeef" pid=1234 childPid=1240 isRunning=true +// Byte 0 is the 0x01 payload discriminator the native side frames it with. +// BEVE-Lite layout: strings are uint64-LE length + UTF-8 bytes, int32 is 4 +// bytes LE, bool is 1 byte. If this test fails, the C++ and Dart sides of the +// wire format have diverged — fix the codec, do not re-bless the bytes. +final _goldenFpInstance = Uint8List.fromList([ + 0x01, // + 0x14, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x6f, 0x72, 0x67, 0x2e, 0x67, 0x6e, 0x6f, 0x6d, 0x65, 0x2e, + 0x43, 0x61, 0x6c, 0x63, 0x75, 0x6c, 0x61, 0x74, 0x6f, 0x72, + 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x34, 0x32, + 0x06, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x78, 0x38, 0x36, 0x5f, 0x36, 0x34, + 0x06, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x73, 0x74, 0x61, 0x62, 0x6c, 0x65, + 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x64, 0x65, 0x61, 0x64, 0x62, 0x65, 0x65, 0x66, + 0xd2, 0x04, 0x00, 0x00, // pid = 1234 + 0xd8, 0x04, 0x00, 0x00, // childPid = 1240 + 0x01, // isRunning = true +]); + void main() { group('FlatpakInstance model', () { test('is exported and constructs', () { @@ -39,16 +83,48 @@ void main() { }); group('FlatpakClient lifecycle API surface', () { - // Signature-only checks — do not invoke the native bridge. - test('launch is a method on FlatpakClient', () { - expect(FlatpakClient.system, isA()); - // launch/stop/listRunning are instance methods; verified by tear-off - // type below without constructing a client (which needs libflatpak). + test('launch returns a Future from an app id', () { + expect( + _launch, + isA Function(FlatpakClient, String)>(), + ); + }); + + test('launch accepts arch/branch/commit named arguments', () { + expect( + _launchWithRef, + isA Function(FlatpakClient, String)>(), + ); + }); + + test('stop returns a Future from an app id', () { + expect(_stop, isA Function(FlatpakClient, String)>()); + }); + + test('listRunning returns a Future>', () { + expect( + _listRunning, + isA> Function(FlatpakClient)>(), + ); + }); + }); + + group('FpInstance wire format', () { + test('decodes the C++ golden buffer', () { + final inst = GlazeCodec.decodeInstance(_goldenFpInstance, 1); + expect(inst.appId, 'org.gnome.Calculator'); + expect(inst.instanceId, '42'); + expect(inst.arch, 'x86_64'); + expect(inst.branch, 'stable'); + expect(inst.commit, 'deadbeef'); + expect(inst.pid, 1234); + expect(inst.childPid, 1240); + expect(inst.isRunning, isTrue); }); - test('FlatpakInstance list type is usable', () { - const list = []; - expect(list, isA>()); + test('golden buffer is exactly the payload the C++ writer emits', () { + // 91 payload bytes + the 1-byte discriminator. + expect(_goldenFpInstance.length, 92); }); }); } From 38d7909581893357cbfa8e711479cae413d171e2 Mon Sep 17 00:00:00 2001 From: Joel Winarske Date: Tue, 25 Aug 2026 07:50:41 -0700 Subject: [PATCH 7/9] Resolve childPid on the instance returned by launch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit flatpak_instance_get_child_pid() returns whatever the instance directory held when the FlatpakInstance was constructed, and launch_full() builds that object before bwrap has written the pid file — so it always read 0, which made the launch dartdoc's promise that callers get the instance details without polling listRunning() true only for instanceId and pid. Re-enumerate flatpak_instance_get_all() on the launch thread, matching the instance id, until a freshly constructed object carries the real pid. libflatpak 1.18.1 exposes no flatpak_instance_new_for_id(), so get_all() is the only way to force the re-read. Bounded at 500ms in 5ms polls, and bails early once the instance has appeared and then vanished, so an app that exits before bwrap publishes the pid cannot park the launch thread. Timing out still yields 0, which is what callers saw before, so this is strictly an improvement rather than a new failure mode. Measured cost on the normal path is nil: launch() round-trips in ~55ms with or without the re-read. Document childPid as best-effort on FlatpakClient.launch, Installation.launch and the FlatpakInstance.childPid field. Verified end to end against org.gnome.Calculator: launch() now returns childPid=3453764, matching what listRunning() reports for the same instance id. Signed-off-by: Joel Winarske --- lib/src/flatpak_client.dart | 3 ++ lib/src/installation.dart | 8 ++++- lib/src/instance.dart | 5 ++++ native/src/installation_reader.cpp | 47 ++++++++++++++++++++++++++++++ 4 files changed, 62 insertions(+), 1 deletion(-) diff --git a/lib/src/flatpak_client.dart b/lib/src/flatpak_client.dart index 27cbe38..1a5de1b 100644 --- a/lib/src/flatpak_client.dart +++ b/lib/src/flatpak_client.dart @@ -81,6 +81,9 @@ class FlatpakClient { /// Launch an installed application in its sandbox ("tap to open"). /// Pass empty [arch]/[branch]/[commit] to use the installed defaults. /// Returns the [FlatpakInstance] libflatpak created for the launch. + /// + /// [FlatpakInstance.childPid] is best-effort — `0` if the app exits before + /// bwrap publishes it. Every other field is always populated. Future launch( String appId, { String arch = '', diff --git a/lib/src/installation.dart b/lib/src/installation.dart index 9093a19..775e2ba 100644 --- a/lib/src/installation.dart +++ b/lib/src/installation.dart @@ -281,7 +281,13 @@ class FlatpakInstallation { /// Launch an installed application in its sandbox. /// Completes when the sandbox has been spawned (non-blocking on the app). /// Returns the [FlatpakInstance] libflatpak created for the launch, so - /// callers get the instanceId/pid immediately instead of polling [listRunning]. + /// callers get the instanceId and pid immediately instead of polling + /// [listRunning]. + /// + /// [FlatpakInstance.childPid] is resolved on a best-effort basis: libflatpak + /// reports it as `0` on a freshly launched instance, so the native side waits + /// briefly for bwrap to write it. It is `0` if the app exits before that or + /// the wait times out; everything else on the instance is always populated. Future launch( String appId, { String arch = '', diff --git a/lib/src/instance.dart b/lib/src/instance.dart index a5a1fdb..14dede3 100644 --- a/lib/src/instance.dart +++ b/lib/src/instance.dart @@ -13,6 +13,11 @@ class FlatpakInstance { final int pid; /// The application process pid inside the sandbox. + /// + /// `0` if it could not be determined — libflatpak does not have it yet at + /// the moment a launch returns, so a launched instance carries it only if + /// bwrap published it while the native side waited. Instances from a listing + /// always carry the real value. final int childPid; /// Whether the instance is still running. diff --git a/native/src/installation_reader.cpp b/native/src/installation_reader.cpp index a571c99..3794c82 100644 --- a/native/src/installation_reader.cpp +++ b/native/src/installation_reader.cpp @@ -418,6 +418,47 @@ static bool resolve_launch_target(FlatpakInstallation* installation, const char* return found; } +// flatpak_instance_get_child_pid() returns the value the instance directory held when the +// FlatpakInstance was constructed. launch_full() hands back an object built before bwrap has +// written the pid file, so reading it there always yields 0. Re-enumerate until a freshly +// constructed object for the same instance id carries the real pid. +// +// Best-effort and bounded: an app that exits before bwrap writes the pid, or a target slow enough +// to miss the deadline, yields 0 — the same value the caller would have seen without this. +static int reread_child_pid(const char* instance_id) { + constexpr int kPollMs = 5; + constexpr int kMaxWaitMs = 500; + bool ever_seen = false; + + for (int waited = 0; waited <= kMaxWaitMs; waited += kPollMs) { + g_autoptr(GPtrArray) all = flatpak_instance_get_all(); + bool seen_now = false; + if (all) { + for (guint i = 0; i < all->len; i++) { + auto* inst = static_cast(all->pdata[i]); + if (g_strcmp0(flatpak_instance_get_id(inst), instance_id) != 0) { + continue; + } + seen_now = true; + int child = flatpak_instance_get_child_pid(inst); + if (child > 0) { + return child; + } + break; + } + } + // Once the instance has appeared and then vanished, the app is gone and no pid is + // coming — stop rather than burning the rest of the budget. + if (seen_now) { + ever_seen = true; + } else if (ever_seen) { + return 0; + } + g_usleep(kPollMs * 1000); + } + return 0; +} + void InstallationReader::launch(Dart_Port port, const char* app_id, const char* arch, const char* branch, const char* commit) { std::string appIdStr = safe_str(app_id); @@ -486,6 +527,12 @@ void InstallationReader::launch_impl(Dart_Port port, const char* app_id, const c info.childPid = flatpak_instance_get_child_pid(instance); info.isRunning = flatpak_instance_is_running(instance); + // Always 0 on the object launch_full() returns; we are on the launch thread, so waiting the + // few ms for bwrap to write it costs the Dart thread nothing. + if (info.childPid <= 0 && !info.instanceId.empty()) { + info.childPid = reread_child_pid(info.instanceId.c_str()); + } + post_glaze(port, 0x01, info); post_sentinel(port); } From 7bd95db7e753e64333d3417c469d20970ba9bf7a Mon Sep 17 00:00:00 2001 From: Joel Winarske Date: Tue, 25 Aug 2026 08:07:26 -0700 Subject: [PATCH 8/9] Harden launcher: stop() correctness, signal identity, shutdown cost MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the reliability, security and performance findings from review of the launcher branch. Reliability: R1 stop() no longer reports "no running instance" for an app that is running. child_pid <= 0 previously dropped the instance while `found` stayed false, so the caller got FlatpakNotFoundException and the app was never signalled. Now falls back to the outer bwrap pid, and tracks matched separately from signalled so "matched but could not signal" reports as 0x03 / FlatpakStopException instead of a not-found. R2 reap_thread() retries waitpid() on EINTR. A single unrestarted call would abandon the bwrap child as a zombie for the lifetime of the host process — the exact leak DO_NOT_REAP + the reaper exist to prevent. The Dart VM delivers SIGPROF; embedders install their own handlers. R3 resolve_launch_target() propagates the list_installed_refs GError rather than discarding it. An unreadable installation reported as "app not installed" sends callers after the wrong problem. R4 The no-pidfd fallback now escalates to SIGKILL. Measured: GTK apps ignore SIGTERM and exit only on the escalation, so a TERM-only fallback would silently fail to stop them on a pre-5.3 kernel. R5 Documented why installation_ needs no mutex (libflatpak documents FlatpakInstallation as concurrency-safe) now that launch_impl touches it from the launch thread. The invariant was previously an undocumented assumption after the single-thread comment was dropped. Security: S1 stop() skips instances whose process has already exited, and verifies process identity via /proc//stat field 22 (start time) before signalling. pidfd pins a process only after the open; the pid came off disk and could have been recycled in between, which would have sent SIGTERM and then SIGKILL to an unrelated process the user owns. The fallback path re-checks identity before the kill for the same reason. Performance: P1 reread_child_pid() backs off geometrically (1,2,4..64ms) instead of polling every 5ms. Each probe re-parses the info file of every running flatpak on the host; this covers the same 500ms in ~13 probes rather than ~100, with the common path (found on the first probe) unchanged. P2 Shutdown cancels the queued launch backlog instead of draining it, so close() costs at most the in-flight launch. Each cancelled request still gets an error frame, so no Dart future hangs. Measured: close() with 5 queued went from ~5 sequential sandbox spawns to 53ms, with 1 launched and 4 cancelled cleanly. P3 Documented why stop()/list_running() stay on the Dart thread rather than joining the launch queue: measured at ~102us (3 instances) to ~161us (9) for list_running and ~727us for a stop matching 6, well inside a frame, and queueing them would make stop() wait on an in-flight sandbox spawn. Verified: ASan/UBSan 4/4, TSAN clean of our code (9 GLib/GDBus reports, same baseline as main), 72/72 Dart, -Wall -Wextra clean, launch latency unchanged at ~47-64ms, and stop() latency A/B'd against the previous commit (1.6-2.0s settled, ~4s mid-startup) — identical either side. Signed-off-by: Joel Winarske --- lib/src/exceptions.dart | 7 + lib/src/flatpak_client.dart | 3 + lib/src/installation.dart | 14 +- native/include/flatpak_bridge.h | 20 +- native/include/installation_reader.h | 42 +++- native/src/installation_reader.cpp | 250 ++++++++++++++++++----- native/test/test_installation_reader.cpp | 57 ++++++ 7 files changed, 325 insertions(+), 68 deletions(-) diff --git a/lib/src/exceptions.dart b/lib/src/exceptions.dart index db574d9..7cba41f 100644 --- a/lib/src/exceptions.dart +++ b/lib/src/exceptions.dart @@ -44,6 +44,13 @@ final class FlatpakRemoteException extends FlatpakException { const FlatpakRemoteException(super.message); } +/// Launching an application failed. final class FlatpakLaunchException extends FlatpakException { const FlatpakLaunchException(super.message); } + +/// Stopping an application failed. Distinct from [FlatpakNotFoundException]: +/// running instances *were* matched, but none of them could be signalled. +final class FlatpakStopException extends FlatpakException { + const FlatpakStopException(super.message); +} diff --git a/lib/src/flatpak_client.dart b/lib/src/flatpak_client.dart index 1a5de1b..c89511b 100644 --- a/lib/src/flatpak_client.dart +++ b/lib/src/flatpak_client.dart @@ -98,6 +98,9 @@ class FlatpakClient { /// /// Returns once SIGTERM has been sent; grace period + SIGKILL escalation /// continue in the background. + /// + /// Throws [FlatpakNotFoundException] if nothing matched, or + /// [FlatpakStopException] if instances matched but none could be signalled. Future stop(String appId) => _installation.stop(appId); /// List running sandbox instances across the host. diff --git a/lib/src/installation.dart b/lib/src/installation.dart index 775e2ba..35d1b78 100644 --- a/lib/src/installation.dart +++ b/lib/src/installation.dart @@ -346,8 +346,12 @@ class FlatpakInstallation { /// Terminate every running instance of [appId] across the host — flatpak /// instances are not scoped to an installation, so instances launched from /// the other installation are matched too. - /// Returns as soon as SIGTERM has been sent to every matched instance. - /// Throws [FlatpakNotFoundException] if no running instance was found. + /// Returns as soon as SIGTERM has been sent to every matched instance; + /// SIGKILL escalation continues in the background. + /// + /// Throws [FlatpakNotFoundException] if no running instance was found, and + /// [FlatpakStopException] if instances were found but none could be + /// signalled — the app is still running in that case. Future stop(String appId) async { final port = ReceivePort('flatpak.stop'); final completer = Completer(); @@ -361,6 +365,12 @@ class FlatpakInstallation { completer.completeError(FlatpakNotFoundException(err)); } port.close(); + case 0x03: + final err = GlazeCodec.decodeError(msg, 1); + if (!completer.isCompleted) { + completer.completeError(FlatpakStopException(err)); + } + port.close(); case 0xFF: if (!completer.isCompleted) completer.complete(); port.close(); diff --git a/native/include/flatpak_bridge.h b/native/include/flatpak_bridge.h index 5ef3bd8..fd2fa2d 100644 --- a/native/include/flatpak_bridge.h +++ b/native/include/flatpak_bridge.h @@ -6,8 +6,10 @@ // Message discriminator byte at offset 0: // 0x01 = success / list-end sentinel // 0x02 = error (UTF-8, uint32_t length-prefix) -// 0x03 = launch error (UTF-8, uint32_t length-prefix) — a launch_full() -// failure, distinct from the 0x02 "app not installed" pre-check. +// 0x03 = lifecycle operation failure (UTF-8, uint32_t length-prefix) — a +// launch_full() failure, an unreadable installation, or a stop() that +// matched running instances but could not signal any. Distinct from +// 0x02, which means nothing matched. // 0x10 = TransactionProgress (glaze-encoded, in-flight during tx_run) // 0x11 = UpdateAvailable (FlatpakMonitor inotify signal) // 0xFF = streaming list end sentinel @@ -48,15 +50,19 @@ void flatpak_reader_fetch_remote_metadata(void* handle, Dart_Port port, const ch // FLATPAK_LAUNCH_FLAGS_DO_NOT_REAP. Returns immediately; the launch runs on the // reader's serial launch thread. On success posts the resulting FlatpakInstance // as a 0x01 FpInstance payload followed by the 0xFF sentinel. On failure posts -// 0x02 if the app is not installed, or 0x03 if launch_full() itself failed. +// 0x02 if the app is not installed, or 0x03 if the installation could not be +// read or launch_full() itself failed. void flatpak_reader_launch(void* handle, Dart_Port port, const char* app_id, const char* arch, const char* branch, const char* commit); // Terminate every running instance matching app_id, host-wide — flatpak // instances are not scoped to an installation, so this also matches instances -// launched from the other installation. Signals the sandboxed app process -// (FlatpakInstance child pid), not the outer bwrap pid, so the app sees the -// SIGTERM and bwrap follows it down. -// Posts 0xFF if at least one was signalled, otherwise 0x02. +// launched from the other installation. Prefers the sandboxed app process +// (FlatpakInstance child pid) over the outer bwrap pid, so the app sees the +// SIGTERM and bwrap follows it down; falls back to the bwrap pid when the child +// pid has not been published yet. Stale instances whose process has already +// exited are skipped. SIGTERM is escalated to SIGKILL after a grace period. +// Posts 0xFF if at least one instance was signalled, 0x02 if nothing matched, +// or 0x03 if instances matched but none could be signalled. void flatpak_reader_stop(void* handle, Dart_Port port, const char* app_id); // List running sandbox instances (FlatpakInstance) via flatpak_instance_get_all(). // Posts each as a 0x01 FpInstance payload, then the 0xFF sentinel. diff --git a/native/include/installation_reader.h b/native/include/installation_reader.h index 9c4a0bd..c8079ab 100644 --- a/native/include/installation_reader.h +++ b/native/include/installation_reader.h @@ -3,10 +3,11 @@ #pragma once #include +#include #include -#include #include #include +#include #include #include "dart_api_dl.h" @@ -33,21 +34,42 @@ class InstallationReader { void drop_caches(); private: + // Thread-safety of installation_: libflatpak documents FlatpakInstallation as safe for + // concurrent operations from multiple threads (flatpak-installation.c SECTION doc), which is + // what lets launch_impl() call into it from the launch thread while the reader's other methods + // call into it from the Dart thread. We deliberately do not add our own mutex: it would have to + // wrap every libflatpak call to be meaningful, and a partial one would only give false + // confidence. This comment is the invariant — if that upstream guarantee is ever in doubt, the + // fix is a mutex around every installation_ use, not just the launch path. FlatpakInstallation* installation_; // ── Launch queue — one dedicated thread, serial ────────────────────── - // flatpak_installation_launch_full() blocks while bubblewrap sets the - // sandbox up, so it must not run on the Dart thread. Same shape as - // TransactionWorker (transaction_bridge.h), deliberately built on the - // standard library rather than an async runtime: every other thread in - // this bridge is a plain std::thread or GThread, and adding a system - // dependency for one serial queue would have to be satisfied by every - // consumer sysroot as well as CI. + // flatpak_installation_launch_full() blocks while bubblewrap sets the sandbox up, so it must + // not run on the Dart thread. Same shape as TransactionWorker (transaction_bridge.h), + // deliberately built on the standard library rather than an async runtime: every other thread + // in this bridge is a plain std::thread or GThread, and adding a system dependency for one + // serial queue would have to be satisfied by every consumer sysroot as well as CI. + // + // stop() and list_running() deliberately stay on the calling (Dart) thread. Both are bounded + // reads of $XDG_RUNTIME_DIR/.flatpak plus, for stop(), a few pidfd syscalls — measured at + // ~102us for list_running() with 3 instances, ~161us with 9, and ~727us for a stop() matching + // 6 instances, all far inside a frame budget. Queueing them behind launches would make stop() + // wait on an in-flight sandbox spawn, which is the opposite of what a stop should do. + struct LaunchRequest { + Dart_Port port; + std::string appId; + std::string arch; + std::string branch; + std::string commit; + }; + std::thread launch_thread_; std::mutex launch_mu_; std::condition_variable launch_cv_; - std::queue> launch_queue_; - bool launch_stop_{false}; + std::queue launch_queue_; + // Written under launch_mu_ so the condition_variable cannot miss a wakeup; also read without + // the lock by reread_child_pid() so an in-flight launch can abandon its poll at shutdown. + std::atomic launch_stop_{false}; void launch_loop(); void launch_impl(Dart_Port port, const char* app_id, const char* arch, const char* branch, diff --git a/native/src/installation_reader.cpp b/native/src/installation_reader.cpp index 3794c82..7a378ea 100644 --- a/native/src/installation_reader.cpp +++ b/native/src/installation_reader.cpp @@ -10,9 +10,12 @@ #include #include +#include #include #include +#include #include +#include #include #include "flatpak_bridge.h" @@ -36,7 +39,9 @@ static void post_error(Dart_Port port, const char* msg) { flatpak_nc::post_framed_error(port, 0x02, msg); } -static void post_launch_error(Dart_Port port, const char* msg) { +// 0x03 — lifecycle operation failure (launch or stop), as distinct from the 0x02 "nothing +// matched" condition that maps to FlatpakNotFoundException on the Dart side. +static void post_op_error(Dart_Port port, const char* msg) { flatpak_nc::post_framed_error(port, 0x03, msg); } @@ -57,7 +62,11 @@ static const char* safe_str(const char* s) { static gpointer reap_thread(gpointer data) { auto pid = static_cast(GPOINTER_TO_INT(data)); int status = 0; - waitpid(pid, &status, 0); + // Retry on EINTR: a single unrestarted waitpid() would abandon the child as a zombie for the + // lifetime of the host process, which is exactly what this reaper exists to prevent. The Dart + // VM's profiler delivers SIGPROF, and embedders install handlers of their own. + while (waitpid(pid, &status, 0) < 0 && errno == EINTR) { + } return nullptr; } @@ -74,11 +83,12 @@ InstallationReader::InstallationReader(FlatpakInstallation* inst) } InstallationReader::~InstallationReader() { - // Drain, then join, then unref: a launch already queued still gets to run - // (and post its reply) against a live installation_ before we let go of it. + // Signal, then join, then unref: the in-flight launch (if any) finishes against a live + // installation_, the queued backlog is cancelled rather than run, and only then do we drop the + // installation reference the launch thread was using. { std::lock_guard lk(launch_mu_); - launch_stop_ = true; + launch_stop_.store(true); } launch_cv_.notify_one(); if (launch_thread_.joinable()) { @@ -89,17 +99,29 @@ InstallationReader::~InstallationReader() { void InstallationReader::launch_loop() { for (;;) { - std::function job; + LaunchRequest req; { std::unique_lock lk(launch_mu_); - launch_cv_.wait(lk, [&] { return launch_stop_ || !launch_queue_.empty(); }); - if (launch_queue_.empty()) { - return; // stopping and drained + launch_cv_.wait(lk, [&] { return launch_stop_.load() || !launch_queue_.empty(); }); + if (launch_stop_.load()) { + // Do not make close() pay for the whole backlog: a queued launch has not spawned + // anything yet, so cancelling it costs nothing but a reply. Each one still gets an + // error frame, so no Dart future is left hanging. Only the in-flight launch (if + // any) has already run to completion by the time we get here. + std::queue pending; + pending.swap(launch_queue_); + lk.unlock(); + while (!pending.empty()) { + post_op_error(pending.front().port, "reader closed before launch started"); + pending.pop(); + } + return; } - job = std::move(launch_queue_.front()); + req = std::move(launch_queue_.front()); launch_queue_.pop(); } - job(); + launch_impl(req.port, req.appId.c_str(), req.arch.c_str(), req.branch.c_str(), + req.commit.c_str()); } } @@ -364,9 +386,13 @@ void InstallationReader::fetch_remote_metadata(Dart_Port port, const char* remot post_sentinel(port); } +// Returns false when no installed ref matches. *out_err is set only when the lookup itself +// failed (as opposed to simply finding nothing), so the caller can tell "app not installed" from +// "could not read the installation". static bool resolve_launch_target(FlatpakInstallation* installation, const char* app_id, const char* hint_arch, const char* hint_branch, - std::string* out_arch, std::string* out_branch) { + std::string* out_arch, std::string* out_branch, + std::string* out_err) { g_autoptr(GError) cerr = nullptr; g_autoptr(FlatpakInstalledRef) current = flatpak_installation_get_current_installed_app(installation, app_id, nullptr, &cerr); @@ -387,6 +413,7 @@ static bool resolve_launch_target(FlatpakInstallation* installation, const char* g_autoptr(GPtrArray) refs = flatpak_installation_list_installed_refs(installation, nullptr, &lerr); if (!refs) { + *out_err = lerr && lerr->message ? lerr->message : "failed to list installed refs"; return false; } bool found = false; @@ -425,12 +452,14 @@ static bool resolve_launch_target(FlatpakInstallation* installation, const char* // // Best-effort and bounded: an app that exits before bwrap writes the pid, or a target slow enough // to miss the deadline, yields 0 — the same value the caller would have seen without this. -static int reread_child_pid(const char* instance_id) { - constexpr int kPollMs = 5; +static int reread_child_pid(const char* instance_id, const std::atomic& cancelled) { constexpr int kMaxWaitMs = 500; + constexpr int kMaxBackoffMs = 64; bool ever_seen = false; + int waited = 0; + int delay_ms = 1; - for (int waited = 0; waited <= kMaxWaitMs; waited += kPollMs) { + for (;;) { g_autoptr(GPtrArray) all = flatpak_instance_get_all(); bool seen_now = false; if (all) { @@ -454,9 +483,18 @@ static int reread_child_pid(const char* instance_id) { } else if (ever_seen) { return 0; } - g_usleep(kPollMs * 1000); + if (waited >= kMaxWaitMs || cancelled.load()) { + return 0; + } + // Back off geometrically. Every probe re-enumerates and re-parses the info file of every + // running flatpak on the host, so a fixed 5ms interval spent ~100 of them to cover 500ms; + // this covers the same window in ~13 while leaving the common case (found on the first or + // second probe) exactly as fast. + int sleep_ms = std::min(delay_ms, kMaxWaitMs - waited); + g_usleep(sleep_ms * 1000); + waited += sleep_ms; + delay_ms = std::min(delay_ms * 2, kMaxBackoffMs); } - return 0; } void InstallationReader::launch(Dart_Port port, const char* app_id, const char* arch, @@ -467,10 +505,7 @@ void InstallationReader::launch(Dart_Port port, const char* app_id, const char* std::string commitStr = safe_str(commit); { std::lock_guard lk(launch_mu_); - launch_queue_.emplace([this, port, appIdStr, archStr, branchStr, commitStr]() { - launch_impl(port, appIdStr.c_str(), archStr.c_str(), branchStr.c_str(), - commitStr.c_str()); - }); + launch_queue_.push(LaunchRequest{port, appIdStr, archStr, branchStr, commitStr}); } launch_cv_.notify_one(); } @@ -484,9 +519,16 @@ void InstallationReader::launch_impl(Dart_Port port, const char* app_id, const c std::string resolved_arch; std::string resolved_branch; if (!use_arch || !use_branch) { + std::string resolve_err; if (!resolve_launch_target(installation_, app_id, use_arch, use_branch, &resolved_arch, - &resolved_branch)) { - post_error(port, "app not installed"); + &resolved_branch, &resolve_err)) { + if (resolve_err.empty()) { + post_error(port, "app not installed"); + } else { + // The installation could not be read at all — surfacing that as "not installed" + // sends the caller looking for the wrong problem. + post_op_error(port, resolve_err.c_str()); + } return; } if (!use_arch) { @@ -507,7 +549,7 @@ void InstallationReader::launch_impl(Dart_Port port, const char* app_id, const c if (err && err->domain == FLATPAK_ERROR && err->code == FLATPAK_ERROR_NOT_INSTALLED) { post_error(port, err->message); } else { - post_launch_error(port, err ? err->message : "launch failed"); + post_op_error(port, err ? err->message : "launch failed"); } return; } @@ -530,15 +572,56 @@ void InstallationReader::launch_impl(Dart_Port port, const char* app_id, const c // Always 0 on the object launch_full() returns; we are on the launch thread, so waiting the // few ms for bwrap to write it costs the Dart thread nothing. if (info.childPid <= 0 && !info.instanceId.empty()) { - info.childPid = reread_child_pid(info.instanceId.c_str()); + info.childPid = reread_child_pid(info.instanceId.c_str(), launch_stop_); } post_glaze(port, 0x01, info); post_sentinel(port); } -// pidfd-based signalling: a pidfd refers to the exact process instance it was opened for, so we can -// send signals to it even if the PID has been recycled. +// Field 22 of /proc//stat is the process start time in clock ticks since boot. Paired with +// the pid it identifies a process *instance*: a recycled pid always carries a different start +// time, so comparing it before and after we pin a process tells us whether we pinned the one we +// meant to. Returns false if the process is gone or /proc is unreadable. +static bool read_start_time(pid_t pid, unsigned long long* out) { + char path[64]; + g_snprintf(path, sizeof(path), "/proc/%d/stat", static_cast(pid)); + g_autofree char* contents = nullptr; + if (!g_file_get_contents(path, &contents, nullptr, nullptr)) { + return false; + } + // Field 2 (comm) is parenthesised and may itself contain spaces and parens, so start scanning + // after the final ')'; the next token is field 3. + const char* p = strrchr(contents, ')'); + if (!p) { + return false; + } + p++; + int field = 2; + while (*p) { + while (*p == ' ') { + p++; + } + if (!*p) { + break; + } + field++; + if (field == 22) { + return sscanf(p, "%llu", out) == 1; + } + while (*p && *p != ' ') { + p++; + } + } + return false; +} + +constexpr int kGraceMs = 1500; + +// pidfd-based signalling. A pidfd pins the exact process instance it was opened for, so once we +// hold one the pid cannot be recycled out from under us. It does NOT cover the window before the +// open: the pid we are about to open came out of an instance file on disk and the process may have +// exited since. read_start_time() below closes that window. static int pidfd_open_compat(pid_t pid) { return static_cast(syscall(SYS_pidfd_open, pid, 0)); } @@ -553,7 +636,6 @@ static int pidfd_send_signal_compat(int pidfd, int sig) { static gpointer stop_escalate_thread(gpointer data) { auto pidfd = GPOINTER_TO_INT(data); struct pollfd pfd = {.fd = pidfd, .events = POLLIN, .revents = 0}; - constexpr int kGraceMs = 1500; auto deadline = std::chrono::steady_clock::now() + std::chrono::milliseconds(kGraceMs); int ret = 0; for (;;) { @@ -581,44 +663,114 @@ static void stop_escalate_async(int pidfd) { g_thread_unref(t); } +// Escalation for the no-pidfd fallback. Measured against GTK apps: gnome-calculator ignores +// SIGTERM outright and exits only on the SIGKILL at the grace deadline, so a fallback that sent +// SIGTERM alone would simply fail to stop such an app rather than degrade gracefully. Without a +// pidfd we cannot pin the process, so re-check the start time before killing: if the pid has been +// recycled during the grace period the kill would land on an unrelated process. +struct FallbackKill { + pid_t pid; + unsigned long long start_time; + bool have_start; +}; + +static gpointer fallback_escalate_thread(gpointer data) { + std::unique_ptr fk(static_cast(data)); + g_usleep(kGraceMs * 1000); + if (kill(fk->pid, 0) != 0) { + return nullptr; // exited during the grace period + } + unsigned long long now = 0; + if (fk->have_start && (!read_start_time(fk->pid, &now) || now != fk->start_time)) { + return nullptr; // pid recycled — not our process any more + } + kill(fk->pid, SIGKILL); + return nullptr; +} + +static void fallback_escalate_async(pid_t pid, unsigned long long start_time, bool have_start) { + auto* fk = new FallbackKill{pid, start_time, have_start}; + GThread* t = g_thread_new("flatpak-stop-fb", fallback_escalate_thread, fk); + g_thread_unref(t); +} + +// Sends SIGTERM to one sandbox process and arms SIGKILL escalation. Returns true only if the +// signal actually reached the intended process. +static bool signal_instance_process(pid_t pid) { + unsigned long long start_before = 0; + const bool have_start = read_start_time(pid, &start_before); + + int pidfd = pidfd_open_compat(pid); + if (pidfd >= 0) { + // The pidfd pins the process from here on. Re-read the start time now that it is pinned: + // if it still matches, the pid was not recycled between the instance file and the open. + unsigned long long start_after = 0; + if (have_start && (!read_start_time(pid, &start_after) || start_after != start_before)) { + close(pidfd); + return false; + } + if (pidfd_send_signal_compat(pidfd, SIGTERM) != 0) { + close(pidfd); + return false; + } + stop_escalate_async(pidfd); // thread takes ownership, closes it + return true; + } + if (errno == ESRCH) { + return false; // already gone + } + // ENOSYS on pre-5.3 kernels, EMFILE, a seccomp filter. Fall back to plain signals. + if (kill(pid, SIGTERM) != 0) { + return false; + } + fallback_escalate_async(pid, start_before, have_start); + return true; +} + void InstallationReader::stop(Dart_Port port, const char* app_id) { g_autoptr(GPtrArray) instances = flatpak_instance_get_all(); - bool found = false; + int matched = 0; + int signalled = 0; if (instances) { for (guint i = 0; i < instances->len; i++) { auto* inst = static_cast(instances->pdata[i]); if (g_strcmp0(flatpak_instance_get_app(inst), app_id) != 0) { continue; } - int child_pid = flatpak_instance_get_child_pid(inst); - if (child_pid <= 0) { + // Skip stale instance directories whose process is already gone, so we never signal a + // pid the kernel may since have handed to something unrelated. + if (!flatpak_instance_is_running(inst)) { continue; } - int pidfd = pidfd_open_compat(child_pid); - if (pidfd < 0) { - // ESRCH means the app already exited — nothing to signal. - // Any other failure (ENOSYS on pre-5.3 kernels, EMFILE, a - // seccomp filter) falls back to a plain SIGTERM. Intentionally - // no SIGKILL escalation here: without a pidfd we cannot wait - // for the exit without racing pid reuse, and the supported - // targets (kernel >= 5.10) always take the pidfd path. An app - // that ignores SIGTERM on such a kernel survives stop(). - if (errno != ESRCH) { - if (kill(child_pid, SIGTERM) == 0) { - found = true; - } - } + matched++; + + // Prefer the sandboxed app process: SIGTERM has to reach the app itself for it to shut + // down on its own terms, and bwrap follows it down. Fall back to the outer bwrap pid + // when the child pid has not been published yet — coarser, but silently skipping a + // running instance while reporting success is worse. + int target = flatpak_instance_get_child_pid(inst); + if (target <= 0) { + target = flatpak_instance_get_pid(inst); + } + if (target <= 0) { continue; } - found = true; - pidfd_send_signal_compat(pidfd, SIGTERM); - stop_escalate_async(pidfd); // thread takes ownership, closes it + if (signal_instance_process(target)) { + signalled++; + } } } - if (!found) { + if (matched == 0) { post_error(port, "no running instance for app_id"); return; } + if (signalled == 0) { + // Matched running instances but could not signal any. This is emphatically not a + // "not found" condition — reporting it as one tells the caller their app is not running + // when it is. + post_op_error(port, "matched running instance(s) but could not signal any"); + return; + } post_sentinel(port); } diff --git a/native/test/test_installation_reader.cpp b/native/test/test_installation_reader.cpp index 6f8b1b1..33ebf9f 100644 --- a/native/test/test_installation_reader.cpp +++ b/native/test/test_installation_reader.cpp @@ -1,6 +1,10 @@ // test_installation_reader.cpp — tests for the installation reader C ABI. #include +#include + +#include +#include #include "flatpak_bridge.h" @@ -18,3 +22,56 @@ TEST(InstallationReaderCABI, CreateSystemMayFail) { flatpak_reader_destroy(handle); } } + +// ── /proc start-time identity check (S1) ──────────────────────────────────── +// read_start_time() is file-static, so exercise the property it relies on: field 22 of +// /proc//stat is stable for a live process and present for our own pid. +namespace { +unsigned long long start_time_of(pid_t pid) { + char path[64]; + snprintf(path, sizeof(path), "/proc/%d/stat", static_cast(pid)); + FILE* f = fopen(path, "re"); + if (!f) { + return 0; + } + char buf[4096]; + size_t n = fread(buf, 1, sizeof(buf) - 1, f); + fclose(f); + buf[n] = '\0'; + const char* p = strrchr(buf, ')'); + if (!p) { + return 0; + } + p++; + int field = 2; + while (*p) { + while (*p == ' ') { + p++; + } + if (!*p) { + break; + } + field++; + if (field == 22) { + unsigned long long v = 0; + return sscanf(p, "%llu", &v) == 1 ? v : 0; + } + while (*p && *p != ' ') { + p++; + } + } + return 0; +} +} // namespace + +TEST(ProcStartTime, StableForLiveProcess) { + unsigned long long a = start_time_of(getpid()); + EXPECT_GT(a, 0u); + EXPECT_EQ(a, start_time_of(getpid())); +} + +TEST(ProcStartTime, ZeroForMissingProcess) { + // Reserved-but-unused high pid; if it happens to exist we only assert we did not crash. + unsigned long long v = start_time_of(0x7FFFFFF); + EXPECT_EQ(v, 0u); +} From 8609d466c4e49be59cab81343a0dd786686193d8 Mon Sep 17 00:00:00 2001 From: Joel Winarske Date: Tue, 25 Aug 2026 08:33:13 -0700 Subject: [PATCH 9/9] Fix implicit widening in g_usleep calls (clang-tidy) bugprone-implicit-widening-of-multiplication-result: both sleeps computed int * int and let the result widen to g_usleep's gulong parameter. Values are small enough that this could not overflow in practice, but the cast makes the arithmetic happen in the destination type. clang-tidy-19 is now clean across all four native sources with the project's .clang-tidy config, and clang-format reports every file correctly formatted. Signed-off-by: Joel Winarske --- native/src/installation_reader.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/native/src/installation_reader.cpp b/native/src/installation_reader.cpp index 7a378ea..a92b902 100644 --- a/native/src/installation_reader.cpp +++ b/native/src/installation_reader.cpp @@ -491,7 +491,7 @@ static int reread_child_pid(const char* instance_id, const std::atomic& ca // this covers the same window in ~13 while leaving the common case (found on the first or // second probe) exactly as fast. int sleep_ms = std::min(delay_ms, kMaxWaitMs - waited); - g_usleep(sleep_ms * 1000); + g_usleep(static_cast(sleep_ms) * 1000); waited += sleep_ms; delay_ms = std::min(delay_ms * 2, kMaxBackoffMs); } @@ -676,7 +676,7 @@ struct FallbackKill { static gpointer fallback_escalate_thread(gpointer data) { std::unique_ptr fk(static_cast(data)); - g_usleep(kGraceMs * 1000); + g_usleep(static_cast(kGraceMs) * 1000); if (kill(fk->pid, 0) != 0) { return nullptr; // exited during the grace period }