diff --git a/internal/relayenv/env_context_impl.go b/internal/relayenv/env_context_impl.go index dc6d5836f..571c0d1b3 100644 --- a/internal/relayenv/env_context_impl.go +++ b/internal/relayenv/env_context_impl.go @@ -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 @@ -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) @@ -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 { diff --git a/internal/relayenv/env_context_impl_test.go b/internal/relayenv/env_context_impl_test.go index 5c0aa671d..56da28926 100644 --- a/internal/relayenv/env_context_impl_test.go +++ b/internal/relayenv/env_context_impl_test.go @@ -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" @@ -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"}}