From 21aa23c7430ce9949d52a98c3f81b829bb5fdf2f Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Fri, 27 Sep 2019 10:40:59 +0200 Subject: winipcfg: make Unregister wait for callbacks to complete Signed-off-by: Jason A. Donenfeld --- tunnel/interfacewatcher.go | 53 +++++++++++++---------- tunnel/winipcfg/interface_change_handler.go | 13 ++++-- tunnel/winipcfg/route_change_handler.go | 13 ++++-- tunnel/winipcfg/unicast_address_change_handler.go | 13 ++++-- 4 files changed, 61 insertions(+), 31 deletions(-) diff --git a/tunnel/interfacewatcher.go b/tunnel/interfacewatcher.go index b7a07f77..d74db0e9 100644 --- a/tunnel/interfacewatcher.go +++ b/tunnel/interfacewatcher.go @@ -116,33 +116,42 @@ func (iw *interfaceWatcher) Configure(device *device.Device, conf *conf.Config, func (iw *interfaceWatcher) Destroy() { iw.setupMutex.Lock() - defer iw.setupMutex.Unlock() - - if iw.tun == nil { - return + routeChangeCallback4 := iw.routeChangeCallback4 + routeChangeCallback6 := iw.routeChangeCallback6 + interfaceChangeCallback := iw.interfaceChangeCallback + tun := iw.tun + iw.setupMutex.Unlock() + + if interfaceChangeCallback != nil { + interfaceChangeCallback.Unregister() + } + if routeChangeCallback4 != nil { + routeChangeCallback4.Unregister() + } + if routeChangeCallback6 != nil { + routeChangeCallback6.Unregister() } - if iw.routeChangeCallback4 != nil { - iw.routeChangeCallback4.Unregister() + iw.setupMutex.Lock() + if interfaceChangeCallback == iw.interfaceChangeCallback { + iw.interfaceChangeCallback = nil + } + if routeChangeCallback4 == iw.routeChangeCallback4 { iw.routeChangeCallback4 = nil } - if iw.routeChangeCallback6 != nil { - iw.routeChangeCallback6.Unregister() + if routeChangeCallback6 == iw.routeChangeCallback6 { iw.routeChangeCallback6 = nil } - if iw.interfaceChangeCallback != nil { - iw.interfaceChangeCallback.Unregister() - iw.interfaceChangeCallback = nil - } - firewall.DisableFirewall() - - // It seems that the Windows networking stack doesn't like it when we destroy interfaces that have active - // routes, so to be certain, just remove everything before destroying. - luid := winipcfg.LUID(iw.tun.LUID()) - luid.FlushRoutes(windows.AF_INET) - luid.FlushIPAddresses(windows.AF_INET) - luid.FlushRoutes(windows.AF_INET6) - luid.FlushIPAddresses(windows.AF_INET6) - luid.FlushDNS() + if tun != nil && iw.tun == tun { + // It seems that the Windows networking stack doesn't like it when we destroy interfaces that have active + // routes, so to be certain, just remove everything before destroying. + luid := winipcfg.LUID(tun.LUID()) + luid.FlushRoutes(windows.AF_INET) + luid.FlushIPAddresses(windows.AF_INET) + luid.FlushRoutes(windows.AF_INET6) + luid.FlushIPAddresses(windows.AF_INET6) + luid.FlushDNS() + } + iw.setupMutex.Unlock() } diff --git a/tunnel/winipcfg/interface_change_handler.go b/tunnel/winipcfg/interface_change_handler.go index 6af5ea3c..9406c18a 100644 --- a/tunnel/winipcfg/interface_change_handler.go +++ b/tunnel/winipcfg/interface_change_handler.go @@ -13,7 +13,8 @@ import ( // InterfaceChangeCallback structure allows interface change callback handling. type InterfaceChangeCallback struct { - cb func(notificationType MibNotificationType, iface *MibIPInterfaceRow) + cb func(notificationType MibNotificationType, iface *MibIPInterfaceRow) + wait sync.WaitGroup } var ( @@ -27,7 +28,7 @@ var ( // registered, the function will silently return. Returned InterfaceChangeCallback.Unregister method should be used // to unregister. func RegisterInterfaceChangeCallback(callback func(notificationType MibNotificationType, iface *MibIPInterfaceRow)) (*InterfaceChangeCallback, error) { - s := &InterfaceChangeCallback{callback} + s := &InterfaceChangeCallback{cb: callback} interfaceChangeAddRemoveMutex.Lock() defer interfaceChangeAddRemoveMutex.Unlock() @@ -59,6 +60,8 @@ func (callback *InterfaceChangeCallback) Unregister() error { removeIt := len(interfaceChangeCallbacks) == 0 && interfaceChangeHandle != 0 interfaceChangeMutex.Unlock() + callback.wait.Wait() + if removeIt { err := cancelMibChangeNotify2(interfaceChangeHandle) if err != nil { @@ -74,7 +77,11 @@ func interfaceChanged(callerContext uintptr, row *MibIPInterfaceRow, notificatio rowCopy := *row interfaceChangeMutex.Lock() for cb := range interfaceChangeCallbacks { - go cb.cb(notificationType, &rowCopy) + cb.wait.Add(1) + go func(cb *InterfaceChangeCallback) { + cb.cb(notificationType, &rowCopy) + cb.wait.Done() + }(cb) } interfaceChangeMutex.Unlock() return 0 diff --git a/tunnel/winipcfg/route_change_handler.go b/tunnel/winipcfg/route_change_handler.go index cafa0e97..1b4bad95 100644 --- a/tunnel/winipcfg/route_change_handler.go +++ b/tunnel/winipcfg/route_change_handler.go @@ -13,7 +13,8 @@ import ( // RouteChangeCallback structure allows route change callback handling. type RouteChangeCallback struct { - cb func(notificationType MibNotificationType, route *MibIPforwardRow2) + cb func(notificationType MibNotificationType, route *MibIPforwardRow2) + wait sync.WaitGroup } var ( @@ -27,7 +28,7 @@ var ( // registered, the function will silently return. Returned RouteChangeCallback.Unregister method should be used // to unregister. func RegisterRouteChangeCallback(callback func(notificationType MibNotificationType, route *MibIPforwardRow2)) (*RouteChangeCallback, error) { - s := &RouteChangeCallback{callback} + s := &RouteChangeCallback{cb: callback} routeChangeAddRemoveMutex.Lock() defer routeChangeAddRemoveMutex.Unlock() @@ -59,6 +60,8 @@ func (callback *RouteChangeCallback) Unregister() error { removeIt := len(routeChangeCallbacks) == 0 && routeChangeHandle != 0 routeChangeMutex.Unlock() + callback.wait.Wait() + if removeIt { err := cancelMibChangeNotify2(routeChangeHandle) if err != nil { @@ -74,7 +77,11 @@ func routeChanged(callerContext uintptr, row *MibIPforwardRow2, notificationType rowCopy := *row routeChangeMutex.Lock() for cb := range routeChangeCallbacks { - go cb.cb(notificationType, &rowCopy) + cb.wait.Add(1) + go func(cb *RouteChangeCallback) { + cb.cb(notificationType, &rowCopy) + cb.wait.Done() + }(cb) } routeChangeMutex.Unlock() return 0 diff --git a/tunnel/winipcfg/unicast_address_change_handler.go b/tunnel/winipcfg/unicast_address_change_handler.go index 1ec34cd4..5f8f2c96 100644 --- a/tunnel/winipcfg/unicast_address_change_handler.go +++ b/tunnel/winipcfg/unicast_address_change_handler.go @@ -13,7 +13,8 @@ import ( // UnicastAddressChangeCallback structure allows unicast address change callback handling. type UnicastAddressChangeCallback struct { - cb func(notificationType MibNotificationType, unicastAddress *MibUnicastIPAddressRow) + cb func(notificationType MibNotificationType, unicastAddress *MibUnicastIPAddressRow) + wait sync.WaitGroup } var ( @@ -27,7 +28,7 @@ var ( // registered, the function will silently return. Returned UnicastAddressChangeCallback.Unregister method should be used // to unregister. func RegisterUnicastAddressChangeCallback(callback func(notificationType MibNotificationType, unicastAddress *MibUnicastIPAddressRow)) (*UnicastAddressChangeCallback, error) { - s := &UnicastAddressChangeCallback{callback} + s := &UnicastAddressChangeCallback{cb: callback} unicastAddressChangeAddRemoveMutex.Lock() defer unicastAddressChangeAddRemoveMutex.Unlock() @@ -59,6 +60,8 @@ func (callback *UnicastAddressChangeCallback) Unregister() error { removeIt := len(unicastAddressChangeCallbacks) == 0 && unicastAddressChangeHandle != 0 unicastAddressChangeMutex.Unlock() + callback.wait.Wait() + if removeIt { err := cancelMibChangeNotify2(unicastAddressChangeHandle) if err != nil { @@ -74,7 +77,11 @@ func unicastAddressChanged(callerContext uintptr, row *MibUnicastIPAddressRow, n rowCopy := *row unicastAddressChangeMutex.Lock() for cb := range unicastAddressChangeCallbacks { - go cb.cb(notificationType, &rowCopy) + cb.wait.Add(1) + go func(cb *UnicastAddressChangeCallback) { + cb.cb(notificationType, &rowCopy) + cb.wait.Done() + }(cb) } unicastAddressChangeMutex.Unlock() return 0 -- cgit v1.2.3-59-g8ed1b