From f2f11bc042ee474363758a7acfe7e477a9bdfd2e Mon Sep 17 00:00:00 2001 From: MHSanaei Date: Wed, 15 Jul 2026 03:33:38 +0200 Subject: [PATCH] fix(xray): synchronize the process version and apiPort fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Start writes p.version and p.apiPort (via refreshVersion/refreshAPIPort) after flipping the process to running, while GetXrayVersion and GetAPIPort read them lock-free from the status and traffic poll goroutines. The struct mutex deliberately excluded these fields, so a restart racing a poll was a real data race — a torn read of the version string header can crash. Extend the mutex to cover version and apiPort, doing the blocking version probe before taking the lock. --- internal/xray/process.go | 37 ++++++++++++++---------- internal/xray/process_race_test.go | 46 ++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 15 deletions(-) diff --git a/internal/xray/process.go b/internal/xray/process.go index 2c1cf47dc..7c1a93fe5 100644 --- a/internal/xray/process.go +++ b/internal/xray/process.go @@ -126,11 +126,12 @@ func NewTestProcess(xrayConfig *Config, configPath string) *Process { } type process struct { - // mu guards the process lifecycle fields (cmd, done, exitErr) which are - // written by Start/startCommand and the waitForCommand goroutine while being - // read concurrently by IsRunning/GetErr/GetResult/Stop from other goroutines - // (status endpoint, check-xray-running job). Snapshot under the lock, then do - // any blocking syscall (Wait/Signal/Kill) on the local copy without holding it. + // mu guards the process lifecycle fields (cmd, done, exitErr) plus version and + // apiPort, which are written by Start/startCommand/refreshVersion/refreshAPIPort + // while being read concurrently by IsRunning/GetErr/GetResult/GetXrayVersion/ + // GetAPIPort/Stop from other goroutines (status endpoint, check-xray-running + // and traffic jobs). Snapshot under the lock, then do any blocking syscall + // (Wait/Signal/Kill) on the local copy without holding it. mu sync.RWMutex cmd *exec.Cmd done chan struct{} @@ -281,11 +282,15 @@ func (p *process) GetResult() string { // GetXrayVersion returns the version string of the Xray process. func (p *process) GetXrayVersion() string { + p.mu.RLock() + defer p.mu.RUnlock() return p.version } // GetAPIPort returns the API port used by the Xray process. func (p *Process) GetAPIPort() int { + p.mu.RLock() + defer p.mu.RUnlock() return p.apiPort } @@ -474,28 +479,30 @@ func (p *Process) GetUptime() uint64 { // refreshAPIPort updates the API port from the inbound configs. func (p *process) refreshAPIPort() { + port := 0 for _, inbound := range p.config.InboundConfigs { if inbound.Tag == "api" { - p.apiPort = inbound.Port + port = inbound.Port break } } + p.mu.Lock() + p.apiPort = port + p.mu.Unlock() } // refreshVersion updates the version string by running the Xray binary with -version. func (p *process) refreshVersion() { + version := "Unknown" cmd := exec.CommandContext(context.Background(), GetBinaryPath(), "-version") - data, err := cmd.Output() - if err != nil { - p.version = "Unknown" - } else { - datas := bytes.Split(data, []byte(" ")) - if len(datas) <= 1 { - p.version = "Unknown" - } else { - p.version = string(datas[1]) + if data, err := cmd.Output(); err == nil { + if datas := bytes.Split(data, []byte(" ")); len(datas) > 1 { + version = string(datas[1]) } } + p.mu.Lock() + p.version = version + p.mu.Unlock() } // Start launches the Xray process with the current configuration. diff --git a/internal/xray/process_race_test.go b/internal/xray/process_race_test.go index 77db0d39d..ef3f06a45 100644 --- a/internal/xray/process_race_test.go +++ b/internal/xray/process_race_test.go @@ -54,3 +54,49 @@ func TestProcessLifecycleFieldsRaceSafe(t *testing.T) { close(stop) wg.Wait() } + +// TestProcessVersionAPIPortRaceSafe writes version/apiPort the way Start's +// refresh helpers do while GetXrayVersion/GetAPIPort read them concurrently. +// Run with -race: an unsynchronized access to either field is reported. +func TestProcessVersionAPIPortRaceSafe(t *testing.T) { + inner := &process{ + logWriter: NewLogWriter(), + config: &Config{InboundConfigs: []InboundConfig{{Tag: "api", Port: 12345}}}, + } + p := &Process{inner} + + var wg sync.WaitGroup + stop := make(chan struct{}) + + wg.Go(func() { + for { + select { + case <-stop: + return + default: + } + p.refreshAPIPort() + inner.mu.Lock() + inner.version = "v1.2.3" + inner.mu.Unlock() + } + }) + + for range 4 { + wg.Go(func() { + for { + select { + case <-stop: + return + default: + } + _ = p.GetAPIPort() + _ = p.GetXrayVersion() + } + }) + } + + time.Sleep(50 * time.Millisecond) + close(stop) + wg.Wait() +}