Undisable ee9 BlockingTest and fix HttpChannel.produceContent

[janbartel]

HttpChannel.produceContent throws NPE if the core request has already been finished when it is called. This caused some of the BlockingTests to fail.

[gregw]

@lorban is this reasonable?

[lorban]

coreContextRequest can only be null after recycle(), so returning null in this case is wrong as that implies a call to needContent() should follow.

I think returning an new ErrorContent(new IllegalStateException("Illegal read from complete request")) would be the right thing to do when coreContextRequest is null.

[janbartel]

I don't see the difference between returning null here, and returning null at line 179 when there is no chunk?

[lorban]

produceContent() returning null has a special meaning: that means there's no content available right away, but more will follow, i.e.: everything is normal, there's just no content to be read right away. Error cases OTOH should always be returned in the form of an ErrorContent object.

When coreContextRequest is null, this means the channel has been recycled, which is an error case.

By the way, this coreContextRequest null check should also be done in needContent() as both needContent() and produceContent() fulfill the same contract about the availability of content.

[janbartel]

@lorban it's not possible to return ErrorContent for needContent(): you can return false or throw an Exception, which would you prefer?

[lorban]

@janbartel as part of my commit, I changed needContent() to make it return true when coreContextRequest is null.

[janbartel]

@lorban are some of these tests racy? When I run BlockingTest in ee9 and ee88 locally on my machine, they all pass. When they run in CI, ee88 all fail with an IllegalStateException due to the channel being recycled. See https://jenkins.webtide.net/blue/organizations/jenkins/jetty.project/detail/PR-12529/13/tests

[lorban]

@janbartel Most of those tests were flaky, so I fixed them. But testBlockingReadAndBlockingWriteGzipped() was failing on a genuine bug in HttpChannelState.ChannelResponse which I fixed too.

But I would appreciate more pairs of eyes (@gregw? @sbordet?) to look at my fix as it's touching delicate code.