Skip to content

Commit

Permalink
fix race condition in init context (#238)
Browse files Browse the repository at this point in the history
There is still a race condition on the init context. This occurs only in the event of a timeout. The goroutine actually continues executing, except we just unblock the main process (similar to js promise.race). So there is a race condition when writing to `context.Error` (1. in the timeout handler, 2. during `client.init`)
To solve, use a mutex lock and clone to context. (Cloning is necessary because even if we use a mutex lock, there is no guarantee that "Timed out" will be the final error set)
  • Loading branch information
kenny-statsig authored Oct 15, 2024
1 parent 7919428 commit 66d7d48
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 9 deletions.
9 changes: 5 additions & 4 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,9 @@ func newClientImpl(sdkKey string, options *Options) (*Client, *initContext) {
Logger().LogStep(StatsigProcessInitialize, "Timed out")
diagnostics.initialize().overall().end().success(false).reason("timeout").mark()
client.initInBackground()
context.Error = errors.New("Timed out")
return client, context
ctx := context.copy() // Goroutines are not terminated upon timeout. Clone context to avoid race condition on setting Error
ctx.setError(errors.New("Timed out"))
return client, ctx
}
} else {
client.init(context)
Expand All @@ -84,8 +85,8 @@ func (c *Client) init(context *initContext) {
c.evaluator.initialize(context)
c.evaluator.store.mu.RLock()
defer c.evaluator.store.mu.RUnlock()
context.Success = c.evaluator.store.source != sourceUninitialized
context.Source = c.evaluator.store.source
context.setSuccess(c.evaluator.store.source != sourceUninitialized)
context.setSource(c.evaluator.store.source)
}

func (c *Client) initInBackground() {
Expand Down
35 changes: 34 additions & 1 deletion statsig_context.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package statsig

import "time"
import (
"sync"
"time"
)

type errorContext struct {
evalContext *evalContext
Expand Down Expand Up @@ -28,8 +31,38 @@ type initContext struct {
Success bool
Error error
Source EvaluationSource
mu sync.RWMutex
}

func newInitContext() *initContext {
return &initContext{Start: time.Now(), Success: false, Source: sourceUninitialized}
}

func (c *initContext) setSuccess(success bool) {
c.mu.Lock()
defer c.mu.Unlock()
c.Success = success
}

func (c *initContext) setError(err error) {
c.mu.Lock()
defer c.mu.Unlock()
c.Error = err
}

func (c *initContext) setSource(source EvaluationSource) {
c.mu.Lock()
defer c.mu.Unlock()
c.Source = source
}

func (c *initContext) copy() *initContext {
c.mu.RLock()
defer c.mu.RUnlock()
return &initContext{
Start: c.Start,
Success: c.Success,
Error: c.Error,
Source: c.Source,
}
}
8 changes: 4 additions & 4 deletions store.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ func (s *store) initialize(context *initContext) {
s.mu.Unlock()
}
} else {
context.Error = errors.New("Failed to parse bootstrap values")
context.setError(errors.New("Failed to parse bootstrap values"))
}
}
if s.lastSyncTime == 0 {
Expand Down Expand Up @@ -296,7 +296,7 @@ func (s *store) fetchConfigSpecsFromAdapter(context *initContext) {
dataAdapterError := DataAdapterError{Err: toError(err), Method: "get"}
Logger().LogError(dataAdapterError)
if context != nil {
context.Error = &dataAdapterError
context.setError(&dataAdapterError)
}
}
}()
Expand Down Expand Up @@ -332,7 +332,7 @@ func (s *store) handleSyncError(err error, context *initContext) {
Logger().LogError(fmt.Sprintf("Failed to initialize from the network. " +
"See https://docs.statsig.com/messages/serverSDKConnection for more information\n"))
s.errorBoundary.logException(err)
context.Error = err
context.setError(err)
} else if failDuration > syncOutdatedMax {
Logger().LogError(fmt.Sprintf("Syncing the server SDK with Statsig network has failed for %dms. "+
"Your sdk will continue to serve gate/config/experiment definitions as of the last successful sync. "+
Expand Down Expand Up @@ -368,7 +368,7 @@ func (s *store) fetchConfigSpecsFromServer(context *initContext) {
}
} else {
if context != nil {
context.Error = errors.New("Failed to parse config specs")
context.setError(errors.New("Failed to parse config specs"))
}
}
}
Expand Down

0 comments on commit 66d7d48

Please sign in to comment.