Jetty 12 - Flaky BlockedWritesWithSmallThreadPoolTest.testServerThreadsBlockedInWrites()

[sbordet]

Jetty version(s)
12+

Description
In Jetty 11, writes from a Handler via response.getOutputStream() are blocking, but internally that blocking call is converted into a non-blocking call typically via:

try (Blocker blocker = _writeBlocker.acquire())
{
    channelWrite(content, complete, blocker);
    blocker.block();
}

where blocker.invocationType==NON_BLOCKING.

For large writes that TCP congest, this means that task SelectableChannelEndPoint._runCompleteWrite is non-blocking; in case all threads are taken, unblocking a write is possible because the task is non-blocking and will be run by the selector thread when it wakes up being write-ready.

In Jetty 12, writes from a Handler receive a Callback whose invocationType depends on the HttpStream implementation.
All of them do not override getInvocationType() so they are all blocking.
This means that task SelectableChannelEndPoint._runCompleteWrite is blocking; in case all threads are taken the task will be queued, rather than executed by the selector thread, therefore locking up the server as it is the task that would have unblocked a write, but there are no threads to execute it.

This is the reason of the flakyness of this test: sometimes there always is a thread that can run the completeWrite task, but in some cases there is not and the test fails.

[sbordet]

@gregw thoughts?

There is a TODO in ChannelCallback.getInvocationType(), but none of the HttpStream implementations are marked as non-blocking, although they mostly are.

The ChannelCallback itself does some possibly blocking operation such as logging to the RequestLog and then delegates to the HttpStream, so it's unclear if it is fully safe to just rely on HttpStream.invocationType.

However, I would start by making HttpStream.getInvocationType() return NON_BLOCKING as they all should be, and then see if we have problems.

The main reasoning is that we have designed a non-blocking Handler API, but the callback passed to it is a BLOCKING callback, which seems not right.

[github-actions]

This issue has been automatically marked as stale because it has been a
full year without activity. It will be closed if no further activity occurs.
Thank you for your contributions.

[github-actions]

This issue has been closed due to it having no activity.

[sbordet]

It's important to fix this.