net-mdns icon indicating copy to clipboard operation
net-mdns copied to clipboard

Dispose UdpClient instances when disposing MulticastService

Open fgheysels opened this issue 5 years ago • 5 comments

Since UdpClient implements IDisposable, and unicastClientIp4 and unicastClientIp6 are members of the MulticastService class, these instances must be disposed when disposing the MulticastService instance.

See issue 93

fgheysels avatar Jan 27 '20 14:01 fgheysels

Codecov Report

Merging #94 into master will decrease coverage by 0.54%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #94      +/-   ##
==========================================
- Coverage   91.11%   90.57%   -0.55%     
==========================================
  Files          10       10              
  Lines         518      520       +2     
==========================================
- Hits          472      471       -1     
- Misses         46       49       +3
Impacted Files Coverage Δ
src/MulticastService.cs 91.66% <100%> (+0.09%) :arrow_up:
src/MulticastClient.cs 82.78% <0%> (-2.46%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update b9f2f81...c6349ff. Read the comment docs.

codecov-io avatar Jan 27 '20 14:01 codecov-io

Kind reminder @richardschneider

fgheysels avatar Feb 16 '20 18:02 fgheysels

I think the NetworkAddressChanged callback needs to be removed as well in the Stop() method:

#if NET461 if (Environment.OSVersion.Platform.ToString().StartsWith("Win")) #else if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) #endif { NetworkChange.NetworkAddressChanged -= OnNetworkAddressChanged; }

To me it also looks like the NetworkAddressChanged callback will update the clients to send/receive upd on, but it will not update the address list used in the response. Those addresses can be specified when creating a ServiceProfile to Advertise, and ends up in the NameServer.Catalog. So only solution I found is to subscribe to NetworkAddressChanged myself, and call Unadvertise,and then Advertise again with a new ServiceProfile. But seems like memory isn't handled properly then...

Since I subscribe to NetworkAddressChanged myself, and re-advertise, the solution for the memory leakage was to remove all use of NetworkAddressChanged from the mdns code. Be aware that NetworkAddressChanged is called twice for every change if you have both ipv4 and ipv6 enabled, so make sure you don't re-advertise too much, or from different threads...