On Mon, 4 Dec 2023 06:30:01 GMT, Prasanta Sadhukhan <psadhuk...@openjdk.org> wrote:
> > That code does not look like double-checked lock, it is something > > different. It checks/init/sets one field and then returns another one. Even > > if both will be marked as volatile the method may return null, since the > > loadedImage is set to true before init of image. > > I think the field `loadedImage` still needs to be volatile as it is accessed > multiple time even though it can be argued to be non-DCL in classical way Yep. It is a DCL, essentially, but it uses a different field to protect access to another field whereas the classic DCL uses the same field that it protects. > Even if both will be marked as volatile the method may return null, since the > loadedImage is set to true before init of image. This is correct. *Good catch!* I missed it. *For correctness*, the assignment https://github.com/openjdk/jdk/blob/96d69bcf8cbf1ab20b1577196120c60e072bbe78/src/java.desktop/share/classes/javax/swing/text/html/CSS.java#L2955 has to be moved below the if-statement: if (!loadedImage) { URL url = CSS.getURL(base, svalue); if (url != null) { image = new ImageIcon(); Image tmpImg = Toolkit.getDefaultToolkit().createImage(url); if (tmpImg != null) { image.setImage(tmpImg); } } loadedImage = true; } Otherwise, the returned `image` may still be in an inconsistent state. This way, `loadedImage` is assigned `true` after `image` is initialised; that way if another thread reads `loadedImage` and its value is `true`, it can safely access `image`. ------------- PR Comment: https://git.openjdk.org/jdk/pull/16917#issuecomment-1838728030
