Switched to using 8192 which is the current value of Segment.SIZE and just have a test check that they are equal.
The reason for doing this is that Segment.SIZE is Kotlin internal so shouldn't be used outside of its module.
Currently this improves 2 flows
1. Known length message which length is greater than 1Mb. Previously the
first buffer was 1Mb, and then many buffers of 4096 bytes (from
CodedOutputStream), now subsequent buffers are also up to 1Mb
2. In case of compression, the first write is always 10 bytes buffer
(gzip header), but worth allocating more space
In 61f19d707a I swapped the signatures to use the version catalog. But I
failed to preserve the `@signature` extension and it all seemed to
work... But in fact all the animalsniffer tasks were completing as
SKIPPED as they lacked signatures. The build.gradle changes in this
commit are to fix that while still using version catalog.
But while it was broken violations crept in. Most violations weren't
too important and we're not surprised went unnoticed. For example, Netty
with TLS has long required the Java 8 API
`setEndpointIdentificationAlgorithm()`, so using `Optional` in the same
code path didn't harm anything in particular. I still swapped it to
Guava's `Optional` to avoid overuse of `@IgnoreJRERequirement`.
One important violation has not been fixed and instead I've disabled the
android signature in api/build.gradle for the moment. The violation is
in StatusException using the `fillInStackTrace` overload of Exception.
This problem [had been noticed][PR11066], but we couldn't figure out
what was going on. AnimalSniffer is now noticing this and agreeing with
the internal linter. There is still a question of why our interop tests
failed to notice this, but given they are no longer running on pre-API
level 24, that may forever be a mystery.
[PR11066]: https://github.com/grpc/grpc-java/pull/11066
When running on the JDK, it is quite normal for Conscrypt not to be
present. We'll end up using the JDK 9 ALPN API and everything will be
fine. On Android, it would be extremely rare for someone to completely
remove the default Android security providers, so the warning was almost
never going to trigger on that platform anyway.
Bazel had the dependency added because of #5046, where Guava was
depending on it as compile-only and Bazel build have "unknown enum
constant" warnings. Guava now has a compile dependency on j2objc, so
this workaround is no longer needed. There are currently no version skew
issues in Gradle, which was the only usage.
Using --runs_per_test=1000, this changes the flake rate of TlsTest from
2% to 0%.
While I believe it is possible to write a reliable test for this
(including noticing the SSLSocket behavior), it was becoming too
invasive so I gave up.
Fixes#11012
* Add option in OkHttpServerBuilder
* Add value as MAX_CONCURRENT_STREAM setting in settings frame sent by the server to the client per connection
* Enforce limit by sending a RST frame with REFUSED_STREAM error
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
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.
Http2OkHttp is now unnecessary, as Http2Test tests OkHttp client to
Netty server. receivedDataForFinishedStream() was the only remaining
unique test and it seems already covered by AbstractInteropTest these
days.
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
* 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 prevents grpc-util from being exposed on the classpath when
compiling code using grpc-okhttp. grpc-core is still needed because of
AbstractManagedChannelImplBuilder.
This allows okhttp to service the Grpc.newServerBuilderForPort() API.
Note that, unlike Netty, it will throw if you try to use
ServerBuilder.forPort().
This fixes android-interop-testing which was broken by c9864a119.
The PipeSocket was convenient and avoided real I/O, but the
shutdown/close while connecting/handshaking tests were triggering a
Socket bug in Java (https://bugs.openjdk.org/browse/JDK-8278326). Using
a real socket doesn't trigger the bug because the test stops sharing
state with the code under test.
Fixes#10228
```
Details
==================
WARNING: ThreadSanitizer: data race (pid=4528)
Write of size 1 at 0x0000cfb9d5f4 by thread T36 (mutexes: write M0):
#0 java.net.Socket.setCreated()V Socket.java:687
#1 java.net.AbstractPlainSocketImpl.create(Z)V AbstractPlainSocketImpl.java:149
#2 java.net.Socket.createImpl(Z)V Socket.java:477
#3 java.net.Socket.getImpl()Ljava/net/SocketImpl; Socket.java:540
#4 java.net.Socket.setTcpNoDelay(Z)V Socket.java:998
#5 io.grpc.okhttp.OkHttpServerTransport.startIo(Lio/grpc/internal/SerializingExecutor;)V OkHttpServerTransport.java:164
#6 io.grpc.okhttp.OkHttpServerTransport.lambda$start$0(Lio/grpc/internal/SerializingExecutor;)V OkHttpServerTransport.java:159
#7 io.grpc.okhttp.OkHttpServerTransport$$Lambda$56.run()V ??
#8 io.grpc.internal.SerializingExecutor.run()V SerializingExecutor.java:133
#9 java.util.concurrent.ThreadPoolExecutor.runWorker(Ljava/util/concurrent/ThreadPoolExecutor$Worker;)V ThreadPoolExecutor.java:1130
#10 java.util.concurrent.ThreadPoolExecutor$Worker.run()V ThreadPoolExecutor.java:630
#11 java.lang.Thread.run()V Thread.java:830
#12 (Generated Stub) <null>
Previous read of size 1 at 0x0000cfb9d5f4 by thread T35 (mutexes: write M1, write M2):
#0 java.net.Socket.close()V Socket.java:1512
#1 io.grpc.okhttp.OkHttpServerTransportTest$PipeSocket.close()V OkHttpServerTransportTest.java:1384
#2 io.grpc.okhttp.OkHttpServerTransportTest.clientCloseDuringHandshake()V OkHttpServerTransportTest.java:290
```
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.
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.
Our tests assume localhost is in /etc/hosts or uses some other form of
local-only resolution. But that wouldn't apply to "host". What was
happening is this was causing a DNS resolution, which would fail, and
the InetSocketAddress would be "unresolved". Thus, the equivalent and
faster code would be `InetSocketAddress.createUnresolved("host", 1234)`.
But there doesn't seem to be any reason to avoid localhost in this test,
so swap to the more typical solution instead.
This should avoid flakes like:
```
io.grpc.okhttp.OkHttpClientTransportTest > invalidAuthorityPropagates FAILED
org.junit.runners.model.TestTimedOutException: test timed out after 10 seconds
at java.base@11.0.17/java.net.Inet6AddressImpl.lookupAllHostAddr(Native Method)
at java.base@11.0.17/java.net.InetAddress$PlatformNameService.lookupAllHostAddr(InetAddress.java:929)
at java.base@11.0.17/java.net.InetAddress.getAddressesFromNameService(InetAddress.java:1529)
at java.base@11.0.17/java.net.InetAddress$NameServiceAddresses.get(InetAddress.java:848)
at java.base@11.0.17/java.net.InetAddress.getAllByName0(InetAddress.java:1519)
at java.base@11.0.17/java.net.InetAddress.getAllByName(InetAddress.java:1378)
at java.base@11.0.17/java.net.InetAddress.getAllByName(InetAddress.java:1306)
at java.base@11.0.17/java.net.InetAddress.getByName(InetAddress.java:1256)
at java.base@11.0.17/java.net.InetSocketAddress.<init>(InetSocketAddress.java:220)
at app//io.grpc.okhttp.OkHttpClientTransportTest.invalidAuthorityPropagates(OkHttpClientTransportTest.java:1687)
```
Alex Panchenko