FEAT(client, server): use native mDNS/DNS-SD API on Windows, if available #4494

davidebeatrici · 2020-09-19T06:25:14Z

This allows:

The client to find servers advertized via zeroconf without the need for Bonjour to be installed.
The server to advertize itself via zeroconf without the need for Bonjour to be installed.

The Win32 API was introduced in the version 10.0.18362.0 (1903/19H1) of Windows SDK. Before that, only the UWP interface was available (introduced in Windows 10 1507).

This commit was successfully tested on Windows 10 1809, which probably means that the API can be used on previous versions as well.

Even if that isn't the case, it's not a problem: the code checks whether the functions we need are exported. If they're not, it falls back to Bonjour.

Q_OS_WIN64 is used instead of Q_OS_WIN because of an issue that appears when certain DNS functions are used in an x86 (32 bit) build: https://developercommunity.visualstudio.com/content/problem/1191345/some-dns-api-functions-cause-lnk2019-errors-in-32.html

This means that until the issue is fixed we can safely use the native mDNS-DNS-SD API only on x86_64 (64 bit).

Krzmbrzl · 2020-09-19T16:18:35Z

FAILED: src/murmur/CMakeFiles/murmur.dir/Zeroconf.cpp.obj 
C:\PROGRA~2\MICROS~1\2019\ENTERP~1\VC\Tools\MSVC\1427~1.291\bin\Hostx64\x86\cl.exe  /nologo /TP -DICE_STATIC_LIBS -DMUMBLE_VERSION=1.4.0~2020-09-19~g5ccd9eb~snapshot -DMUMBLE_VERSION_STRING=1.4.0 -DMURMUR -DQT_ACCESSIBILITY_SUPPORT_LIB -DQT_CORE_LIB -DQT_EVENTDISPATCHER_SUPPORT_LIB -DQT_FONTDATABASE_SUPPORT_LIB -DQT_GUI_LIB -DQT_NETWORK_LIB -DQT_NO_DEBUG -DQT_RESTRICTED_CAST_FROM_ASCII -DQT_SQL_LIB -DQT_THEME_SUPPORT_LIB -DQT_USE_FAST_CONCATENATION -DQT_USE_FAST_OPERATOR_PLUS -DQT_WIDGETS_LIB -DQT_WINDOWSUIAUTOMATION_SUPPORT_LIB -DQT_XML_LIB -DUNICODE -DUSE_ICE -DUSE_QSSLDIFFIEHELLMANPARAMETERS -DUSE_ZEROCONF -DWIN32_LEAN_AND_MEAN -Isrc\murmur\murmur_autogen\include -ID:\a\1\s\src\murmur -ID:\a\1\s\src -ID:\a\1\s\3rdparty\qqbonjour -Isrc\murmur -Isrc -IC:\hostedtoolcache\windows\MumbleBuild\win32-static-1.4.x-2020-07-22-dbd6271-1162\installed\x86-windows-static-md\include -IC:\hostedtoolcache\windows\MumbleBuild\win32-static-1.4.x-2020-07-22-dbd6271-1162\installed\x86-windows-static-md\include\QtCore -IC:\hostedtoolcache\windows\MumbleBuild\win32-static-1.4.x-2020-07-22-dbd6271-1162\installed\x86-windows-static-md\tools\qt5\mkspecs\win32-msvc -IC:\hostedtoolcache\windows\MumbleBuild\win32-static-1.4.x-2020-07-22-dbd6271-1162\installed\x86-windows-static-md\include\QtNetwork -IC:\hostedtoolcache\windows\MumbleBuild\win32-static-1.4.x-2020-07-22-dbd6271-1162\installed\x86-windows-static-md\include\QtXml -IC:\hostedtoolcache\windows\MumbleBuild\win32-static-1.4.x-2020-07-22-dbd6271-1162\installed\x86-windows-static-md\include\QtSql -IC:\hostedtoolcache\windows\MumbleBuild\win32-static-1.4.x-2020-07-22-dbd6271-1162\installed\x86-windows-static-md\include\QtWidgets -IC:\hostedtoolcache\windows\MumbleBuild\win32-static-1.4.x-2020-07-22-dbd6271-1162\installed\x86-windows-static-md\include\QtGui -IC:\hostedtoolcache\windows\MumbleBuild\win32-static-1.4.x-2020-07-22-dbd6271-1162\installed\x86-windows-static-md\include\QtEventDispatcherSupport -IC:\hostedtoolcache\windows\MumbleBuild\win32-static-1.4.x-2020-07-22-dbd6271-1162\installed\x86-windows-static-md\include\QtFontDatabaseSupport -IC:\hostedtoolcache\windows\MumbleBuild\win32-static-1.4.x-2020-07-22-dbd6271-1162\installed\x86-windows-static-md\include\QtThemeSupport -IC:\hostedtoolcache\windows\MumbleBuild\win32-static-1.4.x-2020-07-22-dbd6271-1162\installed\x86-windows-static-md\include\QtAccessibilitySupport -IC:\hostedtoolcache\windows\MumbleBuild\win32-static-1.4.x-2020-07-22-dbd6271-1162\installed\x86-windows-static-md\include\QtWindowsUIAutomationSupport /DWIN32 /D_WINDOWS /GR /EHsc /O2 /Ob2 /DNDEBUG -MD -arch:SSE /GR /Zi /Zo /Oy- -std:c++14 /showIncludes /Fosrc\murmur\CMakeFiles\murmur.dir\Zeroconf.cpp.obj /Fdsrc\murmur\CMakeFiles\murmur.dir\ /FS -c D:\a\1\s\src\murmur\Zeroconf.cpp
D:\a\1\s\src\murmur\Zeroconf.cpp(94): error C2440: '=': cannot convert from 'void (__cdecl *)(const DWORD,void *,DNS_SERVICE_INSTANCE *)' to 'PDNS_SERVICE_REGISTER_COMPLETE'
D:\a\1\s\src\murmur\Zeroconf.cpp(94): note: This conversion requires a reinterpret_cast, a C-style cast or function-style cast

