RFR: 8260366: ExtendedSocketOptions <clinit> can deadlock in some circumstances

Jaikiran Pai jpai at openjdk.java.net
Wed Feb 17 07:25:59 UTC 2021


Can I please get a review for this change which proposes to fix the issue reported in https://bugs.openjdk.java.net/browse/JDK-8260366?

The issue relates to the concurrent classloading of `sun.net.ext.ExtendedSocketOptions` and `jdk.net.ExtendedSocketOptions` leading to a deadlock. This is because the `sun.net.ext.ExtendedSocketOptions` in its static block does a `Class.forName("jdk.net.ExtendedSocketOptions")`. The `jdk.net.ExtendedSocketOptions` in its own static block calls the `register` method on `sun.net.ext.ExtendedSocketOptions`. If 2 threads concurrently try loading these classes (one loading the `sun.net.ext.ExtendedSocketOptions` and the other loading `jdk.net.ExtendedSocketOptions`), it can so happen that each one ends up holding a classloading lock in the static block of the respective class, while waiting for the other thread to release the lock, thus leading to a deadlock. The stacktrace posted in the linked JBS issue has the necessary details on the deadlock.

The commit here breaks this deadlock by moving out the `Class.forName("jdk.net.ExtendedSocketOptions")` call from the static block of `sun.net.ext.ExtendedSocketOptions` to the `getInstance()` method, thus lazily loading (on first call to `getInstance()`) and registering the `jdk.net.ExtendedSocketOptions`.

Extra attention needs to be given to the `sun.net.ext.ExtendedSocketOptions#register(ExtendedSocketOptions extOptions)` method. Before the change in this PR, when the `sun.net.ext.ExtendedSocketOptions` would successfully complete loading, it was guaranteed that the registered `ExtendedSocketOptions` would either be the one registered from the `jdk.net.ExtendedSocketOptions` or a `NoExtendedSocketOptions`. There wasn't any window of chance for any code (be it in the JDK or in application code) to call the `sun.net.ext.ExtendedSocketOptions#register` to register any different/other implementation/instance of the `ExtendedSocketOptions`. However, with this change in the PR, there is now a window of chance where any code in the JDK (or even application code?) can potentially call the `sun.net.ext.ExtendedSocketOptions#register` before either the `jdk.net.ExtendedSocketOptions` is loaded or the `sun.net.ext.ExtendedSocketOptions#getInstance()` method is called, thus allowing for some oth
 er implementation of the `ExtendedSocketOptions` to be registered. However, I'm not sure if it's a practical scenario - although the `sun.net.ext.ExtendedSocketOptions#register` is marked `public`, the comment on that method and the fact that it resides in an internal, not exposed by default class/module, makes me believe that this `register` method isn't supposed to be called by anyone other than the `jdk.net.ExtendedSocketOptions`. If at all this `register` method is allowed to be called from other places, then due to the change in this PR, additional work needs to be probably done in its implementation to allow for the `jdk.net.ExtendedSocketOptions` to be given first priority(?) to be registered first. I'll need input on whether I should worry about this case or if it's fine in its current form in this PR.

This PR also contains a jtreg test which reproduces the issue and verifies the fix.

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

Commit messages:
 - 8260366: ExtendedSocketOptions <clinit> can deadlock in some circumstances

Changes: https://git.openjdk.java.net/jdk/pull/2601/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2601&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8260366
  Stats: 126 lines in 2 files changed: 112 ins; 11 del; 3 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2601.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2601/head:pull/2601

PR: https://git.openjdk.java.net/jdk/pull/2601


More information about the net-dev mailing list