fix(windows): memory leaks

[njeisecke]

Hi Esteban!

This fixes two leaks in the Windows implementation.

Thank you!

[njeisecke]

Ok, this should be the more idiomatic fix. Sorry for the noise.

[scanban]

about allocation retry count, in my use case it's possible that new interfaces appears in 5-10 per second. So one retry might not be enough tbh.

[njeisecke]

Mmh, the documentation https://docs.microsoft.com/en-us/windows/win32/api/iphlpapi/nf-iphlpapi-getadaptersaddresses#return-value is slightly vague. What does "multiple times" mean? Maybe the counter should actually be kept. In my environment, the initial call fails (probably due the various VPN virtual adapters on that machine), the second one works.

[LeoBorai]

Hi guys! Currently I dont have a Windows Environment I could test against.

Changes seems okay so far, have you had any issues with this version?

[LeoBorai]

It would be nice te have any memory usage audition tool integrated in CI! Im curious on the fact that both PR went up almost at same time 😅.

If its possible to share how you guys found these leaks, and if theres a way to automate such auditing, I am happy to integrate as part of existing Windows workflows!

[scanban]

Mmh, the documentation https://docs.microsoft.com/en-us/windows/win32/api/iphlpapi/nf-iphlpapi-getadaptersaddresses#return-value is slightly vague. What does "multiple times" mean? Maybe the counter should actually be kept. In my environment, the initial call fails (probably due the various VPN virtual adapters on that machine), the second one works.

In my use case, it's possible that new interface 'll be added after return from the 1st call to the GetAdaptersAddresses and second call to it. Ofc it's quite rare, but possible. I have multiple devices connected via USB and using RNDIS network interfaces and they are being powered up all at once. So I am getting multiple new interfaces in system in a very short time frame.

[njeisecke]

In my use case, it's possible that new interface 'll be added after return from the 1st call to the GetAdaptersAddresses and second call to it. Ofc it's quite rare, but possible. I have multiple devices connected via USB and using RNDIS network interfaces and they are being powered up all at once. So I am getting multiple new interfaces in system in a very short time frame.

There is no delay between the calls so it would be a rare incident for a third call to return a diifferent list than the second.

To me this sound more like you should provide some mechanism to query the adapter list in some interval und your control.

[scanban]

I am not sure, but I guess that windows are locking its device database during new device instantiation.
Google guys think so too: https://chromium.googlesource.com/chromium/src/+/HEAD/net/base/network_interfaces_win.cc#210

[njeisecke]

This now also contains the @scanban changes relevant to the leak fix.

[LeoBorai]

Great set of improvements @njeisecke and @scanban!

Thanks!