This has a number of small benefits. First, it makes stack traces
easier to read. For example:
Old class names:
```
ServerCalls$1$1.class
ServerCalls$1.class
ServerCalls$2$1.class
ServerCalls$2.class
```
New class names:
```
ServerCalls$StreamingServerCallHandler.class
ServerCalls$StreamingServerCallHandler$StreamingServerCallListener.class
ServerCalls$UnaryServerCallHandler.class
ServerCalls$UnaryServerCallHandler$UnaryServerCallListener.class
```
This is much easier to read quickly, espcially if line numbers don't
match between HEAD and the code that prints the stack trace.
Another benefit of this is that it drops a class file from the jar
(`EmptyServerCallListener`).
Lastly, it makes it easier in the future to test, since the specific
class can be referenced from a test. Traditionally this class
hasn't been easy to test.
Moved the following APIs from `io.grpc.testing.TestUtils` to `io.grpc.internal.TestUtils`:
`InetSocketAddress testServerAddress(String host, int port)`
`InetSocketAddress testServerAddress(int port)`
`List<String> preferredTestCiphers()`
`File loadCert(String name)`
`X509Certificate loadX509Cert(String fileName)`
`SSLSocketFactory newSslSocketFactoryForCa(Provider provider, File certChainFile)`
`void sleepAtLeast(long millis)`
APIs not to be moved:
`ServerInterceptor recordRequestHeadersInterceptor()`
`ServerInterceptor recordServerCallInterceptor()`
Turns out avoiding log() alone in the static initialization is not
enough. isLoggable() can also be overridden and call back to Context.
We take a another approach, save the exception and log it outside of
the initialization block.
currentThread().getContextClassLoader() was originally used to match
ServiceLoader's default. ServiceLoader is mostly used with SPIs that
exist in the bootclasspath and so the class's ClassLoader would not
work. But in gRPC we have a library in the application's ClassLoader.
Context ClassLoader has been causing problems in any case more than one
application ClassLoader exists, like in Servlet containers, Android, and
plugins. To my knowledge, in every case the context class loader is
different from the provider's class loader, the context class loader has
caused breakage, not success. In addition, since we use static fields
for storing the provider results, using anything but the provider's
class loader is asking to have results that vary simply depending on
which thread called gRPC first.
Fixes#2375
It is misnamed if it is @ExperimentalApi. Even being @Internal is
strange, since nothing in io.grpc actually uses it and so it could
actually be in io.grpc.internal.
I'm not trying to fix the API, but just make it slightly more
consistent.
Previously ClientCallImpl's stream listener would call
stream.setDecompressor(), but this has always been a bit strange as the
only case where the call listener calls the stream and forms a bit of a
loop. It also turned out to be racy in the presence of
DelayedClientStream since DelayedClientStream does not guarantee that
the listener has processed before returning.
Now we let the stream handle decompressor selection itself. Compressor
selection on client and server and decompressor selection on server
remain unchanged. Nothing prevents them from being changed, other than
it is currently unnecessary to fix the severe compressionTest flake.
Fixes#2865Fixes#2157
## Bazel
If you prefer to use Bazel:
```
(With Bazel v0.4.5 or above.)
$ bazel build :hello-world-server :hello-world-client
$ # Run the server:
$ bazel-bin/hello-world-server
$ # In another terminal run the client
$ bazel-bin/hello-world-client
```
grpc-protobuf-lite brings in protobuf-lite as a dependency, which has
different versions of classes in protobuf. This causes duplicate classes
on the class path, which easily breaks things. Since lite the lite
runtime/codegen is incompatible with full protobuf, it makes sense to
only depend on one of the two.
grpc-protobuf actually already has a dependency on grpc-protobuf-lite,
but excludes the protobuf-lite dependency, making the dep safe. But this
is more of an implementation detail.
We only change deps so that JavaDoc, code coverage, and similar will
still include grpc-protobuf-lite.
Fixes#2985
This the cause of the flakey serverNotListening test, because the
NOOP_MESSAGE just sits around the pipeline. As a result, the
listener does not fire within the 1s verification timeout.
InternalHandlerSettings is part of "netty:internal" inside google,
which is used to allow controlled exposure of internals.
"netty:internal" depends on "netty", which consists of the rest of the
netty subproject. Therefore, "netty" should not depend on
"netty:internal".
Sadly, the serverNotListening test is still flakey after this change, but this PR fixes a legit problem.
The listener to the connect future depends on the channel pipeline being intact. But the way it is attached allows the connect attempt to fail, and have the entire pipeline being torn down by netty before the .addListener actually runs. The result is that the listener will be attached to an already completed future, and the logic will be applied to an empty pipeline.
The fundamental problem is that there are two threads, the grpc thread, and the netty thread. This PR moves the listener attaching code into the netty thread, guaranteeing the listener is attached before any connection is made. It makes more sense for the code to live inside AbstractBufferingHandler, since handlers are generally free to swallow exceptions (the alternative is to make NettyClientHandler forward exceptions up the pipeline from itself). AbstractBufferingHandler needs the special guarantees, so it will be the one with special code.
Carl Mastrangelo
zpencer
ZHANG Dapeng
Mehrdad Afshari
Bogdan Drutu
Matt Kwong
Eric Anderson
Kun Zhang
Eric Gribkoff