On Tue, 5 Dec 2023 07:59:19 GMT, Prasanta Sadhukhan <psadhuk...@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