vold: Fix devmapper/ptmx fd leak, and give asec unmount more time
Signed-off-by: San Mehat <san@google.com>
diff --git a/Devmapper.cpp b/Devmapper.cpp
index 9dd8ef3..800ee69 100644
--- a/Devmapper.cpp
+++ b/Devmapper.cpp
@@ -167,6 +167,7 @@
free(buffer);
+ close(fd);
return 0;
}
diff --git a/ProcessKiller.c b/ProcessKiller.c
index ad36731..7375ef3 100644
--- a/ProcessKiller.c
+++ b/ProcessKiller.c
@@ -179,8 +179,14 @@
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 @@
|| 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);
}
}
}
diff --git a/Volume.cpp b/Volume.cpp
index 6a81eff..af69bc2 100644
--- a/Volume.cpp
+++ b/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 @@
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 @@
}
LOGW("Volume %s unmount attempt %d failed (%s)",
- getLabel(), i + 1, strerror(errno));
+ getLabel(), i, strerror(errno));
- if (i < 5) {
- usleep(1000 * 250);
- } else {
- KillProcessesWithOpenFiles(getMountpoint(),
- (i < 7 ? 0 : 1),
- NULL, 0);
- usleep(1000 * 250);
- }
+ 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) {
diff --git a/VolumeManager.cpp b/VolumeManager.cpp
index ba92768..d778ce2 100644
--- a/VolumeManager.cpp
+++ b/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 @@
return -1;
}
+#define ASEC_UNMOUNT_RETRIES 10
int VolumeManager::unmountAsec(const char *id) {
char asecFileName[255];
char mountPoint[255];
@@ -335,7 +336,7 @@
}
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 @@
break;
}
LOGW("ASEC %s unmount attempt %d failed (%s)",
- id, i +1, strerror(errno));
+ id, i, strerror(errno));
- if (i >= 5) {
- KillProcessesWithOpenFiles(mountPoint, (i < 7 ? 0 : 1),
- NULL, 0);
- }
- usleep(1000 * 250);
+ 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) {
diff --git a/logwrapper.c b/logwrapper.c
index c96426f..afc6cdb 100644
--- a/logwrapper.c
+++ b/logwrapper.c
@@ -119,6 +119,7 @@
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 @@
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 @@
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;