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

Commit 1389cc47 authored by Phil Burk's avatar Phil Burk Committed by android-build-merger
Browse files

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

am: e1ca8625

Change-Id: I0185a7c824c8e5e61adb5f4da17617721eac2ca7
parents d3643f0f e1ca8625
Loading
Loading
Loading
Loading
+21 −3
Original line number Diff line number Diff line
@@ -16,11 +16,14 @@

// Play sine waves using AAudio.

#include <stdio.h>
#include <stdlib.h>
#include <math.h>
#include <aaudio/AAudio.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 "AAudioSimplePlayer.h"
#include "AAudioArgsParser.h"
@@ -48,6 +51,8 @@ int main(int argc, const char **argv)
    float   *floatData = nullptr;
    int16_t *shortData = nullptr;

    int      testFd = -1;

    // Make printf print immediately so that debug info is not stuck
    // in a buffer if we hang or crash.
    setvbuf(stdout, nullptr, _IONBF, (size_t) 0);
@@ -95,6 +100,9 @@ int main(int argc, const char **argv)
        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.
    printf("call player.start()\n");
    result = player.start();
@@ -176,7 +184,17 @@ int main(int argc, const char **argv)
    }

finish:
    printf("testFd = %d, fcntl before aaudio close returns 0x%08X\n",
           testFd, fcntl(testFd, F_GETFD));
    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[] shortData;
    printf("exiting - AAudio result = %d = %s\n", result, AAudio_convertResultToText(result));
+3 −1
Original line number Diff line number Diff line
@@ -28,6 +28,7 @@
#include "binding/RingBufferParcelable.h"
#include "binding/AudioEndpointParcelable.h"

using android::base::unique_fd;
using android::NO_ERROR;
using android::status_t;
using android::Parcel;
@@ -49,7 +50,8 @@ AudioEndpointParcelable::~AudioEndpointParcelable() {}
 * Add the file descriptor to the table.
 * @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) {
        return AAUDIO_ERROR_OUT_OF_RANGE;
    }
+2 −1
Original line number Diff line number Diff line
@@ -20,6 +20,7 @@
#include <stdint.h>

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

@@ -47,7 +48,7 @@ public:
     * Add the file descriptor to the table.
     * @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;

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

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

#include "binding/SharedMemoryParcelable.h"

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

void SharedMemoryParcelable::setup(int fd, int32_t sizeInBytes) {
    mFd = fd;
void SharedMemoryParcelable::setup(const unique_fd& fd, int32_t sizeInBytes) {
    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;

}

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

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

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

    aaudio_result_t result = AAUDIO_OK;

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

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

int32_t SharedMemoryParcelable::getSizeInBytes() {
@@ -134,17 +145,11 @@ aaudio_result_t SharedMemoryParcelable::validate() {
        ALOGE("SharedMemoryParcelable invalid mSizeInBytes = %d", mSizeInBytes);
        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;
}

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

#include <stdint.h>

#include <sys/mman.h>

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

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

namespace aaudio {

// 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.
 * 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:
    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
    aaudio_result_t resolve(int32_t offsetInBytes, int32_t sizeInBytes, void **regionAddressPtr);
@@ -55,8 +59,6 @@ public:
    // munmap() any mapped memory
    aaudio_result_t close();

    bool isFileDescriptorSafe();

    int32_t getSizeInBytes();

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

#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;
    uint8_t    *mResolvedAddress = MMAP_UNRESOLVED_ADDRESS;
};
Loading