ui: fix thundering herd problem in importing/deleting

4 files changed, 53 insertions, 117 deletions

diff --git a/ui/listview.go b/ui/listview.go
index 78e53fda..c4f5186b 100644
--- a/ui/listview.go
+++ b/ui/listview.go
@@ -8,6 +8,7 @@ package ui
 import (
 	"sort"
 	"strings"
+	"sync/atomic"
 
 	"github.com/lxn/walk"
 	"golang.zx2c4.com/wireguard/windows/service"
@@ -26,6 +27,9 @@ func (t *ListModel) RowCount() int {
 }
 
 func (t *ListModel) Value(row, col int) interface{} {
+	if row < 0 || row >= len(t.tunnels) {
+		return nil
+	}
 	tunnel := t.tunnels[row]
 
 	switch col {
@@ -51,8 +55,9 @@ type ListView struct {
 
 	model *ListModel
 
-	tunnelChangedCB  *service.TunnelChangeCallback
-	tunnelsChangedCB *service.TunnelsChangeCallback
+	tunnelChangedCB        *service.TunnelChangeCallback
+	tunnelsChangedCB       *service.TunnelsChangeCallback
+	tunnelsUpdateSuspended int32
 }
 
 func NewListView(parent walk.Container) (*ListView, error) {
@@ -101,13 +106,17 @@ func (tv *ListView) Dispose() {
 }
 
 func (tv *ListView) StyleCell(style *walk.CellStyle) {
+	row := style.Row()
+	if row < 0 || row >= len(tv.model.tunnels) {
+		return
+	}
+	tunnel := &tv.model.tunnels[row]
+
 	canvas := style.Canvas()
 	if canvas == nil {
 		return
 	}
 
-	tunnel := &tv.model.tunnels[style.Row()]
-
 	b := style.Bounds()
 
 	b.X = b.Height
@@ -147,6 +156,17 @@ func (tv *ListView) onTunnelChange(tunnel *service.Tunnel, state service.TunnelS
 }
 
 func (tv *ListView) onTunnelsChange() {
+	if atomic.LoadInt32(&tv.tunnelsUpdateSuspended) == 0 {
+		tv.Load(true)
+	}
+}
+
+func (tv *ListView) SetSuspendTunnelsUpdate(suspend bool) {
+	if suspend {
+		atomic.AddInt32(&tv.tunnelsUpdateSuspended, 1)
+	} else {
+		atomic.AddInt32(&tv.tunnelsUpdateSuspended, -1)
+	}
 	tv.Load(true)
 }
 
@@ -171,7 +191,7 @@ func (tv *ListView) Load(asyncUI bool) {
 					//TODO: this is inefficient. Use a map here instead.
 					if t.Name == tunnel.Name {
 						tv.model.tunnels = append(tv.model.tunnels[:i], tv.model.tunnels[i+1:]...)
-						tv.model.PublishRowsRemoved(i, i)
+						tv.model.PublishRowsRemoved(i, i) //TODO: Do we have to call that everytime or can we pass a range?
 						break
 					}
 				}
diff --git a/ui/tests/Makefile b/ui/tests/Makefile
deleted file mode 100644
index c71af21d..00000000
--- a/ui/tests/Makefile
+++ /dev/null
@@ -1,30 +0,0 @@
-export CFLAGS := -O3 -Wall -std=gnu11
-export CC := x86_64-w64-mingw32-gcc
-WINDRES := x86_64-w64-mingw32-windres
-export CGO_ENABLED := 1
-export GOOS := windows
-export GOARCH := amd64
-
-DEPLOYMENT_HOST ?= winvm
-DEPLOYMENT_PATH ?= Desktop
-
-all: tests.exe
-
-resources.syso: ../../resources.rc ../../manifest.xml ../icon/icon.ico
-	$(WINDRES) -i $< -o $@ -O coff
-
-rwildcard=$(foreach d,$(wildcard $1*),$(call rwildcard,$d/,$2) $(filter $(subst *,%,$2),$d))
-tests.exe: resources.syso $(call rwildcard,..,*.go *.c *.h)
-	go build -ldflags="-s -w" -v -o $@
-
-deploy: tests.exe
-	-ssh $(DEPLOYMENT_HOST) -- 'taskkill /im tests.exe /f'
-	scp tests.exe $(DEPLOYMENT_HOST):$(DEPLOYMENT_PATH)
-
-run: tests.exe
-	wine tests.exe
-
-clean:
-	rm -rf resources.syso tests.exe
-
-.PHONY: deploy run clean all
diff --git a/ui/tests/confview.go b/ui/tests/confview.go
deleted file mode 100644
index 6907d83d..00000000
--- a/ui/tests/confview.go
+++ /dev/null
@@ -1,70 +0,0 @@
-/* SPDX-License-Identifier: MIT
- *
- * Copyright (C) 2019 WireGuard LLC. All Rights Reserved.
- */
-
-package main
-
-import (
-	"github.com/lxn/walk"
-	"golang.zx2c4.com/wireguard/windows/conf"
-	"golang.zx2c4.com/wireguard/windows/ui"
-	"log"
-	"runtime"
-)
-
-
-const demoConfig = `[Interface]
-PrivateKey = 6KpcbNFK4tKBciKBT2Rj6Z/sHBqxdV+p+nuNA5AlWGI=
-Address = 192.168.4.84/24
-DNS = 8.8.8.8, 8.8.4.4, 1.1.1.1, 1.0.0.1
-
-[Peer]
-PublicKey = JRI8Xc0zKP9kXk8qP84NdUQA04h6DLfFbwJn4g+/PFs=
-Endpoint = demo.wireguard.com:12912
-AllowedIPs = 0.0.0.0/0
-`
-
-func main(){
-	mw, _ := walk.NewMainWindowWithName("Test ConfView")
-	mw.SetSize(walk.Size{600, 800})
-	mw.SetLayout(walk.NewVBoxLayout())
-	cv, err := ui.NewConfView(mw)
-	if err != nil {
-		log.Fatal(err)
-	}
-	config, _ := conf.FromWgQuick(demoConfig, "demo")
-	peer := config.Peers[0]
-	config.Peers = make([]conf.Peer, 0)
-
-	pb1, _ := walk.NewPushButton(mw)
-	pb1.SetText("Add and increment")
-	pb1.Clicked().Attach(func() {
-		config.Interface.ListenPort++
-		config.Peers = append(config.Peers, peer)
-		k,_ :=  conf.NewPrivateKey()
-		config.Peers[len(config.Peers) - 1].PublicKey = *k
-		cv.SetConfiguration(config)
-	})
-	pb2, _ := walk.NewPushButton(mw)
-	pb2.SetText("Remove first peer")
-	pb2.Clicked().Attach(func() {
-		if len(config.Peers) < 1 {
-			return
-		}
-		config.Interface.ListenPort--
-		config.Peers = config.Peers[1:]
-		cv.SetConfiguration(config)
-	})
-	pb3, _ := walk.NewPushButton(mw)
-	pb3.SetText("Toggle MTU")
-	pb3.Clicked().Attach(func() {
-		config.Interface.Mtu = (config.Interface.Mtu + 1) % 2
-		cv.SetConfiguration(config)
-	})
-	mw.SetVisible(true)
-	mw.Show()
-	mw.Activate()
-	mw.Run()
-	runtime.KeepAlive(cv)
-}
\ No newline at end of file
diff --git a/ui/tunnelspage.go b/ui/tunnelspage.go
index 8207099f..18e9e255 100644
--- a/ui/tunnelspage.go
+++ b/ui/tunnelspage.go
@@ -261,6 +261,7 @@ func (tp *TunnelsPage) importFiles(paths []string) {
 		}
 
 		configCount := 0
+		tp.listView.SetSuspendTunnelsUpdate(true)
 		for _, unparsedConfig := range unparsedConfigs {
 			if existingLowerTunnels[strings.ToLower(unparsedConfig.Name)] {
 				lastErr = fmt.Errorf("Another tunnel already exists with the name ‘%s’", unparsedConfig.Name)
@@ -278,7 +279,7 @@ func (tp *TunnelsPage) importFiles(paths []string) {
 			}
 			configCount++
 		}
-		tp.listView.Load(true)
+		tp.listView.SetSuspendTunnelsUpdate(false)
 
 		m, n := configCount, len(unparsedConfigs)
 		switch {
@@ -326,13 +327,6 @@ func (tp *TunnelsPage) addTunnel(config *conf.Config) {
 
 }
 
-func (tp *TunnelsPage) deleteTunnel(tunnel *service.Tunnel) {
-	err := tunnel.Delete()
-	if err != nil {
-		walk.MsgBox(tp.Form(), "Unable to delete tunnel", err.Error(), walk.MsgBoxIconError)
-	}
-}
-
 // Handlers
 
 func (tp *TunnelsPage) onTunnelsViewItemActivated() {
@@ -417,13 +411,35 @@ func (tp *TunnelsPage) onDelete() {
 		}
 		selectTunnelAfter = tp.listView.model.tunnels[max].Name
 	}
-
-	for _, i := range indices {
-		tp.deleteTunnel(&tp.listView.model.tunnels[i])
-	}
 	if len(selectTunnelAfter) > 0 {
 		tp.listView.selectTunnel(selectTunnelAfter)
 	}
+
+	tunnelsToDelete := make([]service.Tunnel, len(indices))
+	for i, j := range indices {
+		tunnelsToDelete[i] = tp.listView.model.tunnels[j]
+
+	}
+	go func() {
+		tp.listView.SetSuspendTunnelsUpdate(true)
+		var errors []error
+		for _, tunnel := range tunnelsToDelete {
+			err := tunnel.Delete()
+			if err != nil && (len(errors) == 0 || errors[len(errors)-1].Error() != err.Error()) {
+				errors = append(errors, err)
+			}
+		}
+		tp.listView.SetSuspendTunnelsUpdate(false)
+		if len(errors) > 0 {
+			tp.listView.Synchronize(func() {
+				if len(errors) == 1 {
+					walk.MsgBox(tp.Form(), "Unable to delete tunnel", fmt.Sprintf("A tunnel was unable to be removed: %s", errors[0].Error()), walk.MsgBoxIconError)
+				} else {
+					walk.MsgBox(tp.Form(), "Unable to delete tunnels", fmt.Sprintf("%d tunnels were unable to be removed.", len(errors)), walk.MsgBoxIconError)
+				}
+			})
+		}
+	}()
 }
 
 func (tp *TunnelsPage) onImport() {