> This PR, addressing 8343791, enhances `Socket#connect()` and effectively 
> makes it close the `Socket` if `SocketImpl#connect()` fails with a
> 
> 1. `SocketTimeoutException`
> 2. `InterruptedIOException` in an interrupted vthread
> 3. `IOException` that is *not* an `UnknownHostException`
> 
> On the other hand, socket will *not* be closed if `SocketImpl#connect()` 
> fails with a
> 
> * `InterruptedIOException` that is neither a `SocketTimeoutException`, nor 
> from an interrupted vthread
> * `UnknownHostException`
> 
> Note that in case of an `UHE`, `Socket` and `SocketImpl` states will match, 
> i.e., `SOCKET_CREATED`.
> 
> One peculiar detail retained from the old code is that 
> `InterruptedIOException` is causing a socket close *iff* we are in an 
> interrupted vhtread. (Guess: To ensure resources within a vthread task scope 
> is released in tandem with the completion of the scope?) I don't know why 
> other `InterruptedIOException` cases are left out.
> 
> Before "8284161: Implementation of Virtual Threads (Preview)" (9583e365), 
> `connect()` failures were getting propagated untouched. 8284161 added the 
> logic to throw `SocketException("Closed by interrupt")` *and* close the 
> socket if `InterruptedIOException` is caught in an interrupted vthread.
> 
> Judging from the last state of the `Socket#connect()` javadoc, I find it very 
> difficult to wrap ones mind around in which cases the socket will be closed. 
> As a user, I'd be in favor of a simpler approach – e.g., `try { 
> socketImpl.connect(); } catch (Exception e) { closeQuietly(e); throw e }` – 
> at the cost of incorporating a backward incompatible behavioral change. I 
> would appreciate your advice on how shall we proceed further.

Volkan Yazıcı has updated the pull request incrementally with one additional 
commit since the last revision:

  Incorporate more review feedback

-------------

Changes:
  - all: https://git.openjdk.org/jdk/pull/22160/files
  - new: https://git.openjdk.org/jdk/pull/22160/files/7e633d8d..424fc8cc

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=22160&range=04
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=22160&range=03-04

  Stats: 62 lines in 2 files changed: 20 ins; 25 del; 17 mod
  Patch: https://git.openjdk.org/jdk/pull/22160.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/22160/head:pull/22160

PR: https://git.openjdk.org/jdk/pull/22160

Reply via email to