commit e72481c5c16d401c776e0ba49b84b80223769b91
author Phil Burk <philburk@google.com> Tue Aug 08 13:20:45 2017 -0700
committer Phil Burk <philburk@google.com> Wed Aug 09 14:11:57 2017 -0700
tree 282c04aa82bba9f854b3125fa8edade0b41f5287
parent e88fd530455d39e4c770870c74c1198820f62bfe

aaudio: fix ownership problems with file descriptors

Use unique_fd to track file descriptors.
Fix extra close of file descriptor in SharedMemoryParcelable.cpp.
This bug was causing unrelated file descriptors to be closed!

Bug: 64311216
Test: write_sine.cpp, FD should survive aaudio close
Change-Id: I4f38c83510a49ea22b79b96d970ee48153417249

services/oboeservice/SharedRingBuffer.cpp

diff --git a/services/oboeservice/SharedRingBuffer.cpp b/services/oboeservice/SharedRingBuffer.cpp
index 6b3fb4c..83b25b3 100644
--- a/services/oboeservice/SharedRingBuffer.cpp
+++ b/services/oboeservice/SharedRingBuffer.cpp
@@ -35,11 +35,6 @@
         munmap(mSharedMemory, mSharedMemorySizeInBytes);
         mSharedMemory = nullptr;
     }
-    if (mFileDescriptor != -1) {
-        ALOGV("SharedRingBuffer: LEAK? close(mFileDescriptor = %d)\n", mFileDescriptor);
-        close(mFileDescriptor);
-        mFileDescriptor = -1;
-    }
 }
 
 aaudio_result_t SharedRingBuffer::allocate(fifo_frames_t   bytesPerFrame,
@@ -49,17 +44,17 @@
     // Create shared memory large enough to hold the data and the read and write counters.
     mDataMemorySizeInBytes = bytesPerFrame * capacityInFrames;
     mSharedMemorySizeInBytes = mDataMemorySizeInBytes + (2 * (sizeof(fifo_counter_t)));
-    mFileDescriptor = ashmem_create_region("AAudioSharedRingBuffer", mSharedMemorySizeInBytes);
-    ALOGV("SharedRingBuffer::allocate() LEAK? mFileDescriptor = %d\n", mFileDescriptor);
-    if (mFileDescriptor < 0) {
+    mFileDescriptor.reset(ashmem_create_region("AAudioSharedRingBuffer", mSharedMemorySizeInBytes));
+    if (mFileDescriptor.get() == -1) {
         ALOGE("SharedRingBuffer::allocate() ashmem_create_region() failed %d", errno);
         return AAUDIO_ERROR_INTERNAL;
     }
+    ALOGV("SharedRingBuffer::allocate() mFileDescriptor = %d\n", mFileDescriptor.get());
 
-    int err = ashmem_set_prot_region(mFileDescriptor, PROT_READ|PROT_WRITE); // TODO error handling?
+    int err = ashmem_set_prot_region(mFileDescriptor.get(), PROT_READ|PROT_WRITE); // TODO error handling?
     if (err < 0) {
         ALOGE("SharedRingBuffer::allocate() ashmem_set_prot_region() failed %d", errno);
-        close(mFileDescriptor);
+        mFileDescriptor.reset();
         return AAUDIO_ERROR_INTERNAL; // TODO convert errno to a better AAUDIO_ERROR;
     }
 
@@ -67,10 +62,10 @@
     mSharedMemory = (uint8_t *) mmap(0, mSharedMemorySizeInBytes,
                          PROT_READ|PROT_WRITE,
                          MAP_SHARED,
-                         mFileDescriptor, 0);
+                         mFileDescriptor.get(), 0);
     if (mSharedMemory == MAP_FAILED) {
         ALOGE("SharedRingBuffer::allocate() mmap() failed %d", errno);
-        close(mFileDescriptor);
+        mFileDescriptor.reset();
         return AAUDIO_ERROR_INTERNAL; // TODO convert errno to a better AAUDIO_ERROR;
     }