From db8eca47ab447acd1ad2858e06377cb89c86b4e1 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Tue, 21 Jul 2015 11:11:32 -0700 Subject: [PATCH] fix not freeing mirrored memory on Windows --- src/os.cpp | 61 +++++++++++++++++++++++++++++---------------- src/os.hpp | 16 +++++++----- src/ring_buffer.cpp | 14 +++++------ src/ring_buffer.hpp | 2 ++ 4 files changed, 58 insertions(+), 35 deletions(-) diff --git a/src/os.cpp b/src/os.cpp index 7eee36e..c3ebae1 100644 --- a/src/os.cpp +++ b/src/os.cpp @@ -16,6 +16,7 @@ #include #include #include +#include #if defined(_WIN32) #define SOUNDIO_OS_WINDOWS @@ -131,6 +132,14 @@ pthread_mutex_t init_mutex = PTHREAD_MUTEX_INITIALIZER; static int page_size; +struct SoundIoOsMirroredMemoryPrivate { + SoundIoOsMirroredMemory pub; +#if defined(SOUNDIO_OS_WINDOWS) + HANDLE handle; +#endif +}; + + double soundio_os_get_time(void) { #if defined(SOUNDIO_OS_WINDOWS) unsigned __int64 time; @@ -603,19 +612,19 @@ int soundio_os_page_size(void) { return page_size; } -int soundio_os_create_mirrored_memory(size_t *capacity, char **out_address) { - *out_address = nullptr; - size_t requested_capacity = *capacity; +struct SoundIoOsMirroredMemory *soundio_os_create_mirrored_memory(size_t requested_capacity) { + SoundIoOsMirroredMemoryPrivate *m = create(); + if (!m) + return nullptr; + SoundIoOsMirroredMemory *mem = &m->pub; - size_t actual_capacity = (requested_capacity / page_size) * page_size; - if (actual_capacity < requested_capacity) - actual_capacity += page_size; + size_t actual_capacity = ceil(requested_capacity / (double)page_size) * page_size; #if defined(SOUNDIO_OS_WINDOWS) BOOL ok; HANDLE hMapFile = CreateFileMapping(INVALID_HANDLE_VALUE, NULL, PAGE_READWRITE, 0, actual_capacity * 2, NULL); if (!hMapFile) - return SoundIoErrorNoMem; + return nullptr; for (;;) { // find a free address space with the correct size @@ -623,7 +632,7 @@ int soundio_os_create_mirrored_memory(size_t *capacity, char **out_address) { if (!address) { ok = CloseHandle(hMapFile); assert(ok); - return SoundIoErrorNoMem; + return nullptr; } // found a big enough address space. hopefully it will remain free @@ -639,7 +648,7 @@ int soundio_os_create_mirrored_memory(size_t *capacity, char **out_address) { } else { ok = CloseHandle(hMapFile); assert(ok); - return SoundIoErrorNoMem; + return nullptr; } } @@ -655,48 +664,56 @@ int soundio_os_create_mirrored_memory(size_t *capacity, char **out_address) { } else { ok = CloseHandle(hMapFile); assert(ok); - return SoundIoErrorNoMem; + return nullptr; } } - *out_address = address; + mem->address = address; break; } #else char *address = (char*)mmap(NULL, actual_capacity * 2, PROT_NONE, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0); if (address == MAP_FAILED) - return SoundIoErrorNoMem; + return nullptr; char *other_address = (char*)mmap(address, actual_capacity, PROT_READ|PROT_WRITE, MAP_ANONYMOUS|MAP_FIXED|MAP_SHARED, -1, 0); if (other_address != address) { munmap(address, 2 * actual_capacity); - return SoundIoErrorNoMem; + return nullptr; } other_address = (char*)mmap(address + actual_capacity, actual_capacity, PROT_READ|PROT_WRITE, MAP_ANONYMOUS|MAP_FIXED|MAP_SHARED, -1, 0); if (other_address != address + actual_capacity) { munmap(address, 2 * actual_capacity); - return SoundIoErrorNoMem; + return nullptr; } - *out_address = address; + mem->address = address; #endif - *capacity = actual_capacity; - - return 0; + mem->capacity = actual_capacity; + return mem; } -void soundio_os_destroy_mirrored_memory(char *address, size_t capacity) { - if (!address) +void soundio_os_destroy_mirrored_memory(struct SoundIoOsMirroredMemory *mem) { + if (!mem) + return; + if (!mem->address) return; #if defined(SOUNDIO_OS_WINDOWS) - // TODO + SoundIoOsMirroredMemoryPrivate *m = (SoundIoOsMirroredMemoryPrivate *)mem; + BOOL ok; + ok = UnmapViewOfFile(mem->address); + assert(ok); + ok = UnmapViewOfFile(mem->address + mem->capacity); + assert(ok); + ok = CloseHandle(m->handle); + assert(ok); #else - int err = munmap(address, 2 * capacity); + int err = munmap(mem->address, 2 * mem->capacity); assert(!err); #endif } diff --git a/src/os.hpp b/src/os.hpp index 4f7c884..fc5ba95 100644 --- a/src/os.hpp +++ b/src/os.hpp @@ -51,11 +51,15 @@ void soundio_os_cond_wait(struct SoundIoOsCond *cond, int soundio_os_page_size(void); -// capacity is replaced with actual capacity which might be modified to be -// a multiple of the system page size -int soundio_os_create_mirrored_memory(size_t *capacity, char **out_address); -// capacity should be the actual capacity value that was given via -// soundio_os_create_mirrored_memory -void soundio_os_destroy_mirrored_memory(char *address, size_t capacity); +// The size of this struct is not part of the API or ABI. +struct SoundIoOsMirroredMemory { + size_t capacity; + char *address; +}; + +// returned capacity might be increased from capacity to be a multiple of the +// system page size +struct SoundIoOsMirroredMemory *soundio_os_create_mirrored_memory(size_t capacity); +void soundio_os_destroy_mirrored_memory(struct SoundIoOsMirroredMemory *mem); #endif diff --git a/src/ring_buffer.cpp b/src/ring_buffer.cpp index 55ba091..4fc7642 100644 --- a/src/ring_buffer.cpp +++ b/src/ring_buffer.cpp @@ -75,14 +75,13 @@ void soundio_ring_buffer_clear(struct SoundIoRingBuffer *rb) { } int soundio_ring_buffer_init(struct SoundIoRingBuffer *rb, int requested_capacity) { - int err; - size_t capacity = requested_capacity; - if ((err = soundio_os_create_mirrored_memory(&capacity, &rb->address))) { + rb->mem = soundio_os_create_mirrored_memory(requested_capacity); + if (!rb->mem) { soundio_ring_buffer_deinit(rb); - return err; + return SoundIoErrorNoMem; } - - rb->capacity = capacity; + rb->address = rb->mem->address; + rb->capacity = rb->mem->capacity; rb->write_offset = 0; rb->read_offset = 0; @@ -90,5 +89,6 @@ int soundio_ring_buffer_init(struct SoundIoRingBuffer *rb, int requested_capacit } void soundio_ring_buffer_deinit(struct SoundIoRingBuffer *rb) { - soundio_os_destroy_mirrored_memory(rb->address, rb->capacity); + soundio_os_destroy_mirrored_memory(rb->mem); + rb->mem = nullptr; } diff --git a/src/ring_buffer.hpp b/src/ring_buffer.hpp index 801b8d8..5abbe79 100644 --- a/src/ring_buffer.hpp +++ b/src/ring_buffer.hpp @@ -9,12 +9,14 @@ #define SOUNDIO_RING_BUFFER_HPP #include "atomics.hpp" +#include "os.hpp" struct SoundIoRingBuffer { char *address; int capacity; atomic_long write_offset; atomic_long read_offset; + SoundIoOsMirroredMemory *mem; }; int soundio_ring_buffer_init(struct SoundIoRingBuffer *rb, int requested_capacity);