Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit c9f2e060 authored by Phil Burk's avatar Phil Burk Committed by Android (Google) Code Review
Browse files

Merge "aaudio: fix ownership problems with file descriptors" into oc-mr1-dev

parents 6b8bbef1 e72481c5
Loading
Loading
Loading
Loading
+21 −3
Original line number Original line Diff line number Diff line
@@ -16,11 +16,14 @@


// Play sine waves using AAudio.
// Play sine waves using AAudio.


#include <stdio.h>
#include <stdlib.h>
#include <math.h>
#include <aaudio/AAudio.h>
#include <aaudio/AAudio.h>
#include <aaudio/AAudioTesting.h>
#include <aaudio/AAudioTesting.h>
#include <asm/fcntl.h>
#include <fcntl.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>

#include "AAudioExampleUtils.h"
#include "AAudioExampleUtils.h"
#include "AAudioSimplePlayer.h"
#include "AAudioSimplePlayer.h"
#include "AAudioArgsParser.h"
#include "AAudioArgsParser.h"
@@ -48,6 +51,8 @@ int main(int argc, const char **argv)
    float   *floatData = nullptr;
    float   *floatData = nullptr;
    int16_t *shortData = nullptr;
    int16_t *shortData = nullptr;


    int      testFd = -1;

    // Make printf print immediately so that debug info is not stuck
    // Make printf print immediately so that debug info is not stuck
    // in a buffer if we hang or crash.
    // in a buffer if we hang or crash.
    setvbuf(stdout, nullptr, _IONBF, (size_t) 0);
    setvbuf(stdout, nullptr, _IONBF, (size_t) 0);
@@ -95,6 +100,9 @@ int main(int argc, const char **argv)
        goto finish;
        goto finish;
    }
    }


    testFd = open("/data/aaudio_temp.raw", O_CREAT | O_RDWR, S_IRWXU);
    printf("testFd = %d, pid = %d\n", testFd, getpid());

    // Start the stream.
    // Start the stream.
    printf("call player.start()\n");
    printf("call player.start()\n");
    result = player.start();
    result = player.start();
@@ -176,7 +184,17 @@ int main(int argc, const char **argv)
    }
    }


finish:
finish:
    printf("testFd = %d, fcntl before aaudio close returns 0x%08X\n",
           testFd, fcntl(testFd, F_GETFD));
    player.close();
    player.close();
    printf("testFd = %d, fcntl after aaudio close returns 0x%08X\n",
           testFd, fcntl(testFd, F_GETFD));
    if (::close(testFd) != 0) {
        printf("ERROR SharedMemoryParcelable::close() of testFd = %d, errno = %s\n",
               testFd, strerror(errno));
    }
    printf("testFd = %d, fcntl after close() returns 0x%08X\n", testFd, fcntl(testFd, F_GETFD));

    delete[] floatData;
    delete[] floatData;
    delete[] shortData;
    delete[] shortData;
    printf("exiting - AAudio result = %d = %s\n", result, AAudio_convertResultToText(result));
    printf("exiting - AAudio result = %d = %s\n", result, AAudio_convertResultToText(result));
+3 −1
Original line number Original line Diff line number Diff line
@@ -28,6 +28,7 @@
#include "binding/RingBufferParcelable.h"
#include "binding/RingBufferParcelable.h"
#include "binding/AudioEndpointParcelable.h"
#include "binding/AudioEndpointParcelable.h"


using android::base::unique_fd;
using android::NO_ERROR;
using android::NO_ERROR;
using android::status_t;
using android::status_t;
using android::Parcel;
using android::Parcel;
@@ -49,7 +50,8 @@ AudioEndpointParcelable::~AudioEndpointParcelable() {}
 * Add the file descriptor to the table.
 * Add the file descriptor to the table.
 * @return index in table or negative error
 * @return index in table or negative error
 */
 */
