mirror of
https://github.com/ngrok/ngrok-operator.git
synced 2026-05-17 16:50:44 +00:00
Move mutex into portBitmap so it is self-contained thread-safe
The external portAllocatorMu in BoundEndpointPoller protected the portAllocator pointer from being swapped while other goroutines called methods on it. Instead, add a Replace method to portBitmap that atomically swaps the internal bitmap state under its own mutex, removing the need for callers to manage locking. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -4,7 +4,6 @@ import (
|
||||
"context"
|
||||
"fmt"
|
||||
"reflect"
|
||||
"sync"
|
||||
"time"
|
||||
|
||||
"github.com/go-logr/logr"
|
||||
@@ -65,11 +64,8 @@ type BoundEndpointPoller struct {
|
||||
// If draining, polling is skipped to prevent creating new resources.
|
||||
DrainState DrainState
|
||||
|
||||
// portAllocatorMu protects concurrent access to portAllocator.
|
||||
// The polling goroutine reassigns it while reconcile action goroutines call methods on it.
|
||||
portAllocatorMu sync.Mutex
|
||||
|
||||
// portAllocator manages the unique port allocations
|
||||
// portAllocator manages the unique port allocations.
|
||||
// portBitmap is internally thread-safe.
|
||||
portAllocator *portBitmap
|
||||
|
||||
// Channel to stop the API polling goroutine
|
||||
@@ -249,9 +245,7 @@ func (r *BoundEndpointPoller) reconcileBoundEndpointsFromAPI(ctx context.Context
|
||||
}
|
||||
|
||||
// reassign port allocations
|
||||
r.portAllocatorMu.Lock()
|
||||
r.portAllocator = currentPortAllocations
|
||||
r.portAllocatorMu.Unlock()
|
||||
r.portAllocator.Replace(currentPortAllocations)
|
||||
|
||||
toCreate, toUpdate, toDelete := r.filterBoundEndpointActions(ctx, existingBoundEndpoints, desiredBoundEndpoints)
|
||||
|
||||
@@ -384,9 +378,7 @@ func (r *BoundEndpointPoller) createBinding(ctx context.Context, desired binding
|
||||
name := hashURL(desired.Spec.GetEndpointURL())
|
||||
|
||||
// allocate a port
|
||||
r.portAllocatorMu.Lock()
|
||||
port, err := r.portAllocator.SetAny()
|
||||
r.portAllocatorMu.Unlock()
|
||||
if err != nil {
|
||||
r.Log.Error(err, "Failed to allocate port for BoundEndpoint", "name", name, "url", desired.Spec.GetEndpointURL())
|
||||
return err
|
||||
@@ -548,11 +540,9 @@ func (r *BoundEndpointPoller) deleteBinding(ctx context.Context, boundEndpoint b
|
||||
log.Info("Deleted BoundEndpoint", "name", boundEndpoint.Name, "url", boundEndpoint.Spec.GetEndpointURL())
|
||||
|
||||
// unset the port allocation
|
||||
r.portAllocatorMu.Lock()
|
||||
if err := r.portAllocator.Unset(boundEndpoint.Spec.Port); err != nil {
|
||||
log.Error(err, "Failed to unset port allocation", "port", boundEndpoint.Spec.Port, "name", boundEndpoint.Name)
|
||||
}
|
||||
r.portAllocatorMu.Unlock()
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -76,3 +76,11 @@ func (pb *portBitmap) NumFree() uint64 {
|
||||
defer pb.mu.Unlock()
|
||||
return pb.ports.Unselected()
|
||||
}
|
||||
|
||||
// Replace atomically swaps the internal bitmap state with that of another portBitmap.
|
||||
// The other portBitmap should not be used concurrently during this call.
|
||||
func (pb *portBitmap) Replace(other *portBitmap) {
|
||||
pb.mu.Lock()
|
||||
defer pb.mu.Unlock()
|
||||
pb.ports = other.ports
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user