vold: Fix devmapper/ptmx fd leak, and give asec unmount more time

Signed-off-by: San Mehat <san@google.com>

Devmapper.cpp

@@ -167,6 +167,7 @@ int Devmapper::create(const char *name, const char *loopFile, const char *key,
 
     free(buffer);
 
+    close(fd);
     return 0;
 }
 

ProcessKiller.c

@@ -179,8 +179,14 @@ static int get_pid(const char* s)
     return result;
 }
 
+/*
+ * Hunt down processes that have files open at the given mount point.
+ * action = 0 to just warn,
+ * action = 1 to SIGHUP,
+ * action = 2 to SIGKILL
+ */
 // hunt down and kill processes that have files open on the given mount point
-void KillProcessesWithOpenFiles(const char* mountPoint, int sigkill, int *excluded, int num_excluded)
+void KillProcessesWithOpenFiles(const char* mountPoint, int action)
 {
     DIR*    dir;
     struct dirent* de;
@@ -202,28 +208,12 @@ void KillProcessesWithOpenFiles(const char* mountPoint, int sigkill, int *exclud
                 || CheckSymLink(pid, mountPoint, "exe", "executable path")      // check executable path
             ) 
         {
-            int i;
-            int hit = 0;
-
-            for (i = 0; i < num_excluded; i++) {
-                if (pid == excluded[i]) {
-                    LOGE("I just need a little more TIME captain!");
-                    hit = 1;
-                    break;
-                }
-            }
-
-            if (!hit) {
-                if (!sigkill) {
-                    kill(pid, SIGTERM);
-                } else {
-                    // Log this as an error since the app should
-                    // have released it's resources long before
-                    // this point.
-
-                    LOGE("Sending SIGKILL to process %d", pid);
-                    kill(pid, SIGKILL);
-                }
+            if (action == 1) {
+                LOGW("Sending SIGHUP to process %d", pid);
+                kill(pid, SIGTERM);
+            } else if (action == 2) {
+                LOGE("Sending SIGKILL to process %d", pid);
+                kill(pid, SIGKILL);
             }
         }
     }

Volume.cpp

@@ -41,7 +41,7 @@
 #include "ResponseCode.h"
 #include "Fat.h"
 
-extern "C" void KillProcessesWithOpenFiles(const char *, int, int, int);
+extern "C" void KillProcessesWithOpenFiles(const char *, int);
 extern "C" void dos_partition_dec(void const *pp, struct dos_partition *d);
 extern "C" void dos_partition_enc(void *pp, struct dos_partition *d);
 
@@ -286,7 +286,7 @@ int Volume::unmountVol() {
 
     setState(Volume::State_Unmounting);
     usleep(1000 * 200); // Give the framework some time to react
-    for (i = 0; i < 10; i++) {
+    for (i = 1; i <= 10; i++) {
         rc = umount(getMountpoint());
         if (!rc)
             break;
@@ -297,16 +297,19 @@ int Volume::unmountVol() {
         }
 
         LOGW("Volume %s unmount attempt %d failed (%s)",
-             getLabel(), i + 1, strerror(errno));
-
-        if (i < 5) {
-            usleep(1000 * 250);
-        } else {
-            KillProcessesWithOpenFiles(getMountpoint(),
-                                       (i < 7 ? 0 : 1),
-                                       NULL, 0);
-            usleep(1000 * 250);
-        }
+             getLabel(), i, strerror(errno));
+
+        int action;
+
+        if (i > 8) {
+            action = 2; // SIGKILL
+        } else if (i > 7) {
+            action = 1; // SIGHUP
+        } else
+            action = 0; // just complain
+
+        KillProcessesWithOpenFiles(getMountpoint(), action);
+        usleep(1000*250);
     }
 
     if (!rc) {

VolumeManager.cpp

@@ -38,7 +38,7 @@
 #include "Fat.h"
 #include "Devmapper.h"
 
-extern "C" void KillProcessesWithOpenFiles(const char *, int, int, int);
+extern "C" void KillProcessesWithOpenFiles(const char *, int);
 
 VolumeManager *VolumeManager::sInstance = NULL;
 
@@ -320,6 +320,7 @@ int VolumeManager::renameAsec(const char *id1, const char *id2) {
     return -1;
 }
 
+#define ASEC_UNMOUNT_RETRIES 10
 int VolumeManager::unmountAsec(const char *id) {
     char asecFileName[255];
     char mountPoint[255];
@@ -335,7 +336,7 @@ int VolumeManager::unmountAsec(const char *id) {
     }
 
     int i, rc;
-    for (i = 0; i < 10; i++) {
+    for (i = 1; i <= ASEC_UNMOUNT_RETRIES; i++) {
         rc = umount(mountPoint);
         if (!rc) {
             break;
@@ -345,13 +346,18 @@ int VolumeManager::unmountAsec(const char *id) {
             break;
         }
         LOGW("ASEC %s unmount attempt %d failed (%s)",
-              id, i +1, strerror(errno));
-
-        if (i >= 5) {
-            KillProcessesWithOpenFiles(mountPoint, (i < 7 ? 0 : 1),
-                                       NULL, 0);
-        }
-        usleep(1000 * 250);
+              id, i, strerror(errno));
+
+        int action;
+        if (i > (ASEC_UNMOUNT_RETRIES - 2))
+            action = 2; // SIGKILL
+        else if (i > (ASEC_UNMOUNT_RETRIES - 3))
+            action = 1; // SIGHUP
+        else
+            action = 0; // Just complain
+
+        KillProcessesWithOpenFiles(mountPoint, action);
+        usleep(1000 * 1000);
     }
 
     if (rc) {

logwrapper.c

@@ -119,6 +119,7 @@ int logwrap(int argc, const char* argv[], int background)
     if (grantpt(parent_ptty) || unlockpt(parent_ptty) ||
             ((child_devname = (char*)ptsname(parent_ptty)) == 0)) {
 	LOG(LOG_ERROR, "logwrapper", "Problem with /dev/ptmx");
+        close(parent_ptty);
 	return -1;
     }
 
@@ -127,6 +128,9 @@ int logwrap(int argc, const char* argv[], int background)
 	LOG(LOG_ERROR, "logwrapper", "Failed to fork");
         return -errno;
     } else if (pid == 0) {
+        /*
+         * Child
+         */
         child_ptty = open(child_devname, O_RDWR);
         if (child_ptty < 0) {
 	    LOG(LOG_ERROR, "logwrapper", "Problem with child ptty");
@@ -160,7 +164,12 @@ int logwrap(int argc, const char* argv[], int background)
 
         child(argc, argv);
     } else {
-        return parent(argv[0], parent_ptty);
+        /*
+         * Parent
+         */
+        int rc = parent(argv[0], parent_ptty);
+        close(parent_ptty);
+        return rc;
     }
 
     return 0;