login: freeze and thaw ARC instance across suspend/resume
This allows us greater control of the ARC instance, such as not
letting the processes wake up in dark resume. Until we implement
something more like Doze mode, this is useful to keep the
instance from becoming unhappy when we suspend and resume a lot.
BUG=b:25410226
TEST=deploy to device, check for relevant log messages;
new unit test
Change-Id: I37193c04bfdd5b5a431af0d2ad2cc6ae809b0269
Reviewed-on: https://chromium-review.googlesource.com/319819
Commit-Ready: Eric Caruso <ejcaruso@chromium.org>
Tested-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
diff --git a/login_manager/session_manager_process_unittest.cc b/login_manager/session_manager_process_unittest.cc
index 52d685f..6dfcbc9 100644
--- a/login_manager/session_manager_process_unittest.cc
+++ b/login_manager/session_manager_process_unittest.cc
@@ -27,9 +27,11 @@
#include "login_manager/mock_file_checker.h"
#include "login_manager/mock_liveness_checker.h"
#include "login_manager/mock_metrics.h"
+#include "login_manager/mock_object_proxy.h"
#include "login_manager/mock_session_manager.h"
#include "login_manager/mock_system_utils.h"
#include "login_manager/system_utils_impl.h"
+#include "power_manager/proto_bindings/suspend.pb.h"
using ::testing::AnyNumber;
using ::testing::AtLeast;
@@ -163,6 +165,66 @@
const pid_t SessionManagerProcessTest::kDummyPid = 4;
const int SessionManagerProcessTest::kExit = 1;
+class HandleSuspendReadinessMethodMatcher
+ : public ::testing::MatcherInterface<dbus::MethodCall*> {
+ public:
+ HandleSuspendReadinessMethodMatcher(int delay_id, int suspend_id)
+ : delay_id_(delay_id),
+ suspend_id_(suspend_id) {}
+
+ virtual bool MatchAndExplain(dbus::MethodCall* method_call,
+ ::testing::MatchResultListener* listener) const {
+ // Make sure we've got the right kind of method call.
+ if (method_call->GetInterface() !=
+ power_manager::kPowerManagerInterface) {
+ *listener << "interface was " << method_call->GetInterface();
+ return false;
+ }
+
+ if (method_call->GetMember() !=
+ power_manager::kHandleSuspendReadinessMethod) {
+ *listener << "method name was " << method_call->GetMember();
+ return false;
+ }
+
+ // Check proto for correctness.
+ power_manager::SuspendReadinessInfo info;
+ dbus::MessageReader reader(method_call);
+ reader.PopArrayOfBytesAsProto(&info);
+ if (info.delay_id() != delay_id_) {
+ *listener << "delay ID was " << info.delay_id();
+ return false;
+ }
+ if (info.suspend_id() != suspend_id_) {
+ *listener << "suspend ID was " << info.suspend_id();
+ return false;
+ }
+
+ return true;
+ }
+
+ virtual void DescribeTo(::std::ostream* os) const {
+ *os << "HandleSuspendReadiness method call with delay ID "
+ << delay_id_ << " and suspend ID " << suspend_id_;
+ }
+
+ virtual void DescribeNegationTo(::std::ostream* os) const {
+ *os << "non-HandleSuspendReadiness method call, or method call "
+ << "not with delay ID " << delay_id_ << " and suspend ID "
+ << suspend_id_;
+ }
+ private:
+ const int delay_id_;
+ const int suspend_id_;
+};
+
+inline testing::Matcher<dbus::MethodCall*> HandleSuspendReadinessMethod(
+ int delay_id,
+ int suspend_id) {
+ return MakeMatcher(
+ new HandleSuspendReadinessMethodMatcher(delay_id, suspend_id));
+}
+
// Browser processes get correctly terminated.
TEST_F(SessionManagerProcessTest, CleanupBrowser) {
FakeBrowserJob* job = CreateMockJobAndInitManager(false);
@@ -289,4 +351,48 @@
ASSERT_EQ(SessionManagerService::MUST_WIPE_DEVICE, manager_->exit_code());
}
+TEST_F(SessionManagerProcessTest, SuspendAndResumeArcInstance) {
+ CreateMockJobAndInitManager(true);
+
+ const int kSuspendDelayId = 1000;
+ const int kSuspendId = 2000;
+ scoped_refptr<MockObjectProxy> powerd_object_proxy(new MockObjectProxy);
+ std::string cgroup_state;
+
+ manager_->test_api().set_powerd_object_proxy(powerd_object_proxy.get());
+ base::FilePath temp_file_path;
+ CHECK(base::CreateTemporaryFile(&temp_file_path));
+ manager_->test_api().set_arc_cgroup_freezer_state_path(temp_file_path);
+ manager_->test_api().set_suspend_delay_id(kSuspendDelayId);
+
+ // Fake the SuspendImminent signal.
+ dbus::Signal suspend_signal(
+ power_manager::kPowerManagerInterface,
+ power_manager::kSuspendImminentSignal);
+ power_manager::SuspendImminent suspend_imminent;
+ suspend_imminent.set_suspend_id(kSuspendId);
+ dbus::MessageWriter suspend_writer(&suspend_signal);
+ suspend_writer.AppendProtoAsArrayOfBytes(suspend_imminent);
+
+ // SuspendImminent should trigger a HandleSuspendReadiness response
+ // after freezing the ARC instance.
+ EXPECT_CALL(*powerd_object_proxy.get(),
+ MockCallMethodAndBlock(
+ HandleSuspendReadinessMethod(kSuspendDelayId, kSuspendId),
+ _));
+
+ manager_->test_api().Suspend(&suspend_signal);
+
+ EXPECT_TRUE(base::ReadFileToString(temp_file_path, &cgroup_state));
+ EXPECT_EQ(cgroup_state, SessionManagerService::kFrozen);
+
+ // SuspendDone should just trigger thawing the instance. We don't
+ // need to worry about faking a message here, since we don't use
+ // the message.
+ manager_->test_api().Resume();
+
+ EXPECT_TRUE(base::ReadFileToString(temp_file_path, &cgroup_state));
+ EXPECT_EQ(cgroup_state, SessionManagerService::kThawed);
+}
+
} // namespace login_manager
diff --git a/login_manager/session_manager_service.cc b/login_manager/session_manager_service.cc
index cb08afa..72a06b5 100644
--- a/login_manager/session_manager_service.cc
+++ b/login_manager/session_manager_service.cc
@@ -40,6 +40,7 @@
#include "login_manager/session_manager_impl.h"
#include "login_manager/system_utils.h"
#include "login_manager/upstart_signal_emitter.h"
+#include "power_manager/proto_bindings/suspend.pb.h"
namespace em = enterprise_management;
namespace login_manager {
@@ -49,6 +50,13 @@
const int kSignals[] = {SIGTERM, SIGINT, SIGHUP};
const int kNumSignals = sizeof(kSignals) / sizeof(int);
+// Constants for susend delays and ARC cgroup control.
+const int kSuspendDelayMs = 1000;
+const char kSuspendDelayDescription[] = "session_manager";
+
+const base::FilePath::CharType kArcCgroupFreezerStatePath[] =
+ FILE_PATH_LITERAL("/sys/fs/cgroup/freezer/android/freezer.state");
+
// I need a do-nothing action for SIGALRM, or using alarm() will kill me.
void DoNothing(int signal) {
}
@@ -69,6 +77,15 @@
proxy->CallMethodAndBlock(&call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT);
}
+void HandleSignalConnected(const std::string& interface,
+ const std::string& signal,
+ bool success) {
+ if (!success) {
+ LOG(ERROR) << "Could not connect to signal " << signal
+ << " on interface " << interface;
+ }
+}
+
} // anonymous namespace
// TODO(mkrebs): Remove CollectChrome timeout and file when
@@ -81,6 +98,11 @@
const char SessionManagerService::kCollectChromeFile[] =
"/mnt/stateful_partition/etc/collect_chrome_crashes";
+// Write these to the ARC cgroup freezer state path to freeze or thaw all
+// of the processes in the instance.
+const char SessionManagerService::kFrozen[] = "FROZEN";
+const char SessionManagerService::kThawed[] = "THAWED";
+
void SessionManagerService::TestApi::ScheduleChildExit(pid_t pid, int status) {
siginfo_t info;
info.si_pid = pid;
@@ -109,6 +131,8 @@
kill_timeout_(base::TimeDelta::FromSeconds(kill_timeout)),
match_rule_(base::StringPrintf("type='method_call', interface='%s'",
kSessionManagerInterface)),
+ arc_cgroup_freezer_state_path_(kArcCgroupFreezerStatePath),
+ suspend_delay_id_(-1),
login_metrics_(metrics),
system_(utils),
nss_(NssUtil::Create()),
@@ -134,7 +158,7 @@
bus_->GetObjectProxy(chromeos::kLibCrosServiceName,
dbus::ObjectPath(chromeos::kLibCrosServicePath));
- dbus::ObjectProxy* powerd_dbus_proxy = bus_->GetObjectProxy(
+ powerd_dbus_proxy_ = bus_->GetObjectProxy(
power_manager::kPowerManagerServiceName,
dbus::ObjectPath(power_manager::kPowerManagerServicePath));
@@ -160,7 +184,7 @@
chromeos::kLibCrosServiceInterface,
chromeos::kLockScreen),
base::Bind(&FireAndBlockOnDBusMethodCall,
- base::Unretained(powerd_dbus_proxy),
+ base::Unretained(powerd_dbus_proxy_),
power_manager::kPowerManagerInterface,
power_manager::kRequestRestartMethod),
&key_gen_,
@@ -180,6 +204,11 @@
adaptor_->ExportDBusMethods(session_manager_dbus_object_);
TakeDBusServiceOwnership();
+
+#ifdef USE_ARC
+ SetUpSuspendHandler();
+#endif
+
return true;
}
@@ -358,6 +387,86 @@
}
}
+bool SessionManagerService::CallPowerdMethod(
+ const std::string& method_name,
+ const google::protobuf::MessageLite& request,
+ google::protobuf::MessageLite* reply_out) {
+ dbus::MethodCall method_call(
+ power_manager::kPowerManagerInterface, method_name);
+ dbus::MessageWriter writer(&method_call);
+ writer.AppendProtoAsArrayOfBytes(request);
+
+ scoped_ptr<dbus::Response> response(
+ powerd_dbus_proxy_->CallMethodAndBlock(
+ &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT));
+ if (!response)
+ return false;
+
+ if (reply_out) {
+ dbus::MessageReader reader(response.get());
+ reader.PopArrayOfBytesAsProto(reply_out);
+ }
+ return true;
+}
+
+void SessionManagerService::SetUpSuspendHandler() {
+ powerd_dbus_proxy_->ConnectToSignal(
+ power_manager::kPowerManagerInterface,
+ power_manager::kSuspendImminentSignal,
+ base::Bind(&SessionManagerService::HandleSuspendImminent,
+ base::Unretained(this)),
+ base::Bind(&HandleSignalConnected));
+ powerd_dbus_proxy_->ConnectToSignal(
+ power_manager::kPowerManagerInterface,
+ power_manager::kSuspendDoneSignal,
+ base::Bind(&SessionManagerService::HandleSuspendDone,
+ base::Unretained(this)),
+ base::Bind(&HandleSignalConnected));
+
+ power_manager::RegisterSuspendDelayRequest request;
+ request.set_timeout(
+ base::TimeDelta::FromMilliseconds(kSuspendDelayMs).ToInternalValue());
+ request.set_description(kSuspendDelayDescription);
+ power_manager::RegisterSuspendDelayReply reply;
+
+ if (!CallPowerdMethod(power_manager::kRegisterSuspendDelayMethod,
+ request, &reply)) {
+ LOG(ERROR) << "Failed to set up suspend handler";
+ return;
+ }
+ LOG(INFO) << "Registered delay " << reply.delay_id();
+ suspend_delay_id_ = reply.delay_id();
+}
+
+void SessionManagerService::SetArcCgroupState(const std::string& state) {
+ // TODO(ejcaruso): Move this to wherever the ARC instance control
+ // lands.
+ LOG(INFO) << "Setting ARC instance state to " << state;
+ if (base::WriteFile(arc_cgroup_freezer_state_path_,
+ state.c_str(), state.size()) < 0) {
+ LOG(WARNING) << "Failed to write to cgroup state file";
+ }
+}
+
+void SessionManagerService::HandleSuspendImminent(dbus::Signal* signal) {
+ power_manager::SuspendImminent info;
+ dbus::MessageReader reader(signal);
+ CHECK(reader.PopArrayOfBytesAsProto(&info));
+ int suspend_id = info.suspend_id();
+
+ SetArcCgroupState(kFrozen);
+
+ power_manager::SuspendReadinessInfo request;
+ request.set_delay_id(suspend_delay_id_);
+ request.set_suspend_id(suspend_id);
+ CallPowerdMethod(power_manager::kHandleSuspendReadinessMethod,
+ request, nullptr);
+}
+
+void SessionManagerService::HandleSuspendDone(dbus::Signal* signal) {
+ SetArcCgroupState(kThawed);
+}
+
// This _must_ be async signal safe. No library calls or malloc'ing allowed.
void SessionManagerService::RevertHandlers() {
struct sigaction action = {};
diff --git a/login_manager/session_manager_service.h b/login_manager/session_manager_service.h
index 6a27bee..d1e4e73 100644
--- a/login_manager/session_manager_service.h
+++ b/login_manager/session_manager_service.h
@@ -17,6 +17,7 @@
#include <brillo/asynchronous_signal_handler.h>
#include <chromeos/dbus/service_constants.h>
#include <dbus/bus.h>
+#include <dbus/message.h>
#include "login_manager/child_exit_handler.h"
#include "login_manager/job_manager.h"
@@ -71,6 +72,10 @@
// Path to magic file that will trigger device wiping on next boot.
static const char kResetFile[];
+ // Constants for setting the ARC instance cgroup state.
+ static const char kFrozen[];
+ static const char kThawed[];
+
// If you want to call any of these setters, you should do so before calling
// any other methods on this class.
class TestApi {
@@ -92,6 +97,18 @@
session_manager_service_->exit_on_child_done_ = do_exit;
}
+ // Sets up powerd and arc cgroup freezer state location
+ // for testing ARC functionality.
+ void set_powerd_object_proxy(dbus::ObjectProxy* proxy) {
+ session_manager_service_->powerd_dbus_proxy_ = proxy;
+ }
+ void set_arc_cgroup_freezer_state_path(base::FilePath path) {
+ session_manager_service_->arc_cgroup_freezer_state_path_ = path;
+ }
+ void set_suspend_delay_id(int id) {
+ session_manager_service_->suspend_delay_id_ = id;
+ }
+
// Executes the CleanupChildren() method on the manager.
void CleanupChildren(int timeout_sec) {
session_manager_service_->CleanupChildren(
@@ -104,6 +121,14 @@
// Trigger and handle SessionManagerImpl initialization.
bool InitializeImpl() { return session_manager_service_->InitializeImpl(); }
+ // Fake messages from powerd.
+ void Suspend(dbus::Signal* signal) {
+ return session_manager_service_->HandleSuspendImminent(signal);
+ }
+ void Resume() {
+ return session_manager_service_->HandleSuspendDone(nullptr);
+ }
+
private:
friend class SessionManagerService;
explicit TestApi(SessionManagerService* session_manager_service)
@@ -186,6 +211,10 @@
// to other needed services. Failure is fatal.
void InitializeDBus();
+ // Initializes suspend delays with powerd and registers callbacks for
+ // suspend and resume.
+ void InitializeSuspendDelays();
+
// Takes ownership of the Session Manager's well-known service name.
// Failure is fatal.
void TakeDBusServiceOwnership();
@@ -206,6 +235,22 @@
// Callback when receiving a termination signal.
bool OnTerminationSignal(const struct signalfd_siginfo& info);
+ // Helper for making powerd calls.
+ bool CallPowerdMethod(const std::string& method_name,
+ const google::protobuf::MessageLite& request,
+ google::protobuf::MessageLite* reply_out);
+
+ // Sets up suspend delay with powerd.
+ void SetUpSuspendHandler();
+
+ // Callbacks for suspend/resume.
+ void HandleSuspendImminent(dbus::Signal* signal);
+ void HandleSuspendDone(dbus::Signal* signal);
+
+ // Sets the ARC instance cgroup state. Can be used to freeze or thaw
+ // the instance.
+ void SetArcCgroupState(const std::string& state);
+
scoped_ptr<BrowserJobInterface> browser_;
bool exit_on_child_done_;
const base::TimeDelta kill_timeout_;
@@ -213,6 +258,15 @@
scoped_refptr<dbus::Bus> bus_;
const std::string match_rule_;
dbus::ExportedObject* session_manager_dbus_object_; // Owned by bus_;
+ dbus::ObjectProxy* powerd_dbus_proxy_; // Owned by bus_.
+
+ // ARC instance related. |arc_cgroup_freezer_state_path_| is the path
+ // to the sysfs file that controls whether the instance's processes
+ // are frozen. |suspend_delay_id_| needs to be passed back to powerd
+ // after we are done freezing the instance to let it know we're ready
+ // to suspend.
+ base::FilePath arc_cgroup_freezer_state_path_;
+ int suspend_delay_id_;
LoginMetrics* login_metrics_; // Owned by the caller.
SystemUtils* system_; // Owned by the caller.
