Skip to content

Commit

Permalink
Don't use a unique_ptr to hold SharedDevices
Browse files Browse the repository at this point in the history
As they're kept in a "global" list, they're subject to being destructed at
process termination if any are left active, when it's not safe to close
devices.
  • Loading branch information
kcat committed Apr 21, 2023
1 parent 4e43fc2 commit 6114139
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 29 deletions.
3 changes: 1 addition & 2 deletions src/buffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -327,8 +327,7 @@ ds::expected<ComPtr<SharedBuffer>,HRESULT> SharedBuffer::Create(const DSBUFFERDE

#define PREFIX "Buffer::"
Buffer::Buffer(DSound8OAL &parent, bool is8, IDirectSoundBuffer *original) noexcept
: mParent{parent}, mContext{parent.getShared().mContext.get()}, mMutex{parent.getMutex()}
, mIs8{is8}
: mParent{parent}, mContext{parent.getShared().mContext}, mMutex{parent.getMutex()}, mIs8{is8}
{
mImmediate.dwSize = sizeof(mImmediate);
mImmediate.vPosition.x = 0.0f;
Expand Down
35 changes: 17 additions & 18 deletions src/dsoundoal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,8 @@ ds::expected<std::unique_ptr<SharedDevice>,HRESULT> CreateDeviceShare(const GUID
shared->mMaxHwSources = maxHw;
shared->mMaxSwSources = totalSources - maxHw;
shared->mExtensions = extensions;
shared->mDevice.reset(aldev.release());
shared->mContext.reset(alctx.release());
shared->mDevice = aldev.release();
shared->mContext = alctx.release();

return shared;
}
Expand All @@ -222,53 +222,52 @@ ds::expected<std::unique_ptr<SharedDevice>,HRESULT> CreateDeviceShare(const GUID

#define CLASS_PREFIX "SharedDevice::"
std::mutex SharedDevice::sDeviceListMutex;
std::vector<std::unique_ptr<SharedDevice>> SharedDevice::sDeviceList;
std::vector<SharedDevice*> SharedDevice::sDeviceList;

auto SharedDevice::GetById(const GUID &deviceId) noexcept
-> ds::expected<ComPtr<SharedDevice>,HRESULT>
{
auto find_id = [&deviceId](std::unique_ptr<SharedDevice> &device)
auto find_id = [&deviceId](SharedDevice *device)
{ return deviceId == device->mId; };

std::unique_lock listlock{sDeviceListMutex};
auto sharediter = std::find_if(sDeviceList.begin(), sDeviceList.end(), find_id);
if(sharediter != sDeviceList.end())
{
(*sharediter)->AddRef();
return ComPtr<SharedDevice>{(*sharediter).get()};
return ComPtr<SharedDevice>{*sharediter};
}

auto shared = CreateDeviceShare(deviceId);
if(!shared) return ds::unexpected(shared.error());

sDeviceList.emplace_back(std::move(shared).value());
return ComPtr<SharedDevice>{sDeviceList.back().get()};
sDeviceList.emplace_back();
sDeviceList.back() = shared->release();
return ComPtr<SharedDevice>{sDeviceList.back()};
}

SharedDevice::~SharedDevice()
{
if(mContext)
{
if(mContext.get() == alcGetThreadContext())
if(mContext == alcGetThreadContext())
alcSetThreadContext(nullptr);
alcDestroyContext(mContext.release());
alcDestroyContext(mContext);
}
if(mDevice)
alcCloseDevice(mDevice.release());
alcCloseDevice(mDevice);
}

#define PREFIX CLASS_PREFIX "dispose "
void SharedDevice::dispose() noexcept
{
std::lock_guard listlock{sDeviceListMutex};

auto find_shared = [this](std::unique_ptr<SharedDevice> &shared)
{ return this == shared.get(); };

auto shared_iter = std::find_if(sDeviceList.begin(), sDeviceList.end(), find_shared);
auto shared_iter = std::find(sDeviceList.begin(), sDeviceList.end(), this);
if(shared_iter != sDeviceList.end())
{
TRACE(PREFIX "Freeing shared device %s\n", GuidPrinter{(*shared_iter)->mId}.c_str());
std::unique_ptr<SharedDevice> device{*shared_iter};
TRACE(PREFIX "Freeing shared device %s\n", GuidPrinter{device->mId}.c_str());
sDeviceList.erase(shared_iter);
}
}
Expand Down Expand Up @@ -351,10 +350,10 @@ ComPtr<Buffer> DSound8OAL::createSecondaryBuffer(IDirectSoundBuffer *original)
#define PREFIX CLASS_PREFIX "notifyThread "
void DSound8OAL::notifyThread() noexcept
{
alcSetThreadContext(mShared->mContext.get());
alcSetThreadContext(mShared->mContext);

ALCint refresh{};
alcGetIntegerv(mShared->mDevice.get(), ALC_REFRESH, 1, &refresh);
alcGetIntegerv(mShared->mDevice, ALC_REFRESH, 1, &refresh);

using namespace std::chrono;
milliseconds waittime{10};
Expand Down Expand Up @@ -781,7 +780,7 @@ HRESULT STDMETHODCALLTYPE DSound8OAL::Initialize(const GUID *deviceId) noexcept
if(!shared) return shared.error();
mShared = std::move(shared).value();

mPrimaryBuffer.setContext(mShared->mContext.get());
mPrimaryBuffer.setContext(mShared->mContext);
mExtensions = mShared->mExtensions;

/* Preallocate some groups for the number of "hardware" buffers we can do.
Expand Down
12 changes: 3 additions & 9 deletions src/dsoundoal.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
#include <atomic>
#include <bitset>
#include <condition_variable>
#include <memory>
#include <mutex>
#include <thread>
#include <type_traits>
Expand All @@ -32,12 +31,7 @@ enum Extensions : uint8_t {

struct SharedDevice {
static std::mutex sDeviceListMutex;
static std::vector<std::unique_ptr<SharedDevice>> sDeviceList;

struct NoDeleter {
template<typename T>
void operator()(T*) noexcept { }
};
static std::vector<SharedDevice*> sDeviceList;

SharedDevice(const GUID &id) : mId{id} { }
SharedDevice(const SharedDevice&) = delete;
Expand Down Expand Up @@ -65,8 +59,8 @@ struct SharedDevice {
const GUID mId;
DWORD mSpeakerConfig{};

std::unique_ptr<ALCdevice,NoDeleter> mDevice;
std::unique_ptr<ALCcontext,NoDeleter> mContext;
ALCdevice *mDevice;
ALCcontext *mContext;

std::atomic<ULONG> mRef{1u};

Expand Down

0 comments on commit 6114139

Please sign in to comment.