Set discardFacades=false in Tomcat 10 Embedded to avoid premature
OutputBuffer recycling. This works around flaky tests in gRPC servlet
transport by ensuring facades are not discarded too early.
Fixes#12524
tomcat10Test is frequently failing on Windows because Tomcat doesn't
support trailers-only. I removed the FIXMEs because we won't workaround
Tomcat in grpc-servlet because:
1. An interceptor that converts trailers-only into headers+trailers
would handle most cases, as it could run directly next to the
application. That is semantically very safe. But an interceptor
doesn't help deadline handling and other transport-caused closures,
as those trailers-only are produced by the transport
2. Having the transport convert trailers-only would work for deadline
handling, but is not semantically safe
3. The other languages didn't find Tomcat to be important enough to
support HEADERS+empty DATA(END_STREAM) as trailers-only on the
client-side
```
Unexpected status: Status{code=INTERNAL, description=Received unexpected EOS on empty DATA frame from server, cause=null}
value of: getCode()
expected: DEADLINE_EXCEEDED
but was : INTERNAL
at app//io.grpc.testing.integration.AbstractInteropTest.assertCodeEquals(AbstractInteropTest.java:2028)
at app//io.grpc.testing.integration.AbstractInteropTest.timeoutOnSleepingServer(AbstractInteropTest.java:1644)
```
Currently there is a race between 2 tasks scheduled to handle request
timeout
* servlet AsyncContext timeout
* gRPC Context
which can cause instability. This change makes the gRPC Context timeout happen first.
f8700a1 stopped using the dependency in our generated code and removed
the dependency the Bazel build. 4f6948f removed mention of the
dependency in our README. This deletes it from our Gradle build and the
examples.
Seems like previously it was not testing all the flows but only: the
write/flush calls are added to the queue by `runOrBuffer` because
`readyAndDrained` is initially `false`.
So,
* `isReady()` is never called
* `isReadyReturnedFalse` never set to true
* `maybeOnWritePossible()` does nothing
All that makes me think that #9917 is caused by some bugs in older
versions of llincheck, can be closed now.
A more real simulation happens if calling `onWritePossible` first via
`initialOnWritePossible`, and then it has found a race condition in
`AsyncServletOutputStreamWriterConcurrencyTest.isReady()`, if
`maybeOnWritePossible()` is executed before `return isReady`.
Additionally updated lincheck,
* it does not need jvmArgs now
* is compatible with java 8 again
* changed asserts from Truth to junit, as lincheck was trying to use
`com.google.common.truth.Subject.equals(Object)`
It still does not detect #12268, but might be needs more time.
Currently `AsyncContext.complete()` is called multiple times in some
flows, e.g. when DEADLINE_EXCEEDED, and that results in
IllegalStateException from servlet container (Tomcat).
If counting IllegalStateException from the tests of the servlet module
here - the number of those is reduced significantly, a few cases still
left, would bet addressed separately.
The test was added in e4e7f3a06 when InProcess stopped returning a
Runnable from start(). In c5a63a1 we realized (indirectly) that there's
no point in using the Runnable any more.
This test failed with Binder (which seems to have been using the
Runnable unnecessarily), and InProcess, Netty, and OkHttp don't use the
Runnable. Instead of fixing it, we'll just move toward stopping using
Runnable.
I'm not removing the Runnable usage from Binder in this commit because
this test is currently causing CI failures and I don't want to do a
behavior change when fixing it.
Returning the runnable did nothing, as both the start method and the
runnable are run within the synchronization context. I believe the
Runnable used to be required in the previous implementation of
ManagedChannelImpl (the lock-based implementation before we created
SynchronizationContext).
This fixes a NPE seen in ServerImpl because the server expects proper
ordering of transport lifecycle events.
```
Uncaught exception in the SynchronizationContext. Panic!
java.lang.NullPointerException: Cannot invoke "java.util.concurrent.Future.cancel(boolean)" because "this.handshakeTimeoutFuture" is null
at io.grpc.internal.ServerImpl$ServerTransportListenerImpl.transportReady(ServerImpl.java:440)
at io.grpc.inprocess.InProcessTransport$4.run(InProcessTransport.java:215)
at io.grpc.SynchronizationContext.drain(SynchronizationContext.java:94)
```
b/338445186
Verifies that latest versions of Tomcat/Undertow/Jetty pass
integration tests - I manually verified that all ignored tests still
fail.
Two tests failed in Jetty, it appears that the integration test
anticipates that the server implementation is willing to send larger
trailers than the client SETTINGS frame allows for. Since the server
refuses to send too large of headers/trailers, the client does not
receive the too-large payloads, and doesn't fail with the expected
message. This change was introduced in Jetty 10.0.15/11.0.11. Those
tests are ignored.
This allows the checkForUpdates task to notice the dependencies and
suggest updates.
I plan to upgrade some of the servers after this change in hopes it
reduces test flakiness.
It is slow, and just tries a bunch of possibilities. Leave it around for
developers to run locally, but it isn't precise enough to run on every
build. This change reduces the build of servlet in isolation by
1 minute. I suspect the gains are larger in a full grpc build because it
is very CPU-intensive; it uses 600% CPU on my 8-thread CPU when running
by itself.
Instead of adding explicit dependsOn to compileJava and sourcesJar,
adding the sync task to the sourceSet automatically propagates the
dependency to all consumers of the code.
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
In d654707 we swapped compiling the uploaded artifacts to Java 11. This
caused ABI issues with ByteBuffer, like clear() returning ByteBuffer
instead of Buffer.
There are source-level approaches to avoid the accidental ABI dependency
on Java 11, but we have no tool able to detect such breakages.
We use Animalsniffer for similar cases, but it fails to detect these[1].
Since we have no tool, source-level approaches can't gain the necessary
confidence that all incompatibility fixes have been resolved.
Java has had javac-level ways to address this, but they used to require
setting bootclasspath. Since Java 9, though, they made it easier and we
can use --release, which does exactly what we need.
Fixes#10432
1. https://github.com/mojohaus/animal-sniffer/issues/77
Also convert to testFixtures, like we did elsewhere.
grpc-testing had previously been listed, but because of the missing
comma, it was not an actual dependency.
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>
Kim Jin Young