Skip to content

Commit

Permalink
Resolve issues with foreground tasks getting cancelled
Browse files Browse the repository at this point in the history
  • Loading branch information
bjester committed Mar 20, 2024
1 parent 3d0cd8b commit 483e729
Show file tree
Hide file tree
Showing 19 changed files with 344 additions and 181 deletions.
1 change: 1 addition & 0 deletions python-for-android/dists/kolibri/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -108,5 +108,6 @@ dependencies {
implementation 'androidx.concurrent:concurrent-futures:1.1.0'
implementation 'androidx.work:work-runtime:2.9.0'
implementation 'androidx.work:work-multiprocess:2.9.0'
implementation "androidx.lifecycle:lifecycle-service:2.7.0"
implementation 'net.sourceforge.streamsupport:java9-concurrent-backport:2.0.5'
}
9 changes: 2 additions & 7 deletions python-for-android/dists/kolibri/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

<uses-permission android:name="android.permission.CHANGE_WIFI_MULTICAST_STATE" />

<uses-permission android:name="android.permission.POST_NOTIFICATIONS" />

<application
android:label="@string/app_name"
Expand Down Expand Up @@ -94,15 +95,9 @@
<service
android:name="androidx.work.impl.foreground.SystemForegroundService"
android:foregroundServiceType="dataSync|connectedDevice"
tools:node="merge"
/>

<service
android:name=".WorkerService"
android:foregroundServiceType="dataSync|connectedDevice"
android:process="@string/task_worker_process"
android:exported="true"
android:permission="android.permission.FOREGROUND_SERVICE_CONNECTED_DEVICE, android.permission.FOREGROUND_SERVICE_DATA_SYNC"
tools:node="merge"
/>

<provider
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package org.kivy.android;

import android.content.Context;