int32_t AudioEndpointParcelable::addFileDescriptor(int fd, int32_t sizeInBytes) {
int32_t AudioEndpointParcelable::addFileDescriptor(const unique_fd& fd,
                                                   int32_t sizeInBytes) {
    if (mNumSharedMemories >= MAX_SHARED_MEMORIES) {
    if (mNumSharedMemories >= MAX_SHARED_MEMORIES) {
        return AAUDIO_ERROR_OUT_OF_RANGE;
        return AAUDIO_ERROR_OUT_OF_RANGE;
    }
    }
+2 −1
Original line number Original line Diff line number Diff line
@@ -20,6 +20,7 @@
#include <stdint.h>
#include <stdint.h>


//#include <sys/mman.h>
//#include <sys/mman.h>
#include <android-base/unique_fd.h>
#include <binder/Parcel.h>
#include <binder/Parcel.h>
#include <binder/Parcelable.h>
#include <binder/Parcelable.h>


@@ -47,7 +48,7 @@ public:
     * Add the file descriptor to the table.
     * Add the file descriptor to the table.
     * @return index in table or negative error
     * @return index in table or negative error
     */
     */
    int32_t addFileDescriptor(int fd, int32_t sizeInBytes);
    int32_t addFileDescriptor(const android::base::unique_fd& fd, int32_t sizeInBytes);


    virtual status_t writeToParcel(Parcel* parcel) const override;
    virtual status_t writeToParcel(Parcel* parcel) const override;


+46 −41
Original line number Original line Diff line number Diff line
@@ -24,11 +24,13 @@
#include <sys/mman.h>
#include <sys/mman.h>
#include <aaudio/AAudio.h>
#include <aaudio/AAudio.h>


#include <android-base/unique_fd.h>
#include <binder/Parcelable.h>
#include <binder/Parcelable.h>
#include <utility/AAudioUtilities.h>
#include <utility/AAudioUtilities.h>


#include "binding/SharedMemoryParcelable.h"
#include "binding/SharedMemoryParcelable.h"


using android::base::unique_fd;
using android::NO_ERROR;
using android::NO_ERROR;
using android::status_t;
using android::status_t;
using android::Parcel;
using android::Parcel;
@@ -39,17 +41,19 @@ using namespace aaudio;
SharedMemoryParcelable::SharedMemoryParcelable() {}
SharedMemoryParcelable::SharedMemoryParcelable() {}
SharedMemoryParcelable::~SharedMemoryParcelable() {};
SharedMemoryParcelable::~SharedMemoryParcelable() {};


void SharedMemoryParcelable::setup(int fd, int32_t sizeInBytes) {
void SharedMemoryParcelable::setup(const unique_fd& fd, int32_t sizeInBytes) {
    mFd = fd;
    mFd.reset(dup(fd.get())); // store a duplicate fd
    ALOGV("SharedMemoryParcelable::setup(%d -> %d, %d) this = %p\n",
          fd.get(), mFd.get(), sizeInBytes, this);
    mSizeInBytes = sizeInBytes;
    mSizeInBytes = sizeInBytes;

}
}


status_t SharedMemoryParcelable::writeToParcel(Parcel* parcel) const {
status_t SharedMemoryParcelable::writeToParcel(Parcel* parcel) const {
    status_t status = parcel->writeInt32(mSizeInBytes);
    status_t status = parcel->writeInt32(mSizeInBytes);
    if (status != NO_ERROR) return status;
    if (status != NO_ERROR) return status;
    if (mSizeInBytes > 0) {
    if (mSizeInBytes > 0) {
        status = parcel->writeDupFileDescriptor(mFd);
        ALOGV("SharedMemoryParcelable::writeToParcel() mFd = %d, this = %p\n", mFd.get(), this);
        status = parcel->writeUniqueFileDescriptor(mFd);
        ALOGE_IF(status != NO_ERROR, "SharedMemoryParcelable writeDupFileDescriptor failed : %d",
        ALOGE_IF(status != NO_ERROR, "SharedMemoryParcelable writeDupFileDescriptor failed : %d",
                 status);
                 status);
    }
    }
@@ -62,15 +66,16 @@ status_t SharedMemoryParcelable::readFromParcel(const Parcel* parcel) {
        return status;
        return status;
    }
    }
    if (mSizeInBytes > 0) {
    if (mSizeInBytes > 0) {
        // Keep the original FD until you are done with the mFd.
        // The Parcel owns the file descriptor and will close it later.
        // If you close it in here then it will prevent mFd from working.
        unique_fd mmapFd;
        int originalFd = parcel->readFileDescriptor();
        status = parcel->readUniqueFileDescriptor(&mmapFd);
        ALOGV("SharedMemoryParcelable::readFromParcel() LEAK? originalFd = %d\n", originalFd);
        if (status != NO_ERROR) {
        mFd = fcntl(originalFd, F_DUPFD_CLOEXEC, 0);
            ALOGE("SharedMemoryParcelable::readFromParcel() readUniqueFileDescriptor() failed : %d",
        ALOGV("SharedMemoryParcelable::readFromParcel() LEAK? mFd = %d\n", mFd);
                  status);
        if (mFd == -1) {
        } else {
            status = -errno;
            // Resolve the memory now while we still have the FD from the Parcel.
            ALOGE("SharedMemoryParcelable readFromParcel fcntl() failed : %d", status);
            // Closing the FD will not affect the shared memory once mmap() has been called.
            status = AAudioConvert_androidToAAudioResult(resolveSharedMemory(mmapFd));
        }
        }
    }
    }
    return status;
    return status;
@@ -85,21 +90,22 @@ aaudio_result_t SharedMemoryParcelable::close() {
        }
        }
        mResolvedAddress = MMAP_UNRESOLVED_ADDRESS;
        mResolvedAddress = MMAP_UNRESOLVED_ADDRESS;
    }
    }
    if (mFd != -1) {
    return AAUDIO_OK;
        ALOGV("SharedMemoryParcelable::close() LEAK? mFd = %d\n", mFd);
        if(::close(mFd) < 0) {
            int err = errno;
            ALOGE("SharedMemoryParcelable close failed for fd = %d, errno = %d (%s)",
                  mFd, err, strerror(err));
}
}
        mFd = -1;

