Re: RFR [14] 8225583: Examine the HttpResponse.BodySubscribers for null handling
Looks good to me. > On 17 Jun 2019, at 18:00, Chris Hegarty wrote: > > I moved the null check to after the signal to the downstream > subscriber. > > Updated webrev: > http://cr.openjdk.java.net/~chegar/8225583/webrev.01/ > > -Chris.
Re: IPv6 multicast binding (Bugs: JDK-8210493 JDK-8215294)
Am 18.06.19 um 08:07 schrieb Alan Bateman: > On 02/05/2019 09:25, Alan Bateman wrote: >> On 02/05/2019 08:44, Andre Naujoks wrote: >>> Hello all. >>> >>> I just noticed, that the fix from Bug JDK-8210493 was reverted for Java >>> 12. With a new bug JDK-8215294 taking over the issue. >> Yes, it caused problems so had to be reverted. In addition to >> JDK-8215294 there is also JDK-8216417 which we expect to set the >> scopeID when binding a TCP socket to an IPv6 link-local address. > Andre - I don't know if you've tracking recent changes but JDK-8216417 > was addressed in jdk-13+25 so that the scope ID is consistently used > when binding, connecting, or sending datagram packets to IPv6 addresses. > In addition, binding to an IPv6 link local address, or > connecting/sending to an IPv6 link local address on the current host, > then a scope ID corresponding to the interface index will be used when > not provided by the application. > > It would be useful if you have cycles to re-do your testing where you > bind to a IPv6 multicast address. If the Inet6Address that you specify > to bind has a scope ID (to identify the network) then it should be used. Hi Alan Thanks for letting me know. We tried the old test-code with the Java Version from Debian experimental and it does indeed work now on linux. Java on Windows still throws an Exception when trying to bind like this. Exception in thread "main" java.net.BindException: Cannot assign requested address: Cannot bind at java.base/java.net.DualStackPlainDatagramSocketImpl.socketBind(Native Method) at java.base/java.net.DualStackPlainDatagramSocketImpl.bind0(DualStackPlainDatagramSocketImpl.java:84) at java.base/java.net.AbstractPlainDatagramSocketImpl.bind(AbstractPlainDatagramSocketImpl.java:117) at java.base/java.net.DatagramSocket.bind(DatagramSocket.java:395) at de.nordsys.test.testipv6.MCastTest.main(MCastTest.java:58) Not sure, if this can be changed or not, but since IPV6_MULTICAST_ALL (or its windows counterpart) is turned off in windows by default, this can be solved on our side with different code depending on the OS. Regards Andre > > -Alan > >
Re: IPv6 multicast binding (Bugs: JDK-8210493 JDK-8215294)
On 18/06/2019 13:53, Andre Naujoks wrote: : We tried the old test-code with the Java Version from Debian experimental and it does indeed work now on linux. Thanks for confirming. Java on Windows still throws an Exception when trying to bind like this. This shouldn't be a big surprise. It's very platform specific as to whether you can bind to a multicast address or not. From the previous mails then I think the only reason you were doing this on Linux is because IP_MULTICAST_ALL doesn't work with IPv6 sockets and you are getting interference on systems with multiple network interfaces. -Alan
Re: IPv6 multicast binding (Bugs: JDK-8210493 JDK-8215294)
Am 18.06.19 um 15:20 schrieb Alan Bateman: > On 18/06/2019 13:53, Andre Naujoks wrote: >> : >> We tried the old test-code with the Java Version from Debian >> experimental and it does indeed work now on linux. > Thanks for confirming. > >> >> Java on Windows still throws an Exception when trying to bind like this. > This shouldn't be a big surprise. It's very platform specific as to > whether you can bind to a multicast address or not. From the previous > mails then I think the only reason you were doing this on Linux is > because IP_MULTICAST_ALL doesn't work with IPv6 sockets and you are > getting interference on systems with multiple network interfaces. Yes, that is the reason. We can work around this quite easily then. > > -Alan
Re: RFR [14] 8225583: Examine the HttpResponse.BodySubscribers for null handling
Hi Chris, Looks good to me. best regards, -- daniel On 17/06/2019 18:00, Chris Hegarty wrote: I moved the null check to after the signal to the downstream subscriber. Updated webrev: http://cr.openjdk.java.net/~chegar/8225583/webrev.01/ -Chris.
Re: RFR [14] 8226303: Examine the HttpRequest.BodyPublishers for exception handling
[ adding net-dev ]
Pavel,
> On 18 Jun 2019, at 14:22, Pavel Rappo wrote:
>
> Hello,
>
> Please review the following change:
>
> http://cr.openjdk.java.net/~prappo/8226303/webrev.00
This looks good. Just a few minor comments:
1) Please add the bug no. and a brief summary, to the test tags.
2) The creation and deletion of a file is necessary to exercise test
scenario, but this could prove to be problematic, at least
intermittently on Windows.
64 Files.delete(file);
65 if (Files.exists(file)) { // just an assumption of the test
66 throw new RuntimeException("File hasn't been deleted");
67 }
Can I suggest that jdk.test.lib.util.FileUtils::deleteFileWithRetry be
used instead of Files::delete, as it is less likely to fail.
-Chris.
Re: RFR [14] 8226303: Examine the HttpRequest.BodyPublishers for exception handling
Hi Chris,
Thanks for adding net-dev. I should've posted there in the first place, but
something went wrong. As for your comments:
1) I will add this before pushing the change:
* @summary Verifies that some of the standard BodyPublishers relay exception
* rather than throw it
* @bug 8226303
Is that okay?
2) Done.
Thanks.
> On 18 Jun 2019, at 16:07, Chris Hegarty wrote:
>
> [ adding net-dev ]
>
> Pavel,
>
>> On 18 Jun 2019, at 14:22, Pavel Rappo wrote:
>>
>> Hello,
>>
>> Please review the following change:
>>
>> http://cr.openjdk.java.net/~prappo/8226303/webrev.00
>
> This looks good. Just a few minor comments:
>
> 1) Please add the bug no. and a brief summary, to the test tags.
>
> 2) The creation and deletion of a file is necessary to exercise test
> scenario, but this could prove to be problematic, at least
> intermittently on Windows.
>
> 64 Files.delete(file);
> 65 if (Files.exists(file)) { // just an assumption of the test
> 66 throw new RuntimeException("File hasn't been deleted");
> 67 }
>
> Can I suggest that jdk.test.lib.util.FileUtils::deleteFileWithRetry be
> used instead of Files::delete, as it is less likely to fail.
>
> -Chris.
Re: RFR [14] 8226303: Examine the HttpRequest.BodyPublishers for exception handling
Pavel,
> On 18 Jun 2019, at 17:56, Pavel Rappo wrote:
>
> Hi Chris,
>
> Thanks for adding net-dev. I should've posted there in the first place, but
> something went wrong. As for your comments:
>
> 1) I will add this before pushing the change:
>
>* @summary Verifies that some of the standard BodyPublishers relay
> exception
>* rather than throw it
>* @bug 8226303
>
> Is that okay?
Yes.
>
> 2) Done.
>
> Thanks.
-Chris.
>> On 18 Jun 2019, at 16:07, Chris Hegarty wrote:
>>
>> [ adding net-dev ]
>>
>> Pavel,
>>
>>> On 18 Jun 2019, at 14:22, Pavel Rappo wrote:
>>>
>>> Hello,
>>>
>>> Please review the following change:
>>>
>>> http://cr.openjdk.java.net/~prappo/8226303/webrev.00
>>
>> This looks good. Just a few minor comments:
>>
>> 1) Please add the bug no. and a brief summary, to the test tags.
>>
>> 2) The creation and deletion of a file is necessary to exercise test
>> scenario, but this could prove to be problematic, at least
>> intermittently on Windows.
>>
>> 64 Files.delete(file);
>> 65 if (Files.exists(file)) { // just an assumption of the test
>> 66 throw new RuntimeException("File hasn't been deleted");
>> 67 }
>>
>> Can I suggest that jdk.test.lib.util.FileUtils::deleteFileWithRetry be
>> used instead of Files::delete, as it is less likely to fail.
>>
>> -Chris.
>
Re: RFR [14] 8226303: Examine the HttpRequest.BodyPublishers for exception handling
Hi Pavel,
Mostly looks good to me.
One remark about the test:
107 CompletableFuture future() {
108 return f.copy();
109 }
I had to read the javadoc to convince myself that this was OK ;-)
If the existing tests all pass reliably you have my review.
best regards,
-- daniel
On 18/06/2019 14:22, Pavel Rappo wrote:
Hello,
Please review the following change:
http://cr.openjdk.java.net/~prappo/8226303/webrev.00
This change increases robustness of convenience BodyPublishers by going the
extra mile and ensuring unlikely thrown exceptions are relayed downstream.
Thanks,
-Pavel
