From 139795aef25b88df98f54c938580beece23e21f5 Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Fri, 3 May 2019 19:32:34 +0200 Subject: ui: fix thundering herd problem in importing/deleting Signed-off-by: Jason A. Donenfeld --- ui/listview.go | 30 ++++++++++++++++++---- ui/tests/Makefile | 30 ---------------------- ui/tests/confview.go | 70 ---------------------------------------------------- ui/tunnelspage.go | 40 +++++++++++++++++++++--------- 4 files changed, 53 insertions(+), 117 deletions(-) delete mode 100644 ui/tests/Makefile delete mode 100644 ui/tests/confview.go (limited to 'ui') 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() { -- cgit v1.2.3-59-g8ed1b