aaudio_result_t SharedMemoryParcelable::resolveSharedMemory(const unique_fd& fd) {
    mResolvedAddress = (uint8_t *) mmap(0, mSizeInBytes, PROT_READ | PROT_WRITE,
                                        MAP_SHARED, fd.get(), 0);
    if (mResolvedAddress == MMAP_UNRESOLVED_ADDRESS) {
        ALOGE("SharedMemoryParcelable mmap() failed for fd = %d, errno = %s",
              fd.get(), strerror(errno));
        return AAUDIO_ERROR_INTERNAL;
    }
    }
    return AAUDIO_OK;
    return AAUDIO_OK;
}
}


aaudio_result_t SharedMemoryParcelable::resolve(int32_t offsetInBytes, int32_t sizeInBytes,
aaudio_result_t SharedMemoryParcelable::resolve(int32_t offsetInBytes, int32_t sizeInBytes,
                                              void **regionAddressPtr) {
                                              void **regionAddressPtr) {

    if (offsetInBytes < 0) {
    if (offsetInBytes < 0) {
        ALOGE("SharedMemoryParcelable illegal offsetInBytes = %d", offsetInBytes);
        ALOGE("SharedMemoryParcelable illegal offsetInBytes = %d", offsetInBytes);
        return AAUDIO_ERROR_OUT_OF_RANGE;
        return AAUDIO_ERROR_OUT_OF_RANGE;
@@ -109,20 +115,25 @@ aaudio_result_t SharedMemoryParcelable::resolve(int32_t offsetInBytes, int32_t s
              offsetInBytes, sizeInBytes, mSizeInBytes);
              offsetInBytes, sizeInBytes, mSizeInBytes);
        return AAUDIO_ERROR_OUT_OF_RANGE;
        return AAUDIO_ERROR_OUT_OF_RANGE;
    }
    }

    aaudio_result_t result = AAUDIO_OK;

    if (mResolvedAddress == MMAP_UNRESOLVED_ADDRESS) {
    if (mResolvedAddress == MMAP_UNRESOLVED_ADDRESS) {
        mResolvedAddress = (uint8_t *) mmap(0, mSizeInBytes, PROT_READ|PROT_WRITE,
        if (mFd.get() != -1) {
                                          MAP_SHARED, mFd, 0);
            result = resolveSharedMemory(mFd);
        if (mResolvedAddress == MMAP_UNRESOLVED_ADDRESS) {
        } else {
            ALOGE("SharedMemoryParcelable mmap failed for fd = %d, errno = %s",
            ALOGE("SharedMemoryParcelable has no file descriptor for shared memory.");
                  mFd, strerror(errno));
            result = AAUDIO_ERROR_INTERNAL;
            return AAUDIO_ERROR_INTERNAL;
        }
        }
    }
    }

    if (result == AAUDIO_OK && mResolvedAddress != MMAP_UNRESOLVED_ADDRESS) {
        *regionAddressPtr = mResolvedAddress + offsetInBytes;
        *regionAddressPtr = mResolvedAddress + offsetInBytes;
        ALOGV("SharedMemoryParcelable mResolvedAddress = %p", mResolvedAddress);
        ALOGV("SharedMemoryParcelable mResolvedAddress = %p", mResolvedAddress);
        ALOGV("SharedMemoryParcelable offset by %d, *regionAddressPtr = %p",
        ALOGV("SharedMemoryParcelable offset by %d, *regionAddressPtr = %p",
              offsetInBytes, *regionAddressPtr);
              offsetInBytes, *regionAddressPtr);
    return AAUDIO_OK;
    }
    return result;
}
}


