Skip to content
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.

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

Backport PRs with "backport" label from master to v1.0.x #2265

Merged
merged 5 commits into from
Sep 14, 2016
Merged
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
1 change: 1 addition & 0 deletions all/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ buildscript {
def subprojects = [
project(':grpc-auth'),
project(':grpc-core'),
project(':grpc-context'),
project(':grpc-netty'),
project(':grpc-okhttp'),
project(':grpc-protobuf'),
Expand Down
14 changes: 14 additions & 0 deletions context/build.gradle
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
plugins {
id "be.insaneprogramming.gradle.animalsniffer" version "1.4.0"
}

description = 'gRPC: Context'

dependencies {
testCompile project(':grpc-testing')
}

// Configure the animal sniffer plugin
animalsniffer {
signature = "org.codehaus.mojo.signature:java16:+@signature"
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,6 @@

package io.grpc;

import com.google.common.base.Preconditions;
import com.google.common.util.concurrent.MoreExecutors;

import java.util.ArrayList;
import java.util.concurrent.Callable;
import java.util.concurrent.Executor;
Expand All @@ -44,8 +41,6 @@
import java.util.logging.Level;
import java.util.logging.Logger;

import javax.annotation.Nullable;

/**
* A context propagation mechanism which can carry scoped-values across API boundaries and between
* threads. Examples of state propagated via context include:
Expand Down Expand Up @@ -275,8 +270,8 @@ public CancellableContext withDeadlineAfter(long duration, TimeUnit unit,
*/
public CancellableContext withDeadline(Deadline deadline,
ScheduledExecutorService scheduler) {
Preconditions.checkNotNull(deadline, "deadline");
Preconditions.checkNotNull(scheduler, "scheduler");
checkNotNull(deadline, "deadline");
checkNotNull(scheduler, "scheduler");
return new CancellableContext(this, deadline, scheduler);
}

Expand Down Expand Up @@ -349,7 +344,7 @@ public Context attach() {
* will still be bound.
*/
public void detach(Context toAttach) {
Preconditions.checkNotNull(toAttach);
checkNotNull(toAttach, "toAttach");
if (toAttach.attach() != this) {
// Log a severe message instead of throwing an exception as the context to attach is assumed
// to be the correct one and the unbalanced state represents a coding mistake in a lower
Expand Down Expand Up @@ -383,7 +378,6 @@ public boolean isCancelled() {
* <p>The cancellation cause is provided for informational purposes only and implementations
* should generally assume that it has already been handled and logged properly.
*/
@Nullable
public Throwable cancellationCause() {
if (parent == null || !cascadesCancellation) {
return null;
Expand All @@ -396,7 +390,6 @@ public Throwable cancellationCause() {
* A context may have an associated {@link Deadline} at which it will be automatically cancelled.
* @return A {@link io.grpc.Deadline} or {@code null} if no deadline is set.
*/
@Nullable
public Deadline getDeadline() {
return DEADLINE_KEY.get(this);
}
Expand All @@ -406,8 +399,8 @@ public Deadline getDeadline() {
*/
public void addListener(final CancellationListener cancellationListener,
final Executor executor) {
Preconditions.checkNotNull(cancellationListener);
Preconditions.checkNotNull(executor);
checkNotNull(cancellationListener, "cancellationListener");
checkNotNull(executor, "executor");
if (canBeCancelled) {
ExecutableListener executableListener =
new ExecutableListener(executor, cancellationListener);
Expand All @@ -420,7 +413,7 @@ public void addListener(final CancellationListener cancellationListener,
// we can cascade listener notification.
listeners = new ArrayList<ExecutableListener>();
listeners.add(executableListener);
parent.addListener(parentListener, MoreExecutors.directExecutor());
parent.addListener(parentListener, DirectExecutor.INSTANCE);
} else {
listeners.add(executableListener);
}
Expand Down Expand Up @@ -685,13 +678,13 @@ public boolean isCurrent() {
}

/**
* Cancel this context and optionally provide a cause for the cancellation. This
* will trigger notification of listeners.
* Cancel this context and optionally provide a cause (can be {@code null}) for the
* cancellation. This will trigger notification of listeners.
*
* @return {@code true} if this context cancelled the context and notified listeners,
* {@code false} if the context was already cancelled.
*/
public boolean cancel(@Nullable Throwable cause) {
public boolean cancel(Throwable cause) {
boolean triggeredCancel = false;
synchronized (this) {
if (!cancelled) {
Expand All @@ -717,7 +710,7 @@ public boolean cancel(@Nullable Throwable cause) {
* @param toAttach context to make current.
* @param cause of cancellation, can be {@code null}.
*/
public void detachAndCancel(Context toAttach, @Nullable Throwable cause) {
public void detachAndCancel(Context toAttach, Throwable cause) {
try {
detach(toAttach);
} finally {
Expand All @@ -741,7 +734,6 @@ public boolean isCancelled() {
return false;
}

@Nullable
@Override
public Throwable cancellationCause() {
if (isCancelled()) {
Expand Down Expand Up @@ -774,7 +766,7 @@ public static class Key<T> {
}

Key(String name, T defaultValue) {
this.name = Preconditions.checkNotNull(name);
this.name = checkNotNull(name, "name");
this.defaultValue = defaultValue;
}

Expand Down Expand Up @@ -838,4 +830,25 @@ public void cancelled(Context context) {
}
}
}

private static <T> T checkNotNull(T reference, Object errorMessage) {
if (reference == null) {
throw new NullPointerException(String.valueOf(errorMessage));
}
return reference;
}

private enum DirectExecutor implements Executor {
INSTANCE;

@Override
public void execute(Runnable command) {
command.run();
}

@Override
public String toString() {
return "Context.DirectExecutor";
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,11 @@

package io.grpc;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;

import io.grpc.internal.LogExceptionRunnable;

import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;
import java.util.logging.Level;
import java.util.logging.Logger;

/**
* An absolute deadline in system time.
Expand All @@ -60,9 +57,9 @@ public static Deadline after(long duration, TimeUnit units) {
return after(duration, units, SYSTEM_TICKER);
}

@VisibleForTesting
// For testing
static Deadline after(long duration, TimeUnit units, Ticker ticker) {
Preconditions.checkNotNull(units);
checkNotNull(units, "units");
return new Deadline(ticker, units.toNanos(duration), true);
}

Expand Down Expand Up @@ -148,8 +145,8 @@ public long timeRemaining(TimeUnit unit) {
* @return {@link ScheduledFuture} which can be used to cancel execution of the task
*/
public ScheduledFuture<?> runOnExpiration(Runnable task, ScheduledExecutorService scheduler) {
Preconditions.checkNotNull(task, "task");
Preconditions.checkNotNull(scheduler, "scheduler");
checkNotNull(task, "task");
checkNotNull(scheduler, "scheduler");
return scheduler.schedule(new LogExceptionRunnable(task),
deadlineNanos - ticker.read(), TimeUnit.NANOSECONDS);
}
Expand Down Expand Up @@ -182,4 +179,42 @@ public long read() {
return System.nanoTime();
}
}

private static <T> T checkNotNull(T reference, Object errorMessage) {
if (reference == null) {
throw new NullPointerException(String.valueOf(errorMessage));
}
return reference;
}

private static class LogExceptionRunnable implements Runnable {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, you could have just dropped it. It wasn't necessary to begin with (in c8648dc). This works too.

private static final Logger log = Logger.getLogger(LogExceptionRunnable.class.getName());

private final Runnable task;

public LogExceptionRunnable(Runnable task) {
this.task = checkNotNull(task, "task");
}

@Override
public void run() {
try {
task.run();
} catch (Throwable t) {
log.log(Level.SEVERE, "Exception while executing runnable " + task, t);
if (t instanceof RuntimeException) {
throw (RuntimeException) t;
} else if (t instanceof Error) {
throw (Error) t;
} else {
throw new RuntimeException(t);
}
}
}

@Override
public String toString() {
return "Deadline.LogExceptionRunnable(" + task + ")";
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public void defaultTickerIsSystemTicker() {
ticker.reset(System.nanoTime());
Deadline reference = Deadline.after(0, TimeUnit.SECONDS, ticker);
// Allow inaccuracy to account for system time advancing during test.
assertAbout(deadline()).that(d).isWithin(20, TimeUnit.MILLISECONDS).of(reference);
assertAbout(deadline()).that(d).isWithin(1, TimeUnit.SECONDS).of(reference);
}

@Test
Expand Down Expand Up @@ -265,7 +265,7 @@ public void toString_before() {
assertEquals("12000 ns from now", d.toString());
}

static class FakeTicker extends Deadline.Ticker {
private static class FakeTicker extends Deadline.Ticker {
private long time;

@Override
Expand Down
3 changes: 2 additions & 1 deletion core/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ description = 'gRPC: Core'

dependencies {
compile libraries.guava,
libraries.jsr305
libraries.jsr305,
project(':grpc-context')
testCompile project(':grpc-testing')
}

Expand Down
19 changes: 7 additions & 12 deletions core/src/main/java/io/grpc/inprocess/InProcessTransport.java
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
import java.util.logging.Level;
import java.util.logging.Logger;

import javax.annotation.CheckReturnValue;
import javax.annotation.concurrent.GuardedBy;
import javax.annotation.concurrent.ThreadSafe;

Expand Down Expand Up @@ -89,8 +90,9 @@ public InProcessTransport(String name) {
.build();
}

@CheckReturnValue
@Override
public synchronized void start(ManagedClientTransport.Listener listener) {
public synchronized Runnable start(ManagedClientTransport.Listener listener) {
this.clientTransportListener = listener;
InProcessServer server = InProcessServer.findServer(name);
if (server != null) {
Expand All @@ -99,31 +101,24 @@ public synchronized void start(ManagedClientTransport.Listener listener) {
if (serverTransportListener == null) {
shutdownStatus = Status.UNAVAILABLE.withDescription("Could not find server: " + name);
final Status localShutdownStatus = shutdownStatus;
Thread shutdownThread = new Thread(new Runnable() {
return new Runnable() {
@Override
public void run() {
synchronized (InProcessTransport.this) {
notifyShutdown(localShutdownStatus);
notifyTerminated();
}
}
});
shutdownThread.setDaemon(true);
shutdownThread.setName("grpc-inprocess-shutdown");
shutdownThread.start();
return;
};
}
Thread readyThread = new Thread(new Runnable() {
return new Runnable() {
@Override
public void run() {
synchronized (InProcessTransport.this) {
clientTransportListener.transportReady();
}
}
});
readyThread.setDaemon(true);
readyThread.setName("grpc-inprocess-ready");
readyThread.start();
};
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,9 @@ class DelayedClientTransport implements ManagedClientTransport {
}

@Override
public void start(Listener listener) {
public Runnable start(Listener listener) {
this.listener = Preconditions.checkNotNull(listener, "listener");
return null;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@

abstract class ForwardingConnectionClientTransport implements ConnectionClientTransport {
@Override
public void start(Listener listener) {
delegate().start(listener);
public Runnable start(Listener listener) {
return delegate().start(listener);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@

import static com.google.common.base.Preconditions.checkNotNull;

import com.google.common.base.Throwables;

import java.util.logging.Level;
import java.util.logging.Logger;

Expand All @@ -56,7 +58,8 @@ public void run() {
task.run();
} catch (Throwable t) {
log.log(Level.SEVERE, "Exception while executing runnable " + task, t);
throw t instanceof RuntimeException ? (RuntimeException) t : new RuntimeException(t);
Throwables.propagateIfPossible(t);
throw new AssertionError(t);
}
}

Expand Down
Loading