Skip to content

fix: Clamp an over-large start wait time instead of overflowing #243

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tanderson-ld wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: Clamp an over-large start wait time instead of overflowing #243

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
19 changes: 15 additions & 4 deletions lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/LDClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import java.io.UnsupportedEncodingException;
import java.security.InvalidKeyException;
import java.security.NoSuchAlgorithmException;
import java.time.Duration;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
Expand Down Expand Up @@ -72,6 +73,10 @@ public final class LDClient implements LDClientInterface {

private static final int EXCESSIVE_INIT_WAIT_MILLIS = 60000;

// The largest start wait the SDK can represent in milliseconds. A longer wait is clamped to it rather than
// converted, because converting it would overflow and throw (and would do so after the data system starts).
private static final Duration MAX_MILLIS_START_WAIT = Duration.ofMillis(Long.MAX_VALUE);

/**
* Creates a new client instance that connects to LaunchDarkly with the default configuration.
* <p>
Expand Down Expand Up @@ -259,15 +264,21 @@ public LDClient(String sdkKey, LDConfig config) {
// Start the data system
Future<Void> startFuture = dataSystem.start();
if (!config.startWait.isZero() && !config.startWait.isNegative()) {
// Convert the start wait to milliseconds, clamping a wait too large to represent to the maximum rather
// than converting it, which would overflow and throw. Only the positive boundary matters here, since
// zero and negative waits are excluded above.
long startWaitMillis = config.startWait.compareTo(MAX_MILLIS_START_WAIT) >= 0
? Long.MAX_VALUE
: config.startWait.toMillis();
if (!dataSystem.isInitialized()) {
baseLogger.info("Waiting up to {} milliseconds for LaunchDarkly client to start...",
config.startWait.toMillis());
if (config.startWait.toMillis() > EXCESSIVE_INIT_WAIT_MILLIS) {
baseLogger.warn("LaunchDarkly client created with start wait time of {} milliseconds. We recommend a timeout of less than {} milliseconds.", config.startWait.toMillis(), EXCESSIVE_INIT_WAIT_MILLIS);
startWaitMillis);
if (startWaitMillis > EXCESSIVE_INIT_WAIT_MILLIS) {
baseLogger.warn("LaunchDarkly client created with start wait time of {} milliseconds. We recommend a timeout of less than {} milliseconds.", startWaitMillis, EXCESSIVE_INIT_WAIT_MILLIS);
}
}
try {
startFuture.get(config.startWait.toMillis(), TimeUnit.MILLISECONDS);
startFuture.get(startWaitMillis, TimeUnit.MILLISECONDS);
} catch (TimeoutException e) {
baseLogger.error("Timeout encountered waiting for LaunchDarkly client initialization");
} catch (Exception e) {
Expand Down
19 changes: 19 additions & 0 deletions lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/LDClientTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,25 @@ public void willWaitForDataSourceIfWaitMillisIsGreaterThanZero() throws Exceptio
mocks.verifyAll();
}

@Test
public void startWaitTooLargeForMillisIsClampedRatherThanOverflowing() throws Exception {
// A start wait time too large to represent in milliseconds must be clamped, not converted directly:
// converting it throws ArithmeticException, and it would do so after the data source has started, leaving
// the data source running with no reference available to close it.
LDConfig.Builder config = new LDConfig.Builder()
.startWait(Duration.ofSeconds(Long.MAX_VALUE));

expect(dataSource.start()).andReturn(initFuture);
expect(initFuture.get(Long.MAX_VALUE, TimeUnit.MILLISECONDS)).andReturn(null);
expect(dataSource.isInitialized()).andReturn(false).anyTimes();
mocks.replayAll();

client = createMockClient(config);
assertFalse(client.isInitialized());

mocks.verifyAll();
}

@Test
public void noWaitForDataSourceIfWaitMillisIsNegative() throws Exception {
LDConfig.Builder config = new LDConfig.Builder()
Expand Down
Loading