RFR: 8299015: Ensure that HttpResponse.BodySubscribers.ofFile writes all bytes

[Daniel Fuchs]

On Mon, 19 Dec 2022 17:09:54 GMT, Chris Hegarty <chegar at openjdk.org> wrote:

>> Hey @ChrisHegarty glad to see you back :-)
>> The proposed changes look reasonable. I will run them in the CI and approve if the tests come back clean.
>> 
>> I am a little puzzled by the description of the issue however as AFAIR the SocketTube will not produce a list that contains more than three ByteBuffers. So unless I'm mistaken the 1024 limit in one onNext() call should never be reached (at least when invoked by the HttpClient stack). I agree this is a bug though, and it should be fixed.
>
>> Hey @ChrisHegarty glad to see you back :-) The proposed changes look reasonable. I will run them in the CI and approve if the tests come back clean.
> 
> Thanks @dfuch.
> 
>> I am a little puzzled by the description of the issue however as AFAIR the SocketTube will not produce a list that contains more than three ByteBuffers. So unless I'm mistaken the 1024 limit in one onNext() call should never be reached (at least when invoked by the HttpClient stack).
> 
> If the server sends the response as chunked, and say 1 byte of response body at a time, then several of these small chunks are effectively merged (by the kernel) into a single packet. On the receiving side, the HTTP Client can receive these in say even one byte buffer that contains several hundreds or even thousands of these chunks. This gets passed to the chunked response handler that parses them and creates a List of byte buffers, one for each small chunk, so there can be many many of these that eventually get sent the response subscriber.
> 
> I managed to intermittently reproduce this on my Mac, by hacking on the test - not pretty. I guess the test could be updated to try to provoke this with the HTTP Client, but it doesn't seem worth it, if we accept that it just needs to be fixed.

@ChrisHegarty 

It looks like we upset something :-) 
The Exception catch is probably too wide now.


subscribing
test BodySubscribersTest.nulls(ofFile): failure
java.lang.AssertionError: Expected NullPointerException to be thrown, but AssertionError was thrown
	at org.testng.Assert.expectThrows(Assert.java:1724)
	at BodySubscribersTest.nulls(BodySubscribersTest.java:118)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
	at java.base/java.lang.reflect.Method.invoke(Method.java:578)
	at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:132)
	at org.testng.internal.TestInvoker.invokeMethod(TestInvoker.java:599)
	at org.testng.internal.TestInvoker.invokeTestMethod(TestInvoker.java:174)
	at org.testng.internal.MethodRunner.runInSequence(MethodRunner.java:46)
	at org.testng.internal.TestInvoker$MethodInvocationAgent.invoke(TestInvoker.java:822)
	at org.testng.internal.TestInvoker.invokeTestMethods(TestInvoker.java:147)
	at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:146)
	at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:128)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at org.testng.TestRunner.privateRun(TestRunner.java:764)
	at org.testng.TestRunner.run(TestRunner.java:585)
	at org.testng.SuiteRunner.runTest(SuiteRunner.java:384)
	at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:378)
	at org.testng.SuiteRunner.privateRun(SuiteRunner.java:337)
	at org.testng.SuiteRunner.run(SuiteRunner.java:286)
	at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:53)
	at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:96)
	at org.testng.TestNG.runSuitesSequentially(TestNG.java:1218)
	at org.testng.TestNG.runSuitesLocally(TestNG.java:1140)
	at org.testng.TestNG.runSuites(TestNG.java:1069)
	at org.testng.TestNG.run(TestNG.java:1037)
	at com.sun.javatest.regtest.agent.TestNGRunner.main(TestNGRunner.java:93)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
	at java.base/java.lang.reflect.Method.invoke(Method.java:578)
	at com.sun.javatest.regtest.agent.MainActionHelper$AgentVMRunnable.run(MainActionHelper.java:312)
	at java.base/java.lang.Thread.run(Thread.java:1623)
Caused by: java.lang.AssertionError: null
	at org.testng.Assert.fail(Assert.java:99)
	at org.testng.Assert.fail(Assert.java:106)
	at BodySubscribersTest$1.cancel(BodySubscribersTest.java:132)
	at java.net.http/jdk.internal.net.http.ResponseSubscribers$PathSubscriber.onNext(ResponseSubscribers.java:307)
	at java.net.http/jdk.internal.net.http.ResponseSubscribers$PathSubscriber.onNext(ResponseSubscribers.java:172)
	at BodySubscribersTest.lambda$nulls$25(BodySubscribersTest.java:118)
	at org.testng.Assert.expectThrows(Assert.java:1714)
	... 29 more
subscribing

-------------

PR: https://git.openjdk.org/jdk/pull/11722