Merge changes I1893a45e,I29bf9acf

* changes:
  Dumpsys also includes service PIDs.
  libbinder: add getDebugPid

7 files changed, 73 insertions, 50 deletions

diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp
index 254e14203e..024a8fd775 100644
--- a/cmds/dumpstate/dumpstate.cpp
+++ b/cmds/dumpstate/dumpstate.cpp
@@ -1057,7 +1057,7 @@ static Dumpstate::RunStatus RunDumpsysTextByPriority(const std::string& title, i
         std::string path(title);
         path.append(" - ").append(String8(service).c_str());
         size_t bytes_written = 0;
-        status_t status = dumpsys.startDumpThread(service, args);
+        status_t status = dumpsys.startDumpThread(service, /* dumpPid = */ true,  args);
         if (status == OK) {
             dumpsys.writeDumpHeader(STDOUT_FILENO, service, priority);
             std::chrono::duration<double> elapsed_seconds;
@@ -1129,7 +1129,7 @@ static Dumpstate::RunStatus RunDumpsysProto(const std::string& title, int priori
             path.append("_HIGH");
         }
         path.append(kProtoExt);
-        status_t status = dumpsys.startDumpThread(service, args);
+        status_t status = dumpsys.startDumpThread(service, /* dumpPid = */ false, args);
         if (status == OK) {
             status = ds.AddZipEntryFromFd(path, dumpsys.getDumpFd(), service_timeout);
             bool dumpTerminated = (status == OK);
diff --git a/cmds/dumpsys/dumpsys.cpp b/cmds/dumpsys/dumpsys.cpp
index 4811927106..68b3907102 100644
--- a/cmds/dumpsys/dumpsys.cpp
+++ b/cmds/dumpsys/dumpsys.cpp
@@ -236,11 +236,13 @@ int Dumpsys::main(int argc, char* const argv[]) {
         return 0;
     }
 
+    const bool dumpPid = !asProto;
+
     for (size_t i = 0; i < N; i++) {
         const String16& serviceName = services[i];
         if (IsSkipped(skippedServices, serviceName)) continue;
 
-        if (startDumpThread(serviceName, args) == OK) {
+        if (startDumpThread(serviceName, dumpPid, args) == OK) {
             bool addSeparator = (N > 1);
             if (addSeparator) {
                 writeDumpHeader(STDOUT_FILENO, serviceName, priorityFlags);
@@ -307,7 +309,7 @@ void Dumpsys::setServiceArgs(Vector<String16>& args, bool asProto, int priorityF
     }
 }
 
-status_t Dumpsys::startDumpThread(const String16& serviceName, const Vector<String16>& args) {
+status_t Dumpsys::startDumpThread(const String16& serviceName, bool dumpPid, const Vector<String16>& args) {
     sp<IBinder> service = sm_->checkService(serviceName);
     if (service == nullptr) {
         aerr << "Can't find service: " << serviceName << endl;
@@ -327,7 +329,20 @@ status_t Dumpsys::startDumpThread(const String16& serviceName, const Vector<Stri
 
     // dump blocks until completion, so spawn a thread..
     activeThread_ = std::thread([=, remote_end{std::move(remote_end)}]() mutable {
-        int err = service->dump(remote_end.get(), args);
+        if (dumpPid) {
+            pid_t pid;
+            status_t status = service->getDebugPid(&pid);
+            if (status == OK) {
+                std::ostringstream pidinfo;
+                pidinfo << "Pid: " << pid << std::endl;
+                WriteStringToFd(pidinfo.str(), remote_end.get());
+            } else {
+                aerr << "Error getting pid status_t (" << status << "): "
+                        << serviceName << endl;
+            }
+        }
+
+        status_t err = service->dump(remote_end.get(), args);
 
         // It'd be nice to be able to close the remote end of the socketpair before the dump
         // call returns, to terminate our reads if the other end closes their copy of the
@@ -335,8 +350,8 @@ status_t Dumpsys::startDumpThread(const String16& serviceName, const Vector<Stri
         // way to do this, though.
         remote_end.reset();
 
-        if (err != 0) {
-            aerr << "Error dumping service info: (" << strerror(err) << ") "
+        if (err != OK) {
+            aerr << "Error dumping service info status_t (" << err << "): "
                  << serviceName << endl;
         }
     });
diff --git a/cmds/dumpsys/dumpsys.h b/cmds/dumpsys/dumpsys.h
index c48a1e959b..8d1291a3b0 100644
--- a/cmds/dumpsys/dumpsys.h
+++ b/cmds/dumpsys/dumpsys.h
@@ -56,12 +56,13 @@ class Dumpsys {
      * the output to a pipe. Thread must be stopped by a subsequent callto {@code
      * stopDumpThread}.
      * @param serviceName
+     * @param dumpPid whether to include a header with service PID information
      * @param args list of arguments to pass to service dump method.
      * @return {@code OK} thread is started successfully.
      *         {@code NAME_NOT_FOUND} service could not be found.
      *         {@code != OK} error
      */
-    status_t startDumpThread(const String16& serviceName, const Vector<String16>& args);
+    status_t startDumpThread(const String16& serviceName, bool dumpPid, const Vector<String16>& args);
 
     /**
      * Writes a section header to a file descriptor.
diff --git a/cmds/dumpsys/tests/dumpsys_test.cpp b/cmds/dumpsys/tests/dumpsys_test.cpp
index cbac839e6f..d0b167e6b0 100644
--- a/cmds/dumpsys/tests/dumpsys_test.cpp
+++ b/cmds/dumpsys/tests/dumpsys_test.cpp
@@ -188,22 +188,6 @@ class DumpsysTest : public Test {
         EXPECT_THAT(status, Eq(0));
     }
 
-    void CallSingleService(const String16& serviceName, Vector<String16>& args, int priorityFlags,
-                           bool supportsProto, std::chrono::duration<double>& elapsedDuration,
-                           size_t& bytesWritten) {
-        CaptureStdout();
-        CaptureStderr();
-        dump_.setServiceArgs(args, supportsProto, priorityFlags);
-        status_t status = dump_.startDumpThread(serviceName, args);
-        EXPECT_THAT(status, Eq(0));
-        status = dump_.writeDump(STDOUT_FILENO, serviceName, std::chrono::milliseconds(500), false,
-                                 elapsedDuration, bytesWritten);
-        EXPECT_THAT(status, Eq(0));
-        dump_.stopDumpThread(/* dumpCompleted = */ true);
-        stdout_ = GetCapturedStdout();
-        stderr_ = GetCapturedStderr();
-    }
-
     void AssertRunningServices(const std::vector<std::string>& services) {
         std::string expected;
         if (services.size() > 1) {
@@ -215,16 +199,13 @@ class DumpsysTest : public Test {
         EXPECT_THAT(stdout_, HasSubstr(expected));
     }
 
-    void AssertOutput(const std::string& expected) {
-        EXPECT_THAT(stdout_, StrEq(expected));
-    }
-
     void AssertOutputContains(const std::string& expected) {
         EXPECT_THAT(stdout_, HasSubstr(expected));
     }
 
     void AssertDumped(const std::string& service, const std::string& dump) {
-        EXPECT_THAT(stdout_, HasSubstr("DUMP OF SERVICE " + service + ":\n" + dump));
+        EXPECT_THAT(stdout_, HasSubstr("DUMP OF SERVICE " + service + ":\n"));
+        EXPECT_THAT(stdout_, HasSubstr(dump));
         EXPECT_THAT(stdout_, HasSubstr("was the duration of dumpsys " + service + ", ending at: "));
     }
 
@@ -232,7 +213,8 @@ class DumpsysTest : public Test {
                                   const char16_t* priorityType) {
         std::string priority = String8(priorityType).c_str();
         EXPECT_THAT(stdout_,
-                    HasSubstr("DUMP OF SERVICE " + priority + " " + service + ":\n" + dump));
+                    HasSubstr("DUMP OF SERVICE " + priority + " " + service + ":\n"));
+        EXPECT_THAT(stdout_, HasSubstr(dump));
         EXPECT_THAT(stdout_, HasSubstr("was the duration of dumpsys " + service + ", ending at: "));
     }
 
@@ -313,7 +295,8 @@ TEST_F(DumpsysTest, DumpRunningService) {
 
     CallMain({"Valet"});
 
-    AssertOutput("Here's your car");
+    AssertOutputContains("Pid: " + std::to_string(getpid()));
+    AssertOutputContains("Here's your car");
 }
 
 // Tests 'dumpsys -t 1 service_name' on a service that times out after 2s
@@ -348,7 +331,7 @@ TEST_F(DumpsysTest, DumpWithArgsRunningService) {
 
     CallMain({"SERVICE", "Y", "U", "NO", "HANDLE", "ARGS"});
 
-    AssertOutput("I DO!");
+    AssertOutputContains("I DO!");
 }
 
 // Tests dumpsys passes the -a flag when called on all services
@@ -539,23 +522,6 @@ TEST_F(DumpsysTest, DumpWithPriorityHighAndProto) {
     AssertDumpedWithPriority("runninghigh2", "dump2", PriorityDumper::PRIORITY_ARG_HIGH);
 }
 
-TEST_F(DumpsysTest, GetBytesWritten) {
-    const char* serviceName = "service2";
-    const char* dumpContents = "dump1";
-    ExpectDump(serviceName, dumpContents);
-
-    String16 service(serviceName);
-    Vector<String16> args;
-    std::chrono::duration<double> elapsedDuration;
-    size_t bytesWritten;
-
-    CallSingleService(service, args, IServiceManager::DUMP_FLAG_PRIORITY_ALL,
-                      /* as_proto = */ false, elapsedDuration, bytesWritten);
-
-    AssertOutput(dumpContents);
-    EXPECT_THAT(bytesWritten, Eq(strlen(dumpContents)));
-}
-
 TEST_F(DumpsysTest, WriteDumpWithoutThreadStart) {
     std::chrono::duration<double> elapsedDuration;
     size_t bytesWritten;
@@ -563,4 +529,4 @@ TEST_F(DumpsysTest, WriteDumpWithoutThreadStart) {
         dump_.writeDump(STDOUT_FILENO, String16("service"), std::chrono::milliseconds(500),
                         /* as_proto = */ false, elapsedDuration, bytesWritten);
     EXPECT_THAT(status, Eq(INVALID_OPERATION));
-}
\ No newline at end of file
+}
diff --git a/libs/binder/Binder.cpp b/libs/binder/Binder.cpp
index 693045e7f3..34b6ea5385 100644
--- a/libs/binder/Binder.cpp
+++ b/libs/binder/Binder.cpp
@@ -99,6 +99,32 @@ status_t IBinder::getExtension(sp<IBinder>* out) {
     return reply.readNullableStrongBinder(out);
 }
 
+status_t IBinder::getDebugPid(pid_t* out) {
+    BBinder* local = this->localBinder();
+    if (local != nullptr) {
+      *out = local->getDebugPid();
+      return OK;
+    }
+
+    BpBinder* proxy = this->remoteBinder();
+    LOG_ALWAYS_FATAL_IF(proxy == nullptr);
+
+    Parcel data;
+    Parcel reply;
+    status_t status = transact(DEBUG_PID_TRANSACTION, data, &reply);
+    if (status != OK) return status;
+
+    int32_t pid;
+    status = reply.readInt32(&pid);
+    if (status != OK) return status;
+
+    if (pid < 0 || pid > std::numeric_limits<pid_t>::max()) {
+        return BAD_VALUE;
+    }
+    *out = pid;
+    return OK;
+}
+
 // ---------------------------------------------------------------------------
 
 class BBinder::Extras
@@ -152,6 +178,9 @@ status_t BBinder::transact(
         case EXTENSION_TRANSACTION:
             err = reply->writeStrongBinder(getExtension());
             break;
+        case DEBUG_PID_TRANSACTION:
+            err = reply->writeInt32(getDebugPid());
+            break;
         default:
             err = onTransact(code, data, reply, flags);
             break;
@@ -250,6 +279,10 @@ sp<IBinder> BBinder::getExtension() {
     return e->mExtension;
 }
 
+pid_t BBinder::getDebugPid() {
+    return getpid();
+}
+
 void BBinder::setExtension(const sp<IBinder>& extension) {
     Extras* e = getOrCreateExtras();
     e->mExtension = extension;
diff --git a/libs/binder/include/binder/Binder.h b/libs/binder/include/binder/Binder.h
index 1095c7f28c..5673d5a871 100644
--- a/libs/binder/include/binder/Binder.h
+++ b/libs/binder/include/binder/Binder.h
@@ -68,6 +68,8 @@ public:
     // This must be called before the object is sent to another process. Not thread safe.
     void                setExtension(const sp<IBinder>& extension);
 
+    pid_t               getDebugPid();
+
 protected:
     virtual             ~BBinder();
 
diff --git a/libs/binder/include/binder/IBinder.h b/libs/binder/include/binder/IBinder.h
index 027e088be6..b12723446d 100644
--- a/libs/binder/include/binder/IBinder.h
+++ b/libs/binder/include/binder/IBinder.h
@@ -59,6 +59,7 @@ public:
         INTERFACE_TRANSACTION   = B_PACK_CHARS('_', 'N', 'T', 'F'),
         SYSPROPS_TRANSACTION    = B_PACK_CHARS('_', 'S', 'P', 'R'),
         EXTENSION_TRANSACTION   = B_PACK_CHARS('_', 'E', 'X', 'T'),
+        DEBUG_PID_TRANSACTION   = B_PACK_CHARS('_', 'P', 'I', 'D'),
 
         // Corresponds to TF_ONE_WAY -- an asynchronous call.
         FLAG_ONEWAY             = 0x00000001
@@ -129,6 +130,11 @@ public:
      */
     status_t                getExtension(sp<IBinder>* out);
 
+    /**
+     * Dump PID for a binder, for debugging.
+     */
+    status_t                getDebugPid(pid_t* outPid);
+
     // NOLINTNEXTLINE(google-default-arguments)
     virtual status_t        transact(   uint32_t code,
                                         const Parcel& data,