Skip to content

Commit

Permalink
api: Fix CallOptions to be properly @Immutable (#9689)
Browse files Browse the repository at this point in the history 
Although CallOptions is annotated by @immutable, its fields are not
final. So it's not truly immutable, namely not safe for unsynchronized
publication.

This commit adds final to all fields of CallOptions. Using internal
builder class to keep flexibility of constructing CallOptions.

Fixes #9658
  • Loading branch information
@pandaapo
pandaapo committed Nov 17, 2022
1 parent 1241946 commit 1b94f48
Showing 1 changed file with 92 additions and 68 deletions.
160 changes: 92 additions & 68 deletions api/src/main/java/io/grpc/CallOptions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,42 +41,68 @@ public final class CallOptions {
/**
* A blank {@code CallOptions} that all fields are not set.
*/
public static final CallOptions DEFAULT = new CallOptions();
public static final CallOptions DEFAULT = new Builder().build();

// Although {@code CallOptions} is immutable, its fields are not final, so that we can initialize
// them outside of constructor. Otherwise the constructor will have a potentially long list of
// unnamed arguments, which is undesirable.
@Nullable
private Deadline deadline;
private final Deadline deadline;

@Nullable
private Executor executor;
private final Executor executor;

@Nullable
private String authority;
private final String authority;

@Nullable
private CallCredentials credentials;
private final CallCredentials credentials;

@Nullable
private String compressorName;
private final String compressorName;

private Object[][] customOptions;
private final Object[][] customOptions;

// Unmodifiable list
private List<ClientStreamTracer.Factory> streamTracerFactories = Collections.emptyList();
private final List<ClientStreamTracer.Factory> streamTracerFactories;

/**
* Opposite to fail fast.
*/
@Nullable
private Boolean waitForReady;
private final Boolean waitForReady;

@Nullable
private Integer maxInboundMessageSize;
private final Integer maxInboundMessageSize;
@Nullable
private Integer maxOutboundMessageSize;
private final Integer maxOutboundMessageSize;

private CallOptions(Builder builder) {
this.deadline = builder.deadline;
this.executor = builder.executor;
this.authority = builder.authority;
this.credentials = builder.credentials;
this.compressorName = builder.compressorName;
this.customOptions = builder.customOptions;
this.streamTracerFactories = builder.streamTracerFactories;
this.waitForReady = builder.waitForReady;
this.maxInboundMessageSize = builder.maxInboundMessageSize;
this.maxOutboundMessageSize = builder.maxOutboundMessageSize;
}

static class Builder {
Deadline deadline;
Executor executor;
String authority;
CallCredentials credentials;
String compressorName;
Object[][] customOptions = new Object[0][2];
// Unmodifiable list
List<ClientStreamTracer.Factory> streamTracerFactories = Collections.emptyList();
Boolean waitForReady;
Integer maxInboundMessageSize;
Integer maxOutboundMessageSize;

private CallOptions build() {
return new CallOptions(this);
}
}

/**
* Override the HTTP/2 authority the channel claims to be connecting to. <em>This is not
Expand All @@ -89,18 +115,18 @@ public final class CallOptions {
*/
@ExperimentalApi("https://github.com/grpc/grpc-java/issues/1767")
public CallOptions withAuthority(@Nullable String authority) {
CallOptions newOptions = new CallOptions(this);
newOptions.authority = authority;
return newOptions;
Builder builder = toBuilder(this);
builder.authority = authority;
return builder.build();
}

/**
* Returns a new {@code CallOptions} with the given call credentials.
*/
public CallOptions withCallCredentials(@Nullable CallCredentials credentials) {
CallOptions newOptions = new CallOptions(this);
newOptions.credentials = credentials;
return newOptions;
Builder builder = toBuilder(this);
builder.credentials = credentials;
return builder.build();
}

/**
Expand All @@ -113,9 +139,9 @@ public CallOptions withCallCredentials(@Nullable CallCredentials credentials) {
*/
@ExperimentalApi("https://github.com/grpc/grpc-java/issues/1704")
public CallOptions withCompression(@Nullable String compressorName) {
CallOptions newOptions = new CallOptions(this);
newOptions.compressorName = compressorName;
return newOptions;
Builder builder = toBuilder(this);
builder.compressorName = compressorName;
return builder.build();
}

/**
Expand All @@ -127,9 +153,9 @@ public CallOptions withCompression(@Nullable String compressorName) {
* @param deadline the deadline or {@code null} for unsetting the deadline.
*/
public CallOptions withDeadline(@Nullable Deadline deadline) {
CallOptions newOptions = new CallOptions(this);
newOptions.deadline = deadline;
return newOptions;
Builder builder = toBuilder(this);
builder.deadline = deadline;
return builder.build();
}

/**
Expand All @@ -156,19 +182,19 @@ public Deadline getDeadline() {
* fails RPCs without sending them if unable to connect.
*/
public CallOptions withWaitForReady() {
CallOptions newOptions = new CallOptions(this);
newOptions.waitForReady = Boolean.TRUE;
return newOptions;
Builder builder = toBuilder(this);
builder.waitForReady = Boolean.TRUE;
return builder.build();
}

/**
* Disables 'wait for ready' feature for the call.
* This method should be rarely used because the default is without 'wait for ready'.
*/
public CallOptions withoutWaitForReady() {
CallOptions newOptions = new CallOptions(this);
newOptions.waitForReady = Boolean.FALSE;
return newOptions;
Builder builder = toBuilder(this);
builder.waitForReady = Boolean.FALSE;
return builder.build();
}

/**
Expand Down Expand Up @@ -208,9 +234,9 @@ public CallCredentials getCredentials() {
* executor specified with {@link ManagedChannelBuilder#executor}.
*/
public CallOptions withExecutor(@Nullable Executor executor) {
CallOptions newOptions = new CallOptions(this);
newOptions.executor = executor;
return newOptions;
Builder builder = toBuilder(this);
builder.executor = executor;
return builder.build();
}

/**
Expand All @@ -221,13 +247,13 @@ public CallOptions withExecutor(@Nullable Executor executor) {
*/
@ExperimentalApi("https://github.com/grpc/grpc-java/issues/2861")
public CallOptions withStreamTracerFactory(ClientStreamTracer.Factory factory) {
CallOptions newOptions = new CallOptions(this);
ArrayList<ClientStreamTracer.Factory> newList =
new ArrayList<>(streamTracerFactories.size() + 1);
newList.addAll(streamTracerFactories);
newList.add(factory);
newOptions.streamTracerFactories = Collections.unmodifiableList(newList);
return newOptions;
Builder builder = toBuilder(this);
builder.streamTracerFactories = Collections.unmodifiableList(newList);
return builder.build();
}

/**
Expand Down Expand Up @@ -319,7 +345,7 @@ public <T> CallOptions withOption(Key<T> key, T value) {
Preconditions.checkNotNull(key, "key");
Preconditions.checkNotNull(value, "value");

CallOptions newOptions = new CallOptions(this);
Builder builder = toBuilder(this);
int existingIdx = -1;
for (int i = 0; i < customOptions.length; i++) {
if (key.equals(customOptions[i][0])) {
Expand All @@ -328,18 +354,18 @@ public <T> CallOptions withOption(Key<T> key, T value) {
}
}

newOptions.customOptions = new Object[customOptions.length + (existingIdx == -1 ? 1 : 0)][2];
System.arraycopy(customOptions, 0, newOptions.customOptions, 0, customOptions.length);
builder.customOptions = new Object[customOptions.length + (existingIdx == -1 ? 1 : 0)][2];
System.arraycopy(customOptions, 0, builder.customOptions, 0, customOptions.length);

if (existingIdx == -1) {
// Add a new option
newOptions.customOptions[customOptions.length] = new Object[] {key, value};
builder.customOptions[customOptions.length] = new Object[] {key, value};
} else {
// Replace an existing option
newOptions.customOptions[existingIdx] = new Object[] {key, value};
builder.customOptions[existingIdx] = new Object[] {key, value};
}

return newOptions;
return builder.build();
}

/**
Expand Down Expand Up @@ -368,10 +394,6 @@ public Executor getExecutor() {
return executor;
}

private CallOptions() {
customOptions = new Object[0][2];
}

/**
* Returns whether <a href="https://github.com/grpc/grpc/blob/master/doc/wait-for-ready.md">
* 'wait for ready'</a> option is enabled for the call. 'Fail fast' is the default option for gRPC
Expand All @@ -392,9 +414,9 @@ Boolean getWaitForReady() {
@ExperimentalApi("https://github.com/grpc/grpc-java/issues/2563")
public CallOptions withMaxInboundMessageSize(int maxSize) {
checkArgument(maxSize >= 0, "invalid maxsize %s", maxSize);
CallOptions newOptions = new CallOptions(this);
newOptions.maxInboundMessageSize = maxSize;
return newOptions;
Builder builder = toBuilder(this);
builder.maxInboundMessageSize = maxSize;
return builder.build();
}

/**
Expand All @@ -403,9 +425,9 @@ public CallOptions withMaxInboundMessageSize(int maxSize) {
@ExperimentalApi("https://github.com/grpc/grpc-java/issues/2563")
public CallOptions withMaxOutboundMessageSize(int maxSize) {
checkArgument(maxSize >= 0, "invalid maxsize %s", maxSize);
CallOptions newOptions = new CallOptions(this);
newOptions.maxOutboundMessageSize = maxSize;
return newOptions;
Builder builder = toBuilder(this);
builder.maxOutboundMessageSize = maxSize;
return builder.build();
}

/**
Expand All @@ -427,19 +449,21 @@ public Integer getMaxOutboundMessageSize() {
}

/**
* Copy constructor.
* Copy CallOptions.
*/
private CallOptions(CallOptions other) {
deadline = other.deadline;
authority = other.authority;
credentials = other.credentials;
executor = other.executor;
compressorName = other.compressorName;
customOptions = other.customOptions;
waitForReady = other.waitForReady;
maxInboundMessageSize = other.maxInboundMessageSize;
maxOutboundMessageSize = other.maxOutboundMessageSize;
streamTracerFactories = other.streamTracerFactories;
private static Builder toBuilder(CallOptions other) {
Builder builder = new Builder();
builder.deadline = other.deadline;
builder.executor = other.executor;
builder.authority = other.authority;
builder.credentials = other.credentials;
builder.compressorName = other.compressorName;
builder.customOptions = other.customOptions;
builder.streamTracerFactories = other.streamTracerFactories;
builder.waitForReady = other.waitForReady;
builder.maxInboundMessageSize = other.maxInboundMessageSize;
builder.maxOutboundMessageSize = other.maxOutboundMessageSize;
return builder;
}

@Override
Expand Down

0 comments on commit 1b94f48

Please sign in to comment.