Javadoc improvements, especially to Context

context/src/main/java/io/grpc/Context.java

@@ -60,11 +60,11 @@ import java.util.logging.Logger;
  *
  * <pre>
  *   Context withCredential = Context.current().withValue(CRED_KEY, cred);
- *   executorService.execute(withCredential.wrap(new Runnable() {
+ *   withCredential.run(new Runnable() {
  *     public void run() {
  *        readUserRecords(userId, CRED_KEY.get());
  *     }
- *   }));
+ *   });
  * </pre>
  *
  * <p>Contexts are also used to represent a scoped unit of work. When the unit of work is done the
@@ -78,7 +78,7 @@ import java.util.logging.Logger;
  * <pre>
  *   CancellableContext withCancellation = Context.current().withCancellation();
  *   try {
- *     executorService.execute(withCancellation.wrap(new Runnable() {
+ *     withCancellation.run(new Runnable() {
  *       public void run() {
  *         while (waitingForData() &amp;&amp; !Context.current().isCancelled()) {}
  *       }
@@ -95,12 +95,13 @@ import java.util.logging.Logger;
  *
  * <p>Notes and cautions on use:
  * <ul>
+ *    <li>Every {@code attach()} should have a {@code detach()} in the same method. And every
+ * CancellableContext should be cancelled at some point. Breaking these rules may lead to memory
+ * leaks.
  *    <li>While Context objects are immutable they do not place such a restriction on the state
  * they store.</li>
  *    <li>Context is not intended for passing optional parameters to an API and developers should
  * take care to avoid excessive dependence on context when designing an API.</li>
- *    <li>If Context is being used in an environment that needs to support class unloading it is the
- * responsibility of the application to ensure that all contexts are properly cancelled.</li>
  * </ul>
  */
 public class Context {
@@ -228,15 +229,16 @@ public class Context {
 
   /**
    * Create a new context which is independently cancellable and also cascades cancellation from
-   * its parent. Callers should ensure that either {@link CancellableContext#cancel(Throwable)}
-   * or {@link CancellableContext#detachAndCancel(Context, Throwable)} are called to notify
-   * listeners and release the resources associated with them.
+   * its parent. Callers <em>must</em> ensure that either {@link
+   * CancellableContext#cancel(Throwable)} or {@link CancellableContext#detachAndCancel(Context,
+   * Throwable)} are called at a later point, in order to allow this context to be garbage
+   * collected.
    *
    * <p>Sample usage:
    * <pre>
    *   Context.CancellableContext withCancellation = Context.current().withCancellation();
    *   try {
-   *     executorService.execute(withCancellation.wrap(new Runnable() {
+   *     withCancellation.run(new Runnable() {
    *       public void run() {
    *         Context current = Context.current();
    *         while (!current.isCancelled()) {
@@ -244,9 +246,8 @@ public class Context {
    *         }
    *       }
    *     });
-   *     doSomethingRelatedWork();
-   *   } catch (Throwable t) {
-   *     withCancellation.cancel(t);
+   *   } finally {
+   *     withCancellation.cancel(null);
    *   }
    * </pre>
    */
@@ -257,13 +258,16 @@ public class Context {
   /**
    * Create a new context which will cancel itself after the given {@code duration} from now.
    * The returned context will cascade cancellation of its parent. Callers may explicitly cancel
-   * the returned context prior to the deadline just as for {@link #withCancellation()},
+   * the returned context prior to the deadline just as for {@link #withCancellation()}. If the unit
+   * of work completes before the deadline, the context should be explicitly cancelled to allow
+   * it to be garbage collected.
    *
    * <p>Sample usage:
    * <pre>
-   *   Context.CancellableContext withDeadline = Context.current().withDeadlineAfter(5,
-   *       TimeUnit.SECONDS, scheduler);
-   *   executorService.execute(withDeadline.wrap(new Runnable() {
+   *   Context.CancellableContext withDeadline = Context.current()
+   *       .withDeadlineAfter(5, TimeUnit.SECONDS, scheduler);
+   *   try {
+   *     withDeadline.run(new Runnable() {
    *       public void run() {
    *         Context current = Context.current();
    *         while (!current.isCancelled()) {
@@ -271,6 +275,9 @@ public class Context {
    *         }
    *       }
    *     });
+   *   } finally {
+   *     withDeadline.cancel(null);
+   *   }
    * </pre>
    */
   public CancellableContext withDeadlineAfter(long duration, TimeUnit unit,
@@ -281,20 +288,26 @@ public class Context {
   /**
    * Create a new context which will cancel itself at the given {@link Deadline}.
    * The returned context will cascade cancellation of its parent. Callers may explicitly cancel
-   * the returned context prior to the deadline just as for {@link #withCancellation()},
+   * the returned context prior to the deadline just as for {@link #withCancellation()}. If the unit
+   * of work completes before the deadline, the context should be explicitly cancelled to allow
+   * it to be garbage collected.
    *
    * <p>Sample usage:
    * <pre>
    *   Context.CancellableContext withDeadline = Context.current()
-   *      .withDeadline(someReceivedDeadline);
-   *   executorService.execute(withDeadline.wrap(new Runnable() {
+   *      .withDeadline(someReceivedDeadline, scheduler);
+   *   try {
+   *     withDeadline.run(new Runnable() {
    *       public void run() {
    *         Context current = Context.current();
-   *       while (!current.isCancelled()) {
+   *         while (!current.isCancelled() &amp;&amp; moreWorkToDo()) {
    *           keepWorking();
    *         }
    *       }
    *     });
+   *   } finally {
+   *     withDeadline.cancel(null);
+   *   }
    * </pre>
    */
   public CancellableContext withDeadline(Deadline deadline,
@@ -310,11 +323,11 @@ public class Context {
    *
    <pre>
    *   Context withCredential = Context.current().withValue(CRED_KEY, cred);
-   *   executorService.execute(withCredential.wrap(new Runnable() {
+   *   withCredential.run(new Runnable() {
    *     public void run() {
    *        readUserRecords(userId, CRED_KEY.get());
    *     }
-   *   }));
+   *   });
    * </pre>
    *
    */
@@ -366,10 +379,19 @@ public class Context {
    * previously current context is returned. It is allowed to attach contexts where {@link
    * #isCancelled()} is {@code true}.
    *
-   * <p>Instead of using {@link #attach()} & {@link #detach(Context)} most use-cases are better
+   * <p>Instead of using {@code attach()} and {@link #detach(Context)} most use-cases are better
    * served by using the {@link #run(Runnable)} or {@link #call(java.util.concurrent.Callable)} to
    * execute work immediately within a context's scope. If work needs to be done in other threads it
    * is recommended to use the 'wrap' methods or to use a propagating executor.
+   *
+   * <p>All calls to {@code attach()} should have a corresponding {@link #detach(Context)} within
+   * the same method:
+   * <pre>{@code Context previous = someContext.attach();
+   * try {
+   *   // Do work
+   * } finally {
+   *   someContext.detach(previous);
+   * }}</pre>
    */
   public Context attach() {
     Context previous = current();
@@ -378,8 +400,7 @@ public class Context {
   }
 
   /**
-   * Detach the current context and attach the provided replacement which should be the context of
-   * the outer scope, thus exit the current scope.
+   * Reverse an {@code attach()}, restoring the previous context and exiting the current scope.
    *
    * <p>This context should be the same context that was previously {@link #attach attached}.  The
    * provided replacement should be what was returned by the same {@link #attach attach()} call.  If
@@ -649,7 +670,9 @@ public class Context {
 
   /**
    * A context which inherits cancellation from its parent but which can also be independently
-   * cancelled and which will propagate cancellation to its descendants.
+   * cancelled and which will propagate cancellation to its descendants. To avoid leaking memory,
+   * every CancellableContext must have a defined lifetime, after which it is guaranteed to be
+   * cancelled.
    */
   public static final class CancellableContext extends Context {
 

context/src/main/java/io/grpc/Deadline.java

@@ -36,7 +36,14 @@ import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.TimeUnit;
 
 /**
- * An absolute deadline in system time.
+ * An absolute point in time, generally for tracking when a task should be completed. A deadline is
+ * immutable except for the passage of time causing it to expire.
+ *
+ * <p>Many systems use timeouts, which are relative to the start of the operation. However, being
+ * relative causes them to be poorly suited for managing higher-level tasks where there are many
+ * components and sub-operations that may not know the time of the initial "start of the operation."
+ * However, a timeout can be converted to a {@code Deadline} at the start of the operation and then
+ * passed to the various components unambiguously.
  */
 public final class Deadline implements Comparable<Deadline> {
   private static final SystemTicker SYSTEM_TICKER = new SystemTicker();

core/src/main/java/io/grpc/Channel.java

@@ -34,15 +34,16 @@ package io.grpc;
 import javax.annotation.concurrent.ThreadSafe;
 
 /**
- * A Channel provides an abstraction over the transport layer that is designed to be consumed
- * by stub implementations. Channel and its associated types {@link ClientCall} and
- * {@link ClientCall.Listener} exchange parsed request and response objects whereas the
- * transport layer only works with serialized data.
+ * A virtual connection to a conceptual endpoint, to perform RPCs. A channel is free to have zero or
+ * many actual connections to the endpoint based on configuration, load, etc. A channel is also free
+ * to determine which actual endpoints to use and may change it every RPC, permitting client-side
+ * load balancing. Applications are generally expected to use stubs instead of calling this class
+ * directly.
  *
  * <p>Applications can add common cross-cutting behaviors to stubs by decorating Channel
  * implementations using {@link ClientInterceptor}. It is expected that most application
  * code will not use this class directly but rather work with stubs that have been bound to a
- * Channel that was decorated during application initialization,
+ * Channel that was decorated during application initialization.
  */
 @ThreadSafe
 public abstract class Channel {

core/src/main/java/io/grpc/ClientCall.java

@@ -236,7 +236,8 @@ public abstract class ClientCall<ReqT, RespT> {
    * just a suggestion and the application is free to ignore it, however doing so may
    * result in excessive buffering within the call.
    *
-   * <p>This implementation always returns {@code true}.
+   * <p>This abstract class's implementation always returns {@code true}. Implementations generally
+   * override the method.
    */
   public boolean isReady() {
     return true;

core/src/main/java/io/grpc/LoadBalancer.java

@@ -198,8 +198,6 @@ public abstract class LoadBalancer {
   /**
    * The main balancing logic.  It <strong>must be thread-safe</strong>. Typically it should only
    * synchronize on its own state, and avoid synchronizing with the LoadBalancer's state.
-   *
-   * <p>Note: Implementations should override exactly one {@code pickSubchannel}.
    */
   @ThreadSafe
   public abstract static class SubchannelPicker {

core/src/main/java/io/grpc/ServerBuilder.java

@@ -129,7 +129,9 @@ public abstract class ServerBuilder<T extends ServerBuilder<T>> {
 
   /**
    * Sets a fallback handler registry that will be looked up in if a method is not found in the
-   * primary registry.
+   * primary registry. The primary registry (configured via {@code addService()}) is faster but
+   * immutable. The fallback registry is more flexible and allows implementations to mutate over
+   * time and load services on-demand.
    *
    * @return this
    * @since 1.0.0

core/src/main/java/io/grpc/ServerTransportFilter.java

@@ -39,8 +39,7 @@ package io.grpc;
  * <p>Multiple filters maybe registered to a server, in which case the output of a filter is the
  * input of the next filter.  For example, what returned by {@link #transportReady} of a filter is
  * passed to the same method of the next filter, and the last filter's return value is the effective
- * transport attributes.  A filter should modify the passed-in attributes instead of creating one
- * from scratch.
+ * transport attributes.
  *
  * <p>{@link Grpc} defines commonly used attributes.
  */

stub/src/main/java/io/grpc/stub/AbstractStub.java

@@ -43,16 +43,20 @@ import io.grpc.ExperimentalApi;
 import io.grpc.ManagedChannelBuilder;
 import java.util.concurrent.TimeUnit;
 import javax.annotation.Nullable;
+import javax.annotation.concurrent.ThreadSafe;
 
 /**
- * Common base type for stub implementations.
+ * Common base type for stub implementations. Stub configuration is immutable; changing the
+ * configuration returns a new stub with updated configuration. Changing the configuration is cheap
+ * and may be done before every RPC, such as would be common when using {@link #withDeadlineAfter}.
  *
- * <p>This is the base class of the stub classes from the generated code. It allows for
- * reconfiguration, e.g., attaching interceptors to the stub.
+ * <p>Configuration is stored in {@link CallOptions} and is passed to the {@link Channel} when
+ * performing an RPC.
  *
  * @since 1.0.0
  * @param <S> the concrete type of this stub.
  */
+@ThreadSafe
 public abstract class AbstractStub<S extends AbstractStub<S>> {
   private final Channel channel;
   private final CallOptions callOptions;

stub/src/main/java/io/grpc/stub/CallStreamObserver.java

@@ -54,8 +54,10 @@ import io.grpc.ExperimentalApi;
 public abstract class CallStreamObserver<V> implements StreamObserver<V> {
 
   /**
-   * If {@code true} indicates that a call to {@link #onNext(Object)} will not require the entire
-   * message to be buffered before it is sent to the peer.
+   * If {@code true}, indicates that the observer is capable of sending additional messages
+   * without requiring excessive buffering internally. This value is just a suggestion and the
+   * application is free to ignore it, however doing so may result in excessive buffering within the
+   * observer.
    */
   public abstract boolean isReady();