Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Create XML parser only once.

Browse Source 
This greatly cuts memory usage: 2.5M -> 0.5M (see bug for traces).

Bug: 200995209
Test: atest SystemConfigTest SystemConfigNamedActorTest
Change-Id: I0b76dc2610afaad3e418ef3115c5e54a05ab334e
Merged-In: I0b76dc2610afaad3e418ef3115c5e54a05ab334e
This commit is contained in:
Alex Buynytskyy
2021-09-23 15:34:51 -07:00
committed by Andrei Onea
parent 56a787cb54
commit 66fc1f1941
3 changed files with 46 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

49
core/java/com/android/server/SystemConfig.java
View File

@@ -509,12 +509,14 @@ public class SystemConfig {
} }
private void readAllPermissions() { private void readAllPermissions() {
final XmlPullParser parser = Xml.newPullParser();
// Read configuration from system // Read configuration from system
readPermissions(Environment.buildPath( readPermissions(parser, Environment.buildPath(
Environment.getRootDirectory(), "etc", "sysconfig"), ALLOW_ALL); Environment.getRootDirectory(), "etc", "sysconfig"), ALLOW_ALL);
// Read configuration from the old permissions dir // Read configuration from the old permissions dir
readPermissions(Environment.buildPath( readPermissions(parser, Environment.buildPath(
Environment.getRootDirectory(), "etc", "permissions"), ALLOW_ALL); Environment.getRootDirectory(), "etc", "permissions"), ALLOW_ALL);
// Vendors are only allowed to customize these // Vendors are only allowed to customize these
@@ -524,18 +526,18 @@ public class SystemConfig {
// For backward compatibility // For backward compatibility
vendorPermissionFlag |= (ALLOW_PERMISSIONS | ALLOW_APP_CONFIGS); vendorPermissionFlag |= (ALLOW_PERMISSIONS | ALLOW_APP_CONFIGS);
} }
readPermissions(Environment.buildPath( readPermissions(parser, Environment.buildPath(
Environment.getVendorDirectory(), "etc", "sysconfig"), vendorPermissionFlag); Environment.getVendorDirectory(), "etc", "sysconfig"), vendorPermissionFlag);
readPermissions(Environment.buildPath( readPermissions(parser, Environment.buildPath(
Environment.getVendorDirectory(), "etc", "permissions"), vendorPermissionFlag); Environment.getVendorDirectory(), "etc", "permissions"), vendorPermissionFlag);
String vendorSkuProperty = SystemProperties.get(VENDOR_SKU_PROPERTY, ""); String vendorSkuProperty = SystemProperties.get(VENDOR_SKU_PROPERTY, "");
if (!vendorSkuProperty.isEmpty()) { if (!vendorSkuProperty.isEmpty()) {
String vendorSkuDir = "sku_" + vendorSkuProperty; String vendorSkuDir = "sku_" + vendorSkuProperty;
readPermissions(Environment.buildPath( readPermissions(parser, Environment.buildPath(
Environment.getVendorDirectory(), "etc", "sysconfig", vendorSkuDir), Environment.getVendorDirectory(), "etc", "sysconfig", vendorSkuDir),
vendorPermissionFlag); vendorPermissionFlag);
readPermissions(Environment.buildPath( readPermissions(parser, Environment.buildPath(
Environment.getVendorDirectory(), "etc", "permissions", vendorSkuDir), Environment.getVendorDirectory(), "etc", "permissions", vendorSkuDir),
vendorPermissionFlag); vendorPermissionFlag);
} }
@@ -543,18 +545,18 @@ public class SystemConfig {
// Allow ODM to customize system configs as much as Vendor, because /odm is another // Allow ODM to customize system configs as much as Vendor, because /odm is another
// vendor partition other than /vendor. // vendor partition other than /vendor.
int odmPermissionFlag = vendorPermissionFlag; int odmPermissionFlag = vendorPermissionFlag;
readPermissions(Environment.buildPath( readPermissions(parser, Environment.buildPath(
Environment.getOdmDirectory(), "etc", "sysconfig"), odmPermissionFlag); Environment.getOdmDirectory(), "etc", "sysconfig"), odmPermissionFlag);
readPermissions(Environment.buildPath( readPermissions(parser, Environment.buildPath(
Environment.getOdmDirectory(), "etc", "permissions"), odmPermissionFlag); Environment.getOdmDirectory(), "etc", "permissions"), odmPermissionFlag);
String skuProperty = SystemProperties.get(SKU_PROPERTY, ""); String skuProperty = SystemProperties.get(SKU_PROPERTY, "");
if (!skuProperty.isEmpty()) { if (!skuProperty.isEmpty()) {
String skuDir = "sku_" + skuProperty; String skuDir = "sku_" + skuProperty;
readPermissions(Environment.buildPath( readPermissions(parser, Environment.buildPath(
Environment.getOdmDirectory(), "etc", "sysconfig", skuDir), odmPermissionFlag); Environment.getOdmDirectory(), "etc", "sysconfig", skuDir), odmPermissionFlag);
readPermissions(Environment.buildPath( readPermissions(parser, Environment.buildPath(
Environment.getOdmDirectory(), "etc", "permissions", skuDir), Environment.getOdmDirectory(), "etc", "permissions", skuDir),
odmPermissionFlag); odmPermissionFlag);
} }
@@ -562,9 +564,9 @@ public class SystemConfig {
// Allow OEM to customize these // Allow OEM to customize these
int oemPermissionFlag = ALLOW_FEATURES | ALLOW_OEM_PERMISSIONS | ALLOW_ASSOCIATIONS int oemPermissionFlag = ALLOW_FEATURES | ALLOW_OEM_PERMISSIONS | ALLOW_ASSOCIATIONS
| ALLOW_VENDOR_APEX; | ALLOW_VENDOR_APEX;
readPermissions(Environment.buildPath( readPermissions(parser, Environment.buildPath(
Environment.getOemDirectory(), "etc", "sysconfig"), oemPermissionFlag); Environment.getOemDirectory(), "etc", "sysconfig"), oemPermissionFlag);
readPermissions(Environment.buildPath( readPermissions(parser, Environment.buildPath(
Environment.getOemDirectory(), "etc", "permissions"), oemPermissionFlag); Environment.getOemDirectory(), "etc", "permissions"), oemPermissionFlag);
// Allow Product to customize these configs // Allow Product to customize these configs
@@ -579,15 +581,15 @@ public class SystemConfig {
// DEVICE_INITIAL_SDK_INT for the devices without product interface enforcement. // DEVICE_INITIAL_SDK_INT for the devices without product interface enforcement.
productPermissionFlag = ALLOW_ALL; productPermissionFlag = ALLOW_ALL;
} }
readPermissions(Environment.buildPath( readPermissions(parser, Environment.buildPath(
Environment.getProductDirectory(), "etc", "sysconfig"), productPermissionFlag); Environment.getProductDirectory(), "etc", "sysconfig"), productPermissionFlag);
readPermissions(Environment.buildPath( readPermissions(parser, Environment.buildPath(
Environment.getProductDirectory(), "etc", "permissions"), productPermissionFlag); Environment.getProductDirectory(), "etc", "permissions"), productPermissionFlag);
// Allow /system_ext to customize all system configs // Allow /system_ext to customize all system configs
readPermissions(Environment.buildPath( readPermissions(parser, Environment.buildPath(
Environment.getSystemExtDirectory(), "etc", "sysconfig"), ALLOW_ALL); Environment.getSystemExtDirectory(), "etc", "sysconfig"), ALLOW_ALL);
readPermissions(Environment.buildPath( readPermissions(parser, Environment.buildPath(
Environment.getSystemExtDirectory(), "etc", "permissions"), ALLOW_ALL); Environment.getSystemExtDirectory(), "etc", "permissions"), ALLOW_ALL);
// Skip loading configuration from apex if it is not a system process. // Skip loading configuration from apex if it is not a system process.
@@ -601,12 +603,13 @@ public class SystemConfig {
if (f.isFile() || f.getPath().contains("@")) { if (f.isFile() || f.getPath().contains("@")) {
continue; continue;
} }
readPermissions(Environment.buildPath(f, "etc", "permissions"), apexPermissionFlag); readPermissions(parser, Environment.buildPath(f, "etc", "permissions"),
apexPermissionFlag);
} }
} }
@VisibleForTesting @VisibleForTesting
public void readPermissions(File libraryDir, int permissionFlag) { public void readPermissions(final XmlPullParser parser, File libraryDir, int permissionFlag) {
// Read permissions from given directory. // Read permissions from given directory.
if (!libraryDir.exists() || !libraryDir.isDirectory()) { if (!libraryDir.exists() || !libraryDir.isDirectory()) {
if (permissionFlag == ALLOW_ALL) { if (permissionFlag == ALLOW_ALL) {
@@ -641,12 +644,12 @@ public class SystemConfig {
continue; continue;
} }
readPermissionsFromXml(f, permissionFlag); readPermissionsFromXml(parser, f, permissionFlag);
} }
// Read platform permissions last so it will take precedence // Read platform permissions last so it will take precedence
if (platformFile != null) { if (platformFile != null) {
readPermissionsFromXml(platformFile, permissionFlag); readPermissionsFromXml(parser, platformFile, permissionFlag);
} }
} }
@@ -655,8 +658,9 @@ public class SystemConfig {
+ permFile + " at " + parser.getPositionDescription()); + permFile + " at " + parser.getPositionDescription());
} }
private void readPermissionsFromXml(File permFile, int permissionFlag) { private void readPermissionsFromXml(final XmlPullParser parser, File permFile,
FileReader permReader = null; int permissionFlag) {
final FileReader permReader;
try { try {
permReader = new FileReader(permFile); permReader = new FileReader(permFile);
} catch (FileNotFoundException e) { } catch (FileNotFoundException e) {
@@ -668,7 +672,6 @@ public class SystemConfig {
final boolean lowRam = ActivityManager.isLowRamDeviceStatic(); final boolean lowRam = ActivityManager.isLowRamDeviceStatic();
try { try {
XmlPullParser parser = Xml.newPullParser();
parser.setInput(permReader); parser.setInput(permReader);
int type; int type;

4
services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigNamedActorTest.kt
View File

@@ -17,6 +17,7 @@
package com.android.server.systemconfig package com.android.server.systemconfig
import android.content.Context import android.content.Context
import android.util.Xml
import androidx.test.InstrumentationRegistry import androidx.test.InstrumentationRegistry
import com.android.server.SystemConfig import com.android.server.SystemConfig
import com.google.common.truth.Truth.assertThat import com.google.common.truth.Truth.assertThat
@@ -227,6 +228,7 @@ class SystemConfigNamedActorTest {
.writeText(this.trimIndent()) .writeText(this.trimIndent())
private fun assertPermissions() = SystemConfig(false).apply { private fun assertPermissions() = SystemConfig(false).apply {
readPermissions(tempFolder.root, 0) val parser = Xml.newPullParser()
readPermissions(parser, tempFolder.root, 0)
}. let { assertThat(it.namedActors) } }. let { assertThat(it.namedActors) }
} }

27
services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigTest.java
View File

@@ -25,6 +25,7 @@ import android.platform.test.annotations.Presubmit;
import android.util.ArrayMap; import android.util.ArrayMap;
import android.util.ArraySet; import android.util.ArraySet;
import android.util.Log; import android.util.Log;
import android.util.Xml;
import androidx.test.filters.SmallTest; import androidx.test.filters.SmallTest;
import androidx.test.runner.AndroidJUnit4; import androidx.test.runner.AndroidJUnit4;
@@ -36,6 +37,7 @@ import org.junit.Rule;
import org.junit.Test; import org.junit.Test;
import org.junit.rules.TemporaryFolder; import org.junit.rules.TemporaryFolder;
import org.junit.runner.RunWith; import org.junit.runner.RunWith;
import org.xmlpull.v1.XmlPullParser;
import java.io.BufferedWriter; import java.io.BufferedWriter;
import java.io.File; import java.io.File;
@@ -76,6 +78,11 @@ public class SystemConfigTest {
} }
} }
private void readPermissions(File libraryDir, int permissionFlag) {
final XmlPullParser parser = Xml.newPullParser();
mSysConfig.readPermissions(parser, libraryDir, permissionFlag);
}
/** /**
* Tests that readPermissions works correctly for the tag: install-in-user-type * Tests that readPermissions works correctly for the tag: install-in-user-type
*/ */
@@ -134,8 +141,8 @@ public class SystemConfigTest {
// Also, make a third file, but with the name folder1/permFile2.xml, to prove no conflicts. // Also, make a third file, but with the name folder1/permFile2.xml, to prove no conflicts.
createTempFile(folder1, "permFile2.xml", contents3); createTempFile(folder1, "permFile2.xml", contents3);
mSysConfig.readPermissions(folder1, /* No permission needed anyway */ 0); readPermissions(folder1, /* No permission needed anyway */ 0);
mSysConfig.readPermissions(folder2, /* No permission needed anyway */ 0); readPermissions(folder2, /* No permission needed anyway */ 0);
Map<String, Set<String>> actualWhite = mSysConfig.getAndClearPackageToUserTypeWhitelist(); Map<String, Set<String>> actualWhite = mSysConfig.getAndClearPackageToUserTypeWhitelist();
Map<String, Set<String>> actualBlack = mSysConfig.getAndClearPackageToUserTypeBlacklist(); Map<String, Set<String>> actualBlack = mSysConfig.getAndClearPackageToUserTypeBlacklist();
@@ -165,7 +172,7 @@ public class SystemConfigTest {
final File folder = createTempSubfolder("folder"); final File folder = createTempSubfolder("folder");
createTempFile(folder, "component-override.xml", contents); createTempFile(folder, "component-override.xml", contents);
mSysConfig.readPermissions(folder, /* No permission needed anyway */ 0); readPermissions(folder, /* No permission needed anyway */ 0);
final ArrayMap<String, Boolean> packageOneExpected = new ArrayMap<>(); final ArrayMap<String, Boolean> packageOneExpected = new ArrayMap<>();
packageOneExpected.put("com.android.package1.Full", true); packageOneExpected.put("com.android.package1.Full", true);
@@ -197,7 +204,7 @@ public class SystemConfigTest {
final File folder = createTempSubfolder("folder"); final File folder = createTempSubfolder("folder");
createTempFile(folder, "staged-installer-whitelist.xml", contents); createTempFile(folder, "staged-installer-whitelist.xml", contents);
mSysConfig.readPermissions(folder, /* Grant all permission flags */ ~0); readPermissions(folder, /* Grant all permission flags */ ~0);
assertThat(mSysConfig.getWhitelistedStagedInstallers()) assertThat(mSysConfig.getWhitelistedStagedInstallers())
.containsExactly("com.android.package1"); .containsExactly("com.android.package1");
@@ -215,7 +222,7 @@ public class SystemConfigTest {
final File folder = createTempSubfolder("folder"); final File folder = createTempSubfolder("folder");
createTempFile(folder, "staged-installer-whitelist.xml", contents); createTempFile(folder, "staged-installer-whitelist.xml", contents);
mSysConfig.readPermissions(folder, /* Grant all permission flags */ ~0); readPermissions(folder, /* Grant all permission flags */ ~0);
assertThat(mSysConfig.getWhitelistedStagedInstallers()) assertThat(mSysConfig.getWhitelistedStagedInstallers())
.containsExactly("com.android.package1"); .containsExactly("com.android.package1");
@@ -238,7 +245,7 @@ public class SystemConfigTest {
IllegalStateException e = expectThrows( IllegalStateException e = expectThrows(
IllegalStateException.class, IllegalStateException.class,
() -> mSysConfig.readPermissions(folder, /* Grant all permission flags */ ~0)); () -> readPermissions(folder, /* Grant all permission flags */ ~0));
assertThat(e).hasMessageThat().contains("Multiple modules installers"); assertThat(e).hasMessageThat().contains("Multiple modules installers");
} }
@@ -257,7 +264,7 @@ public class SystemConfigTest {
final File folder = createTempSubfolder("folder"); final File folder = createTempSubfolder("folder");
createTempFile(folder, "staged-installer-whitelist.xml", contents); createTempFile(folder, "staged-installer-whitelist.xml", contents);
mSysConfig.readPermissions(folder, /* Grant all but ALLOW_APP_CONFIGS flag */ ~0x08); readPermissions(folder, /* Grant all but ALLOW_APP_CONFIGS flag */ ~0x08);
assertThat(mSysConfig.getWhitelistedStagedInstallers()).isEmpty(); assertThat(mSysConfig.getWhitelistedStagedInstallers()).isEmpty();
} }
@@ -277,7 +284,7 @@ public class SystemConfigTest {
final File folder = createTempSubfolder("folder"); final File folder = createTempSubfolder("folder");
createTempFile(folder, "vendor-apex-allowlist.xml", contents); createTempFile(folder, "vendor-apex-allowlist.xml", contents);
mSysConfig.readPermissions(folder, /* Grant all permission flags */ ~0); readPermissions(folder, /* Grant all permission flags */ ~0);
assertThat(mSysConfig.getAllowedVendorApexes()) assertThat(mSysConfig.getAllowedVendorApexes())
.containsExactly("com.android.apex1", "com.installer"); .containsExactly("com.android.apex1", "com.installer");
@@ -297,7 +304,7 @@ public class SystemConfigTest {
final File folder = createTempSubfolder("folder"); final File folder = createTempSubfolder("folder");
createTempFile(folder, "vendor-apex-allowlist.xml", contents); createTempFile(folder, "vendor-apex-allowlist.xml", contents);
mSysConfig.readPermissions(folder, /* Grant all permission flags */ ~0); readPermissions(folder, /* Grant all permission flags */ ~0);
assertThat(mSysConfig.getAllowedVendorApexes()).isEmpty(); assertThat(mSysConfig.getAllowedVendorApexes()).isEmpty();
} }
@@ -317,7 +324,7 @@ public class SystemConfigTest {
final File folder = createTempSubfolder("folder"); final File folder = createTempSubfolder("folder");
createTempFile(folder, "vendor-apex-allowlist.xml", contents); createTempFile(folder, "vendor-apex-allowlist.xml", contents);
mSysConfig.readPermissions(folder, /* Grant all but ALLOW_VENDOR_APEX flag */ ~0x400); readPermissions(folder, /* Grant all but ALLOW_VENDOR_APEX flag */ ~0x400);
assertThat(mSysConfig.getAllowedVendorApexes()).isEmpty(); assertThat(mSysConfig.getAllowedVendorApexes()).isEmpty();
} }