init: don't sync() before shutting down services
Devices in the lab are hitting an issue where they're getting stuck
likely in the sync() call in DoReboot() before we start the reboot
monitor thread and before we shut down services.
It's possible that concurrent writing to RW file systems is causing
this sync() call to take essentially forever. To protect against
this, we need to remove this sync(). Note that we will still call
sync() after shutting down services.
Note that the service shutdown code has a timeout and there is a
reboot monitor thread that will shutdown the device if more than 30
seconds pass above that timeout. This change increases that timeout
to 300 seconds to give the final sync() calls explicitly more time to
finish.
Bug: 150863651
Test: reboot functions normally
Test: put an infinite loop in DoReboot and the the reboot monitor thread
triggers and shuts down the device appropriately
Merged-In: I6fd7d3a25d3225081388e39a14c9fdab21b592ba
Change-Id: I6fd7d3a25d3225081388e39a14c9fdab21b592ba
(cherry picked from commit 10615eb3973a9d7cf9e15b3c8a23907f15021200)
diff --git a/init/reboot.cpp b/init/reboot.cpp
index 081f695..d2dc6d3 100644
--- a/init/reboot.cpp
+++ b/init/reboot.cpp
@@ -102,7 +102,15 @@
if (write_to_property) {
SetProperty(LAST_REBOOT_REASON_PROPERTY, reason);
}
- WriteStringToFile(reason, LAST_REBOOT_REASON_FILE);
+ auto fd = unique_fd(TEMP_FAILURE_RETRY(open(
+ LAST_REBOOT_REASON_FILE, O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC | O_BINARY, 0666)));
+ if (!fd.ok()) {
+ PLOG(ERROR) << "Could not open '" << LAST_REBOOT_REASON_FILE
+ << "' to persist reboot reason";
+ return;
+ }
+ WriteStringToFd(reason, fd);
+ fsync(fd.get());
}
// represents umount status during reboot / shutdown.
@@ -317,9 +325,9 @@
bool* reboot_monitor_run) {
unsigned int remaining_shutdown_time = 0;
- // 30 seconds more than the timeout passed to the thread as there is a final Umount pass
+ // 300 seconds more than the timeout passed to the thread as there is a final Umount pass
// after the timeout is reached.
- constexpr unsigned int shutdown_watchdog_timeout_default = 30;
+ constexpr unsigned int shutdown_watchdog_timeout_default = 300;
auto shutdown_watchdog_timeout = android::base::GetUintProperty(
"ro.build.shutdown.watchdog.timeout", shutdown_watchdog_timeout_default);
remaining_shutdown_time = shutdown_watchdog_timeout + shutdown_timeout.count() / 1000;
@@ -539,26 +547,6 @@
Timer t;
LOG(INFO) << "Reboot start, reason: " << reason << ", reboot_target: " << reboot_target;
- // Ensure last reboot reason is reduced to canonical
- // alias reported in bootloader or system boot reason.
- size_t skip = 0;
- std::vector<std::string> reasons = Split(reason, ",");
- if (reasons.size() >= 2 && reasons[0] == "reboot" &&
- (reasons[1] == "recovery" || reasons[1] == "bootloader" || reasons[1] == "cold" ||
- reasons[1] == "hard" || reasons[1] == "warm")) {
- skip = strlen("reboot,");
- }
- PersistRebootReason(reason.c_str() + skip, true);
- sync();
-
- // If /data isn't mounted then we can skip the extra reboot steps below, since we don't need to
- // worry about unmounting it.
- if (!IsDataMounted()) {
- sync();
- RebootSystem(cmd, reboot_target);
- abort();
- }
-
bool is_thermal_shutdown = cmd == ANDROID_RB_THERMOFF;
auto shutdown_timeout = 0ms;
@@ -591,6 +579,25 @@
// Start reboot monitor thread
sem_post(&reboot_semaphore);
+ // Ensure last reboot reason is reduced to canonical
+ // alias reported in bootloader or system boot reason.
+ size_t skip = 0;
+ std::vector<std::string> reasons = Split(reason, ",");
+ if (reasons.size() >= 2 && reasons[0] == "reboot" &&
+ (reasons[1] == "recovery" || reasons[1] == "bootloader" || reasons[1] == "cold" ||
+ reasons[1] == "hard" || reasons[1] == "warm")) {
+ skip = strlen("reboot,");
+ }
+ PersistRebootReason(reason.c_str() + skip, true);
+
+ // If /data isn't mounted then we can skip the extra reboot steps below, since we don't need to
+ // worry about unmounting it.
+ if (!IsDataMounted()) {
+ sync();
+ RebootSystem(cmd, reboot_target);
+ abort();
+ }
+
// watchdogd is a vendor specific component but should be alive to complete shutdown safely.
const std::set<std::string> to_starts{"watchdogd"};
std::vector<Service*> stop_first;