Skip to content

Improve udp tunnel stability #406

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
magnww wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Improve udp tunnel stability #406

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
d511a10
add a mutex to activatingConn.Wait()
magnww Feb 5, 2023
1edbbeb
retry when ssh tunnel is not established
magnww Feb 5, 2023
db8c112
retry when ssh tunnel is not established
magnww Feb 5, 2023
9d3955f
do not exit the loop when the connection is broken
magnww Feb 6, 2023
f2dd211
remove ssh wait in udp mode
magnww Feb 6, 2023
6bc9491
remove getSSHNoWait()
magnww Aug 20, 2023
d901c92
format
magnww Aug 20, 2023
bddeacc
Fix activatingConnWait() not waiting when connection is lost
magnww Aug 21, 2023
4cf9a12
add tunnel.Close() method to solve possible memory leaks
magnww Aug 21, 2023
afa47e8
Merge branch 'jpillora:master' into master
magnww Jul 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions server/server_handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ func (s *Server) handleWebsocket(w http.ResponseWriter, req *http.Request) {
Socks: s.config.Socks5,
KeepAlive: s.config.KeepAlive,
})
defer tunnel.Close()
//bind
eg, ctx := errgroup.WithContext(req.Context())
eg.Go(func() error {
Expand Down
5 changes: 4 additions & 1 deletion share/tunnel/tunnel.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,10 @@ func New(c Config) *Tunnel {
return t
}

func (t *Tunnel) Close() {
t.activatingConn.DoneAll()

@jpillora jpillora Aug 22, 2023

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need a lock/unlock + nil check?

@magnww magnww Aug 22, 2023

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

activatingConn is a waitGroup, it is value type and thread safe, so no need for lock and nil checks

@jpillora jpillora Aug 23, 2023

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah yea that's right, its a custom wait group (which might be a bad idea itself - anyway)

it should block while disconnected/connecting and it should not-block while connected

DoneAll here would stop blocking, but Tunnel Done means closed?

how does this improve UDP? sorry, I think I'm missing something

@magnww magnww Aug 23, 2023

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the original code, when the connection is lost, DoneAll in the BindSSH method will be called, which causes activatingConnWait() to not work and cannot block.

This leads to high resource usage of runOutbound in udpListener. Calling u.getUDPChan(ctx) will immediately gets nil instead of blocking.

Close is added to correctly release resources in server mode, because DoneAll was removed from BindSSH.

In client mode, there is no need to call DoneAll because the same tunnel is always used.

}

//BindSSH provides an active SSH for use for tunnelling
func (t *Tunnel) BindSSH(ctx context.Context, c ssh.Conn, reqs <-chan *ssh.Request, chans <-chan ssh.NewChannel) error {
//link ctx to ssh-conn
Expand All @@ -76,7 +80,6 @@ func (t *Tunnel) BindSSH(ctx context.Context, c ssh.Conn, reqs <-chan *ssh.Reque
if c.Close() == nil {
t.Debugf("SSH cancelled")
}
t.activatingConn.DoneAll()
}()
//mark active and unblock
t.activeConnMut.Lock()
Expand Down
18 changes: 12 additions & 6 deletions share/tunnel/tunnel_in_proxy_udp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,15 +100,17 @@ func (u *udpListener) runInbound(ctx context.Context) error {
if strings.HasSuffix(err.Error(), "EOF") {
continue
}
return u.Errorf("inbound-udpchan: %w", err)
u.Errorf("inbound-udpchan: %w", err)
continue
}
//send over channel, including source address
b := buff[:n]
if err := uc.encode(addr.String(), b); err != nil {
if strings.HasSuffix(err.Error(), "EOF") {
continue //dropped packet...
}
return u.Errorf("encode error: %w", err)
u.Errorf("encode error: %w", err)
continue
}
//stats
atomic.AddInt64(&u.sent, int64(n))
Expand All @@ -124,24 +126,28 @@ func (u *udpListener) runOutbound(ctx context.Context) error {
if strings.HasSuffix(err.Error(), "EOF") {
continue
}
return u.Errorf("outbound-udpchan: %w", err)
u.Errorf("outbound-udpchan: %w", err)
continue
}
//receive from channel, including source address
p := udpPacket{}
if err := uc.decode(&p); err == io.EOF {
//outbound ssh disconnected, get new connection...
continue
} else if err != nil {
return u.Errorf("decode error: %w", err)
u.Errorf("decode error: %w", err)
continue
}
//write back to inbound udp
addr, err := net.ResolveUDPAddr("udp", p.Src)
if err != nil {
return u.Errorf("resolve error: %w", err)
u.Errorf("resolve error: %w", err)
continue
}
n, err := u.inbound.WriteToUDP(p.Payload, addr)
if err != nil {
return u.Errorf("write error: %w", err)
u.Errorf("write error: %w", err)
continue
}
//stats
atomic.AddInt64(&u.recv, int64(n))
Expand Down