Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

devnexen
Copy link
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
Copy link

thg2k commented Jul 9, 2024

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
Copy link
Member

cmb69 commented Dec 21, 2024

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