Linux/X11: Fix memory leak from created screen images #94473
Conversation
AThousandShips
added
bug
platform:linuxbsd
cherrypick:4.2
| @@ -1641,7 +1641,7 @@ Ref<Image> DisplayServerX11::screen_get_image(int p_screen) const { | |||
| ERR_FAIL_V_MSG(Ref<Image>(), vformat("XImage with RGB mask %x %x %x and depth %d is not supported.", (uint64_t)image->red_mask, (uint64_t)image->green_mask, (uint64_t)image->blue_mask, (int64_t)image->bits_per_pixel)); | |||
| } | |||
| img = Image::create_from_data(width, height, false, Image::FORMAT_RGBA8, img_data); | |||
| XFree(image); | |||
There was a problem hiding this comment.
There's another XFree(image) a few lines above.
There was a problem hiding this comment.
That part actually looks weird, why is it using image right after it was freed? I'm not sure what the original intent was but I think it's safe to just remove that line.
There was a problem hiding this comment.
It's freeing before returning (ERR_FAIL_V_MSG returns).
There was a problem hiding this comment.
I see, but is it alright that ERR_FAIL_V_MSG itself refers to image, like image->red_mask?
There was a problem hiding this comment.
No, that's another bug indeed. I would suggest saving the error message as a String before freeing image, and then pass it to ERR_FAIL_V_MSG.
Allocated XImages are improperly free'd with XFree. The X11 documentation says that XImage should use XDestroyImage to free both the image structure and the data pointed to by the image structure. Also fix a potential use-after-free bug.
akien-mga
force-pushed
the
fix-screen-image-memory-leak
branch
from
253ee7c
to
3636d9d
Compare
akien-mga
changed the title
|
Thanks! And congrats for your first merged Godot contribution 🎉 |
Allocated XImages are improperly free'd with XFree. The X11 documentation says that XImage should use
XDestroyImage to free both the image structure and the data pointed to by the image structure.