Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 19 additions & 4 deletions internal/relayenv/env_context_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,16 @@ func (c *envContextImpl) startSDKClient(sdkKey config.SDKKey, readyCh chan<- Env
client, err := c.sdkClientFactory(sdkKey, c.sdkConfig, c.sdkInitTimeout)
c.mu.Lock()
name := c.identifiers.GetDisplayName()
if client != nil {
// The build above happens without c.mu held, and can take up to c.sdkInitTimeout. If Close() runs to
// completion while it's in flight, Close() has already closed and cleared c.clients and will never
// revisit it (a later Close() call short-circuits on c.closed). Installing this client now would leak
// its streaming connection and goroutines forever, so close it instead of installing it.
discarded := c.closed
if discarded {
if client != nil {
_ = client.Close()
}
} else if client != nil {
c.clients[sdkKey] = client

// The data store instance is created by the SDK when it creates the client. Now that
Expand All @@ -511,10 +520,16 @@ func (c *envContextImpl) startSDKClient(sdkKey config.SDKKey, readyCh chan<- Env
}
c.evaluator = ldeval.NewEvaluatorWithOptions(dataProvider, evalOptions...)
}
c.initErr = err
if !discarded {
c.initErr = err
}
c.mu.Unlock()

if err != nil {
switch {
case discarded:
c.globalLoggers.Infof("SDK key %s finished initializing after the environment %q was already closed; discarding it",
sdkKey.Masked(), name)
case err != nil:
if suppressErrors {
c.globalLoggers.Warnf("Ignoring error initializing LaunchDarkly client for %q: %+v",
name, err)
Expand All @@ -526,7 +541,7 @@ func (c *envContextImpl) startSDKClient(sdkKey config.SDKKey, readyCh chan<- Env
}
return
}
} else {
default:
c.globalLoggers.Infof("Initialized LaunchDarkly client for %q (SDK key %s)", name, sdkKey.Masked())
}
if readyCh != nil {
Expand Down
44 changes: 44 additions & 0 deletions internal/relayenv/env_context_impl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import (
"github.com/launchdarkly/go-sdk-common/v3/ldvalue"
ldevents "github.com/launchdarkly/go-sdk-events/v3"
"github.com/launchdarkly/go-server-sdk-evaluation/v3/ldbuilders"
ld "github.com/launchdarkly/go-server-sdk/v7"
"github.com/launchdarkly/go-server-sdk/v7/ldcomponents"
"github.com/launchdarkly/go-server-sdk/v7/subsystems"
"github.com/launchdarkly/go-server-sdk/v7/subsystems/ldstoreimpl"
Expand Down Expand Up @@ -134,6 +135,49 @@ func TestConstructorWithOnlySDKKey(t *testing.T) {
assert.Nil(t, env.GetInitError())
}

func TestSDKClientBuildIsDiscardedIfEnvClosedFirst(t *testing.T) {
// startSDKClient calls the SDK client factory without holding c.mu, since the build can take a while.
// If Close() runs to completion while a build is still in flight, the late-finishing build must not
// install its client into c.clients - Close() has already closed and cleared that map and will never
// look at it again (a later Close() call short-circuits on c.closed), so a client installed after the
// fact would never be closed, leaking its streaming connection and goroutines.
envConfig := st.EnvMain.Config

mockLog := ldlogtest.NewMockLog()
defer mockLog.DumpIfTestFailed(t)

buildStarted := make(chan struct{})
proceedWithBuild := make(chan struct{})
clientCh := make(chan *testclient.FakeLDClient, 1)
realFactory := testclient.FakeLDClientFactoryWithChannel(true, clientCh)

blockingFactory := func(sdkKey config.SDKKey, sdkConfig ld.Config, timeout time.Duration) (sdks.LDClientContext, error) {
close(buildStarted)
<-proceedWithBuild
return realFactory(sdkKey, sdkConfig, timeout)
}

env := makeBasicEnv(t, envConfig, blockingFactory, mockLog.Loggers, nil)

if !helpers.AssertChannelClosed(t, buildStarted, time.Second, "timed out waiting for SDK client build to start") {
t.FailNow()
}

require.NoError(t, env.Close())

// Let the build finish only after Close() has already run to completion.
close(proceedWithBuild)

client := requireClientReady(t, clientCh)
client.AwaitClose(t, time.Second) // must be closed rather than leaked

envImpl := env.(*envContextImpl)
envImpl.mu.RLock()
_, present := envImpl.clients[envConfig.SDKKey]
envImpl.mu.RUnlock()
assert.False(t, present, "client built after Close() must not be installed into c.clients")
}

func TestConstructorWithJSClientContext(t *testing.T) {
envConfig := st.EnvWithAllCredentials.Config
jsClientContext := JSClientContext{Origins: []string{"origin"}}
Expand Down