aaudio: plug some resource leaks
Close some file descriptors for shared memory that were not getting closed.
Bug: 37947848
Test: see bug for test procedure
Change-Id: Icdc13188b0a49d08b67babff0852c3a4eda8fd2b
Signed-off-by: Phil Burk <philburk@google.com>
diff --git a/media/libaaudio/examples/write_sine/src/write_sine.cpp b/media/libaaudio/examples/write_sine/src/write_sine.cpp
index 9107a7c..885dbbd 100644
--- a/media/libaaudio/examples/write_sine/src/write_sine.cpp
+++ b/media/libaaudio/examples/write_sine/src/write_sine.cpp
@@ -23,7 +23,7 @@
#include "SineGenerator.h"
#define SAMPLE_RATE 48000
-#define NUM_SECONDS 15
+#define NUM_SECONDS 5
#define NANOS_PER_MICROSECOND ((int64_t)1000)
#define NANOS_PER_MILLISECOND (NANOS_PER_MICROSECOND * 1000)
#define NANOS_PER_SECOND (NANOS_PER_MILLISECOND * 1000)
diff --git a/media/libaaudio/src/binding/SharedMemoryParcelable.cpp b/media/libaaudio/src/binding/SharedMemoryParcelable.cpp
index 7e77ca0..1094d9e 100644
--- a/media/libaaudio/src/binding/SharedMemoryParcelable.cpp
+++ b/media/libaaudio/src/binding/SharedMemoryParcelable.cpp
@@ -50,7 +50,8 @@
if (status != NO_ERROR) return status;
if (mSizeInBytes > 0) {
status = parcel->writeDupFileDescriptor(mFd);
- ALOGE_IF(status != NO_ERROR, "SharedMemoryParcelable writeDupFileDescriptor failed : %d", status);
+ ALOGE_IF(status != NO_ERROR, "SharedMemoryParcelable writeDupFileDescriptor failed : %d",
+ status);
}
return status;
}
@@ -61,11 +62,13 @@
return status;
}
if (mSizeInBytes > 0) {
- int originalFD = parcel->readFileDescriptor();
- mFd = fcntl(originalFD, F_DUPFD_CLOEXEC, 0);
+ mOriginalFd = parcel->readFileDescriptor();
+ ALOGV("SharedMemoryParcelable::readFromParcel() LEAK? mOriginalFd = %d\n", mOriginalFd);
+ mFd = fcntl(mOriginalFd, F_DUPFD_CLOEXEC, 0);
+ ALOGV("SharedMemoryParcelable::readFromParcel() LEAK? mFd = %d\n", mFd);
if (mFd == -1) {
status = -errno;
- ALOGE("SharedMemoryParcelable readFileDescriptor fcntl() failed : %d", status);
+ ALOGE("SharedMemoryParcelable readFromParcel fcntl() failed : %d", status);
}
}
return status;
@@ -81,9 +84,15 @@
mResolvedAddress = MMAP_UNRESOLVED_ADDRESS;
}
if (mFd != -1) {
+ ALOGV("SharedMemoryParcelable::close() LEAK? mFd = %d\n", mFd);
::close(mFd);
mFd = -1;
}
+ if (mOriginalFd != -1) {
+ ALOGV("SharedMemoryParcelable::close() LEAK? mOriginalFd = %d\n", mOriginalFd);
+ ::close(mOriginalFd);
+ mOriginalFd = -1;
+ }
return AAUDIO_OK;
}
diff --git a/media/libaaudio/src/binding/SharedMemoryParcelable.h b/media/libaaudio/src/binding/SharedMemoryParcelable.h
index c4feb48..64fb473 100644
--- a/media/libaaudio/src/binding/SharedMemoryParcelable.h
+++ b/media/libaaudio/src/binding/SharedMemoryParcelable.h
@@ -67,8 +67,9 @@
#define MMAP_UNRESOLVED_ADDRESS reinterpret_cast<uint8_t*>(MAP_FAILED)
- int mFd = -1;
- int32_t mSizeInBytes = 0;
+ int mFd = -1;
+ int mOriginalFd = -1;
+ int32_t mSizeInBytes = 0;
uint8_t *mResolvedAddress = MMAP_UNRESOLVED_ADDRESS;
};
diff --git a/services/oboeservice/AAudioServiceStreamBase.cpp b/services/oboeservice/AAudioServiceStreamBase.cpp
index d8882c9..8248f8b 100644
--- a/services/oboeservice/AAudioServiceStreamBase.cpp
+++ b/services/oboeservice/AAudioServiceStreamBase.cpp
@@ -59,6 +59,7 @@
std::lock_guard<std::mutex> lock(mLockUpMessageQueue);
delete mUpMessageQueue;
mUpMessageQueue = nullptr;
+
return AAUDIO_OK;
}
diff --git a/services/oboeservice/AAudioServiceStreamBase.h b/services/oboeservice/AAudioServiceStreamBase.h
index d6b6ee3..9318c2e 100644
--- a/services/oboeservice/AAudioServiceStreamBase.h
+++ b/services/oboeservice/AAudioServiceStreamBase.h
@@ -133,9 +133,6 @@
// This is used by one thread to tell another thread to exit. So it must be atomic.
std::atomic<bool> mThreadEnabled;
-
- int mAudioDataFileDescriptor = -1;
-
aaudio_audio_format_t mAudioFormat = AAUDIO_FORMAT_UNSPECIFIED;
int32_t mFramesPerBurst = 0;
int32_t mSamplesPerFrame = AAUDIO_UNSPECIFIED;
diff --git a/services/oboeservice/AAudioServiceStreamMMAP.cpp b/services/oboeservice/AAudioServiceStreamMMAP.cpp
index 5e4a9b1..cadc2a4 100644
--- a/services/oboeservice/AAudioServiceStreamMMAP.cpp
+++ b/services/oboeservice/AAudioServiceStreamMMAP.cpp
@@ -60,6 +60,13 @@
// FIXME Make closing synchronous.
AudioClock::sleepForNanos(100 * AAUDIO_NANOS_PER_MILLISECOND);
+ if (mAudioDataFileDescriptor != -1) {
+ ALOGV("AAudioServiceStreamMMAP: LEAK? close(mAudioDataFileDescriptor = %d)\n",
+ mAudioDataFileDescriptor);
+ ::close(mAudioDataFileDescriptor);
+ mAudioDataFileDescriptor = -1;
+ }
+
return AAudioServiceStreamBase::close();
}
@@ -164,6 +171,8 @@
: audio_channel_count_from_in_mask(config.channel_mask);
mAudioDataFileDescriptor = mMmapBufferinfo.shared_memory_fd;
+ ALOGV("AAudioServiceStreamMMAP::open LEAK? mAudioDataFileDescriptor = %d\n",
+ mAudioDataFileDescriptor);
mFramesPerBurst = mMmapBufferinfo.burst_size_frames;
mCapacityInFrames = mMmapBufferinfo.buffer_size_frames;
mAudioFormat = AAudioConvert_androidToAAudioDataFormat(config.format);
diff --git a/services/oboeservice/AAudioServiceStreamMMAP.h b/services/oboeservice/AAudioServiceStreamMMAP.h
index a8e63a6..fe75a10 100644
--- a/services/oboeservice/AAudioServiceStreamMMAP.h
+++ b/services/oboeservice/AAudioServiceStreamMMAP.h
@@ -127,6 +127,7 @@
MonotonicCounter mFramesWritten;
MonotonicCounter mFramesRead;
int32_t mPreviousFrameCounter = 0; // from HAL
+ int mAudioDataFileDescriptor = -1;
// Interface to the AudioFlinger MMAP support.
android::sp<android::MmapStreamInterface> mMmapStream;
diff --git a/services/oboeservice/AAudioServiceStreamShared.cpp b/services/oboeservice/AAudioServiceStreamShared.cpp
index b5d9927..713d1f8 100644
--- a/services/oboeservice/AAudioServiceStreamShared.cpp
+++ b/services/oboeservice/AAudioServiceStreamShared.cpp
@@ -200,6 +200,10 @@
mEndpointManager.closeEndpoint(endpoint);
mServiceEndpoint = nullptr;
}
+ if (mAudioDataQueue != nullptr) {
+ delete mAudioDataQueue;
+ mAudioDataQueue = nullptr;
+ }
return AAudioServiceStreamBase::close();
}
diff --git a/services/oboeservice/SharedRingBuffer.cpp b/services/oboeservice/SharedRingBuffer.cpp
index 03c160d..6b3fb4c 100644
--- a/services/oboeservice/SharedRingBuffer.cpp
+++ b/services/oboeservice/SharedRingBuffer.cpp
@@ -33,9 +33,13 @@
if (mSharedMemory != nullptr) {
delete mFifoBuffer;
munmap(mSharedMemory, mSharedMemorySizeInBytes);
- close(mFileDescriptor);
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,
@@ -46,10 +50,12 @@
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) {
ALOGE("SharedRingBuffer::allocate() ashmem_create_region() failed %d", errno);
return AAUDIO_ERROR_INTERNAL;
}
+
int err = ashmem_set_prot_region(mFileDescriptor, PROT_READ|PROT_WRITE); // TODO error handling?
if (err < 0) {
ALOGE("SharedRingBuffer::allocate() ashmem_set_prot_region() failed %d", errno);
@@ -75,9 +81,9 @@
(fifo_counter_t *) &mSharedMemory[SHARED_RINGBUFFER_WRITE_OFFSET];
uint8_t *dataAddress = &mSharedMemory[SHARED_RINGBUFFER_DATA_OFFSET];
- mFifoBuffer = new(std::nothrow) FifoBuffer(bytesPerFrame, capacityInFrames,
+ mFifoBuffer = new FifoBuffer(bytesPerFrame, capacityInFrames,
readCounterAddress, writeCounterAddress, dataAddress);
- return (mFifoBuffer == nullptr) ? AAUDIO_ERROR_NO_MEMORY : AAUDIO_OK;
+ return AAUDIO_OK;
}
void SharedRingBuffer::fillParcelable(AudioEndpointParcelable &endpointParcelable,