dumpstate: enforce oneshot
Ensure the service exits after errors as well as after successful
finish as a oneshot service should.
While at it also move remove(file) to unlink(file) and fix an error
in checking the return value of unlink.
BUG: 123571915
Test: adb shell /data/nativetest64/dumpstate_smoke_test/dumpstate_smoke_test --gtest_filter=DumpstateBinderTest.*
Change-Id: I772b7981cd3b2f7c285ab980495d5539d57ebf46
diff --git a/cmds/dumpstate/tests/dumpstate_smoke_test.cpp b/cmds/dumpstate/tests/dumpstate_smoke_test.cpp
index 570c6c9..b5ad699 100644
--- a/cmds/dumpstate/tests/dumpstate_smoke_test.cpp
+++ b/cmds/dumpstate/tests/dumpstate_smoke_test.cpp
@@ -21,6 +21,10 @@
#include <libgen.h>
#include <android-base/file.h>
+#include <android/os/BnDumpstate.h>
+#include <android/os/BnDumpstateListener.h>
+#include <binder/IServiceManager.h>
+#include <binder/ProcessState.h>
#include <cutils/properties.h>
#include <ziparchive/zip_archive.h>
@@ -34,6 +38,24 @@
using ::testing::Test;
using ::std::literals::chrono_literals::operator""s;
+using android::base::unique_fd;
+
+class DumpstateListener;
+
+namespace {
+
+sp<IDumpstate> GetDumpstateService() {
+ return android::interface_cast<IDumpstate>(
+ android::defaultServiceManager()->getService(String16("dumpstate")));
+}
+
+int OpenForWrite(const std::string& filename) {
+ return TEMP_FAILURE_RETRY(open(filename.c_str(),
+ O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC | O_NOFOLLOW,
+ S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH));
+}
+
+} // namespace
struct SectionInfo {
std::string name;
@@ -46,41 +68,71 @@
* Listens to bugreport progress and updates the user by writing the progress to STDOUT. All the
* section details generated by dumpstate are added to a vector to be used by Tests later.
*/
-class DumpstateListener : public IDumpstateListener {
+class DumpstateListener : public BnDumpstateListener {
public:
- int outFd_, max_progress_;
- std::shared_ptr<std::vector<SectionInfo>> sections_;
DumpstateListener(int fd, std::shared_ptr<std::vector<SectionInfo>> sections)
- : outFd_(fd), max_progress_(5000), sections_(sections) {
+ : out_fd_(fd), sections_(sections) {
}
+
+ DumpstateListener(int fd) : out_fd_(fd) {
+ }
+
binder::Status onProgress(int32_t progress) override {
- dprintf(outFd_, "\rIn progress %d", progress);
+ dprintf(out_fd_, "\rIn progress %d", progress);
return binder::Status::ok();
}
+
binder::Status onError(int32_t error_code) override {
- dprintf(outFd_, "\rError %d", error_code);
+ std::lock_guard<std::mutex> lock(lock_);
+ error_code_ = error_code;
+ dprintf(out_fd_, "\rError code %d", error_code);
return binder::Status::ok();
}
+
binder::Status onFinished() override {
- dprintf(outFd_, "\rFinished");
+ std::lock_guard<std::mutex> lock(lock_);
+ is_finished_ = true;
+ dprintf(out_fd_, "\rFinished");
return binder::Status::ok();
}
+
binder::Status onProgressUpdated(int32_t progress) override {
- dprintf(outFd_, "\rIn progress %d/%d", progress, max_progress_);
+ dprintf(out_fd_, "\rIn progress %d/%d", progress, max_progress_);
return binder::Status::ok();
}
+
binder::Status onMaxProgressUpdated(int32_t max_progress) override {
+ std::lock_guard<std::mutex> lock(lock_);
max_progress_ = max_progress;
return binder::Status::ok();
}
+
binder::Status onSectionComplete(const ::std::string& name, int32_t status, int32_t size_bytes,
int32_t duration_ms) override {
- sections_->push_back({name, status, size_bytes, duration_ms});
+ std::lock_guard<std::mutex> lock(lock_);
+ if (sections_.get() != nullptr) {
+ sections_->push_back({name, status, size_bytes, duration_ms});
+ }
return binder::Status::ok();
}
- IBinder* onAsBinder() override {
- return nullptr;
+
+ bool getIsFinished() {
+ std::lock_guard<std::mutex> lock(lock_);
+ return is_finished_;
}
+
+ int getErrorCode() {
+ std::lock_guard<std::mutex> lock(lock_);
+ return error_code_;
+ }
+
+ private:
+ int out_fd_;
+ int max_progress_ = 5000;
+ int error_code_ = -1;
+ bool is_finished_ = false;
+ std::shared_ptr<std::vector<SectionInfo>> sections_;
+ std::mutex lock_;
};
/**
@@ -293,6 +345,147 @@
SectionExists("DUMPSYS - wifi", /* bytes= */ 100000);
}
+class DumpstateBinderTest : public Test {
+ protected:
+ void SetUp() override {
+ // In case there is a stray service, stop it first.
+ property_set("ctl.stop", "bugreportd");
+ // dry_run results in a faster bugreport.
+ property_set("dumpstate.dry_run", "true");
+ // We need to receive some async calls later. Ensure we have binder threads.
+ ProcessState::self()->startThreadPool();
+ }
+
+ void TearDown() override {
+ property_set("ctl.stop", "bugreportd");
+ property_set("dumpstate.dry_run", "");
+
+ unlink("/data/local/tmp/tmp.zip");
+ unlink("/data/local/tmp/tmp.png");
+ }
+
+ // Waits until listener gets the callbacks.
+ void WaitTillExecutionComplete(DumpstateListener* listener) {
+ // Wait till one of finished, error or timeout.
+ static const int kBugreportTimeoutSeconds = 120;
+ int i = 0;
+ while (!listener->getIsFinished() && listener->getErrorCode() == -1 &&
+ i < kBugreportTimeoutSeconds) {
+ sleep(1);
+ i++;
+ }
+ }
+};
+
+TEST_F(DumpstateBinderTest, Baseline) {
+ // In the beginning dumpstate binder service is not running.
+ sp<android::os::IDumpstate> ds_binder(GetDumpstateService());
+ EXPECT_EQ(ds_binder, nullptr);
+
+ // Start bugreportd, which runs dumpstate binary with -w; which starts dumpstate service
+ // and makes it wait.
+ property_set("dumpstate.dry_run", "true");
+ property_set("ctl.start", "bugreportd");
+
+ // Now we are able to retrieve dumpstate binder service.
+ ds_binder = GetDumpstateService();
+ EXPECT_NE(ds_binder, nullptr);
+
+ // Prepare arguments
+ unique_fd bugreport_fd(OpenForWrite("/bugreports/tmp.zip"));
+ unique_fd screenshot_fd(OpenForWrite("/bugreports/tmp.png"));
+
+ EXPECT_NE(bugreport_fd.get(), -1);
+ EXPECT_NE(screenshot_fd.get(), -1);
+
+ sp<DumpstateListener> listener(new DumpstateListener(dup(fileno(stdout))));
+ android::binder::Status status =
+ ds_binder->startBugreport(123, "com.dummy.package", bugreport_fd, screenshot_fd,
+ Dumpstate::BugreportMode::BUGREPORT_INTERACTIVE, listener);
+ // startBugreport is an async call. Verify binder call succeeded first, then wait till listener
+ // gets expected callbacks.
+ EXPECT_TRUE(status.isOk());
+ WaitTillExecutionComplete(listener.get());
+
+ // Bugreport generation requires user consent, which we cannot get in a test set up,
+ // so instead of getting is_finished_, we are more likely to get a consent error.
+ EXPECT_TRUE(
+ listener->getErrorCode() == IDumpstateListener::BUGREPORT_ERROR_USER_DENIED_CONSENT ||
+ listener->getErrorCode() == IDumpstateListener::BUGREPORT_ERROR_USER_CONSENT_TIMED_OUT);
+
+ // The service should have died on its own, freeing itself up for a new invocation.
+ sleep(2);
+ ds_binder = GetDumpstateService();
+ EXPECT_EQ(ds_binder, nullptr);
+}
+
+TEST_F(DumpstateBinderTest, ServiceDies_OnInvalidInput) {
+ // Start bugreportd, which runs dumpstate binary with -w; which starts dumpstate service
+ // and makes it wait.
+ property_set("ctl.start", "bugreportd");
+ sp<android::os::IDumpstate> ds_binder(GetDumpstateService());
+ EXPECT_NE(ds_binder, nullptr);
+
+ // Prepare arguments
+ unique_fd bugreport_fd(OpenForWrite("/data/local/tmp/tmp.zip"));
+ unique_fd screenshot_fd(OpenForWrite("/data/local/tmp/tmp.png"));
+
+ EXPECT_NE(bugreport_fd.get(), -1);
+ EXPECT_NE(screenshot_fd.get(), -1);
+
+ // Call startBugreport with bad arguments.
+ sp<DumpstateListener> listener(new DumpstateListener(dup(fileno(stdout))));
+ android::binder::Status status =
+ ds_binder->startBugreport(123, "com.dummy.package", bugreport_fd, screenshot_fd,
+ 2000, // invalid bugreport mode
+ listener);
+ EXPECT_EQ(listener->getErrorCode(), IDumpstateListener::BUGREPORT_ERROR_INVALID_INPUT);
+
+ // The service should have died, freeing itself up for a new invocation.
+ sleep(2);
+ ds_binder = GetDumpstateService();
+ EXPECT_EQ(ds_binder, nullptr);
+}
+
+TEST_F(DumpstateBinderTest, SimultaneousBugreportsNotAllowed) {
+ // Start bugreportd, which runs dumpstate binary with -w; which starts dumpstate service
+ // and makes it wait.
+ property_set("dumpstate.dry_run", "true");
+ property_set("ctl.start", "bugreportd");
+ sp<android::os::IDumpstate> ds_binder(GetDumpstateService());
+ EXPECT_NE(ds_binder, nullptr);
+
+ // Prepare arguments
+ unique_fd bugreport_fd(OpenForWrite("/data/local/tmp/tmp.zip"));
+ unique_fd screenshot_fd(OpenForWrite("/data/local/tmp/tmp.png"));
+
+ EXPECT_NE(bugreport_fd.get(), -1);
+ EXPECT_NE(screenshot_fd.get(), -1);
+
+ sp<DumpstateListener> listener1(new DumpstateListener(dup(fileno(stdout))));
+ android::binder::Status status =
+ ds_binder->startBugreport(123, "com.dummy.package", bugreport_fd, screenshot_fd,
+ Dumpstate::BugreportMode::BUGREPORT_INTERACTIVE, listener1);
+ EXPECT_TRUE(status.isOk());
+
+ // try to make another call to startBugreport. This should fail.
+ sp<DumpstateListener> listener2(new DumpstateListener(dup(fileno(stdout))));
+ status = ds_binder->startBugreport(123, "com.dummy.package", bugreport_fd, screenshot_fd,
+ Dumpstate::BugreportMode::BUGREPORT_INTERACTIVE, listener2);
+ EXPECT_FALSE(status.isOk());
+ WaitTillExecutionComplete(listener2.get());
+ EXPECT_EQ(listener2->getErrorCode(), IDumpstateListener::BUGREPORT_ERROR_RUNTIME_ERROR);
+
+ // Meanwhile the first call works as expected. Service should not die in this case.
+ WaitTillExecutionComplete(listener1.get());
+
+ // Bugreport generation requires user consent, which we cannot get in a test set up,
+ // so instead of getting is_finished_, we are more likely to get a consent error.
+ EXPECT_TRUE(
+ listener1->getErrorCode() == IDumpstateListener::BUGREPORT_ERROR_USER_DENIED_CONSENT ||
+ listener1->getErrorCode() == IDumpstateListener::BUGREPORT_ERROR_USER_CONSENT_TIMED_OUT);
+}
+
} // namespace dumpstate
} // namespace os
} // namespace android