From 3f636a038c87f317a728aa3af4d034d726a7f8fd Mon Sep 17 00:00:00 2001 From: Quentin McGaw Date: Mon, 21 Oct 2024 12:45:51 +0200 Subject: [PATCH] wip --- .../configuration/settings/shadowsocks.go | 4 +- internal/shadowsocks/loop.go | 149 +++++++----------- internal/shadowsocks/noop.go | 14 ++ internal/shadowsocks/service.go | 67 ++++++++ internal/shadowsocks/state.go | 93 ++--------- 5 files changed, 155 insertions(+), 172 deletions(-) create mode 100644 internal/shadowsocks/noop.go create mode 100644 internal/shadowsocks/service.go diff --git a/internal/configuration/settings/shadowsocks.go b/internal/configuration/settings/shadowsocks.go index 7e11b74e..2ed2fab1 100644 --- a/internal/configuration/settings/shadowsocks.go +++ b/internal/configuration/settings/shadowsocks.go @@ -22,7 +22,7 @@ func (s Shadowsocks) validate() (err error) { return s.Settings.Validate() } -func (s *Shadowsocks) copy() (copied Shadowsocks) { +func (s *Shadowsocks) Copy() (copied Shadowsocks) { return Shadowsocks{ Enabled: gosettings.CopyPointer(s.Enabled), Settings: s.Settings.Copy(), @@ -32,7 +32,7 @@ func (s *Shadowsocks) copy() (copied Shadowsocks) { // overrideWith overrides fields of the receiver // settings object with any field set in the other // settings. -func (s *Shadowsocks) overrideWith(other Shadowsocks) { +func (s *Shadowsocks) OverrideWith(other Shadowsocks) { s.Enabled = gosettings.OverrideWithPointer(s.Enabled, other.Enabled) s.Settings.OverrideWith(other.Settings) } diff --git a/internal/shadowsocks/loop.go b/internal/shadowsocks/loop.go index 66355619..68bd6868 100644 --- a/internal/shadowsocks/loop.go +++ b/internal/shadowsocks/loop.go @@ -2,14 +2,11 @@ package shadowsocks import ( "context" - "fmt" - "sync" "time" "github.com/qdm12/gluetun/internal/configuration/settings" "github.com/qdm12/gluetun/internal/constants" "github.com/qdm12/gluetun/internal/models" - shadowsockslib "github.com/qdm12/ss-server/pkg/tcpudp" ) type Loop struct { @@ -17,11 +14,10 @@ type Loop struct { // Other objects logger Logger // Internal channels and locks - loopLock sync.Mutex - running chan models.LoopStatus - stop, stopped chan struct{} - start chan struct{} - backoffTime time.Duration + refreshing bool + refresh chan struct{} + changed chan models.LoopStatus + backoffTime time.Duration runCancel context.CancelFunc runDone <-chan struct{} @@ -36,10 +32,8 @@ func NewLoop(settings settings.Shadowsocks, logger Logger) *Loop { settings: settings, }, logger: logger, - start: make(chan struct{}), - running: make(chan models.LoopStatus), - stop: make(chan struct{}), - stopped: make(chan struct{}), + refresh: make(chan struct{}, 1), // capacity of 1 to handle crash auto-restart + changed: make(chan models.LoopStatus), backoffTime: defaultBackoffTime, } } @@ -55,13 +49,6 @@ func (l *Loop) Start(ctx context.Context) (runError <-chan error, err error) { <-ready - if *l.GetSettings().Enabled { - _, err = l.SetStatus(ctx, constants.Running) - if err != nil { - return nil, fmt.Errorf("setting running status: %w", err) - } - } - return nil, nil //nolint:nilnil } @@ -69,80 +56,61 @@ func (l *Loop) run(ctx context.Context, ready, done chan<- struct{}) { defer close(done) close(ready) - select { - case <-l.start: - case <-ctx.Done(): - return - } - - crashed := false - for ctx.Err() == nil { + // What if update and crash at the same time ish? settings := l.GetSettings() - server, err := shadowsockslib.NewServer(settings.Settings, l.logger) - if err != nil { - crashed = true - l.logAndWait(ctx, err) - continue - } - shadowsocksCtx, shadowsocksCancel := context.WithCancel(ctx) - - waitError := make(chan error) - go func() { - waitError <- server.Listen(shadowsocksCtx) - }() - if err != nil { - crashed = true - shadowsocksCancel() - l.logAndWait(ctx, err) - continue - } - - isStableTimer := time.NewTimer(time.Second) - - stayHere := true - for stayHere { - select { - case <-ctx.Done(): - shadowsocksCancel() - <-waitError - close(waitError) - return - case <-isStableTimer.C: - if !crashed { - l.running <- constants.Running - crashed = false - } else { - l.backoffTime = defaultBackoffTime - l.state.setStatusWithLock(constants.Running) - } - case <-l.start: - l.logger.Info("starting") - shadowsocksCancel() - <-waitError - close(waitError) - stayHere = false - case <-l.stop: - l.logger.Info("stopping") - shadowsocksCancel() - <-waitError - close(waitError) - l.stopped <- struct{}{} - case err := <-waitError: // unexpected error - shadowsocksCancel() - close(waitError) - if ctx.Err() != nil { - return - } - l.state.setStatusWithLock(constants.Crashed) - l.logAndWait(ctx, err) - crashed = true - stayHere = false + var service *service + var runError <-chan error + var err error + if *settings.Enabled { + service = newService(settings.Settings, l.logger) + runError, err = service.Start(ctx) + if err != nil { + runErrorCh := make(chan error, 1) + runError = runErrorCh + runErrorCh <- err + } else if l.refreshing { + l.changed <- constants.Running + } else { // auto-restart due to crash + l.state.setStatusWithLock(constants.Running) + l.backoffTime = defaultBackoffTime + } + } else { + if l.refreshing { + l.changed <- constants.Stopped + } else { // auto-restart due to crash + l.state.setStatusWithLock(constants.Stopped) + l.backoffTime = defaultBackoffTime } } - shadowsocksCancel() // repetition for linter only - isStableTimer.Stop() + l.refreshing = false + + select { + case <-l.refresh: + l.refreshing = true + if service != nil { + err = service.Stop() + if err != nil { + l.logger.Error("stopping service: " + err.Error()) + } + } + case err = <-runError: + if l.refreshing { + l.changed <- constants.Crashed + } else { + l.state.setStatusWithLock(constants.Crashed) + } + l.logAndWait(ctx, err) + case <-ctx.Done(): + if service != nil { + err = service.Stop() + if err != nil { + l.logger.Error("stopping service: " + err.Error()) + } + } + return + } } } @@ -162,8 +130,7 @@ func (l *Loop) logAndWait(ctx context.Context, err error) { select { case <-timer.C: case <-ctx.Done(): - if !timer.Stop() { - <-timer.C - } + _ = timer.Stop() + case <-l.refresh: // user-triggered refresh } } diff --git a/internal/shadowsocks/noop.go b/internal/shadowsocks/noop.go new file mode 100644 index 00000000..4860b1d0 --- /dev/null +++ b/internal/shadowsocks/noop.go @@ -0,0 +1,14 @@ +package shadowsocks + +import "context" + +type noopService struct{} + +func (n *noopService) Start(_ context.Context) ( + runError <-chan error, err error) { + return nil, nil +} + +func (n *noopService) Stop() (err error) { + return nil +} diff --git a/internal/shadowsocks/service.go b/internal/shadowsocks/service.go new file mode 100644 index 00000000..d5912d38 --- /dev/null +++ b/internal/shadowsocks/service.go @@ -0,0 +1,67 @@ +package shadowsocks + +import ( + "context" + "fmt" + "time" + + "github.com/qdm12/ss-server/pkg/tcpudp" +) + +type service struct { + // Injected settings + settings tcpudp.Settings + logger Logger + // Internal fields + cancel context.CancelFunc + done <-chan struct{} +} + +func newService(settings tcpudp.Settings, + logger Logger) *service { + return &service{ + settings: settings, + logger: logger, + } +} + +func (s *service) Start(ctx context.Context) (runError <-chan error, err error) { + server, err := tcpudp.NewServer(s.settings, s.logger) + if err != nil { + return nil, fmt.Errorf("creating server: %w", err) + } + + shadowsocksCtx, shadowsocksCancel := context.WithCancel(context.Background()) + s.cancel = shadowsocksCancel + runErrorCh := make(chan error) + done := make(chan struct{}) + s.done = done + go func() { + defer close(done) + err = server.Listen(shadowsocksCtx) + if shadowsocksCtx.Err() == nil { + runErrorCh <- fmt.Errorf("listening: %w", err) + } + }() + + const minStabilityTime = 100 * time.Millisecond + isStableTimer := time.NewTimer(minStabilityTime) + select { + case <-isStableTimer.C: + case err = <-runErrorCh: + return nil, fmt.Errorf("server became unstable within %s: %w", + minStabilityTime, err) + case <-ctx.Done(): + shadowsocksCancel() + <-done + return nil, ctx.Err() + } + + return runErrorCh, nil +} + +func (s *service) Stop() (err error) { + s.cancel() + <-s.done + return nil +} diff --git a/internal/shadowsocks/state.go b/internal/shadowsocks/state.go index 03d5b79c..7b8f9b4f 100644 --- a/internal/shadowsocks/state.go +++ b/internal/shadowsocks/state.go @@ -1,14 +1,10 @@ package shadowsocks import ( - "context" - "errors" - "fmt" "reflect" "sync" "github.com/qdm12/gluetun/internal/configuration/settings" - "github.com/qdm12/gluetun/internal/constants" "github.com/qdm12/gluetun/internal/models" ) @@ -25,95 +21,34 @@ func (s *state) setStatusWithLock(status models.LoopStatus) { s.status = status } +// GetStatus returns the status of the loop for informative purposes. +// In no case it should be used programmatically to avoid any +// TOCTOU race conditions. func (l *Loop) GetStatus() (status models.LoopStatus) { l.state.statusMu.RLock() defer l.state.statusMu.RUnlock() return l.state.status } -var ErrInvalidStatus = errors.New("invalid status") - -func (l *Loop) SetStatus(ctx context.Context, status models.LoopStatus) ( - outcome string, err error, -) { - l.state.statusMu.Lock() - defer l.state.statusMu.Unlock() - existingStatus := l.state.status - - switch status { - case constants.Running: - switch existingStatus { - case constants.Starting, constants.Running, constants.Stopping, constants.Crashed: - return fmt.Sprintf("already %s", existingStatus), nil - } - l.loopLock.Lock() - defer l.loopLock.Unlock() - l.state.status = constants.Starting - l.state.statusMu.Unlock() - l.start <- struct{}{} - - newStatus := constants.Starting // for canceled context - select { - case <-ctx.Done(): - case newStatus = <-l.running: - } - l.state.statusMu.Lock() - l.state.status = newStatus - return newStatus.String(), nil - case constants.Stopped: - switch existingStatus { - case constants.Stopped, constants.Stopping, constants.Starting, constants.Crashed: - return fmt.Sprintf("already %s", existingStatus), nil - } - l.loopLock.Lock() - defer l.loopLock.Unlock() - l.state.status = constants.Stopping - l.state.statusMu.Unlock() - l.stop <- struct{}{} - newStatus := constants.Stopping // for canceled context - select { - case <-ctx.Done(): - case <-l.stopped: - newStatus = constants.Stopped - } - l.state.statusMu.Lock() - l.state.status = newStatus - return status.String(), nil - default: - return "", fmt.Errorf("%w: %s: it can only be one of: %s, %s", - ErrInvalidStatus, status, constants.Running, constants.Stopped) - } -} - func (l *Loop) GetSettings() (settings settings.Shadowsocks) { l.state.settingsMu.RLock() defer l.state.settingsMu.RUnlock() return l.state.settings } -func (l *Loop) SetSettings(ctx context.Context, settings settings.Shadowsocks) ( - outcome string, -) { +func (l *Loop) UpdateSettings(updateSettings settings.Shadowsocks) (outcome string) { l.state.settingsMu.Lock() - settingsUnchanged := reflect.DeepEqual(settings, l.state.settings) + previousSettings := l.state.settings.Copy() + l.state.settings.OverrideWith(updateSettings) + settingsUnchanged := reflect.DeepEqual(previousSettings, l.state.settings) + l.state.settingsMu.Unlock() if settingsUnchanged { - l.state.settingsMu.Unlock() return "settings left unchanged" } - newEnabled := *settings.Enabled - previousEnabled := *l.state.settings.Enabled - l.state.settings = settings - l.state.settingsMu.Unlock() - // Either restart or set changed status - switch { - case !newEnabled && !previousEnabled: - case newEnabled && previousEnabled: - _, _ = l.SetStatus(ctx, constants.Stopped) - _, _ = l.SetStatus(ctx, constants.Running) - case newEnabled && !previousEnabled: - _, _ = l.SetStatus(ctx, constants.Running) - case !newEnabled && previousEnabled: - _, _ = l.SetStatus(ctx, constants.Stopped) - } - return "settings updated" + l.refresh <- struct{}{} + newStatus := <-l.changed + l.state.statusMu.Lock() + l.state.status = newStatus + l.state.statusMu.Unlock() + return "settings updated (service " + newStatus.String() + ")" }