Sanity check IMemory access versus underlying mmap am: 94b0d4e3ab am: ef6908e2b3
am: 97f49e50de
* commit '97f49e50ded651f1ddcf8a9981a7d97fe2816ad7':
Sanity check IMemory access versus underlying mmap
diff --git a/Parcel.cpp b/Parcel.cpp
index a24621b..87ce5d0 100644
--- a/Parcel.cpp
+++ b/Parcel.cpp
@@ -26,7 +26,6 @@
#include <binder/TextOutput.h>
#include <errno.h>
-#include <utils/CallStack.h>
#include <utils/Debug.h>
#include <utils/Log.h>
#include <utils/String8.h>
@@ -36,8 +35,8 @@
#include <cutils/ashmem.h>
#include <private/binder/binder_module.h>
+#include <private/binder/Static.h>
-#include <fcntl.h>
#include <inttypes.h>
#include <stdio.h>
#include <stdlib.h>
@@ -50,6 +49,8 @@
#define LOG_REFS(...)
//#define LOG_REFS(...) ALOG(LOG_DEBUG, "Parcel", __VA_ARGS__)
+#define LOG_ALLOC(...)
+//#define LOG_ALLOC(...) ALOG(LOG_DEBUG, "Parcel", __VA_ARGS__)
// ---------------------------------------------------------------------------
@@ -74,6 +75,10 @@
namespace android {
+static pthread_mutex_t gParcelGlobalAllocSizeLock = PTHREAD_MUTEX_INITIALIZER;
+static size_t gParcelGlobalAllocSize = 0;
+static size_t gParcelGlobalAllocCount = 0;
+
void acquire_object(const sp<ProcessState>& proc,
const flat_binder_object& obj, const void* who)
{
@@ -293,12 +298,28 @@
Parcel::Parcel()
{
+ LOG_ALLOC("Parcel %p: constructing", this);
initState();
}
Parcel::~Parcel()
{
freeDataNoInit();
+ LOG_ALLOC("Parcel %p: destroyed", this);
+}
+
+size_t Parcel::getGlobalAllocSize() {
+ pthread_mutex_lock(&gParcelGlobalAllocSizeLock);
+ size_t size = gParcelGlobalAllocSize;
+ pthread_mutex_unlock(&gParcelGlobalAllocSizeLock);
+ return size;
+}
+
+size_t Parcel::getGlobalAllocCount() {
+ pthread_mutex_lock(&gParcelGlobalAllocSizeLock);
+ size_t count = gParcelGlobalAllocCount;
+ pthread_mutex_unlock(&gParcelGlobalAllocSizeLock);
+ return count;
}
const uint8_t* Parcel::data() const
@@ -768,29 +789,6 @@
status_t Parcel::writeDupFileDescriptor(int fd)
{
int dupFd = dup(fd);
-
- { // Temporary extra debug validation for b/17477219: a Parcel recipient is
- // getting a positive but invalid fd unexpectedly. Trying to track down
- // where it's coming from.
- int dupErrno = dupFd < 0 ? errno : 0;
- int fdFlags = fcntl(fd, F_GETFD);
- int fdFlagsErrno = fdFlags == -1 ? errno : 0;
- int dupFlags = fcntl(dupFd, F_GETFD);
- int dupFlagsErrno = dupFlags == -1 ? errno : 0;
- if (dupFd < 0 || fdFlags == -1 || dupFlags == -1) {
- ALOGE("Parcel::writeDupFileDescriptor failed:\n"
- " fd=%d flags=%d err=%d(%s)\n"
- " dupFd=%d dupErr=%d(%s) flags=%d err=%d(%s)",
- fd, fdFlags, fdFlagsErrno, strerror(fdFlagsErrno),
- dupFd, dupErrno, strerror(dupErrno),
- dupFlags, dupFlagsErrno, strerror(dupFlagsErrno));
- if (fd < 0 || fdFlags == -1) {
- CallStack(LOG_TAG);
- }
- return -errno;
- }
- }
-
if (dupFd < 0) {
return -errno;
}
@@ -1309,23 +1307,11 @@
status_t err = NO_ERROR;
for (size_t i=0 ; i<fd_count && err==NO_ERROR ; i++) {
- int oldfd = this->readFileDescriptor();
- fds[i] = dup(oldfd);
+ fds[i] = dup(this->readFileDescriptor());
if (fds[i] < 0) {
- int dupErrno = errno;
err = BAD_VALUE;
- int flags = fcntl(oldfd, F_GETFD);
- int fcntlErrno = errno;
- const flat_binder_object* flat = readObject(true);
- ALOGE("dup failed in Parcel::read, fd %zu of %zu\n"
- " dup(%d) = %d [errno: %d (%s)]\n"
- " fcntl(%d, F_GETFD) = %d [errno: %d (%s)]\n"
- " flat %p type %d",
- i, fd_count,
- oldfd, fds[i], dupErrno, strerror(dupErrno),
- oldfd, flags, fcntlErrno, strerror(fcntlErrno),
- flat, flat ? flat->type : 0);
- CallStack(LOG_TAG);
+ ALOGE("dup() failed in Parcel::read, i is %zu, fds[i] is %d, fd_count is %zu, error: %s",
+ i, fds[i], fd_count, strerror(errno));
}
}
@@ -1529,11 +1515,20 @@
void Parcel::freeDataNoInit()
{
if (mOwner) {
+ LOG_ALLOC("Parcel %p: freeing other owner data", this);
//ALOGI("Freeing data ref of %p (pid=%d)", this, getpid());
mOwner(this, mData, mDataSize, mObjects, mObjectsSize, mOwnerCookie);
} else {
+ LOG_ALLOC("Parcel %p: freeing allocated data", this);
releaseObjects();
- if (mData) free(mData);
+ if (mData) {
+ LOG_ALLOC("Parcel %p: freeing with %zu capacity", this, mDataCapacity);
+ pthread_mutex_lock(&gParcelGlobalAllocSizeLock);
+ gParcelGlobalAllocSize -= mDataCapacity;
+ gParcelGlobalAllocCount--;
+ pthread_mutex_unlock(&gParcelGlobalAllocSizeLock);
+ free(mData);
+ }
if (mObjects) free(mObjects);
}
}
@@ -1562,6 +1557,11 @@
releaseObjects();
if (data) {
+ LOG_ALLOC("Parcel %p: restart from %zu to %zu capacity", this, mDataCapacity, desired);
+ pthread_mutex_lock(&gParcelGlobalAllocSizeLock);
+ gParcelGlobalAllocSize += desired;
+ gParcelGlobalAllocSize -= mDataCapacity;
+ pthread_mutex_unlock(&gParcelGlobalAllocSizeLock);
mData = data;
mDataCapacity = desired;
}
@@ -1641,6 +1641,12 @@
mOwner(this, mData, mDataSize, mObjects, mObjectsSize, mOwnerCookie);
mOwner = NULL;
+ LOG_ALLOC("Parcel %p: taking ownership of %zu capacity", this, desired);
+ pthread_mutex_lock(&gParcelGlobalAllocSizeLock);
+ gParcelGlobalAllocSize += desired;
+ gParcelGlobalAllocCount++;
+ pthread_mutex_unlock(&gParcelGlobalAllocSizeLock);
+
mData = data;
mObjects = objects;
mDataSize = (mDataSize < desired) ? mDataSize : desired;
@@ -1675,6 +1681,12 @@
if (desired > mDataCapacity) {
uint8_t* data = (uint8_t*)realloc(mData, desired);
if (data) {
+ LOG_ALLOC("Parcel %p: continue from %zu to %zu capacity", this, mDataCapacity,
+ desired);
+ pthread_mutex_lock(&gParcelGlobalAllocSizeLock);
+ gParcelGlobalAllocSize += desired;
+ gParcelGlobalAllocSize -= mDataCapacity;
+ pthread_mutex_unlock(&gParcelGlobalAllocSizeLock);
mData = data;
mDataCapacity = desired;
} else if (desired > mDataCapacity) {
@@ -1705,6 +1717,12 @@
ALOGE("continueWrite: %zu/%p/%zu/%zu", mDataCapacity, mObjects, mObjectsCapacity, desired);
}
+ LOG_ALLOC("Parcel %p: allocating with %zu capacity", this, desired);
+ pthread_mutex_lock(&gParcelGlobalAllocSizeLock);
+ gParcelGlobalAllocSize += desired;
+ gParcelGlobalAllocCount++;
+ pthread_mutex_unlock(&gParcelGlobalAllocSizeLock);
+
mData = data;
mDataSize = mDataPos = 0;
ALOGV("continueWrite Setting data size of %p to %zu", this, mDataSize);
@@ -1717,6 +1735,7 @@
void Parcel::initState()
{
+ LOG_ALLOC("Parcel %p: initState", this);
mError = NO_ERROR;
mData = 0;
mDataSize = 0;
diff --git a/Static.cpp b/Static.cpp
index 2062692..cd9509f 100644
--- a/Static.cpp
+++ b/Static.cpp
@@ -90,7 +90,7 @@
static LibBinderIPCtStatics gIPCStatics;
-// ------------ ServiceManager.cpp
+// ------------ IServiceManager.cpp
Mutex gDefaultServiceManagerLock;
sp<IServiceManager> gDefaultServiceManager;
diff --git a/TextOutput.cpp b/TextOutput.cpp
index db3e858..2ed5188 100644
--- a/TextOutput.cpp
+++ b/TextOutput.cpp
@@ -116,8 +116,8 @@
TextOutput& operator<<(TextOutput& to, const void* val)
{
- char buf[16];
- sprintf(buf, "%p", val);
+ char buf[32];
+ snprintf(buf, sizeof(buf), "%p", val);
to.print(buf, strlen(buf));
return to;
}
diff --git a/include/hwbinder/Parcel.h b/include/hwbinder/Parcel.h
index 2ee99f8..6a69761 100644
--- a/include/hwbinder/Parcel.h
+++ b/include/hwbinder/Parcel.h
@@ -205,7 +205,11 @@
// Explicitly close all file descriptors in the parcel.
void closeFileDescriptors();
-
+
+ // Debugging: get metrics on current allocations.
+ static size_t getGlobalAllocSize();
+ static size_t getGlobalAllocCount();
+
private:
typedef void (*release_func)(Parcel* parcel,
const uint8_t* data, size_t dataSize,