int32_t SharedMemoryParcelable::getSizeInBytes() {
int32_t SharedMemoryParcelable::getSizeInBytes() {
@@ -134,17 +145,11 @@ aaudio_result_t SharedMemoryParcelable::validate() {
        ALOGE("SharedMemoryParcelable invalid mSizeInBytes = %d", mSizeInBytes);
        ALOGE("SharedMemoryParcelable invalid mSizeInBytes = %d", mSizeInBytes);
        return AAUDIO_ERROR_OUT_OF_RANGE;
        return AAUDIO_ERROR_OUT_OF_RANGE;
    }
    }
    if (mSizeInBytes > 0) {
        if (mFd == -1) {
            ALOGE("SharedMemoryParcelable uninitialized mFd = %d", mFd);
            return AAUDIO_ERROR_INTERNAL;
        }
    }
    return AAUDIO_OK;
    return AAUDIO_OK;
}
}


void SharedMemoryParcelable::dump() {
void SharedMemoryParcelable::dump() {
    ALOGD("SharedMemoryParcelable mFd = %d", mFd);
    ALOGD("SharedMemoryParcelable mFd = %d", mFd.get());
    ALOGD("SharedMemoryParcelable mSizeInBytes = %d", mSizeInBytes);
    ALOGD("SharedMemoryParcelable mSizeInBytes = %d", mSizeInBytes);
    ALOGD("SharedMemoryParcelable mResolvedAddress = %p", mResolvedAddress);
    ALOGD("SharedMemoryParcelable mResolvedAddress = %p", mResolvedAddress);
}
}
+18 −14
Original line number Original line Diff line number Diff line
@@ -18,15 +18,12 @@
#define ANDROID_AAUDIO_SHARED_MEMORY_PARCELABLE_H
#define ANDROID_AAUDIO_SHARED_MEMORY_PARCELABLE_H


#include <stdint.h>
#include <stdint.h>

#include <sys/mman.h>
#include <sys/mman.h>

#include <android-base/unique_fd.h>
#include <binder/Parcel.h>
#include <binder/Parcel.h>
#include <binder/Parcelable.h>
#include <binder/Parcelable.h>


using android::status_t;
using android::Parcel;
using android::Parcelable;

namespace aaudio {
namespace aaudio {


// Arbitrary limits for sanity checks. TODO remove after debugging.
// Arbitrary limits for sanity checks. TODO remove after debugging.
@@ -37,17 +34,24 @@ namespace aaudio {
/**
/**
 * This is a parcelable description of a shared memory referenced by a file descriptor.
 * This is a parcelable description of a shared memory referenced by a file descriptor.
 * It may be divided into several regions.
 * It may be divided into several regions.
 * The memory can be shared using Binder or simply shared between threads.
 */
 */
class SharedMemoryParcelable : public Parcelable {
class SharedMemoryParcelable : public android::Parcelable {
public:
public:
    SharedMemoryParcelable();
    SharedMemoryParcelable();
    virtual ~SharedMemoryParcelable();
    virtual ~SharedMemoryParcelable();


    void setup(int fd, int32_t sizeInBytes);
    /**
     * Make a dup() of the fd and store it for later use.
     *
     * @param fd
     * @param sizeInBytes
     */
    void setup(const android::base::unique_fd& fd, int32_t sizeInBytes);


    virtual status_t writeToParcel(Parcel* parcel) const override;
    virtual android::status_t writeToParcel(android::Parcel* parcel) const override;


    virtual status_t readFromParcel(const Parcel* parcel) override;
    virtual android::status_t readFromParcel(const android::Parcel* parcel) override;


    // mmap() shared memory
    // mmap() shared memory
    aaudio_result_t resolve(int32_t offsetInBytes, int32_t sizeInBytes, void **regionAddressPtr);
    aaudio_result_t resolve(int32_t offsetInBytes, int32_t sizeInBytes, void **regionAddressPtr);
@@ -55,8 +59,6 @@ public:
    // munmap() any mapped memory
    // munmap() any mapped memory
    aaudio_result_t close();
    aaudio_result_t close();


    bool isFileDescriptorSafe();

    int32_t getSizeInBytes();
    int32_t getSizeInBytes();


    aaudio_result_t validate();
    aaudio_result_t validate();
@@ -67,7 +69,9 @@ protected:


#define MMAP_UNRESOLVED_ADDRESS    reinterpret_cast<uint8_t*>(MAP_FAILED)
#define MMAP_UNRESOLVED_ADDRESS    reinterpret_cast<uint8_t*>(MAP_FAILED)


    int      mFd = -1;
    aaudio_result_t resolveSharedMemory(const android::base::unique_fd& fd);

    android::base::unique_fd   mFd;
    int32_t     mSizeInBytes = 0;
    int32_t     mSizeInBytes = 0;
    uint8_t    *mResolvedAddress = MMAP_UNRESOLVED_ADDRESS;
    uint8_t    *mResolvedAddress = MMAP_UNRESOLVED_ADDRESS;
};
};
Loading