PawletOS/app_Settings
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix crash in anomaly job service

Browse Source 
If job has been stopped by any reason, we should cancel the background
thread, otherwise it will throw SecurityException when dequeuing from
JobParams.

This CL adds a lock to synchronize the method and stops dequeue work
if job has been canceled.

Change-Id: I7732b7f7d444a55a4b4ba6645cd2c16b6f840a6c
Merged-In: I7732b7f7d444a55a4b4ba6645cd2c16b6f840a6c
Fixes: 77968649
Test: RunSettingsRoboTests
This commit is contained in:
Lei Yu
2018-04-12 16:44:45 -07:00
committed by jackqdyulei
parent 60c513d908
commit 1c95856977
2 changed files with 83 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

41
src/com/android/settings/fuelgauge/batterytip/AnomalyDetectionJobService.java
View File

@@ -38,7 +38,10 @@ import android.os.StatsDimensionsValue;
import android.os.UserHandle;
import android.os.UserManager;
import android.provider.Settings;
import androidx.annotation.VisibleForTesting;
import androidx.annotation.GuardedBy;
import android.util.Log;
import android.util.Pair;
@@ -65,10 +68,13 @@ public class AnomalyDetectionJobService extends JobService {
static final int UID_NULL = -1;
@VisibleForTesting
static final int STATSD_UID_FILED = 1;
@VisibleForTesting
static final long MAX_DELAY_MS = TimeUnit.MINUTES.toMillis(30);
private final Object mLock = new Object();
@GuardedBy("mLock")
private boolean mIsJobCanceled = false;
public static void scheduleAnomalyDetection(Context context, Intent intent) {
final JobScheduler jobScheduler = context.getSystemService(JobScheduler.class);
final ComponentName component = new ComponentName(context,
@@ -102,14 +108,14 @@ public class AnomalyDetectionJobService extends JobService {
.getFactory(this).getMetricsFeatureProvider();
batteryUtils.initBatteryStatsHelper(batteryStatsHelper, null /* bundle */, userManager);
for (JobWorkItem item = params.dequeueWork(); item != null;
item = params.dequeueWork()) {
for (JobWorkItem item = dequeueWork(params); item != null; item = dequeueWork(params)) {
saveAnomalyToDatabase(context, batteryStatsHelper, userManager,
batteryDatabaseManager, batteryUtils, policy, powerWhitelistBackend,
contentResolver, powerUsageFeatureProvider, metricsFeatureProvider,
item.getIntent().getExtras());
completeWork(params, item);
}
jobFinished(params, false /* wantsReschedule */);
});
return true;
@@ -117,7 +123,10 @@ public class AnomalyDetectionJobService extends JobService {
@Override
public boolean onStopJob(JobParameters jobParameters) {
return false;
synchronized (mLock) {
mIsJobCanceled = true;
}
return true; // Need to reschedule
}
@VisibleForTesting
@@ -229,4 +238,26 @@ public class AnomalyDetectionJobService extends JobService {
return anomalyInfo.anomalyType
== StatsManagerConfig.AnomalyType.EXCESSIVE_BACKGROUND_SERVICE;
}
@VisibleForTesting
JobWorkItem dequeueWork(JobParameters parameters) {
synchronized (mLock) {
if (mIsJobCanceled) {
return null;
}
return parameters.dequeueWork();
}
}
@VisibleForTesting
void completeWork(JobParameters parameters, JobWorkItem item) {
synchronized (mLock) {
if (mIsJobCanceled) {
return;
}
parameters.completeWork(item);
}
}
}

58
tests/robotests/src/com/android/settings/fuelgauge/batterytip/AnomalyDetectionJobServiceTest.java
View File

@@ -23,11 +23,14 @@ import static android.os.StatsDimensionsValue.TUPLE_VALUE_TYPE;
import static com.google.common.truth.Truth.assertThat;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyBoolean;
import static org.mockito.Matchers.anyInt;
import static org.mockito.Matchers.anyLong;
import static org.mockito.Matchers.anyString;
import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.spy;
@@ -37,7 +40,9 @@ import static org.robolectric.RuntimeEnvironment.application;
import android.app.StatsManager;
import android.app.job.JobInfo;
import android.app.job.JobParameters;
import android.app.job.JobScheduler;
import android.app.job.JobWorkItem;
import android.content.Context;
import android.content.Intent;
import android.os.Bundle;
@@ -55,6 +60,7 @@ import com.android.settings.fuelgauge.BatteryUtils;
import com.android.settings.overlay.FeatureFactory;
import com.android.settings.testutils.FakeFeatureFactory;
import com.android.settings.testutils.SettingsRobolectricTestRunner;
import com.android.settings.testutils.shadow.ShadowConnectivityManager;
import com.android.settingslib.fuelgauge.PowerWhitelistBackend;
import org.junit.Before;
@@ -62,8 +68,11 @@ import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import org.robolectric.Robolectric;
import org.robolectric.RuntimeEnvironment;
import org.robolectric.Shadows;
import org.robolectric.android.controller.ServiceController;
import org.robolectric.annotation.Config;
import org.robolectric.shadows.ShadowJobScheduler;
import java.util.ArrayList;
@@ -71,6 +80,7 @@ import java.util.List;
import java.util.concurrent.TimeUnit;
@RunWith(SettingsRobolectricTestRunner.class)
@Config(shadows = ShadowConnectivityManager.class)
public class AnomalyDetectionJobServiceTest {
private static final int UID = 12345;
private static final String SYSTEM_PACKAGE = "com.android.system";
@@ -92,6 +102,10 @@ public class AnomalyDetectionJobServiceTest {
private PowerWhitelistBackend mPowerWhitelistBackend;
@Mock
private StatsDimensionsValue mStatsDimensionsValue;
@Mock
private JobParameters mJobParameters;
@Mock
private JobWorkItem mJobWorkItem;
private BatteryTipPolicy mPolicy;
private Bundle mBundle;
@@ -110,11 +124,14 @@ public class AnomalyDetectionJobServiceTest {
mFeatureFactory = FakeFeatureFactory.setupForTest();
when(mBatteryUtils.getAppLongVersionCode(any())).thenReturn(VERSION_CODE);
mAnomalyDetectionJobService = spy(new AnomalyDetectionJobService());
final ServiceController<AnomalyDetectionJobService> controller =
Robolectric.buildService(AnomalyDetectionJobService.class);
mAnomalyDetectionJobService = spy(controller.get());
doNothing().when(mAnomalyDetectionJobService).jobFinished(any(), anyBoolean());
}
@Test
public void testScheduleCleanUp() {
public void scheduleCleanUp() {
AnomalyDetectionJobService.scheduleAnomalyDetection(application, new Intent());
ShadowJobScheduler shadowJobScheduler =
@@ -129,7 +146,7 @@ public class AnomalyDetectionJobServiceTest {
}
@Test
public void testSaveAnomalyToDatabase_systemWhitelisted_doNotSave() {
public void saveAnomalyToDatabase_systemWhitelisted_doNotSave() {
doReturn(UID).when(mAnomalyDetectionJobService).extractUidFromStatsDimensionsValue(any());
doReturn(true).when(mPowerWhitelistBackend).isSysWhitelistedExceptIdle(any(String[].class));
@@ -144,7 +161,7 @@ public class AnomalyDetectionJobServiceTest {
}
@Test
public void testSaveAnomalyToDatabase_systemApp_doNotSaveButLog() {
public void saveAnomalyToDatabase_systemApp_doNotSaveButLog() {
final ArrayList<String> cookies = new ArrayList<>();
cookies.add(SUBSCRIBER_COOKIES_AUTO_RESTRICTION);
mBundle.putStringArrayList(StatsManager.EXTRA_STATS_BROADCAST_SUBSCRIBER_COOKIES, cookies);
@@ -170,7 +187,7 @@ public class AnomalyDetectionJobServiceTest {
}
@Test
public void testSaveAnomalyToDatabase_systemUid_doNotSave() {
public void saveAnomalyToDatabase_systemUid_doNotSave() {
doReturn(Process.SYSTEM_UID).when(
mAnomalyDetectionJobService).extractUidFromStatsDimensionsValue(any());
@@ -185,7 +202,7 @@ public class AnomalyDetectionJobServiceTest {
}
@Test
public void testSaveAnomalyToDatabase_uidNull_doNotSave() {
public void saveAnomalyToDatabase_uidNull_doNotSave() {
doReturn(AnomalyDetectionJobService.UID_NULL).when(
mAnomalyDetectionJobService).extractUidFromStatsDimensionsValue(any());
@@ -200,7 +217,7 @@ public class AnomalyDetectionJobServiceTest {
}
@Test
public void testSaveAnomalyToDatabase_normalAppWithAutoRestriction_save() {
public void saveAnomalyToDatabase_normalAppWithAutoRestriction_save() {
final ArrayList<String> cookies = new ArrayList<>();
cookies.add(SUBSCRIBER_COOKIES_AUTO_RESTRICTION);
mBundle.putStringArrayList(StatsManager.EXTRA_STATS_BROADCAST_SUBSCRIBER_COOKIES, cookies);
@@ -224,9 +241,8 @@ public class AnomalyDetectionJobServiceTest {
Pair.create(MetricsProto.MetricsEvent.FIELD_APP_VERSION_CODE, VERSION_CODE));
}
@Test
public void testSaveAnomalyToDatabase_normalAppWithoutAutoRestriction_save() {
public void saveAnomalyToDatabase_normalAppWithoutAutoRestriction_save() {
final ArrayList<String> cookies = new ArrayList<>();
cookies.add(SUBSCRIBER_COOKIES_NOT_AUTO_RESTRICTION);
mBundle.putStringArrayList(StatsManager.EXTRA_STATS_BROADCAST_SUBSCRIBER_COOKIES, cookies);
@@ -251,7 +267,7 @@ public class AnomalyDetectionJobServiceTest {
}
@Test
public void testExtractUidFromStatsDimensionsValue_extractCorrectUid() {
public void extractUidFromStatsDimensionsValue_extractCorrectUid() {
// Build an integer dimensions value.
final StatsDimensionsValue intValue = mock(StatsDimensionsValue.class);
when(intValue.isValueType(INT_VALUE_TYPE)).thenReturn(true);
@@ -270,7 +286,7 @@ public class AnomalyDetectionJobServiceTest {
}
@Test
public void testExtractUidFromStatsDimensionsValue_wrongFormat_returnNull() {
public void extractUidFromStatsDimensionsValue_wrongFormat_returnNull() {
// Build a float dimensions value
final StatsDimensionsValue floatValue = mock(StatsDimensionsValue.class);
when(floatValue.isValueType(FLOAT_VALUE_TYPE)).thenReturn(true);
@@ -280,4 +296,24 @@ public class AnomalyDetectionJobServiceTest {
assertThat(mAnomalyDetectionJobService.extractUidFromStatsDimensionsValue(
floatValue)).isEqualTo(AnomalyDetectionJobService.UID_NULL);
}
@Test
public void stopJobWhileDequeuingWork_shouldNotCrash() {
when(mJobParameters.dequeueWork()).thenThrow(new SecurityException());
mAnomalyDetectionJobService.onStopJob(mJobParameters);
// Should not crash even job is stopped
mAnomalyDetectionJobService.dequeueWork(mJobParameters);
}
@Test
public void stopJobWhileCompletingWork_shouldNotCrash() {
doThrow(new SecurityException()).when(mJobParameters).completeWork(any());
mAnomalyDetectionJobService.onStopJob(mJobParameters);
// Should not crash even job is stopped
mAnomalyDetectionJobService.completeWork(mJobParameters, mJobWorkItem);
}
}