JDK-8231401 : DatagramSocket.setSoTimeout does not specify IAE when timeout is negative
  • Type: CSR
  • Component: core-libs
  • Sub-Component: java.net
  • Priority: P4
  • Status: Closed
  • Resolution: Approved
  • Fix Versions: 14
  • Submitted: 2019-09-24
  • Updated: 2019-10-01
  • Resolved: 2019-10-01
Related Reports
CSR :  
Relates :  
Relates :  
Description
Summary
-------
Clarifies the behaviour of the `setSoTimeout()` method when given a negative `timeout` value. 

Problem
-------
With this option set to a non-zero `timeout`, a call to `receive()` for this DatagramSocket will block for the specified time. Any value less than zero is invalid, and as such, should throw an `IllegalArgumentException`. However, `setSoTimeout()` does not specify what happens if given a negative `timeout` value.

Solution
--------
The solution is to document that a negative `timeout` value will result in an `IllegalArgumentException` being thrown. As the `setSoTimeout()` method already throws an `IllegalArgumentException`, this change is effectively documenting existing long-standing behaviour. 

However, an explicit check on negative values will now be made in the `setSoTimeout()` method to avoid dependence on implementation for such a check to occur. 

Specification
-------------
 src/java.base/share/classes/java/net/DatagramSocket.java

          *
          * @param timeout the specified timeout in milliseconds.
          * @throws SocketException if there is an error in the underlying protocol, such as an UDP error.
    +    * @throws IllegalArgumentException if {@code timeout} is negative
          * @since   1.1
          * @see #getSoTimeout()
          */
         public synchronized void setSoTimeout(int timeout) throws SocketException {... }

Webrev:
http://cr.openjdk.java.net/~pconcannon/8222829/webrevs/webrev.02/
Comments
Since multiple other classes are affected, I've filed JDK-8231719 as follow-up for 14. Moving this request to Approved.
01-10-2019

Good find, the same contradiction is in Socket.setSoTimeout and ServerSocket.setTimeout javadoc. I checked the JDK 1.1 docs and the same wording existed then so it's been there for a long time. I think we should remove the sentence "The timeout must be > 0" from all three classes. Probably a separate JBS for at least Socket and ServerSocket.
01-10-2019

The existing specs for that method also state: 893 * prior to entering the blocking operation to have effect. The 894 * timeout must be {@code > 0}. 895 * A timeout of zero is interpreted as an infinite timeout. At least read without surrounding context, lines 894 and 895 make contradictory statements on the the values timeout can have. Please propose a correction for this contradiction as part of this bug fix. As a code review comment, if you have not already done so, consider passing down the particular negative value which triggered the exception.
01-10-2019

The same kind of changes have already been made for Socket and ServerSocket - see [JDK-8221248][1] [1]: https://bugs.openjdk.java.net/browse/JDK-8221248
24-09-2019