Signal/Android
1
0
Fork 0
mirror of https://github.com/signalapp/Signal-Android.git synced 2025-12-24 21:15:48 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix foreground service crash with state tracking.

Browse Source
This commit is contained in:
Greyson Parrelli
2023-11-08 06:22:36 -08:00
committed by Cody Henthorne
parent 60a6535a12
commit 5cd2568776
4 changed files with 136 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
app/src/main/java/org/thoughtcrime/securesms/jobs/AttachmentCompressionJob.java
View File

@@ -206,7 +206,9 @@ public final class AttachmentCompressionJob extends BaseJob {
}
try (AttachmentProgressService.Controller notification = AttachmentProgressService.start(context, context.getString(R.string.AttachmentUploadJob_compressing_video_start))) {
notification.setIndeterminate(true);
if (notification != null) {
notification.setIndeterminate(true);
}
try (MediaDataSource dataSource = attachmentDatabase.mediaDataSourceFor(attachment.getAttachmentId(), false)) {
if (dataSource == null) {
@@ -232,7 +234,9 @@ public final class AttachmentCompressionJob extends BaseJob {
try {
try (OutputStream outputStream = ModernEncryptingPartOutputStream.createFor(attachmentSecret, file, true).second) {
transcoder.transcode(percent -> {
notification.setProgress(percent / 100f);
if (notification != null) {
notification.setProgress(percent / 100f);
}
eventBus.postSticky(new PartProgressEvent(attachment,
PartProgressEvent.Type.COMPRESSION,
100,
@@ -261,7 +265,9 @@ public final class AttachmentCompressionJob extends BaseJob {
Log.i(TAG, "Compressing with android in-memory muxer");
try (MediaStream mediaStream = transcoder.transcode(percent -> {
notification.setProgress(percent/100f);
if (notification != null) {
notification.setProgress(percent / 100f);
}
eventBus.postSticky(new PartProgressEvent(attachment,
PartProgressEvent.Type.COMPRESSION,
100,
@@ -289,7 +295,7 @@ public final class AttachmentCompressionJob extends BaseJob {
throw new UndeliverableMessageException("Failed to transcode and cannot skip due to editing", e);
}
}
} catch (UnableToStartException | IOException | MmsException e) {
} catch (IOException | MmsException e) {
throw new UndeliverableMessageException("Failed to transcode", e);
}
return attachment;

7
app/src/main/java/org/thoughtcrime/securesms/jobs/AttachmentUploadJob.kt
View File

@@ -145,12 +145,7 @@ class AttachmentUploadJob private constructor(
private fun getAttachmentNotificationIfNeeded(attachment: Attachment): AttachmentProgressService.Controller? {
return if (attachment.size >= FOREGROUND_LIMIT) {
try {
AttachmentProgressService.start(context, context.getString(R.string.AttachmentUploadJob_uploading_media))
} catch (e: UnableToStartException) {
Log.w(TAG, "Unable to start foreground service", e)
null
}
AttachmentProgressService.start(context, context.getString(R.string.AttachmentUploadJob_uploading_media))
} else {
null
}

22
app/src/main/java/org/thoughtcrime/securesms/service/AttachmentProgressService.kt
View File

@@ -14,13 +14,11 @@ import org.signal.core.util.PendingIntentFlags
import org.signal.core.util.logging.Log
import org.thoughtcrime.securesms.MainActivity
import org.thoughtcrime.securesms.R
import org.thoughtcrime.securesms.jobs.UnableToStartException
import org.thoughtcrime.securesms.notifications.NotificationChannels
import org.thoughtcrime.securesms.notifications.NotificationIds
import java.util.concurrent.CopyOnWriteArraySet
import java.util.concurrent.locks.ReentrantLock
import kotlin.concurrent.withLock
import kotlin.jvm.Throws
/**
* A service to show attachment progress. In order to ensure we only show one status notification,
@@ -55,23 +53,27 @@ class AttachmentProgressService : SafeForegroundService() {
* use to update the notification.
*
* Important: This could fail to start! We do our best to start the service regardless of context,
* but it will fail on some devices, throwing an [UnableToStartException] if it does so.
* but it will fail on some devices. If this happens, the returned [Controller] will be null.
*/
@JvmStatic
@Throws(UnableToStartException::class)
fun start(context: Context, title: String): Controller {
fun start(context: Context, title: String): Controller? {
controllerLock.withLock {
if (controllers.isEmpty()) {
val started = if (controllers.isEmpty()) {
Log.i(TAG, "[start] First controller. Starting.")
SafeForegroundService.start(context, AttachmentProgressService::class.java)
} else {
Log.i(TAG, "[start] No need to start the service again. Already have an active controller.")
true
}
val controller = Controller(context, title)
controllers += controller
onControllersChanged(context)
return controller
return if (started) {
val controller = Controller(context, title)
controllers += controller
onControllersChanged(context)
controller
} else {
null
}
}
}

133
app/src/main/java/org/thoughtcrime/securesms/service/SafeForegroundService.kt
View File

@@ -14,7 +14,9 @@ import androidx.core.app.ServiceCompat
import org.signal.core.util.logging.Log
import org.thoughtcrime.securesms.jobs.ForegroundServiceUtil
import org.thoughtcrime.securesms.jobs.UnableToStartException
import kotlin.jvm.Throws
import java.util.concurrent.locks.ReentrantLock
import javax.annotation.CheckReturnValue
import kotlin.concurrent.withLock
/**
* A simple parent class meant to encourage the safe usage of foreground services.
@@ -29,17 +31,48 @@ abstract class SafeForegroundService : Service() {
private const val ACTION_START = "start"
private const val ACTION_STOP = "stop"
private var states: MutableMap<Class<out SafeForegroundService>, State> = mutableMapOf()
private val stateLock = ReentrantLock()
/**
* Safety starts the target foreground service.
* Important: This operation can fail. If it does, [UnableToStartException] is thrown.
* @return False if we tried to start the service but failed, otherwise true.
*/
@Throws(UnableToStartException::class)
fun start(context: Context, serviceClass: Class<out SafeForegroundService>) {
val intent = Intent(context, serviceClass).apply {
action = ACTION_START
}
@CheckReturnValue
fun start(context: Context, serviceClass: Class<out SafeForegroundService>): Boolean {
stateLock.withLock {
val state = currentState(serviceClass)
ForegroundServiceUtil.startWhenCapable(context, intent)
Log.d(TAG, "[start] Current state: $state")
return when (state) {
State.STARTING,
State.NEEDS_RESTART -> {
Log.d(TAG, "[start] No need to start the service again. Current state: $state")
true
}
State.STOPPED -> {
Log.d(TAG, "[start] Starting service.")
states[serviceClass] = State.STARTING
try {
ForegroundServiceUtil.startWhenCapable(
context = context,
intent = Intent(context, serviceClass).apply { action = ACTION_START }
)
true
} catch (e: UnableToStartException) {
Log.w(TAG, "[start] Failed to start the service!")
states[serviceClass] = State.STOPPED
false
}
}
State.STOPPING -> {
Log.d(TAG, "[start] Attempted to start while the service is stopping. Enqueueing a restart.")
states[serviceClass] = State.NEEDS_RESTART
true
}
}
}
}
/**
@@ -48,16 +81,42 @@ abstract class SafeForegroundService : Service() {
* a start is pending, preventing the posting of a foreground notification.
*/
fun stop(context: Context, serviceClass: Class<out SafeForegroundService>) {
val intent = Intent(context, serviceClass).apply {
action = ACTION_STOP
}
stateLock.withLock {
val state = currentState(serviceClass)
try {
ForegroundServiceUtil.startWhenCapable(context, intent)
} catch (e: UnableToStartException) {
Log.w(TAG, "Failed to start service class $serviceClass", e)
Log.d(TAG, "[stop] Current state: $state")
when (state) {
State.STARTING -> {
Log.d(TAG, "[stop] Stopping service.")
states[serviceClass] = State.STOPPING
try {
ForegroundServiceUtil.startWhenCapable(
context = context,
intent = Intent(context, serviceClass).apply { action = ACTION_STOP }
)
} catch (e: UnableToStartException) {
Log.w(TAG, "Failed to start service class $serviceClass", e)
states[serviceClass] = State.STOPPED
}
}
State.STOPPED,
State.STOPPING -> {
Log.d(TAG, "[stop] No need to stop the service. Current state: $state")
}
State.NEEDS_RESTART -> {
Log.i(TAG, "[stop] Clearing pending restart.")
states[serviceClass] = State.STOPPING
}
}
}
}
private fun currentState(clazz: Class<out SafeForegroundService>): State {
return states.getOrPut(clazz) { State.STOPPED }
}
}
override fun onCreate() {
@@ -74,10 +133,10 @@ abstract class SafeForegroundService : Service() {
when (val action = intent.action) {
ACTION_START -> {
onServiceStarted(intent)
onServiceStartCommandReceived(intent)
}
ACTION_STOP -> {
onServiceStopped(intent)
onServiceStopCommandReceived(intent)
ServiceCompat.stopForeground(this, ServiceCompat.STOP_FOREGROUND_REMOVE)
stopSelf()
}
@@ -88,8 +147,28 @@ abstract class SafeForegroundService : Service() {
}
override fun onDestroy() {
Log.d(tag, "[onDestroy]")
super.onDestroy()
stateLock.withLock {
val state = currentState(javaClass)
Log.d(tag, "[onDestroy] Current state: $state")
when (state) {
State.STOPPED,
State.STARTING,
State.STOPPING -> {
states[javaClass] = State.STOPPED
}
State.NEEDS_RESTART -> {
Log.i(TAG, "[onDestroy] Restarting service!")
states[javaClass] = State.STOPPED
if (!start(this, javaClass)) {
Log.w(TAG, "[onDestroy] Failed to restart service.")
}
}
}
}
}
override fun onBind(intent: Intent?): IBinder? = null
@@ -104,8 +183,22 @@ abstract class SafeForegroundService : Service() {
abstract fun getForegroundNotification(intent: Intent): Notification
/** Event listener for when the service is started via an intent. */
open fun onServiceStarted(intent: Intent) = Unit
open fun onServiceStartCommandReceived(intent: Intent) = Unit
/** Event listener for when the service is stopped via an intent. */
open fun onServiceStopped(intent: Intent) = Unit
open fun onServiceStopCommandReceived(intent: Intent) = Unit
private enum class State {
/** The service is not running. */
STOPPED,
/** We told the service to start (via an intent). It may or may not be actually running yet. */
STARTING,
/** We told the service to stop (via an intent), but it's still technically running. */
STOPPING,
/** We requested that the service be started while it was in the process of stopping. */
NEEDS_RESTART
}
}