Commit 844d52f0 authored by Art Pinch's avatar Art Pinch
Browse files

Fixed kdeconnectd deadlock on Windows caused by systemvolumeplugin

IAudioEndpointVolumeCallback::Release was called in callback functions of IMMNotificationClient (through the call to SystemvolumePlugin::sendSinkList), however https://docs.microsoft.com/en-us/windows/win32/api/mmdeviceapi/nn-mmdeviceapi-immnotificationclient points out that:
*To avoid dead locks, the client should never call IMMDeviceEnumerator::RegisterEndpointNotificationCallback or IMMDeviceEnumerator::UnregisterEndpointNotificationCallback in its implementation of IMMNotificationClient methods.
*The client should never release the final reference on an MMDevice API object during an event callback.

So I moved that part of code to another thread so it will successfully run after callback functions work out and call only if endpoint needs to be released (device was removed), so it won't spawn new thread for every generated event
parent bb619baa
Pipeline #41843 passed with stage
in 5 minutes and 26 seconds
......@@ -23,13 +23,13 @@ K_PLUGIN_CLASS_WITH_JSON(SystemvolumePlugin, "kdeconnect_systemvolume.json")
// Private classes of SystemvolumePlugin
class SystemvolumePlugin::CMMNotificationClient : public IMMNotificationClient
class SystemvolumePlugin::CAudioEndpointVolumeCallback : public IAudioEndpointVolumeCallback
{
LONG _cRef;
public:
CMMNotificationClient(SystemvolumePlugin &x) : enclosing(x), _cRef(1){};
~CMMNotificationClient(){};
public:
CAudioEndpointVolumeCallback(SystemvolumePlugin &x, QString sinkName) : enclosing(x), name(std::move(sinkName)), _cRef(1) {}
~CAudioEndpointVolumeCallback(){};
// IUnknown methods -- AddRef, Release, and QueryInterface
......@@ -68,53 +68,31 @@ class SystemvolumePlugin::CMMNotificationClient : public IMMNotificationClient
return S_OK;
}
// Callback methods for device-event notifications.
HRESULT STDMETHODCALLTYPE OnDefaultDeviceChanged(EDataFlow flow, ERole role, LPCWSTR pwstrDeviceId) override
{
if (flow == eRender)
{
enclosing.sendSinkList();
}
return S_OK;
}
HRESULT STDMETHODCALLTYPE OnDeviceAdded(LPCWSTR pwstrDeviceId) override
{
enclosing.sendSinkList();
return S_OK;
}
HRESULT STDMETHODCALLTYPE OnDeviceRemoved(LPCWSTR pwstrDeviceId) override
{
enclosing.sendSinkList();
return S_OK;
}
// Callback method for endpoint-volume-change notifications.
HRESULT STDMETHODCALLTYPE OnDeviceStateChanged(LPCWSTR pwstrDeviceId, DWORD dwNewState) override
HRESULT STDMETHODCALLTYPE OnNotify(PAUDIO_VOLUME_NOTIFICATION_DATA pNotify) override
{
enclosing.sendSinkList();
return S_OK;
}
NetworkPacket np(PACKET_TYPE_SYSTEMVOLUME);
np.set<int>(QStringLiteral("volume"), (int)(pNotify->fMasterVolume * 100));
np.set<bool>(QStringLiteral("muted"), pNotify->bMuted);
np.set<QString>(QStringLiteral("name"), name);
enclosing.sendPacket(np);
HRESULT STDMETHODCALLTYPE OnPropertyValueChanged(LPCWSTR pwstrDeviceId, const PROPERTYKEY key) override
{
enclosing.sendSinkList();
return S_OK;
}
private:
LONG _cRef;
private:
SystemvolumePlugin &enclosing;
QString name;
};
class SystemvolumePlugin::CAudioEndpointVolumeCallback : public IAudioEndpointVolumeCallback
class SystemvolumePlugin::CMMNotificationClient : public IMMNotificationClient
{
LONG _cRef;
public:
CAudioEndpointVolumeCallback(SystemvolumePlugin &x, QString sinkName) : enclosing(x), name(sinkName), _cRef(1) {}
~CAudioEndpointVolumeCallback(){};
public:
CMMNotificationClient(SystemvolumePlugin &x) : enclosing(x), _cRef(1){};
~CMMNotificationClient(){};
// IUnknown methods -- AddRef, Release, and QueryInterface
......@@ -153,22 +131,89 @@ class SystemvolumePlugin::CAudioEndpointVolumeCallback : public IAudioEndpointVo
return S_OK;
}
// Callback method for endpoint-volume-change notifications.
// Callback methods for device-event notifications.
HRESULT STDMETHODCALLTYPE OnNotify(PAUDIO_VOLUME_NOTIFICATION_DATA pNotify) override
HRESULT STDMETHODCALLTYPE OnDefaultDeviceChanged(EDataFlow flow, ERole role, LPCWSTR pwstrDeviceId) override
{
NetworkPacket np(PACKET_TYPE_SYSTEMVOLUME);
np.set<int>(QStringLiteral("volume"), (int)(pNotify->fMasterVolume * 100));
np.set<bool>(QStringLiteral("muted"), pNotify->bMuted);
np.set<QString>(QStringLiteral("name"), name);
enclosing.sendPacket(np);
if (flow == eRender)
{
enclosing.sendSinkList();
}
return S_OK;
}
HRESULT STDMETHODCALLTYPE OnDeviceAdded(LPCWSTR pwstrDeviceId) override
{
enclosing.sendSinkList();
return S_OK;
}
struct RemovedDeviceThreadData {
QString qDeviceId;
SystemvolumePlugin* plugin;
};
static DWORD WINAPI releaseRemovedDevice(_In_ LPVOID lpParameter) {
auto* data = static_cast<RemovedDeviceThreadData*>(lpParameter);
if (!data->plugin->sinkList.empty())
{
auto idToNameIterator = data->plugin->idToNameMap.find(data->qDeviceId);
if (idToNameIterator == data->plugin->idToNameMap.end()) return 0;
QString& sinkName = idToNameIterator.value();
auto sinkListIterator = data->plugin->sinkList.find(sinkName);
if (sinkListIterator == data->plugin->sinkList.end()) return 0;
auto& sink = sinkListIterator.value();
sink.first->UnregisterControlChangeNotify(sink.second);
sink.first->Release();
sink.second->Release();
data->plugin->idToNameMap.remove(data->qDeviceId);
data->plugin->sinkList.remove(sinkName);
}
data->plugin->sendSinkList();
return 0;
}
HRESULT STDMETHODCALLTYPE OnDeviceRemoved(LPCWSTR pwstrDeviceId) override
{
static RemovedDeviceThreadData data {};
data.qDeviceId = QString::fromWCharArray(pwstrDeviceId);
data.plugin = &enclosing;
DWORD threadId;
HANDLE threadHandle = CreateThread(NULL, 0, releaseRemovedDevice, &data, 0, &threadId);
CloseHandle(threadHandle);
return S_OK;
}
HRESULT STDMETHODCALLTYPE OnDeviceStateChanged(LPCWSTR pwstrDeviceId, DWORD dwNewState) override
{
if (dwNewState == DEVICE_STATE_UNPLUGGED) return OnDeviceRemoved(pwstrDeviceId);
enclosing.sendSinkList();
return S_OK;
}
HRESULT STDMETHODCALLTYPE OnPropertyValueChanged(LPCWSTR pwstrDeviceId, const PROPERTYKEY key) override
{
enclosing.sendSinkList();
return S_OK;
}
private:
private:
LONG _cRef;
SystemvolumePlugin &enclosing;
QString name;
};
SystemvolumePlugin::SystemvolumePlugin(QObject *parent, const QVariantList &args)
......@@ -204,19 +249,8 @@ bool SystemvolumePlugin::sendSinkList()
QJsonDocument document;
QJsonArray array;
HRESULT hr;
if (!sinkList.empty())
{
for (auto const &sink : sinkList)
{
sink.first->UnregisterControlChangeNotify(sink.second);
sink.first->Release();
sink.second->Release();
}
sinkList.clear();
}
IMMDeviceCollection *devices = nullptr;
hr = deviceEnumerator->EnumAudioEndpoints(eRender, DEVICE_STATE_ACTIVE, &devices);
HRESULT hr = deviceEnumerator->EnumAudioEndpoints(eRender, DEVICE_STATE_ACTIVE, &devices);
if (hr != S_OK)
{
......@@ -234,6 +268,7 @@ bool SystemvolumePlugin::sendSinkList()
PROPVARIANT deviceProperty;
QString name;
QString desc;
LPWSTR deviceId;
float volume;
BOOL muted;
......@@ -267,6 +302,8 @@ bool SystemvolumePlugin::sendSinkList()
device->Release();
continue;
}
device->GetId(&deviceId);
endpoint->GetMasterVolumeLevelScalar(&volume);
endpoint->GetMute(&muted);
......@@ -275,9 +312,14 @@ bool SystemvolumePlugin::sendSinkList()
sinkObject.insert(QStringLiteral("maxVolume"), (qint64)100);
// Register Callback
callback = new CAudioEndpointVolumeCallback(*this, name);
sinkList[name] = qMakePair(endpoint, callback);
endpoint->RegisterControlChangeNotify(callback);
QString qDeviceId = QString::fromWCharArray(deviceId);
if (!sinkList.contains(qDeviceId)) {
callback = new CAudioEndpointVolumeCallback(*this, name);
endpoint->RegisterControlChangeNotify(callback);
sinkList[name] = qMakePair(endpoint, callback);
}
idToNameMap[qDeviceId] = name;
device->Release();
array.append(sinkObject);
......
......@@ -41,6 +41,7 @@ class Q_DECL_EXPORT SystemvolumePlugin : public KdeConnectPlugin
IMMDeviceEnumerator* deviceEnumerator;
CMMNotificationClient* deviceCallback;
QMap<QString, QPair<IAudioEndpointVolume *, CAudioEndpointVolumeCallback *>> sinkList;
QMap<QString, QString> idToNameMap;
bool sendSinkList();
};
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment