RFR 8179905: Remove the use of gettimeofday in Networking code

[Vyom Tewari]

Hi ,

Please review the code change for below issue.

Webrev  : http://cr.openjdk.java.net/~vtewari/8179905/webrev0.0/index.html

BugId : https://bugs.openjdk.java.net/browse/JDK-8179905

This issue is duplicate of "JDK-8165437", where pushed code change 
failed on 32 bit platforms so we revert back code changes as part of 
"JDK-8179602".


Please find below is the webrev that was pushed as part of "JDK-8165437".

Webrev  : http://cr.openjdk.java.net/~vtewari/8165437/webrev0.7/index.html

Thanks,

Vyom

Re: RFR 8179905: Remove the use of gettimeofday in Networking code

[Michael McMahon]

This looks fine to me Vyom.

Thanks,
Michael

On 09/05/2017, 10:55, Vyom Tewari wrote:

Hi ,

Please review the code change for below issue.

Webrev  : 
http://cr.openjdk.java.net/~vtewari/8179905/webrev0.0/index.html


BugId : https://bugs.openjdk.java.net/browse/JDK-8179905

This issue is duplicate of "JDK-8165437", where pushed code change 
failed on 32 bit platforms so we revert back code changes as part of 
"JDK-8179602".


Please find below is the webrev that was pushed as part of "JDK-8165437".

Webrev  : 
http://cr.openjdk.java.net/~vtewari/8165437/webrev0.7/index.html


Thanks,

Vyom

RFR [9] 8179021: Latest bugfixes to WebSocket/HPACK from the sandbox repo

[Pavel Rappo]

Hello,

Please review the following change:

  http://cr.openjdk.java.net/~prappo/8179021/webrev.00/

Along with refactoring this change contains a number of critical fixes for
WebSocket. Critical fixes are applied to Receiver, WebSocketImpl and
OpeningHandshake.

Also this change removes 2 convenience methods and 1 constant from WebSocket
interface. Since this interface is a part of incubation feature there shouldn't
be any problem. Those members were initially controversial. 

Thanks,
-Pavel

Re: RFR [9] 8179021: Latest bugfixes to WebSocket/HPACK from the sandbox repo

[Daniel Fuchs]

Hi Pavel,

A few nits:

jdk/incubator/http/WebSocket.java:

line 501: should use {@linkplain

Also (line 46 and 501) the link target #sendClose may cause trouble
if sendClose is overloaded - so I wonder if it may be better to
have the  full prototype there:
   #sendClose(int, String)
(though I guess it's OK as long as there is only 1 sendClose method).

jdk/incubator/http/internal/websocket/OutgoingMessage.java

 75 // maskingKeys, CharsetDecoder and should be here

 what does this means? are some words missing?

jdk/incubator/http/internal/websocket/Transmitter.java

  58  * The supplied handler may be invoked in the calling thread, 
so watch out
  59  * for stack overflow, if there's a possibility of send being 
called again

  60  * from the handler.

Can we reformulate this again?
Something like:

* The supplied handler may be invoked in the calling thread.
* A {@code StackOverflowException} may thus occur if there's a
* possibility that this method is called again by the supplied
* handler.

  93   // or tuned off

I guess you meant 'turned off'

jdk/incubator/http/internal/websocket/WebSocketImpl.java

196 if (alreadyCompleted) {
197 throw new InternalError();
...
321 if (!added) {
322 throw new InternalError();

A comment similar to the other places where InternalError is thrown
would be nice.

best regards,

-- daniel


On 09/05/2017 14:52, Pavel Rappo wrote:

Hello,

Please review the following change:

  http://cr.openjdk.java.net/~prappo/8179021/webrev.00/

Along with refactoring this change contains a number of critical fixes for
WebSocket. Critical fixes are applied to Receiver, WebSocketImpl and
OpeningHandshake.

Also this change removes 2 convenience methods and 1 constant from WebSocket
interface. Since this interface is a part of incubation feature there shouldn't
be any problem. Those members were initially controversial.

Thanks,
-Pavel

Re: RFR 8179905: Remove the use of gettimeofday in Networking code

[Claes Redestad]

Hi,

doesn't this need to consider numerical overflows, e.g., what happens if 
someone sets a timeout value near 0x7fffL before and after 
this change?


/Claes

On 2017-05-09 11:55, Vyom Tewari wrote:

Hi ,

Please review the code change for below issue.

Webrev  : 
http://cr.openjdk.java.net/~vtewari/8179905/webrev0.0/index.html


BugId : https://bugs.openjdk.java.net/browse/JDK-8179905

This issue is duplicate of "JDK-8165437", where pushed code change 
failed on 32 bit platforms so we revert back code changes as part of 
"JDK-8179602".


Please find below is the webrev that was pushed as part of "JDK-8165437".

Webrev  : 
http://cr.openjdk.java.net/~vtewari/8165437/webrev0.7/index.html


Thanks,

Vyom

Re: RFR 8179905: Remove the use of gettimeofday in Networking code

[Vyom Tewari]

Hi Claes,

thanks for review, timeout is "int" so even if you set max(2147483647) 
value that int data type can hold, there will not be any overflow. If 
you try to set very large number like "0x7fff" to int data 
type you will get compile time error(integer number too large: 
7fff).


Thanks,
Vyom

On Tuesday 09 May 2017 11:20 PM, Claes Redestad wrote:

Hi,

doesn't this need to consider numerical overflows, e.g., what happens 
if someone sets a timeout value near 0x7fffL before and 
after this change?


/Claes

On 2017-05-09 11:55, Vyom Tewari wrote:

Hi ,

Please review the code change for below issue.

Webrev  : 
http://cr.openjdk.java.net/~vtewari/8179905/webrev0.0/index.html


BugId : https://bugs.openjdk.java.net/browse/JDK-8179905

This issue is duplicate of "JDK-8165437", where pushed code change 
failed on 32 bit platforms so we revert back code changes as part of 
"JDK-8179602".


Please find below is the webrev that was pushed as part of 
"JDK-8165437".


Webrev  : 
http://cr.openjdk.java.net/~vtewari/8165437/webrev0.7/index.html


Thanks,

Vyom