Linux: Fixed Display.setIcon memory leak.

src/java/org/lwjgl/opengl/DisplayImplementation.java

@@ -242,5 +242,5 @@ public interface DisplayImplementation {
 	 * @param icons Array of icons in RGBA mode
 	 * @return number of icons used.
 	 */
-	public int setIcon(ByteBuffer[] icons); 
+	public int setIcon(ByteBuffer[] icons);
 }

src/java/org/lwjgl/opengl/LinuxDisplay.java

@@ -692,6 +692,27 @@ final class LinuxDisplay implements DisplayImplementation {
 		throw new UnsupportedOperationException();
 	}
 	
+	private static void convertIcon(ByteBuffer icon, int width, int height) {
+		int x = 0;
+		int y = 5;
+		byte r,g,b,a;
+
+		int depth = 4;
+
+		for (y = 0; y < height; y++) {
+			for (x = 0; x < width; x++) {
+				r = icon.get((x*4)+(y*width*4));
+				g = icon.get((x*4)+(y*width*4)+1);
+				b = icon.get((x*4)+(y*width*4)+2);
+				a = icon.get((x*4)+(y*width*4)+3);
+
+				icon.put((x*depth)+(y*width*depth), b); // blue
+				icon.put((x*depth)+(y*width*depth)+1, g); // green
+				icon.put((x*depth)+(y*width*depth)+2, r);
+				icon.put((x*depth)+(y*width*depth)+3, a);
+			}
+		}
+	}
 
 	/**
 	 * Sets one or more icons for the Display.
@@ -700,23 +721,36 @@ final class LinuxDisplay implements DisplayImplementation {
 	 * <li>Linux (and similar platforms) expect one 32x32 icon.</li>
 	 * <li>Mac OS X should be supplied one 128x128 icon</li>
 	 * </ul>
-	 * The implementation will use the supplied ByteBuffers with image data in RGBA and perform any conversions nescesarry for the specific platform.
+	 * The implementation will use the supplied ByteBuffers with image data in RGBA and perform any conversions necessary for the specific platform.
 	 *
 	 * @param icons Array of icons in RGBA mode
 	 * @return number of icons used.
 	 */
 	public int setIcon(ByteBuffer[] icons) {
-		for (int i=0;i<icons.length;i++) {
+		lockAWT();
-			int size = icons[i].limit() / 4;
+		try {
-			
+			incDisplay();
-			if (((int) Math.sqrt(size)) == 32) {
+			try {
-				nSetWindowIcon(icons[i]);
+				for (int i=0;i<icons.length;i++) {
-				return 1;
+					int size = icons[i].limit() / 4;
+					int dimension = (int)Math.sqrt(size);
+					if (dimension == 32) {
+						convertIcon(icons[i], dimension, dimension);
+						nSetWindowIcon(icons[i], icons[i].capacity(), dimension, dimension);
+						return 1;
+					}
+				}
+				return 0;
+			} finally {
+				decDisplay();
 			}
+		} catch (LWJGLException e) {
+			LWJGLUtil.log("Failed to set display icon: " + e);
+			return 0;
+		} finally {
+			unlockAWT();
 		}
-		
-		return 0;
 	}
 	
-	private static native int nSetWindowIcon(ByteBuffer icon);
+	private static native void nSetWindowIcon(ByteBuffer icon, int icons_size, int width, int height);
 }

src/native/linux/org_lwjgl_opengl_Display.c

@@ -74,6 +74,7 @@ static Atom delete_atom;
 static Colormap cmap;
 static Window current_win;
 static int current_depth;
+static Pixmap current_icon_pixmap;	
 
 static Visual *current_visual;
 
@@ -316,6 +317,13 @@ JNIEXPORT void JNICALL Java_org_lwjgl_opengl_LinuxDisplay_nSetTitle(JNIEnv * env
 	free(title);
 }
 
+static void freeIconPixmap() {
+	if (current_icon_pixmap != 0) {
+		XFreePixmap(getDisplay(), current_icon_pixmap);
+		current_icon_pixmap = 0;
+	}
+}
+
 static void destroyWindow(JNIEnv *env) {
 	if (glx_window != None) {
 		lwjgl_glXDestroyWindow(getDisplay(), glx_window);
@@ -323,6 +331,7 @@ static void destroyWindow(JNIEnv *env) {
 	}
 	XDestroyWindow(getDisplay(), current_win);
 	XFreeColormap(getDisplay(), cmap);
+	freeIconPixmap();
 	setRepeatMode(env, AutoRepeatModeDefault);
 }
 
@@ -513,61 +522,53 @@ JNIEXPORT void JNICALL Java_org_lwjgl_opengl_LinuxDisplay_nUnlockAWT(JNIEnv *env
 	jawt.Unlock(env);
 }
 
-static int setIcon(char *data,int width,int height) {
+static void setIcon(JNIEnv *env, char *data, int icon_size, int width,int height) {
 	XWMHints* win_hints;
-	int x = 0;
+	freeIconPixmap();
-	int y = 5;
+	current_icon_pixmap = XCreatePixmap(getDisplay(), getCurrentWindow(), width, height, current_depth);
-	char r,g,b,a;
+	/* We need to copy the image data since XDestroyImage will also free its data buffer, which can't be allowed
-	
+	 * since the data buffer is managed by the jvm (it's the storage for the direct ByteBuffer)
-	int depth = 4;
+	 */
-	
+	char *icon_copy = (char *)malloc(sizeof(*icon_copy)*icon_size);
-	for (y=0;y<height;y++) {
+
-		for (x=0;x<width;x++) {
+	if (icon_copy == NULL) {
-			r = data[(x*4)+(y*width*4)];
+		throwException(env, "malloc failed");
-			g = data[(x*4)+(y*width*4)+1];
+		return;
-			b = data[(x*4)+(y*width*4)+2];
+	}
-			a = data[(x*4)+(y*width*4)+3];
+	memcpy(icon_copy, data, icon_size);
-			
+	XImage *image = XCreateImage(getDisplay(), current_visual, current_depth, ZPixmap, 0, icon_copy, width, height, 32, 0);
-			data[(x*depth)+(y*width*depth)] = b; // blue
+	if (image == NULL) {
-			data[(x*depth)+(y*width*depth)+1] = g; // green
+		freeIconPixmap();
-			data[(x*depth)+(y*width*depth)+2] = r;
+		free(icon_copy);
-			data[(x*depth)+(y*width*depth)+3] = a;
+		throwException(env, "XCreateImage failed");
-		}
+		return;
 	}
 	
-	Pixmap icon_pixmap = XCreatePixmap(getDisplay(), getCurrentWindow(), width, height, current_depth);	
+	GC gc = XCreateGC(getDisplay(), current_icon_pixmap, 0, NULL);
-	
+	XPutImage(getDisplay(), current_icon_pixmap, gc, image, 0, 0, 0, 0, width, height);
-	XImage *image = XCreateImage(getDisplay(), current_visual, current_depth, ZPixmap, 0, data, width, height, 32, 0);
+	XFreeGC(getDisplay(), gc);
-	
+	XDestroyImage(image);
-	GC gc = XCreateGC(getDisplay(), icon_pixmap, 0, NULL);
+	// We won't free icon_copy because it is freed by XDestroyImage
-	
-	XPutImage(getDisplay(), icon_pixmap, gc, image, 0, 0, 0, 0, width, height);
 	
 	win_hints = XAllocWMHints();
-	if (!win_hints) {
+	if (win_hints == NULL) {
-    		return -1;
+		throwException(env, "XAllocWMHints failed");
+		return;
 	}
 	
-    	win_hints->flags = IconPixmapHint;               
+	win_hints->flags = IconPixmapHint;               
-	win_hints->icon_pixmap = icon_pixmap;
+	win_hints->icon_pixmap = current_icon_pixmap;
 	
 	XSetWMHints(getDisplay(), getCurrentWindow(), win_hints);
 	XFree(win_hints);
 	XFlush(getDisplay());
-	
-	return 0;
 }
 
-JNIEXPORT jint JNICALL Java_org_lwjgl_opengl_LinuxDisplay_nSetWindowIcon
+JNIEXPORT void JNICALL Java_org_lwjgl_opengl_LinuxDisplay_nSetWindowIcon
-  (JNIEnv *env, jclass clazz, jobject iconBuffer)
+  (JNIEnv *env, jclass clazz, jobject iconBuffer, jint icon_size, jint width, jint height)
 {
 	char *imgData = (char *)(*env)->GetDirectBufferAddress(env, iconBuffer);
 
-	if (setIcon(imgData,32,32) == 0) 
+	setIcon(env, imgData, icon_size, width, height);
-	{
-		return 1;
-	}
-	
-	return 0;
 }