jetty/jetty.project

`RetainableByteBuffer.DynamicCapacity` enters a corrupt state when released

lorban opened this issue · 0 comments

Jetty version(s)
12.1.x

Description
HttpClientTransportOverHTTP2Test.testClientStopsServerDoesNotCloseClientCloses() illustrates the problem.

When you run that test, it passes but dumps the following stack trace to stderr:

java.lang.IllegalStateException: already released ReferenceCounter@7bd4937b[r=0]
	at org.eclipse.jetty.io.Retainable$ReferenceCounter.lambda$release$2(Retainable.java:226)
	at java.base/java.util.concurrent.atomic.AtomicInteger.updateAndGet(AtomicInteger.java:281)
	at org.eclipse.jetty.io.Retainable$ReferenceCounter.release(Retainable.java:223)
	at org.eclipse.jetty.io.Retainable$Wrapper.release(Retainable.java:149)
	at org.eclipse.jetty.io.RetainableByteBuffer$DynamicCapacity.release(RetainableByteBuffer.java:1913)
	at org.eclipse.jetty.http2.tests.HttpClientTransportOverHTTP2Test$13.writeFrames(HttpClientTransportOverHTTP2Test.java:602)
	at org.eclipse.jetty.http2.tests.HttpClientTransportOverHTTP2Test$13.onHeaders(HttpClientTransportOverHTTP2Test.java:588)
	at org.eclipse.jetty.http2.parser.BodyParser.notifyHeaders(BodyParser.java:115)
	at org.eclipse.jetty.http2.parser.HeadersBodyParser.onHeaders(HeadersBodyParser.java:269)
	at org.eclipse.jetty.http2.parser.HeadersBodyParser.onHeaders(HeadersBodyParser.java:264)
	at org.eclipse.jetty.http2.parser.HeadersBodyParser.parse(HeadersBodyParser.java:208)
	at org.eclipse.jetty.http2.parser.Parser.parseBody(Parser.java:229)
	at org.eclipse.jetty.http2.parser.Parser.parse(Parser.java:156)
	at org.eclipse.jetty.http2.parser.ServerParser.parse(ServerParser.java:121)
	at org.eclipse.jetty.http2.tests.HttpClientTransportOverHTTP2Test.testClientStopsServerDoesNotCloseClientCloses(HttpClientTransportOverHTTP2Test.java:618)
	...

This happens because writeFrames() is called more than once, and itself calls RetainableByteBuffer.DynamicCapacity.release().

The first time writeFrames() is called, everything works as expected: release() figures out it's time to cleanup and does it well.
The second time, release() relies on a ReferenceCounter instance held it a superclass that was not reset by the first call, hence it throws the above exception to complain that release was already performed. Unfortunately, in the meantime the dynamic capacity accumulator was asked to accumulate, and it did without error. At this point in time, RetainableByteBuffer.DynamicCapacity contains lifecycled resources that it does not correctly track and is incapable of releasing.