ayo/kitty
1
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Graphics: Fix deleted but not freed images without any references being incorrectly freed on a subsequent delete command

Browse source
This commit is contained in:
Kovid Goyal 2024-12-18 09:25:06 +05:30
parent 9f3f5984b4
commit 8facaf4fa0
No known key found for this signature in database
GPG key ID: 06BC317B515ACE7C
4 changed files with 34 additions and 11 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
docs/changelog.rst
View file

@ -91,6 +91,8 @@ Detailed list of changes
- themes kitten: When using the *Default* theme as an auto switch theme include all the actual settings values (:iss:`8124`)
- Graphics: Fix deleted but not freed images without any references being incorrectly freed on a subsequent delete command (:disc:`8129`)
0.38.0 [2024-12-15]
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

22
kitty/graphics.c
View file

@ -1894,9 +1894,9 @@ remove_ref(Image *img, ImageRef *ref) {
}
static void
filter_refs(GraphicsManager *self, const void* data, bool free_images, bool (*filter_func)(const ImageRef*, Image*, const void*, CellPixelSize), CellPixelSize cell, bool only_first_image) {
bool matched = false;
filter_refs(GraphicsManager *self, const void* data, bool free_images, bool (*filter_func)(const ImageRef*, Image*, const void*, CellPixelSize), CellPixelSize cell, bool only_first_image, bool free_only_matched) {
for (image_map_itr ii = vt_first(&self->images_by_internal_id); !vt_is_end(ii); ) { Image *img = ii.data->val;
bool matched = false;
for (ref_map_itr ri = vt_first(&img->refs_by_internal_id); !vt_is_end(ri); ) { ImageRef *ref = ri.data->val;
if (filter_func(ref, img, data, cell)) {
ri = remove_ref_itr(img, ri);
@ -1904,7 +1904,7 @@ filter_refs(GraphicsManager *self, const void* data, bool free_images, bool (*fi
matched = true;
} else ri = vt_next(ri);
}
if (!vt_size(&img->refs_by_internal_id) && (free_images || img->client_id == 0)) ii = remove_image_itr(self, ii);
if ((!free_only_matched || matched) && !vt_size(&img->refs_by_internal_id) && (free_images || img->client_id == 0)) ii = remove_image_itr(self, ii);
else ii = vt_next(ii);
if (only_first_image && matched) break;
}
@ -2004,13 +2004,13 @@ void
grman_remove_cell_images(GraphicsManager *self, int32_t top, int32_t bottom) {
CellPixelSize dummy = {0};
int32_t data[] = {top, bottom};
filter_refs(self, data, false, cell_image_row_filter_func, dummy, false);
filter_refs(self, data, false, cell_image_row_filter_func, dummy, false, true);
}
void
grman_remove_all_cell_images(GraphicsManager *self) {
CellPixelSize dummy = {0};
filter_refs(self, NULL, false, cell_image_filter_func, dummy, false);
filter_refs(self, NULL, false, cell_image_filter_func, dummy, false, true);
}
@ -2034,7 +2034,7 @@ clear_all_filter_func(const ImageRef *ref UNUSED, Image UNUSED *img, const void
void
grman_clear(GraphicsManager *self, bool all, CellPixelSize cell) {
filter_refs(self, NULL, true, all ? clear_all_filter_func : clear_filter_func, cell, false);
filter_refs(self, NULL, true, all ? clear_all_filter_func : clear_filter_func, cell, false, false);
}
static bool
@ -2091,7 +2091,7 @@ handle_delete_command(GraphicsManager *self, const GraphicsCommand *g, Cursor *c
if (self->currently_loading.loading_for.image_id) free_load_data(&self->currently_loading);
GraphicsCommand d;
switch (g->delete_action) {
#define I(u, data, func) filter_refs(self, data, g->delete_action == u, func, cell, false); *is_dirty = true; break
#define I(u, data, func) filter_refs(self, data, g->delete_action == u, func, cell, false, true); *is_dirty = true; break
#define D(l, u, data, func) case l: case u: I(u, data, func)
#define G(l, u, func) D(l, u, g, func)
case 0:
@ -2121,12 +2121,14 @@ handle_delete_command(GraphicsManager *self, const GraphicsCommand *g, Cursor *c
}
} break;
case 'f':
case 'F':
if (handle_delete_frame_command(self, g, is_dirty) != NULL) {
filter_refs(self, g, true, id_filter_func, cell, true);
case 'F': {
Image *img = handle_delete_frame_command(self, g, is_dirty);
if (img != NULL) {
remove_image(self, img);
*is_dirty = true;
}
break;
}
default:
REPORT_ERROR("Unknown graphics command delete action: %c", g->delete_action);
break;

3
kitty/screen.c
View file

@ -184,7 +184,8 @@ screen_reset(Screen *self) {
linebuf_clear(self->linebuf, BLANK_CHAR);
historybuf_clear(self->historybuf);
clear_hyperlink_pool(self->hyperlink_pool);
grman_clear(self->grman, false, self->cell_size);
grman_clear(self->main_grman, false, self->cell_size); // dont delete images in scrollback
grman_clear(self->alt_grman, true, self->cell_size);
self->modes = empty_modes;
self->saved_modes = empty_modes;
self->active_hyperlink_id = 0;

18
kitty_tests/graphics.py
View file

@ -1055,6 +1055,11 @@ def delete(ac=None, **kw):
delete()
self.ae(s.grman.image_count, 1)
self.ae(len(layers(s)), 0)
delete('A')
self.ae(s.grman.image_count, 1)
s.reset()
self.ae(s.grman.image_count, 0)
put_image(s, cw, ch)
self.ae(s.grman.image_count, 1)
delete('A')
self.ae(s.grman.image_count, 0)
@ -1104,6 +1109,18 @@ def delete(ac=None, **kw):
self.ae(s.grman.image_count, 0)
self.assertEqual(s.grman.disk_cache.total_size, 0)
# test delete but not free
s.reset()
iid = 9999999
self.ae(put_image(s, cw, ch, id=iid), (iid, 'OK'))
self.ae(put_ref(s, id=iid), (iid, ('OK', f'i={iid}')))
self.ae(put_image(s, cw, ch, id=iid+1), (iid+1, 'OK'))
self.ae(put_ref(s, id=iid+1), (iid+1, ('OK', f'i={iid+1}')))
delete('i', i=iid)
self.ae(s.grman.image_count, 2)
delete('I', i=iid+1)
self.ae(s.grman.image_count, 1)
def test_animation_frame_loading(self):
s = self.create_screen()
g = s.grman
@ -1210,6 +1227,7 @@ def expand(*rows):
self.assertIsNone(li(a='d', d='f', i=1))
img = g.image_for_client_id(1)
self.assertEqual(img['data'], b'5' * 36)
self.ae(g.image_count, 1)
self.assertIsNone(li(a='d', d='F', i=1))
self.ae(g.image_count, 0)
self.assertEqual(g.disk_cache.total_size, 0)