/**
* This class is used to provide the Context to the python-side in a way that minimizes us
* potentially creating memory leaks by holding a static reference to the context. It implements
* AutoCloseable so that it can be used in a try-with-resources block and automatically release
* the static instance and the instance's context.
*/
public class PythonProvider implements AutoCloseable {
private static final ThreadLocal<PythonProvider> localInstance = new ThreadLocal<>();
private final Context context;

public PythonProvider(Context context) {
this.context = context;
localInstance.set(this);
}

public Context getContext() {
return context;
}


public void close() {
localInstance.remove();
}

public static PythonProvider create(Context context) {
if (isActive()) {
throw new RuntimeException("PythonProviders cannot be nested");
}
return new PythonProvider(context);
}

public static PythonProvider get() {
if (!isActive()) {
throw new RuntimeException("PythonProvider not initialized");
}
return PythonProvider.localInstance.get();
}

public static boolean isActive() {
return localInstance.get() != null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@

import androidx.annotation.NonNull;

import java.io.File;

/**
* Ideally this would be called `PythonWorkerImpl` but the name is used in the native code.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@
import android.content.Context;

import org.kivy.android.PythonActivity;
import org.learningequality.Kolibri.WorkerService;
import org.kivy.android.PythonProvider;

public class ContextUtil {
public static Context getApplicationContext() {
if (isServiceContext()) {
return WorkerService.mService.getApplicationContext();
if (PythonProvider.isActive()) {
return PythonProvider.get().getContext();
}
if (isActivityContext()) {
return PythonActivity.mActivity.getApplicationContext();
Expand All @@ -19,8 +19,4 @@ public static Context getApplicationContext() {
public static boolean isActivityContext() {
return PythonActivity.mActivity != null;
}

public static boolean isServiceContext() {
return WorkerService.mService != null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,42 +7,27 @@
import androidx.annotation.NonNull;
import androidx.work.WorkerParameters;

import org.learningequality.notification.Manager;
import org.learningequality.notification.NotificationRef;
import org.learningequality.task.Worker;
import org.kivy.android.PythonWorker;

import org.learningequality.Kolibri.task.TaskWorkerImpl;

/**
* Background worker that runs a Python task in a background thread. This will likely be run by the
* SystemJobService.
*/
final public class BackgroundWorker extends androidx.work.Worker implements Worker {
final public class BackgroundWorker extends Worker {
private static final String TAG = "Kolibri.BackgroundWorker";
private final PythonWorker workerImpl;

public BackgroundWorker(
@NonNull Context context, @NonNull WorkerParameters workerParams
) {
super(context, workerParams);
workerImpl = new PythonWorker(context, "TaskWorker", "taskworker.py");
}

/**
* Parent worker class will call this method on a background thread automatically.
*/
@Override
@NonNull
public Result doWork() {
Log.d(TAG, "Running background task " + getId());
final String id = getId().toString();
final String arg = getArgument();
Result r = workerImpl.execute(id, arg) ? Result.success() : Result.failure();
hideNotification();
return r;
}

@Override
public void onStopped() {
Log.d(TAG, "Stopping background remote task " + getId());
super.onStopped();
hideNotification();
protected TaskWorkerImpl getWorkerImpl() {
Log.d(TAG, "Starting background task: " + getId());
return new TaskWorkerImpl(getId(), getApplicationContext());
}
}
Original file line number Diff line number Diff line change
@@ -1,102 +1,60 @@
package org.learningequality.Kolibri;

import android.annotation.SuppressLint;
import android.app.Notification;
import android.content.pm.ServiceInfo;
import android.util.Log;

import androidx.annotation.NonNull;
import androidx.concurrent.futures.CallbackToFutureAdapter;
import androidx.work.ForegroundInfo;
import androidx.work.impl.utils.futures.SettableFuture;
import androidx.work.multiprocess.RemoteListenableWorker;

import com.google.common.util.concurrent.ListenableFuture;

import org.learningequality.Kolibri.task.TaskWorkerImpl;
import org.learningequality.notification.Builder;
import org.learningequality.notification.NotificationRef;
import org.learningequality.task.Worker;
import org.kivy.android.PythonWorker;

import java.util.concurrent.Future;
import java.util.concurrent.ThreadPoolExecutor;

final public class ForegroundWorker extends RemoteListenableWorker implements Worker {
final public class ForegroundWorker extends Worker {
private static final String TAG = "Kolibri.ForegroundWorker";
private final PythonWorker workerImpl;

public ForegroundWorker(
@NonNull android.content.Context context,
@NonNull androidx.work.WorkerParameters workerParams
) {
super(context, workerParams);
workerImpl = new PythonWorker(context, "TaskWorker", "taskworker.py");
}

@SuppressLint("RestrictedApi")
@Override
@NonNull
public ListenableFuture<Result> startRemoteWork() {
Log.d(TAG, "Running foreground remote task " + getId());
final SettableFuture<Result> future = SettableFuture.create();
final String id = getId().toString();
final String arg = getArgument();

// See executor defined in configuration
final ThreadPoolExecutor executor = (ThreadPoolExecutor) getBackgroundExecutor();
// This is somewhat similar to what the plain `Worker` class does, except that we
// use `submit` instead of `execute` so we can propagate cancellation
// See https://android.googlesource.com/platform/frameworks/support/+/60ae0eec2a32396c22ad92502cde952c80d514a0/work/workmanager/src/main/java/androidx/work/Worker.java
final Future<?> threadFuture = executor.submit(() -> {
try {
Result r = workerImpl.execute(id, arg) ? Result.success() : Result.failure();
future.set(r);
} catch (Exception e) {
Log.e(TAG, "Exception in remote python work for " + id, e);
future.setException(e);
}
});

// If `RunnableFuture` was a `ListenableFuture` we could simply use `future.setFuture` to
// propagate the result and cancellation, but instead add listener to propagate
// cancellation to python thread, using the task executor which should invoke this in the
// main thread (where this was originally called from)
future.addListener(() -> {
synchronized (future) {
if (future.isCancelled()) {
Log.i(TAG, "Interrupting python thread");
synchronized (threadFuture) {
threadFuture.cancel(true);
}
}

if (future.isDone()) {
hideNotification();
}
}
}, getTaskExecutor().getMainThreadExecutor());
return future;
protected TaskWorkerImpl getWorkerImpl() {
Log.d(TAG, "Starting foreground task: " + getId());
return new TaskWorkerImpl(getId(), getApplicationContext());
}

@Override
public void onStopped() {
Log.d(TAG, "Stopping foreground remote task " + getId());
super.onStopped();
hideNotification();
@NonNull
public Result doWork() {
Log.d(TAG, "Setting task as foreground: " + getId());
setForegroundAsync(getForegroundInfo());
return super.doWork();
}

@NonNull
public ForegroundInfo getForegroundInfo() {
NotificationRef ref = WorkerService.buildNotificationRef();
Builder builder = new Builder(getApplicationContext(), ref);
NotificationRef ref = getNotificationRef();
Notification lastNotification = this.getLastNotification();
if (lastNotification == null) {
// build default notification
lastNotification = new Builder(getApplicationContext(), ref).build();
}
// If API level is at least 29
if (android.os.Build.VERSION.SDK_INT >= android.os.Build.VERSION_CODES.Q) {
return new ForegroundInfo(
ref.getId(),
builder.build(),
lastNotification,
ServiceInfo.FOREGROUND_SERVICE_TYPE_MANIFEST
);
}

return new ForegroundInfo(ref.getId(), builder.build());
return new ForegroundInfo(ref.getId(), lastNotification);
}

@Override
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,9 @@
import androidx.work.OutOfQuotaPolicy;
import androidx.work.WorkInfo;
import androidx.work.WorkQuery;
import androidx.work.multiprocess.RemoteListenableWorker;

import org.learningequality.Kolibri.BackgroundWorker;
import org.learningequality.Kolibri.ForegroundWorker;
import org.learningequality.Kolibri.WorkerService;
import org.learningequality.Kolibri.sqlite.JobStorage;
import org.learningequality.task.Worker;

Expand Down Expand Up @@ -161,16 +159,6 @@ private Data buildInputData() {
String dataArgument = id == null ? "" : id;
Data.Builder builder = new Data.Builder()
.putString(Worker.ARGUMENT_WORKER_ARGUMENT, dataArgument);

if (longRunning || expedite) {
builder.putString(
RemoteListenableWorker.ARGUMENT_PACKAGE_NAME, "org.learningequality.Kolibri"
)
.putString(
RemoteListenableWorker.ARGUMENT_CLASS_NAME,
WorkerService.class.getName()
);
}
Data data = builder.build();
Log.v(TAG, "Worker request data: " + data.toString());
return data;
Expand Down
Loading

0 comments on commit 483e729

Please sign in to comment.