From 8b1d526a72c1e6705b03f4b3267ee02fe84ce765 Mon Sep 17 00:00:00 2001 From: Wei Wang Date: Tue, 15 Nov 2016 23:58:55 -0800 Subject: [PATCH 1/2] Revert "Revert "init: warn slow action"" This reverts commit 1802d11cc766f04e75e93c7f98dcf52fb2e64149. Test: grep init log Bug: 32712851 --- init/action.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/init/action.cpp b/init/action.cpp index ccc18cf9e..e9b2a0dfe 100644 --- a/init/action.cpp +++ b/init/action.cpp @@ -118,14 +118,16 @@ void Action::ExecuteCommand(const Command& command) const { Timer t; int result = command.InvokeFunc(); - // TODO: this should probably be changed to "if (failed || took a long time)"... - if (android::base::GetMinimumLogSeverity() <= android::base::DEBUG) { + double duration_ms = t.duration() * 1000; + // Any action longer than 50ms will be warned to user as slow operation + if (duration_ms > 50.0 || + android::base::GetMinimumLogSeverity() <= android::base::DEBUG) { std::string trigger_name = BuildTriggersString(); std::string cmd_str = command.BuildCommandString(); std::string source = command.BuildSourceString(); LOG(INFO) << "Command '" << cmd_str << "' action=" << trigger_name << source - << " returned " << result << " took " << t.duration() << "s"; + << " returned " << result << " took " << duration_ms << "ms."; } } From d67a4abc647d5ed7235ff7ee1695b31340e63a1c Mon Sep 17 00:00:00 2001 From: Wei Wang Date: Wed, 16 Nov 2016 12:08:30 -0800 Subject: [PATCH 2/2] init: fix undefined behavior in ExecuteCommand ExecuteCommand may change command_ vector which leads undefined behavior This bug is found when adding logs in ExecuteCommand printing our Command class fields Bug: 32838381 Test: on emulator Change-Id: I96468bd2192ca80013871a3a6ac4132149363fff --- init/action.cpp | 5 ++++- init/builtins.cpp | 6 +++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/init/action.cpp b/init/action.cpp index e9b2a0dfe..acbb12e8e 100644 --- a/init/action.cpp +++ b/init/action.cpp @@ -105,7 +105,10 @@ std::size_t Action::NumCommands() const { } void Action::ExecuteOneCommand(std::size_t command) const { - ExecuteCommand(commands_[command]); + // We need a copy here since some Command execution may result in + // changing commands_ vector by importing .rc files through parser + Command cmd = commands_[command]; + ExecuteCommand(cmd); } void Action::ExecuteAllCommands() const { diff --git a/init/builtins.cpp b/init/builtins.cpp index ebdc8c993..d10f7198e 100644 --- a/init/builtins.cpp +++ b/init/builtins.cpp @@ -589,9 +589,9 @@ static int do_mount_all(const std::vector& args) { for (na = args.size() - 1; na > 1; --na) { if (args[na] == "--early") { - path_arg_end = na; - queue_event = false; - mount_mode = MOUNT_MODE_EARLY; + path_arg_end = na; + queue_event = false; + mount_mode = MOUNT_MODE_EARLY; } else if (args[na] == "--late") { path_arg_end = na; import_rc = false;