Merge "Revert "Remove UID requirement for set sched_fifo for sensor thread""
diff --git a/cmds/dumpstate/DumpstateUtil.cpp b/cmds/dumpstate/DumpstateUtil.cpp
index 26702c4..7488827 100644
--- a/cmds/dumpstate/DumpstateUtil.cpp
+++ b/cmds/dumpstate/DumpstateUtil.cpp
@@ -103,6 +103,12 @@
return *this;
}
+CommandOptions::CommandOptionsBuilder& CommandOptions::CommandOptionsBuilder::AsRootIfAvailable() {
+ if (!PropertiesHelper::IsUserBuild())
+ values.account_mode_ = SU_ROOT;
+ return *this;
+}
+
CommandOptions::CommandOptionsBuilder& CommandOptions::CommandOptionsBuilder::DropRoot() {
values.account_mode_ = DROP_ROOT;
return *this;
diff --git a/cmds/dumpstate/DumpstateUtil.h b/cmds/dumpstate/DumpstateUtil.h
index 5a8ce5b..698ceff 100644
--- a/cmds/dumpstate/DumpstateUtil.h
+++ b/cmds/dumpstate/DumpstateUtil.h
@@ -89,6 +89,8 @@
CommandOptionsBuilder& Always();
/* Sets the command's PrivilegeMode as `SU_ROOT` */
CommandOptionsBuilder& AsRoot();
+ /* If !IsUserBuild(), sets the command's PrivilegeMode as `SU_ROOT` */
+ CommandOptionsBuilder& AsRootIfAvailable();
/* Sets the command's PrivilegeMode as `DROP_ROOT` */
CommandOptionsBuilder& DropRoot();
/* Sets the command's OutputMode as `REDIRECT_TO_STDERR` */
diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp
index f84d86d..24b4c99 100644
--- a/cmds/dumpstate/dumpstate.cpp
+++ b/cmds/dumpstate/dumpstate.cpp
@@ -972,14 +972,15 @@
RunCommand(
"HARDWARE HALS",
{"lshal", std::string("--debug=") + kLsHalDebugPath},
- CommandOptions::AS_ROOT);
+ CommandOptions::WithTimeout(10).AsRootIfAvailable().Build());
ds.AddZipEntry("lshal-debug.txt", kLsHalDebugPath);
unlink(kLsHalDebugPath.c_str());
} else {
RunCommand(
- "HARDWARE HALS", {"lshal", "--debug"}, CommandOptions::AS_ROOT);
+ "HARDWARE HALS", {"lshal", "--debug"},
+ CommandOptions::WithTimeout(10).AsRootIfAvailable().Build());
}
RunCommand("PRINTENV", {"printenv"});
diff --git a/cmds/dumpstate/tests/dumpstate_test.cpp b/cmds/dumpstate/tests/dumpstate_test.cpp
index 1c19268..52698b2 100644
--- a/cmds/dumpstate/tests/dumpstate_test.cpp
+++ b/cmds/dumpstate/tests/dumpstate_test.cpp
@@ -477,6 +477,48 @@
EXPECT_THAT(err, StrEq("stderr\n"));
}
+TEST_F(DumpstateTest, RunCommandAsRootIfAvailableOnUserBuild) {
+ if (!IsStandalone()) {
+ // TODO: temporarily disabled because it might cause other tests to fail after dropping
+ // to Shell - need to refactor tests to avoid this problem)
+ MYLOGE("Skipping DumpstateTest.RunCommandAsRootIfAvailableOnUserBuild() on test suite\n")
+ return;
+ }
+ if (!PropertiesHelper::IsUserBuild()) {
+ // Emulates user build if necessarily.
+ SetBuildType("user");
+ }
+
+ DropRoot();
+
+ EXPECT_EQ(0, RunCommand("", {kSimpleCommand, "--uid"},
+ CommandOptions::WithTimeout(1).AsRootIfAvailable().Build()));
+
+ EXPECT_THAT(out, StrEq("2000\nstdout\n"));
+ EXPECT_THAT(err, StrEq("stderr\n"));
+}
+
+TEST_F(DumpstateTest, RunCommandAsRootIfAvailableOnDebugBuild) {
+ if (!IsStandalone()) {
+ // TODO: temporarily disabled because it might cause other tests to fail after dropping
+ // to Shell - need to refactor tests to avoid this problem)
+ MYLOGE("Skipping DumpstateTest.RunCommandAsRootIfAvailableOnDebugBuild() on test suite\n")
+ return;
+ }
+ if (PropertiesHelper::IsUserBuild()) {
+ ALOGI("Skipping RunCommandAsRootNonUserBuild on user builds\n");
+ return;
+ }
+
+ DropRoot();
+
+ EXPECT_EQ(0, RunCommand("", {kSimpleCommand, "--uid"},
+ CommandOptions::WithTimeout(1).AsRootIfAvailable().Build()));
+
+ EXPECT_THAT(out, StrEq("0\nstdout\n"));
+ EXPECT_THAT(err, StrEq("stderr\n"));
+}
+
TEST_F(DumpstateTest, DumpFileNotFoundNoTitle) {
EXPECT_EQ(-1, DumpFile("", "/I/cant/believe/I/exist"));
EXPECT_THAT(out,
@@ -1053,6 +1095,51 @@
EXPECT_THAT(err, StrEq("stderr\n"));
}
+
+TEST_F(DumpstateUtilTest, RunCommandAsRootIfAvailableOnUserBuild) {
+ if (!IsStandalone()) {
+ // TODO: temporarily disabled because it might cause other tests to fail after dropping
+ // to Shell - need to refactor tests to avoid this problem)
+ MYLOGE("Skipping DumpstateUtilTest.RunCommandAsRootIfAvailableOnUserBuild() on test suite\n")
+ return;
+ }
+ CreateFd("RunCommandAsRootIfAvailableOnUserBuild.txt");
+ if (!PropertiesHelper::IsUserBuild()) {
+ // Emulates user build if necessarily.
+ SetBuildType("user");
+ }
+
+ DropRoot();
+
+ EXPECT_EQ(0, RunCommand("", {kSimpleCommand, "--uid"},
+ CommandOptions::WithTimeout(1).AsRootIfAvailable().Build()));
+
+ EXPECT_THAT(out, StrEq("2000\nstdout\n"));
+ EXPECT_THAT(err, StrEq("stderr\n"));
+}
+
+TEST_F(DumpstateUtilTest, RunCommandAsRootIfAvailableOnDebugBuild) {
+ if (!IsStandalone()) {
+ // TODO: temporarily disabled because it might cause other tests to fail after dropping
+ // to Shell - need to refactor tests to avoid this problem)
+ MYLOGE("Skipping DumpstateUtilTest.RunCommandAsRootIfAvailableOnDebugBuild() on test suite\n")
+ return;
+ }
+ CreateFd("RunCommandAsRootIfAvailableOnDebugBuild.txt");
+ if (PropertiesHelper::IsUserBuild()) {
+ ALOGI("Skipping RunCommandAsRootNonUserBuild on user builds\n");
+ return;
+ }
+
+ DropRoot();
+
+ EXPECT_EQ(0, RunCommand("", {kSimpleCommand, "--uid"},
+ CommandOptions::WithTimeout(1).AsRootIfAvailable().Build()));
+
+ EXPECT_THAT(out, StrEq("0\nstdout\n"));
+ EXPECT_THAT(err, StrEq("stderr\n"));
+}
+
TEST_F(DumpstateUtilTest, RunCommandDropRoot) {
if (!IsStandalone()) {
// TODO: temporarily disabled because it might cause other tests to fail after dropping
diff --git a/cmds/installd/InstalldNativeService.cpp b/cmds/installd/InstalldNativeService.cpp
index 60c89a9..bac83a4 100644
--- a/cmds/installd/InstalldNativeService.cpp
+++ b/cmds/installd/InstalldNativeService.cpp
@@ -519,6 +519,9 @@
if (access(path.c_str(), F_OK) == 0) {
if (delete_dir_contents(path) != 0) {
res = error("Failed to delete contents of " + path);
+ } else if ((flags & (FLAG_CLEAR_CACHE_ONLY | FLAG_CLEAR_CODE_CACHE_ONLY)) == 0) {
+ remove_path_xattr(path, kXattrInodeCache);
+ remove_path_xattr(path, kXattrInodeCodeCache);
}
}
}
diff --git a/cmds/installd/utils.cpp b/cmds/installd/utils.cpp
index 7c05417..8a0e137 100644
--- a/cmds/installd/utils.cpp
+++ b/cmds/installd/utils.cpp
@@ -733,6 +733,12 @@
}
}
+void remove_path_xattr(const std::string& path, const char* inode_xattr) {
+ if (removexattr(path.c_str(), inode_xattr) && errno != ENODATA) {
+ PLOG(ERROR) << "Failed to remove xattr " << inode_xattr << " at " << path;
+ }
+}
+
/**
* Validate that the path is valid in the context of the provided directory.
* The path is allowed to have at most one subdirectory and no indirections
diff --git a/cmds/installd/utils.h b/cmds/installd/utils.h
index 070da84..ac6a488 100644
--- a/cmds/installd/utils.h
+++ b/cmds/installd/utils.h
@@ -120,6 +120,7 @@
int write_path_inode(const std::string& parent, const char* name, const char* inode_xattr);
std::string read_path_inode(const std::string& parent, const char* name, const char* inode_xattr);
+void remove_path_xattr(const std::string& path, const char* inode_xattr);
int validate_system_app_path(const char* path);
bool validate_secondary_dex_path(const std::string& pkgname, const std::string& dex_path,
diff --git a/services/sensorservice/Android.bp b/services/sensorservice/Android.bp
index 8d381b1..a7f3a52 100644
--- a/services/sensorservice/Android.bp
+++ b/services/sensorservice/Android.bp
@@ -14,6 +14,7 @@
"RecentEventLogger.cpp",
"RotationVectorSensor.cpp",
"SensorDevice.cpp",
+ "SensorDeviceUtils.cpp",
"SensorDirectConnection.cpp",
"SensorEventConnection.cpp",
"SensorFusion.cpp",
diff --git a/services/sensorservice/SensorDevice.cpp b/services/sensorservice/SensorDevice.cpp
index da3b275..fdb69d3 100644
--- a/services/sensorservice/SensorDevice.cpp
+++ b/services/sensorservice/SensorDevice.cpp
@@ -26,11 +26,10 @@
#include <cinttypes>
#include <thread>
-using android::hardware::hidl_vec;
-
using namespace android::hardware::sensors::V1_0;
using namespace android::hardware::sensors::V1_0::implementation;
-
+using android::hardware::hidl_vec;
+using android::SensorDeviceUtils::HidlServiceRegistrationWaiter;
namespace android {
// ---------------------------------------------------------------------------
@@ -52,7 +51,8 @@
}
}
-SensorDevice::SensorDevice() : mHidlTransportErrors(20) {
+SensorDevice::SensorDevice()
+ : mHidlTransportErrors(20), mRestartWaiter(new HidlServiceRegistrationWaiter()) {
if (!connectHidlService()) {
return;
}
@@ -87,35 +87,29 @@
}
bool SensorDevice::connectHidlService() {
- // SensorDevice may wait upto 100ms * 10 = 1s for hidl service.
- constexpr auto RETRY_DELAY = std::chrono::milliseconds(100);
+ // SensorDevice will wait for HAL service to start if HAL is declared in device manifest.
size_t retry = 10;
- while (true) {
- int initStep = 0;
+ while (retry-- > 0) {
mSensors = ISensors::getService();
- if (mSensors != nullptr) {
- ++initStep;
- // Poke ISensor service. If it has lingering connection from previous generation of
- // system server, it will kill itself. There is no intention to handle the poll result,
- // which will be done since the size is 0.
- if(mSensors->poll(0, [](auto, const auto &, const auto &) {}).isOk()) {
- // ok to continue
- break;
- }
- // hidl service is restarting, pointer is invalid.
- mSensors = nullptr;
- }
-
- if (--retry <= 0) {
- ALOGE("Cannot connect to ISensors hidl service!");
+ if (mSensors == nullptr) {
+ // no sensor hidl service found
break;
}
- // Delay 100ms before retry, hidl service is expected to come up in short time after
- // crash.
- ALOGI("%s unsuccessful, try again soon (remaining retry %zu).",
- (initStep == 0) ? "getService()" : "poll() check", retry);
- std::this_thread::sleep_for(RETRY_DELAY);
+
+ mRestartWaiter->reset();
+ // Poke ISensor service. If it has lingering connection from previous generation of
+ // system server, it will kill itself. There is no intention to handle the poll result,
+ // which will be done since the size is 0.
+ if(mSensors->poll(0, [](auto, const auto &, const auto &) {}).isOk()) {
+ // ok to continue
+ break;
+ }
+
+ // hidl service is restarting, pointer is invalid.
+ mSensors = nullptr;
+ ALOGI("%s unsuccessful, remaining retry %zu.", __FUNCTION__, retry);
+ mRestartWaiter->wait();
}
return (mSensors != nullptr);
}
@@ -173,7 +167,7 @@
}
status_t SensorDevice::initCheck() const {
- return mSensors != NULL ? NO_ERROR : NO_INIT;
+ return mSensors != nullptr ? NO_ERROR : NO_INIT;
}
ssize_t SensorDevice::poll(sensors_event_t* buffer, size_t count) {
diff --git a/services/sensorservice/SensorDevice.h b/services/sensorservice/SensorDevice.h
index fd6cee6..6d75051 100644
--- a/services/sensorservice/SensorDevice.h
+++ b/services/sensorservice/SensorDevice.h
@@ -17,6 +17,7 @@
#ifndef ANDROID_SENSOR_DEVICE_H
#define ANDROID_SENSOR_DEVICE_H
+#include "SensorDeviceUtils.h"
#include "SensorServiceUtils.h"
#include <sensor/Sensor.h>
@@ -39,12 +40,9 @@
namespace android {
// ---------------------------------------------------------------------------
-using SensorServiceUtil::Dumpable;
-using hardware::Return;
-class SensorDevice : public Singleton<SensorDevice>, public Dumpable {
+class SensorDevice : public Singleton<SensorDevice>, public SensorServiceUtil::Dumpable {
public:
-
class HidlTransportErrorLog {
public:
@@ -105,7 +103,7 @@
private:
friend class Singleton<SensorDevice>;
- sp<android::hardware::sensors::V1_0::ISensors> mSensors;
+ sp<hardware::sensors::V1_0::ISensors> mSensors;
Vector<sensor_t> mSensorList;
std::unordered_map<int32_t, sensor_t*> mConnectedDynamicSensors;
@@ -174,6 +172,8 @@
}
return std::move(ret);
}
+ //TODO(b/67425500): remove waiter after bug is resolved.
+ sp<SensorDeviceUtils::HidlServiceRegistrationWaiter> mRestartWaiter;
bool isClientDisabled(void* ident);
bool isClientDisabledLocked(void* ident);
diff --git a/services/sensorservice/SensorDeviceUtils.cpp b/services/sensorservice/SensorDeviceUtils.cpp
new file mode 100644
index 0000000..b1344be
--- /dev/null
+++ b/services/sensorservice/SensorDeviceUtils.cpp
@@ -0,0 +1,70 @@
+/*
+ * Copyright (C) 2017 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 "SensorDeviceUtils.h"
+
+#include <android/hardware/sensors/1.0/ISensors.h>
+#include <utils/Log.h>
+
+#include <chrono>
+#include <thread>
+
+using ::android::hardware::Void;
+using namespace android::hardware::sensors::V1_0;
+
+namespace android {
+namespace SensorDeviceUtils {
+
+HidlServiceRegistrationWaiter::HidlServiceRegistrationWaiter()
+ : mRegistered(ISensors::registerForNotifications("default", this)) {
+}
+
+Return<void> HidlServiceRegistrationWaiter::onRegistration(
+ const hidl_string &fqName, const hidl_string &name, bool preexisting) {
+ ALOGV("onRegistration fqName %s, name %s, preexisting %d",
+ fqName.c_str(), name.c_str(), preexisting);
+
+ {
+ std::lock_guard<std::mutex> lk(mLock);
+ mRestartObserved = true;
+ }
+ mCondition.notify_all();
+ return Void();
+}
+
+void HidlServiceRegistrationWaiter::reset() {
+ std::lock_guard<std::mutex> lk(mLock);
+ mRestartObserved = false;
+}
+
+bool HidlServiceRegistrationWaiter::wait() {
+ constexpr int DEFAULT_WAIT_MS = 100;
+ constexpr int TIMEOUT_MS = 1000;
+
+ if (!mRegistered) {
+ ALOGW("Cannot register service notification, use default wait(%d ms)", DEFAULT_WAIT_MS);
+ std::this_thread::sleep_for(std::chrono::milliseconds(DEFAULT_WAIT_MS));
+ // not sure if service is actually restarted
+ return false;
+ }
+
+ std::unique_lock<std::mutex> lk(mLock);
+ return mCondition.wait_for(lk, std::chrono::milliseconds(TIMEOUT_MS),
+ [this]{return mRestartObserved;});
+}
+
+} // namespace SensorDeviceUtils
+} // namespace android
diff --git a/services/sensorservice/SensorDeviceUtils.h b/services/sensorservice/SensorDeviceUtils.h
new file mode 100644
index 0000000..da36928
--- /dev/null
+++ b/services/sensorservice/SensorDeviceUtils.h
@@ -0,0 +1,60 @@
+/*
+ * Copyright (C) 2017 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 ANDROID_SENSOR_DEVICE_UTIL
+#define ANDROID_SENSOR_DEVICE_UTIL
+
+#include <android/hidl/manager/1.0/IServiceNotification.h>
+
+#include <condition_variable>
+#include <thread>
+
+using ::android::hardware::hidl_string;
+using ::android::hardware::Return;
+using ::android::hidl::manager::V1_0::IServiceNotification;
+
+namespace android {
+namespace SensorDeviceUtils {
+
+class HidlServiceRegistrationWaiter : public IServiceNotification {
+public:
+
+ HidlServiceRegistrationWaiter();
+
+ Return<void> onRegistration(const hidl_string &fqName,
+ const hidl_string &name,
+ bool preexisting) override;
+
+ void reset();
+
+ /**
+ * Wait for service restart
+ *
+ * @return true if service is restart since last reset(); false otherwise.
+ */
+ bool wait();
+private:
+ const bool mRegistered;
+
+ std::mutex mLock;
+ std::condition_variable mCondition;
+ bool mRestartObserved;
+};
+
+} // namespace SensorDeviceUtils
+} // namespace android;
+
+#endif // ANDROID_SENSOR_SERVICE_UTIL