Jetty-12 client calls onDataAvailable with producing thread

[gregw]

Jetty version(s)
12

Description

The HttpClient is calling onDataAvailable from within the parser, which is called from the producing thread. This can be the last producing thread in the selector and it is invoked with invokeNonBlocking, yet it still blocks.
The producer should not call application code. It should produce tasks which call application code.

The following stack illustrates the problem and is from org.eclipse.jetty.http2.tests.ReverseProxyTest#testServerBigDownloadSlowClient

"client-2655" #2655 prio=5 os_prio=0 cpu=120840.12ms elapsed=5079.80s tid=0x00007f2474c625f0 nid=0x198bd sleeping [0x00007f240dff7000]
   java.lang.Thread.State: TIMED_WAITING (sleeping)
	at java.lang.Thread.sleep(java.base@17.0.2/Native Method)
	at java.lang.Thread.sleep(java.base@17.0.2/Thread.java:337)
	at java.util.concurrent.TimeUnit.sleep(java.base@17.0.2/TimeUnit.java:446)
	at org.eclipse.jetty.http2.tests.ReverseProxyTest$6.onDataAvailable(ReverseProxyTest.java:186)
	at org.eclipse.jetty.http2.internal.HTTP2Stream.notifyDataAvailable(HTTP2Stream.java:779)
	at org.eclipse.jetty.http2.internal.HTTP2Stream.processData(HTTP2Stream.java:502)
	at org.eclipse.jetty.http2.internal.HTTP2Stream.offerAndProcess(HTTP2Stream.java:442)
	at org.eclipse.jetty.http2.internal.HTTP2Stream.onData(HTTP2Stream.java:428)
	at org.eclipse.jetty.http2.internal.HTTP2Stream.process(HTTP2Stream.java:353)
	at org.eclipse.jetty.http2.internal.HTTP2Session.onData(HTTP2Session.java:264)
	at org.eclipse.jetty.http2.internal.HTTP2Connection$ParserListener.onData(HTTP2Connection.java:408)
	at org.eclipse.jetty.http2.internal.parser.BodyParser.notifyData(BodyParser.java:103)
	at org.eclipse.jetty.http2.internal.parser.DataBodyParser.onData(DataBodyParser.java:145)
	at org.eclipse.jetty.http2.internal.parser.DataBodyParser.emptyBody(DataBodyParser.java:55)
	at org.eclipse.jetty.http2.internal.parser.Parser.parseBody(Parser.java:190)
	at org.eclipse.jetty.http2.internal.parser.Parser.parse(Parser.java:123)
	at org.eclipse.jetty.http2.internal.HTTP2Connection$HTTP2Producer.produce(HTTP2Connection.java:280)
	at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.produceTask(AdaptiveExecutionStrategy.java:512)
	at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.tryProduce(AdaptiveExecutionStrategy.java:258)
	at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.produce(AdaptiveExecutionStrategy.java:201)
	at org.eclipse.jetty.http2.internal.HTTP2Connection.produce(HTTP2Connection.java:210)
	at org.eclipse.jetty.http2.internal.HTTP2Connection.onFillable(HTTP2Connection.java:157)
	at org.eclipse.jetty.http2.internal.HTTP2Connection$FillableCallback.succeeded(HTTP2Connection.java:380)
	at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:100)
	at org.eclipse.jetty.io.SelectableChannelEndPoint$1.run(SelectableChannelEndPoint.java:53)
	at org.eclipse.jetty.util.thread.Invocable.invokeNonBlocking(Invocable.java:156)
	at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.invokeAsNonBlocking(AdaptiveExecutionStrategy.java:495)
	at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.consumeTask(AdaptiveExecutionStrategy.java:431)
	at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.tryProduce(AdaptiveExecutionStrategy.java:293)
	at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.produce(AdaptiveExecutionStrategy.java:201)
	at org.eclipse.jetty.io.ManagedSelector$$Lambda$471/0x0000000800d27c10.run(Unknown Source)
	at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:933)
	at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:1077)
	at java.lang.Thread.run(java.base@17.0.2/Thread.java:833)

How to reproduce?

org.eclipse.jetty.http2.tests.ReverseProxyTest#testServerBigDownloadSlowClient

[gregw]

Note that if somehow the HTTP2Stream could be made to go via the new Content.Source, then the Runnable passed to demand can be an Invocable and thus could be called directly by the producer, but only if it declared itself non-blocking or either. However, it would still need to be a produced task rather than a production.

[gregw]

An analysis with @sbordet and @lorban reveal that this issue is at least partially caused by a bad test, which was always demanding in onDataAvailable, even if EOF had been reached. That was always going to loop forever, but in this case the loop happened on the selector thread.

Commit a812b86 fixes the test. However, this issue is still left open, as we should not be calling application code from a producing thread in HTTP2

[sbordet]

@gregw a fix is in #11429 but only for the upper HTTP layer.

The reason is that to make this work in the low HTTP/2 layer I would have to change the signature of many HTTP/2 low level API methods.
For example Stream.Listener.onDataAvailable() now returns void but should return a Runnable, etc.

So the contract would be that the low layer must always be non-blocking, but the upper layer may be blocking as internally we can wrap the calls to the upper layer to return a Runnable, which is what I have done in #11429.