Skip to content

imageclone fix leaks when tile and/or brush are set.#884

Open
devnexen wants to merge 1 commit into
libgd:masterfrom
devnexen:gdimagecloneleaks
Open

imageclone fix leaks when tile and/or brush are set.#884
devnexen wants to merge 1 commit into
libgd:masterfrom
devnexen:gdimagecloneleaks

Conversation

@devnexen

Copy link
Copy Markdown
Contributor

those are cloned themselves which make things complicated to track to release manually thus taking care of cloned parts internally.

those are cloned themselves which make things complicated to track
to release manually thus taking care of cloned parts internally.
@thg2k

thg2k commented Jul 9, 2024

Copy link
Copy Markdown

I believe this might cause some troubles if the cloned image brush/tile are later set to a new image.

The only really clean solution I can think of is to clone the tiles and brushes (non recursively) on set and always free them on image destroy.

@cmb69

cmb69 commented Dec 21, 2024

Copy link
Copy Markdown
Member

I believe this might cause some troubles if the cloned image brush/tile are later set to a new image.

Right. And what happens if a tile/brush is set on an image, but the tile/brush is destroyed? Do we consider this a programmer error? And what happens if the same image is set as tile and brush?

The only really clean solution I can think of is to clone the tiles and brushes (non recursively) on set and always free them on image destroy.

Alternatively it should be possible to implement ref-counting for images. That might break some client's expectations, though (e.g. gdImageDestroy() might not actually destroy the image). However, cloning the tiles/brushes when cloning an image might also break some expectations (e.g. clone image → change tile/brush → draw with tile/brush).

Given that libgd is relatively low-level, it might be better to not change its behavior, but to adapt consumers (e.g. PHP).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants