Properly close fd backing a MemoryIntArray

Use ParcelFileDescriptor only as an IPC transport
to make sure MemoryIntArray manges its backing fd.

Bug:30310689

Change-Id: Ib3cc13ef4ae2a744e5f7a96099570e0431847bce
(cherry picked from commit fe2462f3a6)
This commit is contained in:
Svetoslav Ganov
2016-08-29 11:14:05 -07:00
parent 689c92db9d
commit e257d6e167
2 changed files with 24 additions and 34 deletions

View File

@@ -54,7 +54,7 @@ public final class MemoryIntArray implements Parcelable, Closeable {
private final int mOwnerPid; private final int mOwnerPid;
private final boolean mClientWritable; private final boolean mClientWritable;
private final long mMemoryAddr; private final long mMemoryAddr;
private ParcelFileDescriptor mFd; private int mFd;
/** /**
* Creates a new instance. * Creates a new instance.
@@ -71,22 +71,23 @@ public final class MemoryIntArray implements Parcelable, Closeable {
mOwnerPid = Process.myPid(); mOwnerPid = Process.myPid();
mClientWritable = clientWritable; mClientWritable = clientWritable;
final String name = UUID.randomUUID().toString(); final String name = UUID.randomUUID().toString();
mFd = ParcelFileDescriptor.fromFd(nativeCreate(name, size)); mFd = nativeCreate(name, size);
mMemoryAddr = nativeOpen(mFd.getFd(), true, clientWritable); mMemoryAddr = nativeOpen(mFd, true, clientWritable);
} }
private MemoryIntArray(Parcel parcel) throws IOException { private MemoryIntArray(Parcel parcel) throws IOException {
mOwnerPid = parcel.readInt(); mOwnerPid = parcel.readInt();
mClientWritable = (parcel.readInt() == 1); mClientWritable = (parcel.readInt() == 1);
mFd = parcel.readParcelable(null); ParcelFileDescriptor pfd = parcel.readParcelable(null);
if (mFd == null) { if (pfd == null) {
throw new IOException("No backing file descriptor"); throw new IOException("No backing file descriptor");
} }
mFd = pfd.detachFd();
final long memoryAddress = parcel.readLong(); final long memoryAddress = parcel.readLong();
if (isOwner()) { if (isOwner()) {
mMemoryAddr = memoryAddress; mMemoryAddr = memoryAddress;
} else { } else {
mMemoryAddr = nativeOpen(mFd.getFd(), false, mClientWritable); mMemoryAddr = nativeOpen(mFd, false, mClientWritable);
} }
} }
@@ -108,7 +109,7 @@ public final class MemoryIntArray implements Parcelable, Closeable {
public int get(int index) throws IOException { public int get(int index) throws IOException {
enforceNotClosed(); enforceNotClosed();
enforceValidIndex(index); enforceValidIndex(index);
return nativeGet(mFd.getFd(), mMemoryAddr, index, isOwner()); return nativeGet(mFd, mMemoryAddr, index, isOwner());
} }
/** /**
@@ -124,7 +125,7 @@ public final class MemoryIntArray implements Parcelable, Closeable {
enforceNotClosed(); enforceNotClosed();
enforceWritable(); enforceWritable();
enforceValidIndex(index); enforceValidIndex(index);
nativeSet(mFd.getFd(), mMemoryAddr, index, value, isOwner()); nativeSet(mFd, mMemoryAddr, index, value, isOwner());
} }
/** /**
@@ -134,7 +135,7 @@ public final class MemoryIntArray implements Parcelable, Closeable {
*/ */
public int size() throws IOException { public int size() throws IOException {
enforceNotClosed(); enforceNotClosed();
return nativeSize(mFd.getFd()); return nativeSize(mFd);
} }
/** /**
@@ -145,9 +146,8 @@ public final class MemoryIntArray implements Parcelable, Closeable {
@Override @Override
public void close() throws IOException { public void close() throws IOException {
if (!isClosed()) { if (!isClosed()) {
ParcelFileDescriptor pfd = mFd; nativeClose(mFd, mMemoryAddr, isOwner());
mFd = null; mFd = -1;
nativeClose(pfd.getFd(), mMemoryAddr, isOwner());
} }
} }
@@ -155,7 +155,7 @@ public final class MemoryIntArray implements Parcelable, Closeable {
* @return Whether this array is closed and shouldn't be used. * @return Whether this array is closed and shouldn't be used.
*/ */
public boolean isClosed() { public boolean isClosed() {
return mFd == null; return mFd == -1;
} }
@Override @Override
@@ -171,10 +171,15 @@ public final class MemoryIntArray implements Parcelable, Closeable {
@Override @Override
public void writeToParcel(Parcel parcel, int flags) { public void writeToParcel(Parcel parcel, int flags) {
ParcelFileDescriptor pfd = ParcelFileDescriptor.adoptFd(mFd);
try {
parcel.writeInt(mOwnerPid); parcel.writeInt(mOwnerPid);
parcel.writeInt(mClientWritable ? 1 : 0); parcel.writeInt(mClientWritable ? 1 : 0);
parcel.writeParcelable(mFd, 0); parcel.writeParcelable(pfd, flags & ~Parcelable.PARCELABLE_WRITE_RETURN_VALUE);
parcel.writeLong(mMemoryAddr); parcel.writeLong(mMemoryAddr);
} finally {
pfd.detachFd();
}
} }
@Override @Override
@@ -189,19 +194,12 @@ public final class MemoryIntArray implements Parcelable, Closeable {
return false; return false;
} }
MemoryIntArray other = (MemoryIntArray) obj; MemoryIntArray other = (MemoryIntArray) obj;
if (mFd == null) { return mFd == other.mFd;
if (other.mFd != null) {
return false;
}
} else if (mFd.getFd() != other.mFd.getFd()) {
return false;
}
return true;
} }
@Override @Override
public int hashCode() { public int hashCode() {
return mFd != null ? mFd.hashCode() : 1; return mFd;
} }
private boolean isOwner() { private boolean isOwner() {

View File

@@ -160,16 +160,8 @@ static jint android_util_MemoryIntArray_size(JNIEnv* env, jobject clazz, jint fd
return -1; return -1;
} }
// Use ASHMEM_GET_SIZE to find out if the fd refers to an ashmem region.
// ASHMEM_GET_SIZE should succeed for all ashmem regions, and the kernel
// should return ENOTTY for all other valid file descriptors
int ashmemSize = ashmem_get_size_region(fd); int ashmemSize = ashmem_get_size_region(fd);
if (ashmemSize < 0) { if (ashmemSize < 0) {
if (errno == ENOTTY) {
// ENOTTY means that the ioctl does not apply to this object,
// i.e., it is not an ashmem region.
return -1;
}
// Some other error, throw exception // Some other error, throw exception
jniThrowIOException(env, errno); jniThrowIOException(env, errno);
return -1; return -1;