Is it safe to leak this ImageObserver in the constructor?
I am working on a class that contains an image and draws it centered at 0, 0; to do this, it extracts the height and width of the image and bases its display offset on those values. But when creating the ImageObserver in case the image is not fully loaded yet, I leak this
in the constructor:
public class Sprite extends SimplePaintable implements ImageObserver {
private final Image sprite;
private int xOffset;
private boolean xOffsetSet;
private int yOffset;
private boolean yOffsetSet;
public Sprite(Image sprite) {
this.sprite = sprite;
//warning: leaking this in constructor
int width = sprite.getWidth(this);
xOffset = width/2;
xOffsetSet = width != -1;
int height = sprite.getHeight(this);
yOffset = height/2;
yOffsetSet = height != -1;
}
@Override
public boolean imageUpdate(Image img, int infoflags, int x, int y, int width, int height) {
assert img == sprite;
if ((infoflags & WIDTH) != 0) {
xOffset = width / 2;
xOffsetSet = true;
}
if ((infoflags & HEIGHT) != 0) {
yOffset = height / 2;
yOffsetSet = true;
}
return !(xOffsetSet && yOffsetSet);
}
...
At first I thought it was good, since only the offset variables were uninitialized and their defaults were good for (not) displaying an unloaded image, but then I realized that if a call loaded as soon as getWidth(this)
was called, it could theoretically would be called imageUpdate
before the constructor completes, causing the offsets to be correctly set to imageUpdate
and then disabled by the constructor. Is this a problem, or are the images being uploaded synchronously on EDT only? If that's a concern, will the imageUpdate a method synchronized
prevent it from executing until the constructor finishes?
source to share
The leak this
in the constructor is just a warning as it could lead to a problem. If the variables from are super
initialized, but this
changes them later, but super
leaks itself, then someone is working with the variables that are not fully initialized yet.
If you are sure that nobody will access any variables while building, and there is no other way to do this (because the external iE libraries don't let you do this correctly), then it's okay to ignore this warning.
In your case, it is theoretically possible that during a call, the sprite.getWidth(this)
image calls the observer to update the progress immediately, so it imageUpdate
gets called before the constructor completes. In this case, the offset variables will be overwritten after the initialization of the constructor. And no, synchronized will not prevent the problem, since no one else is holding the lock at this point.
There are several ways to get around this:
-
Use a BufferedImage which doesn't require an observer for getWidth / getHeight. for a full download, which in some cases may result in a slight delay (if the iE is loaded over the network).
-
Correct user blocking:
private final Object offsetLock = new Object(); public Sprite(Image sprite) { this.sprite = sprite; synchronized(offsetLock) { int width = sprite.getWidth(this); xOffset = width/2; xOffsetSet = width != -1; int height = sprite.getHeight(this); yOffset = height/2; yOffsetSet = height != -1; } } @Override public boolean imageUpdate(Image img, int infoflags, int x, int y, int width, int height) { assert img == sprite; if ((infoflags & WIDTH) != 0) { synchronized(offsetLock) { xOffset = width / 2; xOffsetSet = true; } } if ((infoflags & HEIGHT) != 0) { synchronized(offsetLock) { yOffset = height / 2; yOffsetSet = true; } } return xOffsetSet && yOffsetSet; }
Downside: This will cause sync, which may cause a slight delay. You usually won't notice if it is called only a few times, but in a time-critical cycle it can be noticeable.
-
Use an external function to update the data after the build is complete:
public Sprite(Image sprite) { this.sprite = sprite; } protected updateOffsets() { updateWidth(sprite.getWidth(this)); updateHeight(sprite.getHeight(this)); } protected updateWidth(final int newWidth) { if (newWidth != -1) { xOffset = newWidth/2; xOffsetSet = true; } } protected updateHeight(final int newHeight) { if (newHeight!= -1) { yOffset = newHeight/2; yOffsetSet = true; } } @Override public boolean imageUpdate(Image img, int infoflags, int x, int y, int width, int height) { assert img == sprite; if ((infoflags & WIDTH) != 0) { updateWidth(width); } if ((infoflags & HEIGHT) != 0) { updateHeight(height); } return xOffsetSet && yOffsetSet; }
Downside: Someone has to call a method
updateOffsets()
, and before that the object is not fully initialized, which can lead to errors or require you to write a build method, which further complicates the situation.
source to share