Add locking around boot image generation.
If zygote aborts due to an error, it will restart and
spawn another dex2oat process while the old one is still
running. If this happens fast enough, the system will
eventually need a kernel reboot since neither the zygote
nor dex2oat are killable.
This brings boot image generation in line with dex2oat
generation, which uses a similar pattern of advisory
locking.
bug: 15415316
Change-Id: Iaccd274d3d96ab002b04e246ec4b3ef9a422ff7c
diff --git a/build/Android.gtest.mk b/build/Android.gtest.mk
index b32fc9b..e305dc8 100644
--- a/build/Android.gtest.mk
+++ b/build/Android.gtest.mk
@@ -25,6 +25,7 @@
runtime/base/hex_dump_test.cc \
runtime/base/histogram_test.cc \
runtime/base/mutex_test.cc \
+ runtime/base/scoped_flock_test.cc \
runtime/base/timing_logger_test.cc \
runtime/base/unix_file/fd_file_test.cc \
runtime/base/unix_file/mapped_file_test.cc \
diff --git a/runtime/Android.mk b/runtime/Android.mk
index 7a832c1..8d532c7 100644
--- a/runtime/Android.mk
+++ b/runtime/Android.mk
@@ -26,6 +26,7 @@
base/hex_dump.cc \
base/logging.cc \
base/mutex.cc \
+ base/scoped_flock.cc \
base/stringpiece.cc \
base/stringprintf.cc \
base/timing_logger.cc \
diff --git a/runtime/base/scoped_flock.cc b/runtime/base/scoped_flock.cc
new file mode 100644
index 0000000..c0bce84
--- /dev/null
+++ b/runtime/base/scoped_flock.cc
@@ -0,0 +1,75 @@
+/*
+ * Copyright (C) 2011 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "scoped_flock.h"
+
+#include <sys/file.h>
+#include <sys/stat.h>
+
+#include "base/logging.h"
+#include "base/stringprintf.h"
+#include "base/unix_file/fd_file.h"
+
+namespace art {
+
+bool ScopedFlock::Init(const char* filename, std::string* error_msg) {
+ while (true) {
+ file_.reset(OS::OpenFileWithFlags(filename, O_CREAT | O_RDWR));
+ if (file_.get() == NULL) {
+ *error_msg = StringPrintf("Failed to open file '%s': %s", filename, strerror(errno));
+ return false;
+ }
+ int flock_result = TEMP_FAILURE_RETRY(flock(file_->Fd(), LOCK_EX));
+ if (flock_result != 0) {
+ *error_msg = StringPrintf("Failed to lock file '%s': %s", filename, strerror(errno));
+ return false;
+ }
+ struct stat fstat_stat;
+ int fstat_result = TEMP_FAILURE_RETRY(fstat(file_->Fd(), &fstat_stat));
+ if (fstat_result != 0) {
+ *error_msg = StringPrintf("Failed to fstat file '%s': %s", filename, strerror(errno));
+ return false;
+ }
+ struct stat stat_stat;
+ int stat_result = TEMP_FAILURE_RETRY(stat(filename, &stat_stat));
+ if (stat_result != 0) {
+ PLOG(WARNING) << "Failed to stat, will retry: " << filename;
+ // ENOENT can happen if someone racing with us unlinks the file we created so just retry.
+ continue;
+ }
+ if (fstat_stat.st_dev != stat_stat.st_dev || fstat_stat.st_ino != stat_stat.st_ino) {
+ LOG(WARNING) << "File changed while locking, will retry: " << filename;
+ continue;
+ }
+ return true;
+ }
+}
+
+File* ScopedFlock::GetFile() {
+ CHECK(file_.get() != NULL);
+ return file_.get();
+}
+
+ScopedFlock::ScopedFlock() { }
+
+ScopedFlock::~ScopedFlock() {
+ if (file_.get() != NULL) {
+ int flock_result = TEMP_FAILURE_RETRY(flock(file_->Fd(), LOCK_UN));
+ CHECK_EQ(0, flock_result);
+ }
+}
+
+} // namespace art
diff --git a/runtime/base/scoped_flock.h b/runtime/base/scoped_flock.h
new file mode 100644
index 0000000..26b4eb0
--- /dev/null
+++ b/runtime/base/scoped_flock.h
@@ -0,0 +1,51 @@
+/*
+ * Copyright (C) 2011 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef ART_RUNTIME_BASE_SCOPED_FLOCK_H_
+#define ART_RUNTIME_BASE_SCOPED_FLOCK_H_
+
+#include <memory>
+#include <string>
+
+#include "base/macros.h"
+#include "os.h"
+
+namespace art {
+
+class ScopedFlock {
+ public:
+ ScopedFlock();
+
+ // Attempts to acquire an exclusive file lock (see flock(2)) on the file
+ // at filename, and blocks until it can do so.
+ //
+ // Returns true if the lock could be acquired, or false if an error
+ // occurred. It is an error if the file does not exist, or if its inode
+ // changed (usually due to a new file being created at the same path)
+ // between attempts to lock it.
+ bool Init(const char* filename, std::string* error_msg);
+
+ // Returns the (locked) file associated with this instance.
+ File* GetFile();
+ ~ScopedFlock();
+ private:
+ std::unique_ptr<File> file_;
+ DISALLOW_COPY_AND_ASSIGN(ScopedFlock);
+};
+
+} // namespace art
+
+#endif // ART_RUNTIME_BASE_SCOPED_FLOCK_H_
diff --git a/runtime/base/scoped_flock_test.cc b/runtime/base/scoped_flock_test.cc
new file mode 100644
index 0000000..8fa181a
--- /dev/null
+++ b/runtime/base/scoped_flock_test.cc
@@ -0,0 +1,41 @@
+/*
+ * Copyright (C) 2014 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "scoped_flock.h"
+#include "common_runtime_test.h"
+
+#include "gtest/gtest.h"
+
+namespace art {
+
+class ScopedFlockTest : public CommonRuntimeTest {};
+
+TEST_F(ScopedFlockTest, TestLocking) {
+ ScratchFile scratch_file;
+ std::string error_msg;
+
+ // NOTE: Locks applied using flock(2) and fcntl(2) are oblivious
+ // to each other, so attempting to query locks set by flock using
+ // using fcntl(,F_GETLK,) will not work. see kernel doc at
+ // Documentation/filesystems/locks.txt.
+ ScopedFlock file_lock;
+ ASSERT_TRUE(file_lock.Init(scratch_file.GetFilename().c_str(),
+ &error_msg));
+
+ ASSERT_FALSE(file_lock.Init("/guaranteed/not/to/exist", &error_msg));
+}
+
+} // namespace art
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 330b110..28164cd 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -27,6 +27,7 @@
#include "base/casts.h"
#include "base/logging.h"
+#include "base/scoped_flock.h"
#include "base/stl_util.h"
#include "base/unix_file/fd_file.h"
#include "class_linker-inl.h"
@@ -701,60 +702,6 @@
return dex_file;
}
-class ScopedFlock {
- public:
- ScopedFlock() {}
-
- bool Init(const char* filename, std::string* error_msg) {
- while (true) {
- file_.reset(OS::OpenFileWithFlags(filename, O_CREAT | O_RDWR));
- if (file_.get() == NULL) {
- *error_msg = StringPrintf("Failed to open file '%s': %s", filename, strerror(errno));
- return false;
- }
- int flock_result = TEMP_FAILURE_RETRY(flock(file_->Fd(), LOCK_EX));
- if (flock_result != 0) {
- *error_msg = StringPrintf("Failed to lock file '%s': %s", filename, strerror(errno));
- return false;
- }
- struct stat fstat_stat;
- int fstat_result = TEMP_FAILURE_RETRY(fstat(file_->Fd(), &fstat_stat));
- if (fstat_result != 0) {
- *error_msg = StringPrintf("Failed to fstat file '%s': %s", filename, strerror(errno));
- return false;
- }
- struct stat stat_stat;
- int stat_result = TEMP_FAILURE_RETRY(stat(filename, &stat_stat));
- if (stat_result != 0) {
- PLOG(WARNING) << "Failed to stat, will retry: " << filename;
- // ENOENT can happen if someone racing with us unlinks the file we created so just retry.
- continue;
- }
- if (fstat_stat.st_dev != stat_stat.st_dev || fstat_stat.st_ino != stat_stat.st_ino) {
- LOG(WARNING) << "File changed while locking, will retry: " << filename;
- continue;
- }
- return true;
- }
- }
-
- File& GetFile() {
- return *file_;
- }
-
- ~ScopedFlock() {
- if (file_.get() != NULL) {
- int flock_result = TEMP_FAILURE_RETRY(flock(file_->Fd(), LOCK_UN));
- CHECK_EQ(0, flock_result);
- }
- }
-
- private:
- std::unique_ptr<File> file_;
-
- DISALLOW_COPY_AND_ASSIGN(ScopedFlock);
-};
-
const DexFile* ClassLinker::FindOrCreateOatFileForDexLocation(
const char* dex_location,
uint32_t dex_location_checksum,
@@ -785,7 +732,7 @@
// Generate the output oat file for the dex file
VLOG(class_linker) << "Generating oat file " << oat_location << " for " << dex_location;
- if (!GenerateOatFile(dex_location, scoped_flock.GetFile().Fd(), oat_location, &error_msg)) {
+ if (!GenerateOatFile(dex_location, scoped_flock.GetFile()->Fd(), oat_location, &error_msg)) {
CHECK(!error_msg.empty());
error_msgs->push_back(error_msg);
return nullptr;
diff --git a/runtime/gc/space/image_space.cc b/runtime/gc/space/image_space.cc
index 3d35c00..61633cd 100644
--- a/runtime/gc/space/image_space.cc
+++ b/runtime/gc/space/image_space.cc
@@ -18,6 +18,7 @@
#include "base/stl_util.h"
#include "base/unix_file/fd_file.h"
+#include "base/scoped_flock.h"
#include "gc/accounting/space_bitmap-inl.h"
#include "mirror/art_method.h"
#include "mirror/class-inl.h"
@@ -148,7 +149,17 @@
std::string image_filename;
std::string error_msg;
bool is_system = false;
- if (FindImageFilename(image_location, image_isa, &image_filename, &is_system)) {
+ const bool found_image = FindImageFilename(image_location, image_isa, &image_filename,
+ &is_system);
+
+ // Note that we must not use the file descriptor associated with
+ // ScopedFlock::GetFile to Init the image file. We want the file
+ // descriptor (and the associated exclusive lock) to be released when
+ // we leave Create.
+ ScopedFlock image_lock;
+ image_lock.Init(image_filename.c_str(), &error_msg);
+
+ if (found_image) {
ImageSpace* space = ImageSpace::Init(image_filename.c_str(), image_location, !is_system,
&error_msg);
if (space != nullptr) {
