Skip to content

Commit db2b666

Browse files
Fix dt_image_update_final_size() and dt_image_get_final_size() (shutdown part 3)
In dt_image_update_final_size() we **must not** use dt_cache_release() if the entry is NULL. In dt_image_get_final_size() we kept a pointer to a dt_image_t struct and later used it after the original dt_image_t has been released. If system is under heavy cache load this could very well fail - and it does.
1 parent f771efe commit db2b666

File tree

1 file changed

+27
-24
lines changed

1 file changed

+27
-24
lines changed

src/common/image.c

Lines changed: 27 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -851,43 +851,44 @@ void dt_image_update_final_size(const dt_imgid_t imgid)
851851
&ww, &hh);
852852

853853
dt_image_t *imgtmp = dt_image_cache_get(darktable.image_cache, imgid, 'w');
854-
if(!imgtmp || (ww == imgtmp->final_width && hh == imgtmp->final_height))
855-
{
856-
dt_cache_release(&darktable.image_cache->cache, imgtmp->cache_entry);
857-
}
858-
else
854+
if(imgtmp)
859855
{
856+
const gboolean changed = (ww != imgtmp->final_width) || (hh != imgtmp->final_height);
860857
imgtmp->final_width = ww;
861858
imgtmp->final_height = hh;
862859
dt_image_cache_write_release(darktable.image_cache, imgtmp, DT_IMAGE_CACHE_RELAXED);
863-
DT_CONTROL_SIGNAL_RAISE(DT_SIGNAL_METADATA_UPDATE);
864-
DT_CONTROL_SIGNAL_RAISE(DT_SIGNAL_DEVELOP_IMAGE_CHANGED);
860+
if(changed)
861+
{
862+
dt_print(DT_DEBUG_PIPE, "[dt_image_update_final_size] for ID=%i, updated to %ix%i", imgid, ww, hh);
863+
DT_CONTROL_SIGNAL_RAISE(DT_SIGNAL_METADATA_UPDATE);
864+
DT_CONTROL_SIGNAL_RAISE(DT_SIGNAL_DEVELOP_IMAGE_CHANGED);
865+
}
865866
}
866867
}
867-
dt_print(DT_DEBUG_PIPE, "[dt_image_update_final_size] for ID=%i, updated to %ix%i", imgid, ww, hh);
868868
}
869869

870870
gboolean dt_image_get_final_size(const dt_imgid_t imgid, int *width, int *height)
871871
{
872+
if(!dt_is_valid_imgid(imgid)) return TRUE;
872873
// get the img strcut
873-
dt_image_t *imgtmp = dt_image_cache_get(darktable.image_cache, imgid, 'r');
874-
dt_image_t img = *imgtmp;
875-
dt_image_cache_read_release(darktable.image_cache, imgtmp);
876-
if(!imgtmp)
874+
dt_image_t *timg = dt_image_cache_get(darktable.image_cache, imgid, 'r');
875+
if(!timg)
877876
{
878877
*width = 0;
879878
*height = 0;
880879
return FALSE;
881880
}
882881

883882
// if we already have computed them
884-
if(img.final_height > 0 && img.final_width > 0)
883+
const gboolean available = timg->final_height > 0 && timg->final_width > 0;
884+
if(available)
885885
{
886-
*width = img.final_width;
887-
*height = img.final_height;
886+
*width = timg->final_width;
887+
*height = timg->final_height;
888888
dt_print(DT_DEBUG_PIPE, "[dt_image_get_final_size] for ID=%i from cache %ix%i", imgid, *width, *height);
889-
return FALSE;
890889
}
890+
dt_image_cache_read_release(darktable.image_cache, timg);
891+
if(available) return FALSE;
891892

892893
// we have to do the costly pipe run to get the final image size
893894
dt_print(DT_DEBUG_PIPE, "[dt_image_get_final_size] calculate it for ID=%i", imgid);
@@ -896,8 +897,9 @@ gboolean dt_image_get_final_size(const dt_imgid_t imgid, int *width, int *height
896897
dt_dev_load_image(&dev, imgid);
897898

898899
dt_dev_pixelpipe_t pipe;
899-
int wd = dev.image_storage.width, ht = dev.image_storage.height;
900-
gboolean res = dt_dev_pixelpipe_init_dummy(&pipe, wd, ht);
900+
int wd = dev.image_storage.width;
901+
int ht = dev.image_storage.height;
902+
const gboolean res = dt_dev_pixelpipe_init_dummy(&pipe, wd, ht);
901903
if(res)
902904
{
903905
// set mem pointer to 0, won't be used.
@@ -909,16 +911,17 @@ gboolean dt_image_get_final_size(const dt_imgid_t imgid, int *width, int *height
909911
&pipe.processed_height);
910912
wd = pipe.processed_width;
911913
ht = pipe.processed_height;
912-
res = TRUE;
913914
dt_dev_pixelpipe_cleanup(&pipe);
914915
}
915916
dt_dev_cleanup(&dev);
916917

917-
imgtmp = dt_image_cache_get(darktable.image_cache, imgid, 'w');
918-
imgtmp->final_width = *width = wd;
919-
imgtmp->final_height = *height = ht;
920-
dt_image_cache_write_release(darktable.image_cache, imgtmp, DT_IMAGE_CACHE_RELAXED);
921-
918+
dt_image_t * imgwr = dt_image_cache_get(darktable.image_cache, imgid, 'w');
919+
if(imgwr)
920+
{
921+
imgwr->final_width = *width = wd;
922+
imgwr->final_height = *height = ht;
923+
dt_image_cache_write_release(darktable.image_cache, imgwr, DT_IMAGE_CACHE_RELAXED);
924+
}
922925
return res;
923926
}
924927

0 commit comments

Comments
 (0)