Skip to content

Commit

Permalink
[LEIP-299] Fix ShutdownFinishedHandler
Browse files Browse the repository at this point in the history
  • Loading branch information
hb0 committed Jan 16, 2025
1 parent 2903b6a commit 388dee6
Show file tree
Hide file tree
Showing 9 changed files with 65 additions and 30 deletions.
24 changes: 23 additions & 1 deletion datacapturing/lint-baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,18 @@
errorLine2=" ^">
<location
file="src/main/kotlin/de/cyface/datacapturing/DataCapturingService.kt"
line="748"
line="747"
column="13"/>
</issue>

<issue
id="UnspecifiedRegisterReceiverFlag"
message="`finishedHandler` \&#xA;is missing `RECEIVER_EXPORTED` or `RECEIVER_NOT_EXPORTED` flag for unprotected \&#xA;broadcasts registered for de.cyface.service_stopped"
errorLine1=" context!!.registerReceiver("
errorLine2=" ^">
<location
file="src/main/kotlin/de/cyface/datacapturing/DataCapturingService.kt"
line="807"
column="13"/>
</issue>

Expand All @@ -37,4 +48,15 @@
file="$GRADLE_USER_HOME/caches/modules-2/files-2.1/com.google.http-client/google-http-client/1.42.3/e0feb1bd93ad9fb1e064706cff96e32b41a57b9c/google-http-client-1.42.3.jar"/>
</issue>

<issue
id="KaptUsageInsteadOfKsp"
message="This library supports using KSP instead of kapt, which greatly improves performance. Learn more: https://developer.android.com/studio/build/migrate-to-ksp"
errorLine1=" kapt &quot;androidx.room:room-compiler:$rootProject.ext.roomVersion&quot;"
errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~">
<location
file="build.gradle"
line="109"
column="5"/>
</issue>

