Skip to content

Commit

Permalink
Fix unsynchronized use of TacticsServer.filterMatches
Browse files Browse the repository at this point in the history
  • Loading branch information
rod-hynes committed Nov 4, 2024
1 parent 0247401 commit d58a1c0
Showing 1 changed file with 26 additions and 10 deletions.
36 changes: 26 additions & 10 deletions psiphon/common/tactics/tactics.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ import (
"net/http"
"sort"
"strings"
"sync"
"time"

"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common"
Expand Down Expand Up @@ -255,7 +256,7 @@ type Server struct {
apiParameterValidator common.APIParameterValidator

cachedTacticsData *lrucache.Cache
filterMatches []bool
filterMatches *sync.Pool
}

const (
Expand Down Expand Up @@ -468,6 +469,9 @@ func NewServer(
return errors.Trace(err)
}

// Server.ReloadableFile.RWMutex is the mutex for accessing
// these and other Server fields.

// Modify actual traffic rules only after validation
server.RequestPublicKey = newServer.RequestPublicKey
server.RequestPrivateKey = newServer.RequestPrivateKey
Expand All @@ -477,15 +481,23 @@ func NewServer(

// Any cached, merged tactics data is flushed when the
// configuration changes.
//
// A single filterMatches, used in getTactics, is allocated here
// to avoid allocating a slice per getTactics call.
//
// Server.ReloadableFile.RLock/RUnlock is the mutex for accessing
// these and other Server fields.

server.cachedTacticsData.Flush()
server.filterMatches = make([]bool, len(server.FilteredTactics))

// A pool of filterMatches, used in getTactics, is used to avoid
// allocating a slice for every getTactics call.
//
// A pointer to a slice is used with sync.Pool to avoid an
// allocation on Put, as would happen if passing in a slice
// instead of a pointer; see
// https://github.com/dominikh/go-tools/issues/1042#issuecomment-869064445

server.filterMatches = &sync.Pool{
New: func() any {
b := make([]bool, len(server.FilteredTactics))
return &b
},
}

server.initLookups()

Expand Down Expand Up @@ -874,8 +886,12 @@ func (server *Server) getTactics(
var aggregatedValues map[string]int
filterMatchCount := 0

// Use the preallocated slice to avoid an allocation per getTactics call.
filterMatches := server.filterMatches
// Use the filterMatches buffer pool to avoid an allocation per getTactics
// call.
b := server.filterMatches.Get().(*[]bool)
filterMatches := *b
clear(filterMatches)
defer server.filterMatches.Put(b)

for filterIndex, filteredTactics := range server.FilteredTactics {

Expand Down

0 comments on commit d58a1c0

Please sign in to comment.