Avoid using flock on profiles. am: ea2c68d27e am: 8449493515
Original change: https://android-review.googlesource.com/c/platform/art/+/2421123
Change-Id: I6f6e1b3c2b681a374b9cac6a0fc44faaa26a9319
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
diff --git a/libprofile/Android.bp b/libprofile/Android.bp
index 55bdcd1..cb9aa95 100644
--- a/libprofile/Android.bp
+++ b/libprofile/Android.bp
@@ -39,6 +39,7 @@
"libz",
],
static_libs: [
+ "libmodules-utils-build",
// ZipArchive support, the order matters here to get all symbols.
"libziparchive",
],
diff --git a/libprofile/profile/profile_compilation_info.cc b/libprofile/profile/profile_compilation_info.cc
index bb48713..b1371db 100644
--- a/libprofile/profile/profile_compilation_info.cc
+++ b/libprofile/profile/profile_compilation_info.cc
@@ -25,6 +25,7 @@
#include <algorithm>
#include <cerrno>
#include <climits>
+#include <cstdio>
#include <cstdlib>
#include <iostream>
#include <numeric>
@@ -33,11 +34,13 @@
#include <vector>
#include "android-base/file.h"
-
+#include "android-base/scopeguard.h"
+#include "android-base/unique_fd.h"
#include "base/arena_allocator.h"
#include "base/bit_utils.h"
#include "base/dumpable.h"
#include "base/file_utils.h"
+#include "base/globals.h"
#include "base/logging.h" // For VLOG.
#include "base/malloc_arena_pool.h"
#include "base/os.h"
@@ -52,6 +55,10 @@
#include "dex/descriptors_names.h"
#include "dex/dex_file_loader.h"
+#ifdef ART_TARGET_ANDROID
+#include "android-modules-utils/sdk_level.h"
+#endif
+
namespace art {
const uint8_t ProfileCompilationInfo::kProfileMagic[] = { 'p', 'r', 'o', '\0' };
@@ -806,6 +813,63 @@
bool ProfileCompilationInfo::Save(const std::string& filename, uint64_t* bytes_written) {
ScopedTrace trace(__PRETTY_FUNCTION__);
+
+#ifndef ART_TARGET_ANDROID
+ return SaveFallback(filename, bytes_written);
+#else
+ // Prior to U, SELinux policy doesn't allow apps to create profile files.
+ if (!android::modules::sdklevel::IsAtLeastU()) {
+ return SaveFallback(filename, bytes_written);
+ }
+
+ std::string tmp_filename = filename + ".XXXXXX.tmp";
+ // mkstemps creates the file with permissions 0600, which is the desired permissions, so there's
+ // no need to chmod.
+ android::base::unique_fd fd(mkostemps(tmp_filename.data(), /*suffixlen=*/4, O_CLOEXEC));
+ if (fd.get() < 0) {
+ PLOG(WARNING) << "Failed to create temp profile file for " << filename;
+ return false;
+ }
+
+ // In case anything goes wrong.
+ auto remove_tmp_file = android::base::make_scope_guard([&]() {
+ if (unlink(tmp_filename.c_str()) != 0) {
+ PLOG(WARNING) << "Failed to remove temp profile file " << tmp_filename;
+ }
+ });
+
+ bool result = Save(fd.get());
+ if (!result) {
+ VLOG(profiler) << "Failed to save profile info to temp profile file " << tmp_filename;
+ return false;
+ }
+
+ fd.reset();
+
+ // Move the temp profile file to the final location.
+ if (rename(tmp_filename.c_str(), filename.c_str()) != 0) {
+ PLOG(WARNING) << "Failed to commit profile file " << filename;
+ return false;
+ }
+
+ remove_tmp_file.Disable();
+
+ int64_t size = OS::GetFileSizeBytes(filename.c_str());
+ if (size != -1) {
+ VLOG(profiler) << "Successfully saved profile info to " << filename << " Size: " << size;
+ if (bytes_written != nullptr) {
+ *bytes_written = static_cast<uint64_t>(size);
+ }
+ } else {
+ VLOG(profiler) << "Saved profile info to " << filename
+ << " but failed to get size: " << strerror(errno);
+ }
+
+ return true;
+#endif
+}
+
+bool ProfileCompilationInfo::SaveFallback(const std::string& filename, uint64_t* bytes_written) {
std::string error;
#ifdef _WIN32
int flags = O_WRONLY;
@@ -842,6 +906,9 @@
if (bytes_written != nullptr) {
*bytes_written = static_cast<uint64_t>(size);
}
+ } else {
+ VLOG(profiler) << "Saved profile info to " << filename
+ << " but failed to get size: " << strerror(errno);
}
} else {
VLOG(profiler) << "Failed to save profile info to " << filename;
diff --git a/libprofile/profile/profile_compilation_info.h b/libprofile/profile/profile_compilation_info.h
index 76cbf9a..c74f478 100644
--- a/libprofile/profile/profile_compilation_info.h
+++ b/libprofile/profile/profile_compilation_info.h
@@ -480,9 +480,12 @@
// Save the profile data to the given file descriptor.
bool Save(int fd);
- // Save the current profile into the given file. The file will be cleared before saving.
+ // Save the current profile into the given file. Overwrites any existing data.
bool Save(const std::string& filename, uint64_t* bytes_written);
+ // A fallback implementation of `Save` that uses a flock.
+ bool SaveFallback(const std::string& filename, uint64_t* bytes_written);
+
// Return the number of dex files referenced in the profile.
size_t GetNumberOfDexFiles() const {
return info_.size();
diff --git a/runtime/jit/profiling_info_test.cc b/runtime/jit/profiling_info_test.cc
index 0bd21aa..021bebf 100644
--- a/runtime/jit/profiling_info_test.cc
+++ b/runtime/jit/profiling_info_test.cc
@@ -189,7 +189,7 @@
// Check that what we saved is in the profile.
ProfileCompilationInfo info1;
- ASSERT_TRUE(info1.Load(GetFd(profile)));
+ ASSERT_TRUE(info1.Load(profile.GetFilename(), /*clear_if_invalid=*/false));
ASSERT_EQ(info1.GetNumberOfMethods(), main_methods.size());
{
ScopedObjectAccess soa(self);
@@ -209,7 +209,7 @@
// Check that what we saved is in the profile (methods form Main and Second).
ProfileCompilationInfo info2;
- ASSERT_TRUE(info2.Load(GetFd(profile)));
+ ASSERT_TRUE(info2.Load(profile.GetFilename(), /*clear_if_invalid=*/false));
ASSERT_EQ(info2.GetNumberOfMethods(), main_methods.size() + second_methods.size());
{
ScopedObjectAccess soa(self);
@@ -249,7 +249,7 @@
// Check that what we saved is in the profile.
ProfileCompilationInfo info;
- ASSERT_TRUE(info.Load(GetFd(profile)));
+ ASSERT_TRUE(info.Load(profile.GetFilename(), /*clear_if_invalid=*/false));
ASSERT_EQ(info.GetNumberOfMethods(), main_methods.size());
{
ScopedObjectAccess soa(self);