Handles Netty write frame failures caused by issues in the Netty
itself.
Normally we don't need to do anything on frame write failures because
the cause of a failed future would be an IO error that resulted in
the stream closure. Prior to this PR we treated these issues as a
noop, except the initial headers write on the client side.
However, a case like netty/netty#13805 (a bug in generating next
stream id) resulted in an unclosed stream on our side. This PR adds
write frame future failure handlers that ensures the stream is
cancelled, and the cause is propagated via Status.
Fixes#10849
The recommended way to load dependencies from `rules_jvm_external`
is to make use of the `@maven` workspace, and the most readable
way of doing that is to use the `artifact` macro provides.
This removes the need to generate the "compat" namespaces, which
`rules_jvm_external` provided for backwards compatibility with
older releases. This change also sets things up for supporting
`bzlmod`: this requires all workspaces accessed by a library to
be named "up front" in the `MODULE.bazel` file. This way, the
only repo that needs to be exported is `@maven`, rather than the
current huge list.
* Allow the queued byte threshold for a Stream to be ready to be configurable
- on clients this is exposed by setting a CallOption
- on servers this is configured by calling a method on ServerCall or ServerStreamListener
`project(':grpc-netty').configurations` requires the grpc-netty project
to be configured, which requires evaluationDependsOn. Without
evaluationDependsOn, project loading order is arbitrary and you can get
random errors after small configuration changes.
SelfSignedCertificate is not available on Java 17 because
OpenJdkSelfSignedCertGenerator is not available. This only impacted
tests.
AccessController is being removed, and these locations are doing simple
reflection which is unlikely to require it even when a security policy
is in effect. There's other places we do reflection without the
AccessController, so either no security policies care or the users can
update their policies to allow it.
`-link` does I/O to download the package list, for every javadoc
invocation. There is no caching, so this happens many times per build.
Swap to offline mode to avoid spamming the servers, and avoid build
failures if the servers aren't entirely healthy.
It wasn't actually being used. Since Java 8u252 in early 2020 we've been
using ALPN from the JDK. The Jetty ALPN Agent has been a noop.
We do keep the Jetty ALPN support in the code and tests, but we don't
have the infrastructure to actually run it.
Add the 'fake' dependency to grpc-netty instead of grpc-core.
grpc-okhttp already depends on grpc-util and probably would be fine
without round_robin on Android.
There's not actually a circular dependency, but some tools can't handle
the compile vs runtime distinction. Such tools are broken, but fixes
have been slow and this approach works with no real downfalls.
Works around #10576#10701
* netty: improve server handling of writes to reset streams
A server stream can be reset by the client while server writes are still queued. After the stream is reset, the netty connection will forget the stream object. The `NettyServerHandler` must deal with that situation. `sendResponseHandlers` already had some code to do that. This change standardizes that code and adds it to `sendGrpcFrame`. This fixes a potential bug where a `SendGrpcFrameCommand` with `endOfStream=true` would raise an `AssertionError` if written to a reset stream. (This bug is not currently reachable because `endOfStream=false` for all server `SendGrpcFrameCommand` objects.)
* Do not call into the encoder when we know the stream is gone.
6efa9ee3 added `volatile` to `attributes` after TSAN detected a data
race that was added in 91d15ce4. The race was because attributes may be
read from another thread after `transportReady()`, and the
post-filtering assignment occurred after `transportReady()`. The code
now filters the attributes separately so they are updated before calling
`transportReady()`.
Original TSAN failure:
```
Read of size 4 at 0x0000cd44769c by thread T23:
#0 io.grpc.netty.NettyClientHandler.getAttributes()Lio/grpc/Attributes; NettyClientHandler.java:327
#1 io.grpc.netty.NettyClientTransport.getAttributes()Lio/grpc/Attributes; NettyClientTransport.java:363
#2 io.grpc.netty.NettyClientTransport.newStream(Lio/grpc/MethodDescriptor;Lio/grpc/Metadata;Lio/grpc/CallOptions;[Lio/grpc/ClientStreamTracer;)Lio/grpc/internal/ClientStream; NettyClientTransport.java:183
#3 io.grpc.internal.MetadataApplierImpl.apply(Lio/grpc/Metadata;)V MetadataApplierImpl.java:74
#4 io.grpc.auth.GoogleAuthLibraryCallCredentials$1.onSuccess(Ljava/util/Map;)V GoogleAuthLibraryCallCredentials.java:141
#5 com.google.auth.oauth2.OAuth2Credentials$FutureCallbackToMetadataCallbackAdapter.onSuccess(Lcom/google/auth/oauth2/OAuth2Credentials$OAuthValue;)V OAuth2Credentials.java:534
#6 com.google.auth.oauth2.OAuth2Credentials$FutureCallbackToMetadataCallbackAdapter.onSuccess(Ljava/lang/Object;)V OAuth2Credentials.java:525
...
Previous write of size 4 at 0x0000cd44769c by thread T24:
#0 io.grpc.netty.NettyClientHandler$FrameListener.onSettingsRead(Lio/netty/channel/ChannelHandlerContext;Lio/netty/handler/codec/http2/Http2Settings;)V NettyClientHandler.java:920
#1 io.netty.handler.codec.http2.DefaultHttp2ConnectionDecoder$FrameReadListener.onSettingsRead(Lio/netty/channel/ChannelHandlerContext;Lio/netty/handler/codec/http2/Http2Settings;)V DefaultHttp2ConnectionDecoder.java:515
...
```
The KeepAliveManager clamps the keepalive and keepalive time values such that the result is larger than the minimum values specified here. Therefore, a second check here is unnecessary.
All the changes outside libs.versions.toml and examples were
because of ErrorProne. It didn't actually find anything to fix; signal
vs noise has gotten pretty bad with the newer checks.
Status was changed for ErrorProne's SuperCallToObjectMethod. With the
old code it didn't notice the trivial implementation. The fail-for-test
code wasn't used, so it was easiest to just remove it.
Some of the libs had their versions inlined; now that we have
:checkForUpdates it isn't much of a risk for versions to diverge when
there's only a few artifacts sharing a version. If we need 4+ artifacts
to have the same version, then it makes sense to still use a shared
version.
Dependencies not upgraded: google-auth-libray, mockito, netty, cronet
The behavior purposefully mirrors that of Netty's
AbstractHttp2ConnectionHandlerBuilder.decoderEnforceMaxRstFramesPerWindow().
That API is not available to our code as we extend the
Http2ConnectionHandler, but we want our API to be able to delegate to
Netty's in the future if that ever becomes possible.
Huffman in the datacenter doesn't add much value in the common cases. It could be useful to turn on huffman based on the connection latency (say, >10ms means "assume cross-datacenter") but the Netty API doesn't lend it to that. The savings here aren't huge and it is expensive; the table provides the biggest savings.
* core, netty, okhttp: implement new logic for nameResolverFactory API in channelBuilder
fix ManagedChannelImpl to use NameResolverRegistry instead of NameResolverFactory
fix the ManagedChannelImplBuilder and remove nameResolverFactory
* Integrate target parsing and NameResolverProvider searching
Actually creating the name resolver is now delayed to the end of
ManagedChannelImpl.getNameResolver; we don't want to call into the name
resolver to determine if we should use the name resolver.
Added getDefaultScheme() to NameResolverRegistry to avoid needing
NameResolver.Factory.
---------
Co-authored-by: Eric Anderson <ejona@google.com>
This breaks the ABI of the classes listed below.
Users that recompiled their code using grpc-java [`v1.36.0`]
(https://github.com/grpc/grpc-java/releases/tag/v1.36.0) (released on
Feb 23, 2021) and later, ARE NOT AFFECTED.
Users that compiled their source using grpc-java earlier than
[`v1.36.0`]
(https://github.com/grpc/grpc-java/releases/tag/v1.36.0) need to
recompile when upgrading to grpc-java `v1.59.0`. Otherwise the code
will fail on runtime with `NoSuchMethodError`. For example, code:
```java
NettyChannelBuilder.forTarget("localhost:100").maxRetryAttempts(2);
```
Will fail with
> `java.lang.NoSuchMethodError: 'io.grpc.internal.AbstractManagedChannelImplBuilder
io.grpc.netty.NettyChannelBuilder.maxRetryAttempts(int)'`
**Affected classes**
Class `AbstractManagedChannelImplBuilder` is deleted, and no longer in
the class hierarchy of the channel builders:
- `io.grpc.netty.NettyChannelBuilder`
- `io.grpc.okhttp.OkhttpChannelBuilder`
- `grpc.cronet.CronetChannelBuilder`
This breaks the ABI of the classes listed below.
Users that recompiled their code using grpc-java [`v1.36.0`]
(https://github.com/grpc/grpc-java/releases/tag/v1.36.0) (released on
Feb 23, 2021) and later, ARE NOT AFFECTED.
Users that compiled their source using grpc-java earlier than
[`v1.36.0`]
(https://github.com/grpc/grpc-java/releases/tag/v1.36.0) need to
recompile when upgrading to grpc-java `v1.59.0`. Otherwise the code
will fail on runtime with `NoSuchMethodError`. For example, code:
```java
NettyServerBuilder.forPort(80).directExecutor();
```
Will fail with
> `java.lang.NoSuchMethodError: 'io.grpc.internal.AbstractServerImplBuilder
io.grpc.netty.NettyServerBuilder.directExecutor()'`
**Affected classes**
Class `AbstractServerImplBuilder` is deleted, and no longer in the
class hierarchy of the server builders:
- `io.grpc.netty.NettyServerBuilder`
- `io.grpc.inprocess.InProcessServerBuilder`
When a memory leak occurs, it is really helpful to have access records
to understand where the buffer was being held when it leaked. retain()
when we create the NettyReadableBuffer already creates an access record
the ByteBuf, so here we track when the ByteBuf is passed to another
thread.
See #8330
Most changes are migrating from conventions to the equivalent
extensions. JMH, AppEngine, and Jib plugins trigger future compatibility
warnings with `--warning-mode all`.
The movement of configurations was to allow sourceSets to create the
configurations and then we just configure them. When configurations were
before sourceSets, we'd implicitly create the configuration.
The examples were _not_ updated to the newer Gradle, although the
non-Android examples work with the newer Gradle. The Android examples
use an older Android Gradle Plugin which will need to be upgraded first.
https://github.com/grpc/grpc-java/issues/10445
* Eliminate NPE by skipping further processing when stream is defined, but doesn't have a property for streamKey (header processing identified an error)
Fixes#10364
* Add unit test for missing content type
Ciphers have been "fast enough" for testing since early Java 8 updates;
we haven't needed to override ciphers since we dropped Java 7 support.
Java 8u252 had ALPN, so Conscrypt or Jetty ALPN hasn't been necessary
for basic testing for a while. We still want specialized testing for
Conscrypt, but only tests testing Conscrypt need to care.
This avoids the (often missing) evaluationDependsOn and fixes using
results from other projects without propagating those through
Configuration. It also reduces the number of useless classes pulled in
by down-stream tests, reducing the probability of rebuilds.
The expectation of fixtures is they help testing down-stream code that
use the classes in main. That applies to all the classes here except for
FakeClock and StaticTestingClassLoader. It would also apply to many
internal classes in grpc-testing, but let's consider cleaning that up
future work.
The pinning is unreliable in Maven and ignored by Gradle. I'm not at all
convinced that we are pinning/not pinning in appropriate projects. The
pinning also serves less of a purpose since we started encouraging the
BOM and grpc-netty-shaded. Netty's HTTP/2 API has also become somewhat
stable compared to its earlier history. If we notice an up-tick in
version skew, we can reinstate it.
The pinning is annoying in the build.gradle code and causes Maven/Gradle
to download the version list once a day, which can be troublesome to
users unaware of how to tell the tools to work offline.
It also opens our users to platform issues like seen in #10043
and #10086 where Maven Central's version list was incorrectly generated.
Or like #9664 where Gradle Plugin's repository caches packages from
JCenter but the version list is not as cachable so exposed us to JCenter
instability.
This fixes#8357, by way of "we think we won't worry any more." See
90db93b9 when it was originally introduced. And issues
like #8337, #3634.
TlsTesting.loadCert() is a public API and so should be preferred over
our internal utility. It avoids creating a temp file that has to be
deleted by a shutdown hook. Usages that needed a file were not migrated.
* Made dependency on build.gradle in protobuf tasks relative so as to make downstream tasks cacheable
* Made transformer cacheable
* Add property name to path to root build.gradle
Co-authored-by: Nelson Osacky <nelson@osacky.com>
Full end to end implementation of gRPC server as a Servlet including tests and examples
Co-authored-by: Penn (Dapeng) Zhang <zdapeng@google.com>
Co-authored-by: Chengyuan Zhang <chengyuanzhang@google.com>
ExpectedException is deprecated, so I fixed the new warnings. However,
we are still using ExpectedException many places and had previously
supressed the warning. See
https://github.com/grpc/grpc-java/issues/7467 . I did not fix those
existing instances that had suppressed the warning, since it is
unrelated to the upgrade and we have been free to fix them at any time
since we dropped Java 7.
* Revert "Revert "netty:Auto adjust BDP ping frequency (#9650)" (#9821)"
This reverts commit a2bbe84198.
* Eliminate half RTT delay in sending BDP Pings when starting up.
* Eliminate delay for bdp ping when current read would push us over the threshold.
Sergii Tkachenko