Merge "Add an option to write the trace to a file" into nyc-dev
diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp
index bb485bf..30ddec3 100644
--- a/cmds/dumpstate/dumpstate.cpp
+++ b/cmds/dumpstate/dumpstate.cpp
@@ -201,7 +201,8 @@
execl("/system/bin/atrace", "/system/bin/atrace", "--async_dump", nullptr);
// execl should never return, but if it did, we need to exit.
MYLOGD("execl on '/system/bin/atrace --async_dump' failed: %s", strerror(errno));
- exit(EXIT_FAILURE);
+ // Must call _exit (instead of exit), otherwise it will corrupt the zip file.
+ _exit(EXIT_FAILURE);
} else {
close(pipefd[1]); // close the write end of the pipe in the parent
add_zip_entry_from_fd("systrace.txt", pipefd[0]); // write output to zip file
diff --git a/cmds/dumpstate/utils.cpp b/cmds/dumpstate/utils.cpp
index 8129852..8c0e840 100644
--- a/cmds/dumpstate/utils.cpp
+++ b/cmds/dumpstate/utils.cpp
@@ -746,7 +746,8 @@
// it's safer to exit dumpstate.
MYLOGD("execvp on command '%s' failed (error: %s)", command, strerror(errno));
fflush(stdout);
- exit(EXIT_FAILURE);
+ // Must call _exit (instead of exit), otherwise it will corrupt the zip file.
+ _exit(EXIT_FAILURE);
}
/* handle parent case */
diff --git a/cmds/installd/commands.cpp b/cmds/installd/commands.cpp
index ff5a3b8..95bc4b9 100644
--- a/cmds/installd/commands.cpp
+++ b/cmds/installd/commands.cpp
@@ -161,43 +161,75 @@
return StringPrintf("%s/%s", profile_dir.c_str(), PRIMARY_PROFILE_NAME);
}
-static bool unlink_reference_profile(const char* pkgname) {
+static bool clear_profile(const std::string& profile) {
+ fd_t fd = open(profile.c_str(), O_WRONLY | O_NOFOLLOW);
+ if (fd < 0) {
+ if (errno != ENOENT) {
+ PLOG(WARNING) << "Could not open profile " << profile;
+ return false;
+ } else {
+ // Nothing to clear. That's ok.
+ return true;
+ }
+ }
+
+ if (flock(fd, LOCK_EX | LOCK_NB) != 0) {
+ if (errno != EWOULDBLOCK) {
+ PLOG(WARNING) << "Error locking profile " << profile;
+ }
+ // This implies that the app owning this profile is running
+ // (and has acquired the lock).
+ //
+ // If we can't acquire the lock bail out since clearing is useless anyway
+ // (the app will write again to the profile).
+ //
+ // Note:
+ // This does not impact the this is not an issue for the profiling correctness.
+ // In case this is needed because of an app upgrade, profiles will still be
+ // eventually cleared by the app itself due to checksum mismatch.
+ // If this is needed because profman advised, then keeping the data around
+ // until the next run is again not an issue.
+ //
+ // If the app attempts to acquire a lock while we've held one here,
+ // it will simply skip the current write cycle.
+ return false;
+ }
+
+ bool truncated = ftruncate(fd, 0) == 0;
+ if (!truncated) {
+ PLOG(WARNING) << "Could not truncate " << profile;
+ }
+ if (flock(fd, LOCK_UN) != 0) {
+ PLOG(WARNING) << "Error unlocking profile " << profile;
+ }
+ return truncated;
+}
+
+static bool clear_reference_profile(const char* pkgname) {
std::string reference_profile_dir = create_data_ref_profile_package_path(pkgname);
std::string reference_profile = create_primary_profile(reference_profile_dir);
- if (unlink(reference_profile.c_str()) != 0) {
- if (errno != ENOENT) {
- PLOG(WARNING) << "Could not unlink " << reference_profile;
- return false;
- }
- }
- return true;
+ return clear_profile(reference_profile);
}
-static bool unlink_current_profile(const char* pkgname, userid_t user) {
+static bool clear_current_profile(const char* pkgname, userid_t user) {
std::string profile_dir = create_data_user_profile_package_path(user, pkgname);
std::string profile = create_primary_profile(profile_dir);
- if (unlink(profile.c_str()) != 0) {
- if (errno != ENOENT) {
- PLOG(WARNING) << "Could not unlink " << profile;
- return false;
- }
- }
- return true;
+ return clear_profile(profile);
}
-static bool unlink_current_profiles(const char* pkgname) {
+static bool clear_current_profiles(const char* pkgname) {
bool success = true;
std::vector<userid_t> users = get_known_users(/*volume_uuid*/ nullptr);
for (auto user : users) {
- success &= unlink_current_profile(pkgname, user);
+ success &= clear_current_profile(pkgname, user);
}
return success;
}
int clear_app_profiles(const char* pkgname) {
bool success = true;
- success &= unlink_reference_profile(pkgname);
- success &= unlink_current_profiles(pkgname);
+ success &= clear_reference_profile(pkgname);
+ success &= clear_current_profiles(pkgname);
return success ? 0 : -1;
}
@@ -226,7 +258,7 @@
delete_dir_contents(path);
}
if (!only_cache) {
- if (!unlink_current_profile(pkgname, userid)) {
+ if (!clear_current_profile(pkgname, userid)) {
res |= -1;
}
}
@@ -234,6 +266,12 @@
return res;
}
+static int destroy_app_reference_profile(const char *pkgname) {
+ return delete_dir_contents_and_dir(
+ create_data_ref_profile_package_path(pkgname),
+ /*ignore_if_missing*/ true);
+}
+
static int destroy_app_current_profiles(const char *pkgname, userid_t userid) {
return delete_dir_contents_and_dir(
create_data_user_profile_package_path(userid, pkgname),
@@ -246,9 +284,7 @@
for (auto user : users) {
result |= destroy_app_current_profiles(pkgname, user);
}
- result |= delete_dir_contents_and_dir(
- create_data_ref_profile_package_path(pkgname),
- /*ignore_if_missing*/ true);
+ result |= destroy_app_reference_profile(pkgname);
return result;
}
@@ -262,6 +298,10 @@
res |= delete_dir_contents_and_dir(
create_data_user_de_package_path(uuid, userid, pkgname));
destroy_app_current_profiles(pkgname, userid);
+ // TODO(calin): If the package is still installed by other users it's probably
+ // beneficial to keep the reference profile around.
+ // Verify if it's ok to do that.
+ destroy_app_reference_profile(pkgname);
}
return res;
}
@@ -1192,8 +1232,8 @@
/* parent */
int return_code = wait_child(pid);
bool need_to_compile = false;
- bool delete_current_profiles = false;
- bool delete_reference_profile = false;
+ bool should_clear_current_profiles = false;
+ bool should_clear_reference_profile = false;
if (!WIFEXITED(return_code)) {
LOG(WARNING) << "profman failed for package " << pkgname << ": " << return_code;
} else {
@@ -1201,35 +1241,35 @@
switch (return_code) {
case PROFMAN_BIN_RETURN_CODE_COMPILE:
need_to_compile = true;
- delete_current_profiles = true;
- delete_reference_profile = false;
+ should_clear_current_profiles = true;
+ should_clear_reference_profile = false;
break;
case PROFMAN_BIN_RETURN_CODE_SKIP_COMPILATION:
need_to_compile = false;
- delete_current_profiles = false;
- delete_reference_profile = false;
+ should_clear_current_profiles = false;
+ should_clear_reference_profile = false;
break;
case PROFMAN_BIN_RETURN_CODE_BAD_PROFILES:
LOG(WARNING) << "Bad profiles for package " << pkgname;
need_to_compile = false;
- delete_current_profiles = true;
- delete_reference_profile = true;
+ should_clear_current_profiles = true;
+ should_clear_reference_profile = true;
break;
case PROFMAN_BIN_RETURN_CODE_ERROR_IO: // fall-through
case PROFMAN_BIN_RETURN_CODE_ERROR_LOCKING:
// Temporary IO problem (e.g. locking). Ignore but log a warning.
LOG(WARNING) << "IO error while reading profiles for package " << pkgname;
need_to_compile = false;
- delete_current_profiles = false;
- delete_reference_profile = false;
+ should_clear_current_profiles = false;
+ should_clear_reference_profile = false;
break;
default:
// Unknown return code or error. Unlink profiles.
LOG(WARNING) << "Unknown error code while processing profiles for package " << pkgname
<< ": " << return_code;
need_to_compile = false;
- delete_current_profiles = true;
- delete_reference_profile = true;
+ should_clear_current_profiles = true;
+ should_clear_reference_profile = true;
break;
}
}
@@ -1237,11 +1277,11 @@
if (close(reference_profile_fd) != 0) {
PLOG(WARNING) << "Failed to close fd for reference profile";
}
- if (delete_current_profiles) {
- unlink_current_profiles(pkgname);
+ if (should_clear_current_profiles) {
+ clear_current_profiles(pkgname);
}
- if (delete_reference_profile) {
- unlink_reference_profile(pkgname);
+ if (should_clear_reference_profile) {
+ clear_reference_profile(pkgname);
}
return need_to_compile;
}
@@ -1423,9 +1463,9 @@
// Use app images only if it is enabled (by a set image format) and we are compiling
// profile-guided (so the app image doesn't conservatively contain all classes).
if (profile_guided && have_app_image_format) {
- // Recreate is false since we want to avoid deleting the image in case dex2oat decides to
- // not compile anything.
- image_fd = open_output_file(image_path, /*recreate*/false);
+ // Recreate is true since we do not want to modify a mapped image. If the app is already
+ // running and we modify the image file, it can cause crashes (b/27493510).
+ image_fd = open_output_file(image_path, /*recreate*/true);
if (image_fd < 0) {
// Could not create application image file. Go on since we can compile without it.
ALOGE("installd could not create '%s' for image file during dexopt\n", image_path);
@@ -1511,7 +1551,7 @@
close(reference_profile_fd);
// We failed to compile. Unlink the reference profile. Current profiles are already unlinked
// when profmoan advises compilation.
- unlink_reference_profile(pkgname);
+ clear_reference_profile(pkgname);
}
if (swap_fd >= 0) {
close(swap_fd);