</issues>
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ class DataCapturingServiceTest {
val condition = lock.newCondition()
val shutDownFinishedHandler = TestShutdownFinishedHandler(
lock,
condition, MessageCodes.LOCAL_BROADCAST_SERVICE_STOPPED
condition, MessageCodes.GLOBAL_BROADCAST_SERVICE_STOPPED
)
oocut!!.stop(shutDownFinishedHandler)

Expand Down Expand Up @@ -258,7 +258,7 @@ class DataCapturingServiceTest {
val condition = lock.newCondition()
val shutDownFinishedHandler = TestShutdownFinishedHandler(
lock, condition,
MessageCodes.LOCAL_BROADCAST_SERVICE_STOPPED
MessageCodes.GLOBAL_BROADCAST_SERVICE_STOPPED
)
oocut!!.pause(shutDownFinishedHandler)
checkThatStopped(shutDownFinishedHandler, measurementIdentifier)
Expand Down Expand Up @@ -316,7 +316,7 @@ class DataCapturingServiceTest {
val condition = lock.newCondition()
val shutDownFinishedHandler = TestShutdownFinishedHandler(
lock, condition,
MessageCodes.LOCAL_BROADCAST_SERVICE_STOPPED
MessageCodes.GLOBAL_BROADCAST_SERVICE_STOPPED
)
oocut!!.stop(shutDownFinishedHandler)
checkThatStopped(shutDownFinishedHandler, measurementIdentifier)
Expand Down Expand Up @@ -489,21 +489,21 @@ We should consider refactoring the code before to use startCommandReceived as in
val condition4 = lock4.newCondition()
val shutDownFinishedHandler1 = TestShutdownFinishedHandler(
lock4, condition4,
MessageCodes.LOCAL_BROADCAST_SERVICE_STOPPED
MessageCodes.GLOBAL_BROADCAST_SERVICE_STOPPED
)
// Do not reuse the lock/condition!
val lock5: Lock = ReentrantLock()
val condition5 = lock5.newCondition()
val shutDownFinishedHandler2 = TestShutdownFinishedHandler(
lock5, condition5,
MessageCodes.LOCAL_BROADCAST_SERVICE_STOPPED
MessageCodes.GLOBAL_BROADCAST_SERVICE_STOPPED
)
// Do not reuse the lock/condition!
val lock6: Lock = ReentrantLock()
val condition6 = lock6.newCondition()
val shutDownFinishedHandler3 = TestShutdownFinishedHandler(
lock6, condition6,
MessageCodes.LOCAL_BROADCAST_SERVICE_STOPPED
MessageCodes.GLOBAL_BROADCAST_SERVICE_STOPPED
)

// First Start/stop without waiting
Expand Down Expand Up @@ -674,7 +674,7 @@ We should consider refactoring the code before to use startCommandReceived as in
TestShutdownFinishedHandler(
lock,
condition,
MessageCodes.LOCAL_BROADCAST_SERVICE_STOPPED
MessageCodes.GLOBAL_BROADCAST_SERVICE_STOPPED
)
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,10 @@ import de.cyface.persistence.SetupException
import de.cyface.persistence.exception.NoSuchMeasurementException
import de.cyface.persistence.model.Modality
import de.cyface.synchronization.CyfaceAuthenticator
import de.cyface.synchronization.settings.DefaultSynchronizationSettings
import de.cyface.testutils.SharedTestUtils.clearPersistenceLayer
import kotlinx.coroutines.runBlocking
import org.hamcrest.CoreMatchers
import org.hamcrest.MatcherAssert
import org.json.JSONObject
import org.junit.After
import org.junit.Before
import org.junit.Rule
Expand Down Expand Up @@ -208,7 +206,7 @@ class PingPongTest {
// Stop Capturing
val shutdownHandler = TestShutdownFinishedHandler(
lock!!, condition!!,
MessageCodes.LOCAL_BROADCAST_SERVICE_STOPPED
MessageCodes.GLOBAL_BROADCAST_SERVICE_STOPPED
)
dcs!!.stop(shutdownHandler)

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2017-2022 Cyface GmbH
* Copyright 2017-2025 Cyface GmbH
*
* This file is part of the Cyface SDK for Android.
*
Expand All @@ -26,7 +26,7 @@
*
* @author Klemens Muthmann
* @author Armin Schnabel
* @version 5.0.0
* @version 6.0.0
* @since 2.0.0
*/
public class MessageCodes {
Expand Down Expand Up @@ -92,11 +92,13 @@ public class MessageCodes {
*/
public static final String GLOBAL_BROADCAST_PONG = "de.cyface.pong";
/**
* Local (i.e. inner process communication) Broadcast action identifier sent by the {@link DataCapturingService}
* Global (inter-process) action identifier sent by the {@link DataCapturingService}
* after it has received a inter-process {@link MessageCodes#SERVICE_STOPPED} from the
* {@link DataCapturingBackgroundService} that it has successfully stopped.
* <p>
* Using global broadcast as local broadcast broke ShutdownFinishedHandler. [LEIP-299]
*/
public static final String LOCAL_BROADCAST_SERVICE_STOPPED = "de.cyface.service_stopped";
public static final String GLOBAL_BROADCAST_SERVICE_STOPPED = "de.cyface.service_stopped";

/**
* Private constructor for utility class.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ public void checkIsRunningAsync(final long timeout, final @NonNull TimeUnit unit
Handler receiverHandler = new Handler(pongReceiverThread.getLooper());
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) {
context.get().registerReceiver(this, new IntentFilter(pongActionId), null,
receiverHandler, Context.RECEIVER_EXPORTED);
receiverHandler, Context.RECEIVER_EXPORTED); // Does not work with NOT_EXPORTED
} else {
context.get().registerReceiver(this, new IntentFilter(pongActionId), null,
receiverHandler);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public void onReceive(@NonNull final Context context, @NonNull final Intent inte
shutDownFinished(measurementIdentifier);

try {
LocalBroadcastManager.getInstance(context).unregisterReceiver(this);
context.unregisterReceiver(this);
} catch (IllegalArgumentException e) {
Log.w(TAG, "Probably tried to deregister shut down finished broadcast receiver twice.", e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ public void onCreate() {
// Allows other parties to ping this service to see if it is running
pingReceiver = new PingReceiver(GLOBAL_BROADCAST_PING, GLOBAL_BROADCAST_PONG);
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) {
// Does not work with NOT_EXPORTED
registerReceiver(pingReceiver, new IntentFilter(GLOBAL_BROADCAST_PING), Context.RECEIVER_EXPORTED);
} else {
registerReceiver(pingReceiver, new IntentFilter(GLOBAL_BROADCAST_PING));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2017-2023 Cyface GmbH
* Copyright 2017-2025 Cyface GmbH
*
* This file is part of the Cyface SDK for Android.
*
Expand Down Expand Up @@ -35,7 +35,6 @@ import android.os.Messenger
import android.os.RemoteException
import android.util.Log
import androidx.core.app.ActivityCompat
import androidx.localbroadcastmanager.content.LocalBroadcastManager
import de.cyface.datacapturing.backend.DataCapturingBackgroundService
import de.cyface.datacapturing.exception.CorruptedMeasurementException
import de.cyface.datacapturing.exception.DataCapturingException
Expand Down Expand Up @@ -78,7 +77,7 @@ import java.util.concurrent.locks.ReentrantLock
*
* @author Klemens Muthmann
* @author Armin Schnabel
* @version 20.0.0
* @version 21.0.0
* @since 1.0.0
* @property context The context (i.e. `Activity`) handling this service.
* @property authority The `ContentProvider` authority required to request a sync operation in the
Expand Down Expand Up @@ -742,7 +741,7 @@ abstract class DataCapturingService(
context!!.registerReceiver(
startUpFinishedHandler,
IntentFilter(MessageCodes.GLOBAL_BROADCAST_SERVICE_STARTED),
Context.RECEIVER_NOT_EXPORTED
Context.RECEIVER_EXPORTED
)
} else {
context!!.registerReceiver(
Expand Down Expand Up @@ -798,10 +797,18 @@ abstract class DataCapturingService(
Constants.TAG,
"Registering finishedHandler for service stop synchronization broadcast."
)
LocalBroadcastManager.getInstance(context!!).registerReceiver(
finishedHandler,
IntentFilter(MessageCodes.LOCAL_BROADCAST_SERVICE_STOPPED)
)
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) {
context!!.registerReceiver(
finishedHandler,
IntentFilter(MessageCodes.GLOBAL_BROADCAST_SERVICE_STOPPED),
Context.RECEIVER_EXPORTED, // Does not work with NOT_EXPORTED or local broadcast [LEIP-299]
)
} else {
context!!.registerReceiver(
finishedHandler,
IntentFilter(MessageCodes.GLOBAL_BROADCAST_SERVICE_STOPPED),
)
}
val serviceWasActive: Boolean
try {
// For some reasons we have to call the unbind here.
Expand Down Expand Up @@ -836,15 +843,15 @@ abstract class DataCapturingService(
context: Context?, measurementIdentifier: Long,
stoppedSuccessfully: Boolean
) {
val stoppedBroadcastIntent = Intent(MessageCodes.LOCAL_BROADCAST_SERVICE_STOPPED)
val stoppedBroadcastIntent = Intent(MessageCodes.GLOBAL_BROADCAST_SERVICE_STOPPED)
// The measurement id should always be set, also if `stoppedSuccessfully=false` [STAD-333]
Validate.isTrue(measurementIdentifier > 0L)
stoppedBroadcastIntent.putExtra(BundlesExtrasCodes.MEASUREMENT_ID, measurementIdentifier)
stoppedBroadcastIntent.putExtra(
BundlesExtrasCodes.STOPPED_SUCCESSFULLY,
stoppedSuccessfully
)
LocalBroadcastManager.getInstance(context!!).sendBroadcast(stoppedBroadcastIntent)
context!!.sendBroadcast(stoppedBroadcastIntent)
}

/**
Expand Down Expand Up @@ -1252,7 +1259,10 @@ abstract class DataCapturingService(
)
)

MessageCodes.SERVICE_STOPPED, MessageCodes.SERVICE_STOPPED_ITSELF -> listener.onCapturingStopped()
// Don't call onCapturingStopped() as this is only intended for self-stopped events
MessageCodes.SERVICE_STOPPED -> {}
MessageCodes.SERVICE_STOPPED_ITSELF -> listener.onCapturingStopped()

else -> listener.onErrorState(
DataCapturingException(
context.getString(R.string.unknown_message_error, messageCode)
Expand Down Expand Up @@ -1290,7 +1300,7 @@ abstract class DataCapturingService(
val condition = lock.newCondition()
val synchronizationReceiver = StopSynchronizer(
lock, condition,
MessageCodes.LOCAL_BROADCAST_SERVICE_STOPPED
MessageCodes.GLOBAL_BROADCAST_SERVICE_STOPPED
)
// The background service already received a stopSelf command but as it's still
// bound to this service it should be still alive. We unbind it from this service via the
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2018-2024 Cyface GmbH
* Copyright 2018-2025 Cyface GmbH
*
* This file is part of the Cyface SDK for Android.
*
Expand Down Expand Up @@ -85,6 +85,7 @@ public static void sendErrorIntent(final Context context, final int errorCode, f
final Intent intent = new Intent(ERROR_INTENT);
intent.putExtra(HTTP_CODE_EXTRA, httpCode);
intent.putExtra(ERROR_CODE_EXTRA, errorCode);
// Other than ShutdownFinishedHandler, this seem to work with local broadcast
LocalBroadcastManager.getInstance(context).sendBroadcast(intent);
Log.d(TAG, String.format("ErrorHandler (err %s, httpErr %s): %s", errorCode, httpCode, message));
}
Expand All @@ -103,6 +104,7 @@ public static void sendErrorIntent(final Context context, final int errorCode, @
final Intent intent = new Intent(ERROR_INTENT);
intent.putExtra(ERROR_CODE_EXTRA, errorCode);
intent.putExtra(ERROR_BACKGROUND_EXTRA, fromBackground);
// Other than ShutdownFinishedHandler, this seem to work with local broadcast
LocalBroadcastManager.getInstance(context).sendBroadcast(intent);
if (message != null) {
Log.d(TAG, message);
Expand Down

0 comments on commit 388dee6

Please sign in to comment.