RFR: 8319925: CSS.BackgroundImage incorrectly uses double-checked locking [v2]

Alexey Ivanov aivanov at openjdk.org
Tue Dec 5 14:12:35 UTC 2023


On Tue, 5 Dec 2023 07:59:19 GMT, Prasanta Sadhukhan <psadhukhan at openjdk.org> wrote:

>> CSS.BackgroundImage.getImage uses double-checked locking but the loadedImage field isn't declared as volatile. Without the volatile modifier, double-checked locking implementation is broken.
>
> Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Use image field for DCL

> > if the URL is invalid, the image isn't loaded
> 
> As per your change, if URL is invalid ie url = null, image is not loaded but `loadedImage` is set to true so it will not give another chance to load the URL again via `CSS.getURL` just in case user decides to call setBase with a valid URL (after finding `getImage `returning null)

Exactly! The image is **never** given a chance to load for a second time.

It is how the code has always worked.

With your proposed change, the image has a chance to re-load if and only if `url == null`. And it tries re-loading over and over again until `url` becomes non-`null`. At the same time, the change to `base` is ignored if the `url` wasn't `null`… even though the image itself may not have been loaded successfully.

I do not think this is correct or desirable.

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

PR Comment: https://git.openjdk.org/jdk/pull/16917#issuecomment-1840866434


More information about the client-libs-dev mailing list