dashboard / erock/pico / fix: tunkit memory leaks #93 rss

open · opened on 2025-12-16T04:16:46Z by erock
Help
checkout latest patchset:
ssh pr.pico.sh print pr-93 | git am -3
checkout any patchset in a patch request:
ssh pr.pico.sh print ps-X | git am -3
add changes to patch request:
git format-patch main --stdout | ssh pr.pico.sh pr add 93
add review to patch request:
git format-patch main --stdout | ssh pr.pico.sh pr add --review 93
accept PR:
ssh pr.pico.sh pr accept 93
close PR:
ssh pr.pico.sh pr close 93

Logs

erock created pr with ps-170 on 2025-12-16T04:16:46Z

Patchsets

ps-170 by erock on 2025-12-16T04:16:46Z

Patchset ps-170

fix: tunkit memory leaks

Eric Bower
2025-12-16T04:13:11Z
Back to top
- Context leak: Added defer cancel() immediately after creating the context to ensure it's always cancelled when the function returns
- Goroutine coordination: Removed the outer goroutine wrapper - the handler now runs synchronously, waiting for io.Copy goroutines to complete via wg.Wait() before calling handler.Close()
- Simplified cleanup: Moved connection closing to after wg.Wait() to ensure both copy operations complete before closing resources
- Removed dead code: The <-ctx.Done() was waiting forever since nothing called cancel() - now the function naturally completes when the io.Copy operations finish (when either side closes the connection)
+30 -41 pkg/tunkit/ptun.go link
 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
diff --git a/pkg/tunkit/ptun.go b/pkg/tunkit/ptun.go
index 9faa754..df256b9 100644
--- a/pkg/tunkit/ptun.go
+++ b/pkg/tunkit/ptun.go
@@ -53,6 +53,8 @@ func LocalForwardHandler(handler Tunnel) pssh.SSHServerChannelMiddleware {
 		}
 
 		origCtx, cancel := context.WithCancel(context.Background())
+		defer cancel()
+
 		ctx := &pssh.SSHServerConnSession{
 			Channel:       ch,
 			SSHServerConn: sc,
@@ -62,51 +64,38 @@ func LocalForwardHandler(handler Tunnel) pssh.SSHServerChannelMiddleware {
 
 		go ssh.DiscardRequests(reqs)
 
+		downConn, err := handler.CreateConn(ctx)
+		if err != nil {
+			log.Error("unable to connect to conn", "err", err)
+			_ = ch.Close()
+			return err
+		}
+
+		var wg sync.WaitGroup
+		wg.Add(2)
+
 		go func() {
-			downConn, err := handler.CreateConn(ctx)
-			if err != nil {
-				log.Error("unable to connect to conn", "err", err)
-				_ = ch.Close()
-				return
+			defer wg.Done()
+			_, err := io.Copy(ch, downConn)
+			if err != nil && !errors.Is(err, net.ErrClosed) {
+				log.Error("io copy", "err", err)
 			}
-			defer func() {
-				_ = downConn.Close()
-			}()
-
-			var wg sync.WaitGroup
-			wg.Add(2)
-
-			go func() {
-				defer wg.Done()
-				defer func() {
-					_ = ch.CloseWrite()
-					_ = downConn.Close()
-				}()
-				_, err := io.Copy(ch, downConn)
-				if err != nil {
-					if !errors.Is(err, net.ErrClosed) {
-						log.Error("io copy", "err", err)
-					}
-				}
-			}()
-			go func() {
-				defer wg.Done()
-				defer func() {
-					_ = ch.Close()
-					_ = downConn.Close()
-				}()
-				_, err := io.Copy(downConn, ch)
-				if err != nil {
-					if !errors.Is(err, net.ErrClosed) {
-						log.Error("io copy", "err", err)
-					}
-				}
-			}()
-
-			wg.Wait()
+			_ = ch.CloseWrite()
 		}()
 
-		<-ctx.Done()
+		go func() {
+			defer wg.Done()
+			_, err := io.Copy(downConn, ch)
+			if err != nil && !errors.Is(err, net.ErrClosed) {
+				log.Error("io copy", "err", err)
+			}
+			_ = downConn.Close()
+		}()
+
+		wg.Wait()
+		_ = ch.Close()
+		_ = downConn.Close()
+
 		err = handler.Close(ctx)
 		if err != nil {
 			log.Error("tunnel handler error", "err", err)