Krzmbrzl

A few more documentation comments would be nice as well :)

src/CMakeLists.txt

src/mumble/CMakeLists.txt

src/mumble/ConnectDialog.cpp

src/mumble/ConnectDialog.h

src/mumble/Zeroconf.h

src/murmur/Zeroconf.h

davidebeatrici · 2020-09-20T01:03:16Z

Sorry for the many force-pushes.

Interestingly, the error you mentioned only happened with the x86 build, I had to specify WINAPI for the callback functions.

However, there's now an issue I can reproduce locally:

Zeroconf.cpp.obj : error LNK2019: unresolved external symbol _DnsServiceConstructInstance referenced in function "public: bool __thiscall Zeroconf::registerService(class BonjourRecord const &,unsigned short)" (?registerService@Zeroconf@@QAE_NABVBonjourRecord@@G@Z)
Zeroconf.cpp.obj : error LNK2019: unresolved external symbol _DnsServiceFreeInstance referenced in function "protected: static void __stdcall Zeroconf::callbackRegisterComplete(unsigned long,void *,struct _DNS_SERVICE_INSTANCE *)" (?callbackRegisterComplete@Zeroconf@@KGXKPAXPAU_DNS_SERVICE_INSTANCE@@@Z)
Zeroconf.cpp.obj : error LNK2019: unresolved external symbol _DnsServiceRegister referenced in function "public: bool __thiscall Zeroconf::registerService(class BonjourRecord const &,unsigned short)" (?registerService@Zeroconf@@QAE_NABVBonjourRecord@@G@Z)
Zeroconf.cpp.obj : error LNK2019: unresolved external symbol _DnsServiceDeRegister referenced in function "public: bool __thiscall Zeroconf::unregisterService(void)" (?unregisterService@Zeroconf@@QAE_NXZ)
Zeroconf.cpp.obj : error LNK2019: unresolved external symbol _DnsServiceRegisterCancel referenced in function "public: bool __thiscall Zeroconf::unregisterService(void)" (?unregisterService@Zeroconf@@QAE_NXZ)

The symbols appear to be present in the x86 dnsapi.lib.

davidebeatrici · 2020-09-20T01:56:10Z

I can reproduce with https://github.com/andrewtj/windns-dnssd-exercise:

windns dnssd exercise.obj : error LNK2019: unresolved external symbol _DnsServiceConstructInstance referenced in function _main
windns dnssd exercise.obj : error LNK2019: unresolved external symbol _DnsServiceFreeInstance referenced in function "void __stdcall ResolveCallback(unsigned long,void *,struct _DNS_SERVICE_INSTANCE *)" (?ResolveCallback@@YGXKPAXPAU_DNS_SERVICE_INSTANCE@@@Z)
windns dnssd exercise.obj : error LNK2019: unresolved external symbol _DnsServiceRegister referenced in function _main
windns dnssd exercise.obj : error LNK2019: unresolved external symbol _DnsServiceDeRegister referenced in function _main

@andrewtj Did you perhaps try to build the project for x86 back when you worked on it? If yes, did it link successfully?

I would like to know whether it's a long standing issue or one that appeared in a recent version of the SDK.

andrewtj · 2020-09-20T04:16:17Z

@davidebeatrici I never tried x86 and fwiw I got the impression the API isn't finished.

