If ALPN.class isn't in the bootclasspath, we don't want to find it as it
won't work. Also, if someone has fiddled with ClassLoaders, we really
want to make sure we get the proper ALPN class.
Passing "null" to Class.forName() means "bootstrap class loader". In
fact, ClassLoader.getParent() says, "Some implementations may use null
to represent the bootstrap class loader."
We must load the Proxy in the bootstrap class loader as well, because
our class loader may not have access to ALPN.{Client,Server}Provider,
for the same reasons as above. Even if we have multiple instances of
gRPC (due to class loaders), combined they will only create two Proxy
classes: one for ALPN.ClientProvider and one for ALPN.ServerProvider.
In benchmarks we would see grpc talking up tens of gigabytes of memory. A heap dump
revealed that streams would not get cleaned up and stick around in memory forever.
If a newStream is called while MAX_CONCURRENT_STREAMS is reached, then get the call blocked and the request queued, until:
1. a running stream closed, and this newStream() request is accepted.
2. receives go-away from server, fail all pending new stream requests.
3. the available ids are exhausted.
1. Adds <property name="separateLineBetweenGroups" value="true"/> to CustomImportOrder to enfore blank line between imports groups.
2. Uses checkstyle 6.5, which fixed a bug of "CustomImportOrder checks import sorting according to ASCII order instead of case-insensitive alphabetical order".
The ServerImpl import was removed because it wasn't used as far as
checkstyle was able to determine. However, it was being used to resolve
JavaDoc references. Instead of re-adding the import just make the
reference fully qualified to prevent the two systems from continuing to
disagree on whether it is needed.
The synchronization was simply using the wrong object, so no
synchronization was actually occurring. In addition, reference of cached
outside of synchronized block could permit using a partially constructed
cached object, as the reference may be set before instantiation
completes.
goingAway() is called before onGoAwayRead() in Netty:
b7f57223c1/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java (L521)
The test before checked that the stream went away, but not that the
GOAWAY code influenced our Status, as UNAVAILABLE is the default
internally.
The UNAVAILABLE default has also been changed to include a message so
that we can determine where the Status came from in case it is triggered
again in the future.
We were seeing errors on Travis like:
> Process 'Gradle Test Executor 2' finished with non-zero exit value 137
That doesn't make much sense, other than maybe the OOM killer killing
our processes. Turning off parallel execution seemed to fix the problem,
so we'll just assume memory was the actual problem and doing fewer
things in parallel reduces our maximum memory usage.
Travis documentation seems to agree with that being a likely cause:
http://docs.travis-ci.com/user/common-build-problems/#My-build-script-is-killed-without-any-error
readlink -f allows the script to be run via a symlink yet still
function. However, OS X doesn't have the -f flag. We don't really care
too much about symlinking to the scripts, so just drop readlink -f.
Parallel doesn't mix well with deployment, so if that becomes part of
Travis, we would need to pass -Dorg.gradle.parallel=false to the gradle
command line. But otherwise parallel builds have been solid.
Failing disabled by default, but setting checkstyle.ignoreFailures=false
in ~/.gradle/gradle.properties enables failing. Travis will always run
with such failing enabled.
Also add OkHttpReadableBuffer unit test, and increase the test string
length to expose the bug.
Also use array equality assertions in the base test, instead of
comparisons whose return value is discarded.
Fixes#231.
For code style, we either need a comment describing why an exception is
ignored or to actually handle the exception. In this case there doesn't
seem to be a strong reason to ignore the exception, but it isn't all
that important either, so just log at INFO.
When the description is null, the exception message would be in the form
of "INTERNAL: null", which isn't very attractive, and makes it seem like
an error in itself. If the description is null, just use "INTERNAL".