> 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 with a new target base due to a merge or a rebase. The pull request now contains eight commits: - Merge master - Catch all `bind()` and `connect()` exceptions in ctor - Use safe-`close()` before throwing `UHE` in `connect()` - Update docs for `connect()` not closing on `UHE` - Incorporate more review feedback - Apply review feedback from Alan - Apply review suggestions by Alan & Daniel - 8343791: Close `Socket` on `connect()` failures ------------- Changes: https://git.openjdk.org/jdk/pull/22160/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=22160&range=08 Stats: 428 lines in 4 files changed: 408 ins; 12 del; 8 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