Krzmbrzl · 2020-09-20T06:42:57Z

The MinGW build fails with

CMake Error at src/mumble/CMakeLists.txt:826 (message):

  DNS-SD library not found!

Additionally I think that your last 2 commits should actually only be a single commit since they are about the same thing (pretty much) 🤔

davidebeatrici · 2020-09-21T02:53:53Z

Issue reported: https://developercommunity.visualstudio.com/content/problem/1191345/some-dns-api-functions-cause-lnk2019-errors-in-32.html

@andrewtj I read your blog post (https://andrewtj.com/log/dnssd-apis).

First, registering a service requires specifying a hostname and the only way to obtain the system's mDNS hostname is to do a reverse lookup. Clumsy, but workable.

GetComputerNameEx() with ComputerNameDnsHostname seems to work perfectly.

More of an issue is that browse callbacks are only invoked once instead of whenever a service appears or disappears from the network. This could be worked around with a query, however multicast queries do not seem to be consistently updated as RRSets change.

This is indeed a limitation, however in our case it's not a big issue: closing the connect dialog and opening it again forces a refresh.

andrewtj · 2020-09-21T03:22:41Z

GetComputerNameEx() with ComputerNameDnsHostname seems to work perfectly.

I think that might get out of sync if there's a naming conflict (due to duelling clients, bugs, or infrastructure issues).

davidebeatrici · 2020-09-21T03:32:22Z

I agree. However, doesn't Avahi/mDNSResponder retrieve the local hostname too?

andrewtj · 2020-09-21T05:35:48Z

I agree. However, doesn't Avahi/mDNSResponder retrieve the local hostname too?

They're generally configured with the local hostname but they can be configured with another name and will change their name if there's a conflict.

Krzmbrzl

The x86 build still fails with

   Creating library src\murmur\murmur.lib and object src\murmur\murmur.exp
Zeroconf.cpp.obj : error LNK2019: unresolved external symbol _DnsServiceConstructInstance referenced in function "public: bool __thiscall Zeroconf::registerService(class BonjourRecord const &,unsigned short)" (?registerService@Zeroconf@@QAE_NABVBonjourRecord@@G@Z)
Zeroconf.cpp.obj : error LNK2019: unresolved external symbol _DnsServiceFreeInstance referenced in function "protected: static void __stdcall Zeroconf::callbackRegisterComplete(unsigned long,void *,struct _DNS_SERVICE_INSTANCE *)" (?callbackRegisterComplete@Zeroconf@@KGXKPAXPAU_DNS_SERVICE_INSTANCE@@@Z)
Zeroconf.cpp.obj : error LNK2019: unresolved external symbol _DnsServiceRegister referenced in function "public: bool __thiscall Zeroconf::registerService(class BonjourRecord const &,unsigned short)" (?registerService@Zeroconf@@QAE_NABVBonjourRecord@@G@Z)
Zeroconf.cpp.obj : error LNK2019: unresolved external symbol _DnsServiceDeRegister referenced in function "public: bool __thiscall Zeroconf::unregisterService(void)" (?unregisterService@Zeroconf@@QAE_NXZ)
Zeroconf.cpp.obj : error LNK2019: unresolved external symbol _DnsServiceRegisterCancel referenced in function "public: bool __thiscall Zeroconf::unregisterService(void)" (?unregisterService@Zeroconf@@QAE_NXZ)
murmur.exe : fatal error LNK1120: 5 unresolved externals

And one more thing: According to our commit guidelines, multiple scopes are to be separated using a comma and not a slash. Therefore it should be REFAC(client, server) ☝️

davidebeatrici · 2020-09-21T19:29:58Z

They're generally configured with the local hostname but they can be configured with another name and will change their name if there's a conflict.

Ah, I see. Thank you for the explanation.

@Krzmbrzl Renamed the PR, I'll also rename the commit as soon as the x86 build issue is fixed (see the Microsoft Developer Community issue I linked in #4494 (comment) for updates).

…3rdparty This should make it more clear that we don't include Bonjour-related stuff in our project. We use external libraries depending on the OS. For compatibility, the server option is still called "bonjour". We should probably add a new option called "zeroconf" and then handle the old one if present in the configuration file.

…able This allows: - The client to find servers advertized via zeroconf without the need for Bonjour to be installed. - The server to advertize itself via zeroconf without the need for Bonjour to be installed. The Win32 API was introduced in the version 10.0.18362.0 (1903/19H1) of Windows SDK. Before that, only the UWP interface was available (introduced in Windows 10 1507). This commit was successfully tested on Windows 10 1809, which probably means that the API can be used on previous versions as well. Even if that isn't the case, it's not a problem: if the code fails to load the required symbols, it falls back to Bonjour. "Q_OS_WIN64" is used instead of "Q_OS_WIN" because of an issue that appears when certain DNS functions are used in an x86 (32 bit) build: https://developercommunity.visualstudio.com/content/problem/1191345/some-dns-api-functions-cause-lnk2019-errors-in-32.html This means that until the issue is fixed we can safely use the native mDNS-DNS-SD API only on x86_64 (64 bit).

src/mumble/Zeroconf.cpp

davidebeatrici · 2020-09-22T22:41:15Z

Turns out the issue is caused by a bug in the Windows SDK: missing WINAPI modifier in the declaration of the functions that trigger LNK2019.

https://developercommunity.visualstudio.com/comments/1194102/view.html

davidebeatrici added client windows server labels Sep 19, 2020

davidebeatrici linked an issue Sep 19, 2020 that may be closed by this pull request

Mumble should use Windows 10's own DNS-SD implementation instead of Bonjour #3024

Closed

davidebeatrici requested a review from Krzmbrzl September 19, 2020 06:26

Krzmbrzl requested changes Sep 19, 2020

View reviewed changes

davidebeatrici force-pushed the zeroconf-winapi branch 8 times, most recently from ca3b5f8 to c828996 Compare September 20, 2020 00:13

davidebeatrici force-pushed the zeroconf-winapi branch from c828996 to 2099a95 Compare September 21, 2020 03:42

Krzmbrzl reviewed Sep 21, 2020

View reviewed changes

davidebeatrici changed the title ~~FEAT(client/server): use native mDNS/DNS-SD API on Windows, if available~~ FEAT(client, server): use native mDNS/DNS-SD API on Windows, if available Sep 21, 2020

davidebeatrici force-pushed the zeroconf-winapi branch 2 times, most recently from 76947b9 to 732b35d Compare September 22, 2020 05:34

davidebeatrici force-pushed the zeroconf-winapi branch from 732b35d to 44aacb0 Compare September 22, 2020 05:36

davidebeatrici force-pushed the zeroconf-winapi branch from 44aacb0 to ee731f8 Compare September 22, 2020 05:38

davidebeatrici requested a review from Krzmbrzl September 22, 2020 06:06

Krzmbrzl approved these changes Sep 22, 2020

View reviewed changes

src/mumble/Zeroconf.cpp Show resolved Hide resolved

davidebeatrici merged commit 69dfdd2 into mumble-voip:master Sep 22, 2020

skissane mentioned this pull request Nov 25, 2020

Suggestion: add notice to README that this library is no longer required on newer builds of Windows 10 stammen/dnssd-uwp#10

Open

fwcd mentioned this pull request Apr 19, 2024

Support for native Windows mDNS/DNS-SD stack jbagg/QtZeroConf#66

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FEAT(client, server): use native mDNS/DNS-SD API on Windows, if available #4494

FEAT(client, server): use native mDNS/DNS-SD API on Windows, if available #4494

davidebeatrici commented Sep 19, 2020 •

edited

Loading

Krzmbrzl commented Sep 19, 2020

Krzmbrzl left a comment

davidebeatrici commented Sep 20, 2020 •

edited

Loading

davidebeatrici commented Sep 20, 2020

andrewtj commented Sep 20, 2020

Krzmbrzl commented Sep 20, 2020

davidebeatrici commented Sep 21, 2020

andrewtj commented Sep 21, 2020

davidebeatrici commented Sep 21, 2020

andrewtj commented Sep 21, 2020

Krzmbrzl left a comment

davidebeatrici commented Sep 21, 2020

davidebeatrici commented Sep 22, 2020

FEAT(client, server): use native mDNS/DNS-SD API on Windows, if available #4494

FEAT(client, server): use native mDNS/DNS-SD API on Windows, if available #4494

Conversation

davidebeatrici commented Sep 19, 2020 • edited Loading

Krzmbrzl commented Sep 19, 2020

Krzmbrzl left a comment

Choose a reason for hiding this comment

davidebeatrici commented Sep 20, 2020 • edited Loading

davidebeatrici commented Sep 20, 2020

andrewtj commented Sep 20, 2020

Krzmbrzl commented Sep 20, 2020

davidebeatrici commented Sep 21, 2020

andrewtj commented Sep 21, 2020

davidebeatrici commented Sep 21, 2020

andrewtj commented Sep 21, 2020

Krzmbrzl left a comment

Choose a reason for hiding this comment

davidebeatrici commented Sep 21, 2020

davidebeatrici commented Sep 22, 2020

davidebeatrici commented Sep 19, 2020 •

edited

Loading

davidebeatrici commented Sep 20, 2020 •

edited

Loading