diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ea6a8a..13ed396 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,19 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Fixed +- **Issue #95 — Android foreground admission:** foreground promotion and worker execution + now share one `maxConcurrentTasks` permit. Waiting workers do not promote themselves. + Cancellation remains under WorkManager's control, preserving periodic scheduling; + debug logs include the phase, work ID, stop reason, and run attempt count. + Verified on an Android 14 emulator (`maxConcurrentTasks: 1`, 6 foreground Dart + workers): promotions were spread one per ~0.6 s instead of 6 within ~130 ms. + The reported cancellation itself is not reproduced and its cause remains + unconfirmed. iOS has no foreground-service promotion and already admits through + `ConcurrencyLimiter` before execution, so it is unaffected. + ## [1.10.0] - 2026-11-08 Adds an opt-in to refuse unsigned `native_wm` remote commands. Backwards compatible: default behaviour is unchanged. diff --git a/android/src/main/kotlin/dev/brewkits/native_workmanager/SimpleAndroidWorkerFactory.kt b/android/src/main/kotlin/dev/brewkits/native_workmanager/SimpleAndroidWorkerFactory.kt index 3bd0aa3..1f25f24 100644 --- a/android/src/main/kotlin/dev/brewkits/native_workmanager/SimpleAndroidWorkerFactory.kt +++ b/android/src/main/kotlin/dev/brewkits/native_workmanager/SimpleAndroidWorkerFactory.kt @@ -123,13 +123,21 @@ class SimpleAndroidWorkerFactory( } } - override fun createWorker(workerClassName: String): AndroidWorker? { + override fun createWorker(workerClassName: String): AndroidWorker? = + createWorker(workerClassName) {} + + // Foreground promotion must use the same permit as execution, never a nested permit. + internal fun createWorker( + workerClassName: String, + onAdmitted: suspend () -> Unit + ): AndroidWorker? { val worker = resolveWorker(workerClassName) ?: return null // issue_57: wrap every worker (built-in, registry, and factory-chain) so chain step // output data reaches ChainStore regardless of which path created it. No-op for // standalone (non-chain) tasks — see ChainResultCapturingWorker's doc comment. return dev.brewkits.native_workmanager.concurrency.ConcurrencyLimitedWorker( - dev.brewkits.native_workmanager.workers.ChainResultCapturingWorker(worker, context) + dev.brewkits.native_workmanager.workers.ChainResultCapturingWorker(worker, context), + onAdmitted ) } diff --git a/android/src/main/kotlin/dev/brewkits/native_workmanager/concurrency/TaskConcurrency.kt b/android/src/main/kotlin/dev/brewkits/native_workmanager/concurrency/TaskConcurrency.kt index 2c6f289..85bd977 100644 --- a/android/src/main/kotlin/dev/brewkits/native_workmanager/concurrency/TaskConcurrency.kt +++ b/android/src/main/kotlin/dev/brewkits/native_workmanager/concurrency/TaskConcurrency.kt @@ -28,7 +28,13 @@ internal object TaskConcurrency { } } -internal class ConcurrencyLimitedWorker(private val delegate: AndroidWorker) : AndroidWorker { +internal class ConcurrencyLimitedWorker( + private val delegate: AndroidWorker, + private val onAdmitted: suspend () -> Unit = {} +) : AndroidWorker { override suspend fun doWork(input: String?, env: WorkerEnvironment): WorkerResult = - TaskConcurrency.limiter.withPermit { delegate.doWork(input, env) } + TaskConcurrency.limiter.withPermit { + onAdmitted() + delegate.doWork(input, env) + } } diff --git a/android/src/main/kotlin/dev/brewkits/native_workmanager/workers/ForegroundNativeWorker.kt b/android/src/main/kotlin/dev/brewkits/native_workmanager/workers/ForegroundNativeWorker.kt index d392e85..73ab7f7 100644 --- a/android/src/main/kotlin/dev/brewkits/native_workmanager/workers/ForegroundNativeWorker.kt +++ b/android/src/main/kotlin/dev/brewkits/native_workmanager/workers/ForegroundNativeWorker.kt @@ -40,16 +40,20 @@ class ForegroundNativeWorker( override suspend fun doWork(): Result { NativeLogger.d("ForegroundNativeWorker: starting doWork for $taskId ($workerClassName)") - try { - setForeground(getForegroundInfo()) - } catch (e: kotlinx.coroutines.CancellationException) { - throw e - } catch (e: Exception) { - NativeLogger.e("ForegroundNativeWorker: Failed to set foreground status. This might be due to background-start restrictions on Android 12+.", e) - // We continue as background work if possible, but the OS might kill us. - } + var phase = "waiting-for-admission" val workerFactory = SimpleAndroidWorkerFactory(context) - val worker = workerFactory.createWorker(workerClassName) + val worker = workerFactory.createWorker(workerClassName) { + phase = "foreground-promotion" + try { + setForeground(getForegroundInfo()) + } catch (e: kotlinx.coroutines.CancellationException) { + throw e + } catch (e: Exception) { + NativeLogger.e("ForegroundNativeWorker: Failed to set foreground status. This might be due to background-start restrictions on Android 12+.", e) + // Retain the existing background fallback when promotion is rejected. + } + phase = "worker-execution" + } if (worker == null) { NativeLogger.e("ForegroundNativeWorker: Worker class not found: $workerClassName") emitToBus(false, "Worker class not found: $workerClassName") @@ -97,6 +101,13 @@ class ForegroundNativeWorker( } } } catch (e: kotlinx.coroutines.CancellationException) { + // WorkManager owns rescheduling of stopped periodic runs. Never translate + // cancellation into callback failure, retry, or a failure event. + NativeLogger.d( + "ForegroundNativeWorker: cancelled taskId=$taskId workId=$id " + + "phase=$phase isStopped=$isStopped stopReason=$stopReason " + + "runAttemptCount=$runAttemptCount" + ) throw e } catch (e: Exception) { NativeLogger.e("ForegroundNativeWorker: Error executing worker", e) @@ -116,6 +127,8 @@ class ForegroundNativeWorker( outputData = outputData ) ) + } catch (e: kotlinx.coroutines.CancellationException) { + throw e } catch (e: Exception) { NativeLogger.e("ForegroundNativeWorker: Failed to emit to TaskEventBus", e) } diff --git a/android/src/test/kotlin/dev/brewkits/native_workmanager/concurrency/ForegroundAdmissionTest.kt b/android/src/test/kotlin/dev/brewkits/native_workmanager/concurrency/ForegroundAdmissionTest.kt new file mode 100644 index 0000000..df000c4 --- /dev/null +++ b/android/src/test/kotlin/dev/brewkits/native_workmanager/concurrency/ForegroundAdmissionTest.kt @@ -0,0 +1,149 @@ +package dev.brewkits.native_workmanager.concurrency + +import android.content.Context +import androidx.test.core.app.ApplicationProvider +import androidx.work.Data +import androidx.work.ForegroundUpdater +import androidx.work.WorkerParameters +import com.google.common.util.concurrent.SettableFuture +import dev.brewkits.kmpworkmanager.background.domain.* +import dev.brewkits.native_workmanager.NativeLogger +import dev.brewkits.native_workmanager.SimpleAndroidWorkerFactory +import dev.brewkits.native_workmanager.workers.ForegroundNativeWorker +import kotlinx.coroutines.* +import kotlinx.coroutines.test.* +import org.junit.* +import org.junit.Assert.* +import org.junit.runner.RunWith +import org.mockito.Mockito.* +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config +import org.robolectric.shadows.ShadowLog +import java.util.UUID +import kotlin.coroutines.EmptyCoroutineContext + +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [28]) +@OptIn(ExperimentalCoroutinesApi::class) +class ForegroundAdmissionTest { + private lateinit var context: Context + private val events = mutableListOf() + + @Before fun setup() { + context = ApplicationProvider.getApplicationContext() + TaskConcurrency.configure(context, 1) + SimpleAndroidWorkerFactory.registerWorker("issue95") { + object : AndroidWorker { + override suspend fun doWork(input: String?, env: WorkerEnvironment): WorkerResult { + events += "callback:$input" + delay(100) + events += "complete:$input" + return WorkerResult.Success() + } + } + } + } + + @After fun cleanup() { + SimpleAndroidWorkerFactory.unregisterWorker("issue95") + TaskConcurrency.configure(context, 4) + } + + private fun worker(name: String, promotion: SettableFuture? = null): ForegroundNativeWorker { + val data = Data.Builder() + .putString("workerClassName", "issue95").putString("taskId", name) + .putString("inputJson", name) + .putString("fgsConfigJson", "{\"showCancelButton\":false}").build() + val updater = ForegroundUpdater { _, _, _ -> + events += "promote:$name" + promotion ?: SettableFuture.create().also { it.set(null) } + } + val params = WorkerParameters( + UUID.randomUUID(), data, emptyList(), WorkerParameters.RuntimeExtras(), + 0, 0, java.util.concurrent.Executor { it.run() }, EmptyCoroutineContext, + mock(androidx.work.impl.utils.taskexecutor.TaskExecutor::class.java), + object : androidx.work.WorkerFactory() { + override fun createWorker(context: Context, name: String, params: WorkerParameters): + androidx.work.ListenableWorker? = null + }, + androidx.work.ProgressUpdater { _, _, _ -> SettableFuture.create().also { it.set(null) } }, + updater + ) + return ForegroundNativeWorker(context, params) + } + + @Test fun promotion_and_callbacks_share_one_permit() = runTest { + val jobs = listOf(launch { worker("first").doWork() }, launch { worker("second").doWork() }) + runCurrent() + assertEquals(listOf("promote:first", "callback:first"), events) + jobs.joinAll() + assertEquals(listOf("promote:first", "callback:first", "complete:first", + "promote:second", "callback:second", "complete:second"), events) + } + + @Test fun background_work_delays_foreground_promotion() = runTest { + val background = SimpleAndroidWorkerFactory(context).createWorker("issue95")!! + val first = launch { background.doWork("background", WorkerEnvironment()) } + val foreground = launch { worker("foreground").doWork() } + runCurrent() + assertEquals(listOf("callback:background"), events) + joinAll(first, foreground) + assertEquals(listOf("callback:background", "complete:background", + "promote:foreground", "callback:foreground", "complete:foreground"), events) + } + + @Test fun rejected_promotion_preserves_background_fallback_and_releases_permit() = runTest { + val rejected = SettableFuture.create().also { + it.setException(IllegalStateException("Foreground start rejected")) + } + worker("first", rejected).doWork() + worker("next").doWork() + assertEquals(listOf("promote:first", "callback:first", "complete:first", + "promote:next", "callback:next", "complete:next"), events) + } + + @Test fun cancelled_waiter_never_promotes_and_active_cancellation_releases_permit() = runTest { + val first = launch { worker("first").doWork() } + val waiting = launch { worker("waiting").doWork() } + runCurrent() + waiting.cancelAndJoin() + first.cancelAndJoin() + worker("next").doWork() + assertEquals(listOf("promote:first", "callback:first", "promote:next", + "callback:next", "complete:next"), events) + assertTrue(first.isCancelled) + assertTrue(waiting.isCancelled) + } + + @Test fun stopped_work_logs_reason_and_phase_and_propagates_cancellation() = runTest { + val wasEnabled = NativeLogger.enabled + NativeLogger.enabled = true + try { + val pending = SettableFuture.create() + val stopped = worker("stopped", pending) + val job = launch { stopped.doWork() } + runCurrent() + stopped.stop(7) + job.cancelAndJoin() + assertTrue(job.isCancelled) + val log = ShadowLog.getLogs().map { it.msg }.first { + it.contains("cancelled taskId=stopped") + } + assertTrue(log.contains("phase=foreground-promotion")) + assertTrue(log.contains("isStopped=true stopReason=7")) + assertEquals(listOf("promote:stopped"), events) + } finally { + NativeLogger.enabled = wasEnabled + } + } + + @Test fun cancellation_during_promotion_releases_permit_without_starting_callback() = runTest { + val pending = SettableFuture.create() + val first = launch { worker("first", pending).doWork() } + runCurrent() + first.cancelAndJoin() + worker("next").doWork() + assertEquals(listOf("promote:first", "promote:next", "callback:next", "complete:next"), events) + assertTrue(first.isCancelled) + } +} diff --git a/example/integration_test/device_integration_test.dart b/example/integration_test/device_integration_test.dart index 140bcfa..4f98108 100644 --- a/example/integration_test/device_integration_test.dart +++ b/example/integration_test/device_integration_test.dart @@ -508,6 +508,117 @@ void main() { } }); + // Issue #95: ForegroundNativeWorker used to call setForeground() BEFORE taking + // a TaskConcurrency permit, so with maxConcurrentTasks: 1 every waiting worker + // still promoted itself to a foreground service and booted its Flutter engine. + // Now the order is: wait for permit -> promote -> run callback -> release. + // Run with --dart-define=ISSUE84_LIMIT=1. NOTE: this is a smoke test, NOT a + // red/green guard — it also passes on the pre-fix code (the limiter already + // serialized callbacks). Dart cannot observe promotion order; that regression + // is guarded by ForegroundAdmissionTest (JVM). On-device the order is visible in + // logcat: `adb logcat | grep "Moving WorkSpec"` shows 6 promotions within + // ~130 ms before the fix, and one per ~0.6 s (serialized) after it. + testWidgets('issue_95: foreground Dart workers are admitted under the limit', + (tester) async { + const count = 6; + final results = >[]; + final files = []; + for (var i = 0; i < count; i++) { + final id = _id('issue95_$i'); + final file = File('${tmpDir.path}/$id.json'); + files.add(file); + results.add(_waitEvent(id)); + await NativeWorkManager.enqueue( + taskId: id, + trigger: const TaskTrigger.oneTime(), + worker: DartWorker( + callbackId: 'issue84_probe', input: {'path': file.path}), + constraints: const Constraints( + foregroundNotificationConfig: ForegroundNotificationConfig( + title: 'issue_95', + body: 'admission test', + showCancelButton: false, + ), + ), + ); + } + for (final event in await Future.wait(results) + .timeout(const Duration(seconds: 120))) { + expect(event?.success, isTrue, + reason: 'FGS Dart worker must not be cancelled at startup: $event'); + } + final boundaries = <(int, int)>[]; + for (final file in files) { + final iv = jsonDecode(file.readAsStringSync()) as Map; + boundaries.add((iv['start'] as int, 1)); + boundaries.add((iv['end'] as int, -1)); + } + boundaries.sort((a, b) => + a.$1 != b.$1 ? a.$1.compareTo(b.$1) : a.$2.compareTo(b.$2)); + var running = 0; + var peak = 0; + for (final b in boundaries) { + running += b.$2; + if (running > peak) peak = running; + } + print('issue_95: limit=$_issue84Limit peak=$peak completed=${files.length}'); + expect(running, 0); + expect(peak, lessThanOrEqualTo(_issue84Limit)); + }); + + // Issue #95: foreground (promoted) and plain background workers share ONE + // limiter. A mixed batch must still respect the limit and all must succeed; + // a foreground worker queued behind a background one must wait, not promote. + testWidgets('issue_95: mixed foreground and background workers share the limit', + (tester) async { + final results = >[]; + final files = []; + for (var i = 0; i < 6; i++) { + final id = _id('issue95mix_$i'); + final file = File('${tmpDir.path}/$id.json'); + files.add(file); + results.add(_waitEvent(id)); + final input = {'path': file.path}; + await NativeWorkManager.enqueue( + taskId: id, + trigger: const TaskTrigger.oneTime(), + worker: i.isEven + ? NativeWorker.custom(className: 'Issue84Probe', input: input) + : DartWorker(callbackId: 'issue84_probe', input: input), + constraints: i % 3 == 0 + ? const Constraints() + : const Constraints( + foregroundNotificationConfig: ForegroundNotificationConfig( + title: 'issue_95', + body: 'mixed admission', + showCancelButton: false, + ), + ), + ); + } + for (final event in await Future.wait(results) + .timeout(const Duration(seconds: 120))) { + expect(event?.success, isTrue, reason: 'mixed task must complete: $event'); + } + final boundaries = <(int, int)>[]; + for (final file in files) { + final iv = jsonDecode(file.readAsStringSync()) as Map; + boundaries.add((iv['start'] as int, 1)); + boundaries.add((iv['end'] as int, -1)); + } + boundaries.sort((a, b) => + a.$1 != b.$1 ? a.$1.compareTo(b.$1) : a.$2.compareTo(b.$2)); + var running = 0; + var peak = 0; + for (final b in boundaries) { + running += b.$2; + if (running > peak) peak = running; + } + print('issue_95 mixed: limit=$_issue84Limit peak=$peak'); + expect(running, 0); + expect(peak, lessThanOrEqualTo(_issue84Limit)); + }); + // ════════════════════════════════════════════════════════════ // GROUP 1 – Trigger Types // Verifies the bug fix: trigger was previously hardcoded to diff --git a/example/lib/main.dart b/example/lib/main.dart index e3aa197..b974016 100644 --- a/example/lib/main.dart +++ b/example/lib/main.dart @@ -107,6 +107,7 @@ void main() async { debugMode: true, dartWorkers: { 'customTask': customTaskCallback, + 'fgsAdmissionDemo': fgsAdmissionDemoCallback, 'heavyTask': heavyTaskCallback, 'longRunningTask': longRunningTaskCallback, 'cancellableTask': cancellableTaskCallback, diff --git a/example/lib/pages/fgs_bypass_demo_page.dart b/example/lib/pages/fgs_bypass_demo_page.dart index 3336939..476c5de 100644 --- a/example/lib/pages/fgs_bypass_demo_page.dart +++ b/example/lib/pages/fgs_bypass_demo_page.dart @@ -1,7 +1,16 @@ import 'package:flutter/material.dart'; import 'package:native_workmanager/native_workmanager.dart'; +import 'dart:async'; import 'dart:math'; +/// Issue #95 demo callback: holds the execution slot for 3 s so the admission +/// order is visible. Registered as 'fgsAdmissionDemo' in main.dart. +@pragma('vm:entry-point') +Future fgsAdmissionDemoCallback(Map? input) async { + await Future.delayed(const Duration(seconds: 3)); + return true; +} + class FgsBypassDemoPage extends StatefulWidget { const FgsBypassDemoPage({super.key}); @@ -22,6 +31,48 @@ class _FgsBypassDemoPageState extends State { }); } + /// Issue #95: enqueue several FGS Dart workers at once. Each waits for an + /// execution slot BEFORE promoting to foreground, so with a small + /// `maxConcurrentTasks` they complete one batch at a time and none is + /// cancelled during Flutter-engine start-up. + Future _runAdmissionDemo() async { + const count = 6; + setState(() => _isScheduling = true); + final run = Random().nextInt(100000); + final pending = {}; + late final StreamSubscription sub; + sub = NativeWorkManager.events.listen((e) { + if (pending.remove(e.taskId)) { + _addLog('${e.success ? "✅" : "❌"} ${e.taskId} finished ' + '(${pending.length} left)'); + if (pending.isEmpty) sub.cancel(); + } + }); + try { + for (var i = 0; i < count; i++) { + final id = 'fgs_admission_${run}_$i'; + pending.add(id); + await NativeWorkManager.enqueue( + taskId: id, + worker: DartWorker(callbackId: 'fgsAdmissionDemo'), + constraints: const Constraints( + foregroundNotificationConfig: ForegroundNotificationConfig( + title: 'Admission demo', + body: 'Waiting for a slot, then promoting', + showCancelButton: false, + ), + ), + ); + } + _addLog('Enqueued $count FGS Dart tasks — expect batches, no failures.'); + } catch (e) { + sub.cancel(); + _addLog('❌ Error: $e'); + } finally { + setState(() => _isScheduling = false); + } + } + Future _runFgsTask() async { setState(() => _isScheduling = true); final taskId = "fgs_task_${Random().nextInt(1000)}"; @@ -113,6 +164,12 @@ class _FgsBypassDemoPageState extends State { : const Icon(Icons.bolt), label: const Text("RUN HIGH-PRIORITY FGS TASK"), ), + const SizedBox(height: 12), + OutlinedButton.icon( + onPressed: _isScheduling ? null : _runAdmissionDemo, + icon: const Icon(Icons.queue), + label: const Text("RUN 6 FGS DART TASKS (ADMISSION, #95)"), + ), const SizedBox(height: 20), const Text( "Execution Logs:",