Improve shutdown of non-persistent HTTP/1 connections #12212

[gregw]

Fix #12212 by improving shutdown of non-persistent HTTP/1 connections

• shutdown in SendCallback
• keep failsafe in Stream callback
• improve chunk toString clarity
• added failsafe release in onFillable, to handle case when endpoint is closed.

[gregw]

@lachlan-roberts @sbordet @lorban This work builds on #12213 by moving the shutdownOutput to the SendCallback, rather than wait for the Stream callback completion (which may be long after the response is completed).

Whilst working on this, I noticed that onFillable could exit without releasing the request buffer. So I've added a failsafe release. However, it would be good to review the release logic of the class as part of the review of this PR.
I also improved the chunk toString output, as debugging was difficult with the previous verbose toString.

I believe this change may help with the HTTP/0.9 issues we have seen

[sbordet]

@gregw I fail to exactly understand what this PR is fixing.

Is there a new test case to be added, or an existing one that fails?

[gregw]

@gregw I fail to exactly understand what this PR is fixing.

Did you read #12212 where it says:

In previous versions of jetty, we did the shutdown as the last write completed. We are now doing it as the handling callback is completed, which may be very much later than when the response is sent.
I think we should do the shutdown as soon as possible.
@sbordet thoughts?

So this is really just reverting to the behaviour that we've had for decades.

Is there a new test case to be added, or an existing one that fails?

I've written a test to check that the shutdown by a write(last) is not delayed until after callback.succeeded on the handle callback.

[lorban]

Code wise, I believe this change is good.

But the biggest issue here is IMHO that the difference between the last write and succeeding the callback is a bit tricky to implement and may need to be better javadoc'ed.

[gregw]

@sbordet @lorban fixing the buffer release in HttpConnection proved to be a game of whack-a-bug!
So I have removed the majority of the release "fixes" from this PR and they need to be addressed in different one. This PR now just moves the shutdown back to the sendCallback, but leaves a failsafe in the stream (apparently needed for some SSL tests).
I'd like to get this merged for this release and then look at the release issues next cycle.

The problems with release include:

• the releaseRequestBuffer only does a conditional release of an empty buffer, when is some cases (e.g. failure, upgrade) we want to always release.
• the releaseRequestBuffer method throws if the release is not the last one, which is counter to the concept of allowing chunks to be held with a retain.
• when we release because the buffer has been retained, we do not release if it is not empty (e.g. if another request has been pipelined).

I think it is all mostly working, but it is fragile to changes. I think it needs a bit more time to review and perhaps a big catchall release called when the stream is completed (i.e. after both succeed/fail AND the last write has completed AND the handler thread has returned).

[sbordet]

@gregw I reduced to the minimum the changes necessary, and filed #12227 for further work.

Also, @lorban fixed a number of instabilities in #12226.