Re: [JDK-8016521] IPV6 address selection
On 12/04/16 21:27, Bernd Eckenfels wrote: Hello, a while back I brought up the discussion that there is no preferIPV6=system (or similar) setting which allows to turn off the reordering of address families by Java. Because only the OS can try to correctly do target address determinaton. A Bug was opened and it was excluded from Java 8 - would there be a window for Java 9, still? https://bugs.openjdk.java.net/browse/JDK-8016521 I added a comment to this back in 2013, and I think it is still applicable. I assigned the bug and targeted it to 9. It should be relatively straight forward. -Chris.
Re: RFR: 8154238: Drop code to support Windows XP in windows socket impl
On 14/04/16 13:49, Claes Redestad wrote: Hi, more code in the Windows socket implementation that can be dropped Bug: https://bugs.openjdk.java.net/browse/JDK-8154238 Webrev: http://cr.openjdk.java.net/~redestad/8154238/webrev.01/ The changes look fine. Maybe the bug description should be updated a before pushing, it looks like it affects only legacy networking code and not NIO. Maybe "... and async channels" ? -Chris.
Re: RFR: 8154238: Drop code to support Windows XP in windows socket impl
On 18/04/2016 10:45, Chris Hegarty wrote: The changes look fine. Maybe the bug description should be updated a before pushing, it looks like it affects only legacy networking code and not NIO. Maybe "... and async channels" ? The changes to the NIO code look okay but probably should be split into their own issue for tracking purposes. -Alan
Re: RFR: 8154238: Drop code to support Windows XP in windows socket impl
On 04/18/2016 11:45 AM, Chris Hegarty wrote: On 14/04/16 13:49, Claes Redestad wrote: Hi, more code in the Windows socket implementation that can be dropped Bug: https://bugs.openjdk.java.net/browse/JDK-8154238 Webrev: http://cr.openjdk.java.net/~redestad/8154238/webrev.01/ The changes look fine. Thanks Chris! Maybe the bug description should be updated a before pushing, it looks like it affects only legacy networking code and not NIO. Maybe "... and async channels" ? Not sure where you saw NIO mentioned. Does "Drop code to support Windows XP in windows socket and async channel impls" sound better? Thanks! /Claes
Re: RFR: 8154238: Drop code to support Windows XP in windows socket impl
On 04/18/2016 12:01 PM, Alan Bateman wrote: On 18/04/2016 10:45, Chris Hegarty wrote: The changes look fine. Maybe the bug description should be updated a before pushing, it looks like it affects only legacy networking code and not NIO. Maybe "... and async channels" ? The changes to the NIO code look okay but probably should be split into their own issue for tracking purposes. Ah, now I get what Chris meant. Sure. I'll break those off to a separate bug. /Claes
Re: RFR: 8154238: Drop code to support Windows XP in windows socket impl
On 18/04/16 11:01, Alan Bateman wrote: On 18/04/2016 10:45, Chris Hegarty wrote: The changes look fine. Maybe the bug description should be updated a before pushing, it looks like it affects only legacy networking code and not NIO. Maybe "... and async channels" ? The changes to the NIO code look okay but probably should be split into their own issue for tracking purposes. That works for me. -Chris.
Re: RFR: 8154238: Drop code to support Windows XP in windows socket impl
On 04/18/2016 12:06 PM, Chris Hegarty wrote: On 18/04/16 11:01, Alan Bateman wrote: On 18/04/2016 10:45, Chris Hegarty wrote: The changes look fine. Maybe the bug description should be updated a before pushing, it looks like it affects only legacy networking code and not NIO. Maybe "... and async channels" ? The changes to the NIO code look okay but probably should be split into their own issue for tracking purposes. That works for me. -Chris. New webrev only with the PlainSocket changes: http://cr.openjdk.java.net/~redestad/8154238/webrev.02/ I'll file a bug and send out a RFR for the other changes shortly /Claes
Re: RFR: 8154238: Drop code to support Windows XP in windows socket impl
On 18/04/16 11:09, Claes Redestad wrote: On 04/18/2016 12:06 PM, Chris Hegarty wrote: On 18/04/16 11:01, Alan Bateman wrote: On 18/04/2016 10:45, Chris Hegarty wrote: The changes look fine. Maybe the bug description should be updated a before pushing, it looks like it affects only legacy networking code and not NIO. Maybe "... and async channels" ? The changes to the NIO code look okay but probably should be split into their own issue for tracking purposes. That works for me. -Chris. New webrev only with the PlainSocket changes: http://cr.openjdk.java.net/~redestad/8154238/webrev.02/ Looks fine. -Chris I'll file a bug and send out a RFR for the other changes shortly /Claes
ALPN and HTTP/2 client
Hi, I am missing where the ALPN negotiation is performed in the new HTTP/2 client code, can you direct me at where this is done ? The HTTP/2 spec requires to close the connection with INADEQUATE_SECURITY if the cipher does not meet the HTTP/2 requirements, but I don't see this code ? SSLEngine.getApplicationProtocol() seems only to be used for debugging ? Thanks ! -- Simone Bordet http://bordet.blogspot.com --- Finally, no matter how good the architecture and design are, to deliver bug-free software with optimal performance and reliability, the implementation technique must be flawless. Victoria Livschitz
ServiceLoader for HTTP/2 and WebSocket clients
Hi, is there any plan to make the new HTTP/2 and WebSocket client implementation replaceable via a ServiceLoader mechanism ? Thanks ! -- Simone Bordet http://bordet.blogspot.com --- Finally, no matter how good the architecture and design are, to deliver bug-free software with optimal performance and reliability, the implementation technique must be flawless. Victoria Livschitz
Re: RFR JDK-8153353: HPACK implementation
> On 13 Apr 2016, at 10:10, Chris Hegarty wrote: > > A general comment; Quite a number of classes, especially Encoder and Decoder, > would benefit from a few relatively lightweight comments, e.g. IntegerReader I would like to address it after the initial push. If possible. Other than that, I have updated the implementation: http://cr.openjdk.java.net/~prappo/8153353/webrev.01/ There's also a javadoc now: http://cr.openjdk.java.net/~prappo/8153353/javadoc.01/ Removed some FIXMEs and TODOs, updated tests. > StringReader.java > > reset() can probably just reset its readers, regardless of huffman, since > the > readers are always guaranteed to be present ( and reset is minimal ). > > General comment; in many cases it would be useful to capture, at least, the > ‘state’ > when throwing InternalError. Done. > IntegerReader.java > > reset() assumes that there is no bugs in the code. Should is assign 0 to r, > value, and maxValue as well. It will be done in the next invocation of 'configure'. > Q: is it necessary to call configure after reset? Yes. > ISO_8859_1.java > > Sorry, I don’ get 'outstanding'. Are you catching RuntimeException somewhere, > or do you expect a user of the Decoder to do some special exception handling > ? Removed. It was a leftover from a failure-atomic behaviour, which is not needed here. > HpackException.java > > Do you need this ??? ( FIXME comment ) Removed, thanks. Instead, a sandwich is thrown: UncheckedIOException(ProtocolException).
RFR: 8154454: Fix compilation issue in PlainSocketImpl
Hi,
a small omission in JDK-8154238 cause Windows builds to fail. Sorry
about that, see patch to fix this below (I was 100% certain I had run this
through JPRT last week)
Bug: https://bugs.openjdk.java.net/browse/JDK-8154454
Thanks!
/Claes
diff -r 3459ee432728
src/java.base/windows/classes/java/net/PlainSocketImpl.java
--- a/src/java.base/windows/classes/java/net/PlainSocketImpl.java Mon
Apr 18 14:01:03 2016 +0200
+++ b/src/java.base/windows/classes/java/net/PlainSocketImpl.java Mon
Apr 18 16:35:03 2016 +0200
@@ -80,7 +80,7 @@
* Constructs an instance with the given file descriptor.
*/
PlainSocketImpl(FileDescriptor fd) {
-if (useDualStackImpl) {
+if (!preferIPv4Stack) {
impl = new DualStackPlainSocketImpl(fd, exclusiveBind);
} else {
impl = new TwoStacksPlainSocketImpl(fd, exclusiveBind);
Re: RFR: 8154454: Fix compilation issue in PlainSocketImpl
Looks ok Claes.
-Chris.
On 18/04/16 15:38, Claes Redestad wrote:
Hi,
a small omission in JDK-8154238 cause Windows builds to fail. Sorry
about that, see patch to fix this below (I was 100% certain I had run this
through JPRT last week)
Bug: https://bugs.openjdk.java.net/browse/JDK-8154454
Thanks!
/Claes
diff -r 3459ee432728
src/java.base/windows/classes/java/net/PlainSocketImpl.java
--- a/src/java.base/windows/classes/java/net/PlainSocketImpl.java Mon
Apr 18 14:01:03 2016 +0200
+++ b/src/java.base/windows/classes/java/net/PlainSocketImpl.java Mon
Apr 18 16:35:03 2016 +0200
@@ -80,7 +80,7 @@
* Constructs an instance with the given file descriptor.
*/
PlainSocketImpl(FileDescriptor fd) {
-if (useDualStackImpl) {
+if (!preferIPv4Stack) {
impl = new DualStackPlainSocketImpl(fd, exclusiveBind);
} else {
impl = new TwoStacksPlainSocketImpl(fd, exclusiveBind);
Re: RFR: 8154454: Fix compilation issue in PlainSocketImpl
Thanks for the quick review Chris,
I'll push as soon as JPRT runs look green.
/Claes
On 04/18/2016 04:45 PM, Chris Hegarty wrote:
Looks ok Claes.
-Chris.
On 18/04/16 15:38, Claes Redestad wrote:
Hi,
a small omission in JDK-8154238 cause Windows builds to fail. Sorry
about that, see patch to fix this below (I was 100% certain I had run
this
through JPRT last week)
Bug: https://bugs.openjdk.java.net/browse/JDK-8154454
Thanks!
/Claes
diff -r 3459ee432728
src/java.base/windows/classes/java/net/PlainSocketImpl.java
--- a/src/java.base/windows/classes/java/net/PlainSocketImpl.java Mon
Apr 18 14:01:03 2016 +0200
+++ b/src/java.base/windows/classes/java/net/PlainSocketImpl.java Mon
Apr 18 16:35:03 2016 +0200
@@ -80,7 +80,7 @@
* Constructs an instance with the given file descriptor.
*/
PlainSocketImpl(FileDescriptor fd) {
-if (useDualStackImpl) {
+if (!preferIPv4Stack) {
impl = new DualStackPlainSocketImpl(fd, exclusiveBind);
} else {
impl = new TwoStacksPlainSocketImpl(fd, exclusiveBind);
Re: RFR JDK-8153353: HPACK implementation
On 18/04/16 15:30, Pavel Rappo wrote: On 13 Apr 2016, at 10:10, Chris Hegarty wrote: A general comment; Quite a number of classes, especially Encoder and Decoder, would benefit from a few relatively lightweight comments, e.g. IntegerReader I would like to address it after the initial push. That's fine. > If possible. Other than that, I have updated the implementation: http://cr.openjdk.java.net/~prappo/8153353/webrev.01/ I'm happy with this version. -Chris. There's also a javadoc now: http://cr.openjdk.java.net/~prappo/8153353/javadoc.01/ Removed some FIXMEs and TODOs, updated tests. StringReader.java reset() can probably just reset its readers, regardless of huffman, since the readers are always guaranteed to be present ( and reset is minimal ). General comment; in many cases it would be useful to capture, at least, the ‘state’ when throwing InternalError. Done. IntegerReader.java reset() assumes that there is no bugs in the code. Should is assign 0 to r, value, and maxValue as well. It will be done in the next invocation of 'configure'. Q: is it necessary to call configure after reset? Yes. ISO_8859_1.java Sorry, I don’ get 'outstanding'. Are you catching RuntimeException somewhere, or do you expect a user of the Decoder to do some special exception handling ? Removed. It was a leftover from a failure-atomic behaviour, which is not needed here. HpackException.java Do you need this ??? ( FIXME comment ) Removed, thanks. Instead, a sandwich is thrown: UncheckedIOException(ProtocolException).
Re: ALPN and HTTP/2 client
Hi, ALPN is set in the HttpConnection class. Checking the exact ciphers selected is not implemented yet. That will come later. I am currently updating the implementation to get rid of the two threads per connection limitation and will have a new webrev then. So I will address other review comments after that. We don't have a plan to make the implementation pluggable by ServiceLoader. I can add it to the list of API issues we can look at after the first integration though. Michael On 18/04/16 14:55, Simone Bordet wrote: Hi, I am missing where the ALPN negotiation is performed in the new HTTP/2 client code, can you direct me at where this is done ? The HTTP/2 spec requires to close the connection with INADEQUATE_SECURITY if the cipher does not meet the HTTP/2 requirements, but I don't see this code ? SSLEngine.getApplicationProtocol() seems only to be used for debugging ? Thanks !
Re: ALPN and HTTP/2 client
Hi, On Mon, Apr 18, 2016 at 10:09 PM, Michael McMahon wrote: > Hi, > > ALPN is set in the HttpConnection class. > > Checking the exact ciphers selected is not implemented > yet. That will come later. I am guessing checking for the actual application protocol too ? Sending "h2" via ALPN does not mean that the server will speak "h2", but I have not seen the code that handles the application protocol negotiation on the client (i.e. the code that reads what application protocol the server chose, and arrange to speak that protocol). > I am currently updating the implementation to get > rid of the > two threads per connection limitation and will have a new webrev then. > So I will address other review comments after that. Great ! I hope you can sneak in a completely non-blocking implementation :) > We don't have a plan to make the implementation pluggable by ServiceLoader. > I can add it to the list of API issues we can look at after the first > integration though. So can you expand on the process and times for this integration ? Like draft1 end of April, May integration in mainline JDK 9, June API review, etc. ? I'm presenting the API at a conference this week, would be great to have a 10k meters view of the availability for early adopters. Thanks ! -- Simone Bordet http://bordet.blogspot.com --- Finally, no matter how good the architecture and design are, to deliver bug-free software with optimal performance and reliability, the implementation technique must be flawless. Victoria Livschitz
