Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 3 additions & 2 deletions endpoints/ac/msghandler.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ const (
)

func (a *UdpAC) HandleUdpACOperations(ppd *core.PacketParserData) (err error) {
defer a.wg.Done()

acId := a.config.ACId
dopMsg := &common.ServerACOpsMsg{}
artMsg := &common.ACOpsResultMsg{}
Expand Down Expand Up @@ -611,6 +609,7 @@ func (a *UdpAC) HandleAccessControl(au *common.AgentUser, srcAddrs []*common.Net

func (a *UdpAC) tcpTempAccessHandler(listener *net.TCPListener, timeoutSec int, dstAddrs []*common.NetAddress, openTimeSec int) {
defer a.wg.Done()
defer a.recoverUDPHandler(core.NHP_ACC)
defer listener.Close()

// accept only the first incoming tcp connection
Expand Down Expand Up @@ -745,6 +744,7 @@ func (a *UdpAC) tcpTempAccessHandler(listener *net.TCPListener, timeoutSec int,

func (a *UdpAC) udpTempAccessHandler(conn *net.UDPConn, timeoutSec int, dstAddrs []*common.NetAddress, openTimeSec int) {
defer a.wg.Done()
defer a.recoverUDPHandler(core.NHP_ACC)
defer conn.Close()
// listen to accept and handle only one incoming connection
startTime := time.Now()
Expand Down Expand Up @@ -908,6 +908,7 @@ func (a *UdpAC) udpTempAccessHandler(conn *net.UDPConn, timeoutSec int, dstAddrs
}

func (a *UdpAC) tempConnTerminator(conn net.Conn, ctx context.Context) {
defer a.recoverUDPHandler(core.NHP_ACC)
select {
case <-a.signals.stop:
conn.Close()
Expand Down
57 changes: 57 additions & 0 deletions endpoints/ac/udp_handler_panic_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
package ac

import (
"sync/atomic"
"testing"
"time"

"github.com/OpenNHP/opennhp/nhp/core"
)

func TestRunUDPHandlerRecoversPanicAndReleasesWaitGroup(t *testing.T) {
a := &UdpAC{config: &Config{ACId: "test-ac"}}
a.wg.Add(1)

done := make(chan struct{})
go func() {
a.runUDPHandler(core.NHP_AOP, func() { panic("malformed packet") })
close(done)
}()

select {
case <-done:
case <-time.After(time.Second):
t.Fatal("panicking UDP handler did not return")
}

waitDone := make(chan struct{})
go func() {
a.wg.Wait()
close(waitDone)
}()
select {
case <-waitDone:
case <-time.After(time.Second):
t.Fatal("panicking UDP handler did not release UdpAC wait group")
}
}

func TestRunUDPHandlerHappyPath(t *testing.T) {
a := &UdpAC{}
a.wg.Add(1)
var called atomic.Bool

a.runUDPHandler(core.NHP_AOP, func() { called.Store(true) })
a.wg.Wait()

if !called.Load() {
t.Fatal("handler was not called")
}
}

func TestRecoverUDPHandlerNilConfigIsSafe(t *testing.T) {
a := &UdpAC{}
a.wg.Add(1)
a.runUDPHandler(core.NHP_AOP, func() { panic("nil config") })
a.wg.Wait()
}
26 changes: 24 additions & 2 deletions endpoints/ac/udpac.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"net"
"path/filepath"
"runtime/debug"
"sync"
"sync/atomic"
"time"
Expand Down Expand Up @@ -523,15 +524,36 @@ func (a *UdpAC) recvMessageRoutine() {
switch ppd.HeaderType {
case core.NHP_AOP:
// deal with NHP_AOP message
p := ppd
a.wg.Add(1)
go func(p *core.PacketParserData) {
go a.runUDPHandler(p.HeaderType, func() {
_ = a.HandleUdpACOperations(p)
}(ppd)
})
}
}
}
}

// runUDPHandler contains panics from input-driven handler goroutines. A
// malformed packet or an unexpected nil in one handler must drop that request,
// not terminate the entire access-controller process.
func (a *UdpAC) runUDPHandler(headerType int, handler func()) {
defer a.wg.Done()
defer a.recoverUDPHandler(headerType)
handler()
}

func (a *UdpAC) recoverUDPHandler(headerType int) {
if recovered := recover(); recovered != nil {
acID := "unknown"
if a != nil && a.config != nil && a.config.ACId != "" {
acID = a.config.ACId
}
log.Error("ac(%s)[%s] UDP handler panic recovered: %v\n%s",
acID, core.HeaderTypeToString(headerType), recovered, debug.Stack())
}
}

// keep interaction between ac and server in certain time interval to keep outwards ip path active
func (a *UdpAC) maintainServerConnectionRoutine() {
defer a.wg.Done()
Expand Down
Loading