RFR: 8222968 ByteArrayPublisher is not thread-safe resulting in broken re-use of HttpRequests

[Michael McMahon]

Hi Chris,


On 27/06/2019, 11:50, Chris Hegarty wrote:
> Michael,
>
>> On 27 Jun 2019, at 11:08, Michael McMahon<michael.x.mcmahon at oracle.com>  wrote:
>>
>> Could I get the following change reviewed please:
>>
>> The change is as suggested in the report and the test case
>> is based on the one provided.
>>
>> http://cr.openjdk.java.net/~michaelm/8222968/webrev.1/index.html
> If I’m not mistaken, then after this change there will be no explicit
> reference to the PullPublisher. There is an implicit reference from the
> nested Subscription. I think this should be ok, but just want to ensure
> that this has been considered.

Yes, that is the case. Though once the publisher is created and used
we don't care about it. The subscription does and it keeps the reference 
alive
in this particular implementation.

> On the test:
>
> 1) Can you please split the long line, L48. Maybe import the nested
>     BodyPublisher and BodyPublishers types. And move the byte array
>     initialization to a constant declaration ( that way the test can also
>     assert that it received the correct response body )

Sure.

> 2) Given the teststabilization work that is ongoing, the test should
>     probably use the loopback, for both binding and connecting. Also,
>     rather than using the IPv4 loopback literal, defer to the loopback
>     InetAddress to retrieve the host address ( which may be IPv6 ).

Right, good point

Thanks,

Michael
> -Chris.