When handing off a picture to a worker thread, copy it first

as otherwise we crash when the picture is freed before the
worker thread (to load from the net or to compute hashes)
is finished

Signed-off-by: Robert C. Helling <helling@atdotde.de>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
This commit is contained in:
Robert C. Helling 2016-03-15 21:31:59 +01:00 committed by Dirk Hohndel
parent 3c7e14a18f
commit 8a59d78faa
5 changed files with 30 additions and 6 deletions

View file

@ -3374,7 +3374,7 @@ void dive_set_geodata_from_picture(struct dive *dive, struct picture *picture)
}
}
static void picture_free(struct picture *picture)
void picture_free(struct picture *picture)
{
if (!picture)
return;
@ -3383,6 +3383,18 @@ static void picture_free(struct picture *picture)
free(picture);
}
// When handling pictures in different threads, we need to copy them so we don't
// run into problems when the main thread frees the picture.
struct picture *clone_picture(struct picture *src)
{
struct picture *dst;
dst = alloc_picture();
copy_pl(src, dst);
return dst;
}
void dive_remove_picture(char *filename)
{
struct picture **picture = &current_dive->picture_list;

View file

@ -376,6 +376,7 @@ struct picture {
for (struct picture *picture = (_divestruct).picture_list; picture; picture = picture->next)
extern struct picture *alloc_picture();
extern struct picture *clone_picture(struct picture *src);
extern bool dive_check_picture_time(struct dive *d, int shift_time, timestamp_t timestamp);
extern void dive_create_picture(struct dive *d, char *filename, int shift_time, bool match_all);
extern void dive_add_picture(struct dive *d, struct picture *newpic);
@ -385,6 +386,7 @@ extern bool picture_check_valid(char *filename, int shift_time);
extern void picture_load_exif_data(struct picture *p);
extern timestamp_t picture_get_timestamp(char *filename);
extern void dive_set_geodata_from_picture(struct dive *d, struct picture *pic);
extern void picture_free(struct picture *picture);
extern int explicit_first_cylinder(struct dive *dive, struct divecomputer *dc);
extern int get_depth_at_time(struct divecomputer *dc, unsigned int time);

View file

@ -17,6 +17,11 @@ ImageDownloader::ImageDownloader(struct picture *pic)
picture = pic;
}
ImageDownloader::~ImageDownloader()
{
picture_free(picture);
}
void ImageDownloader::load(bool fromHash){
QUrl url;
if(fromHash)
@ -79,21 +84,21 @@ SHashedImage::SHashedImage(struct picture *picture) : QImage()
if (filename.isNull()) {
// That didn't produce a local filename.
// Try the cloud server
QtConcurrent::run(loadPicture, picture, true);
QtConcurrent::run(loadPicture, clone_picture(picture), true);
} else {
// Load locally from translated file name
load(filename);
if (!isNull()) {
// Make sure the hash still matches the image file
QtConcurrent::run(updateHash, picture);
QtConcurrent::run(updateHash, clone_picture(picture));
} else {
// Interpret filename as URL
QtConcurrent::run(loadPicture, picture, false);
QtConcurrent::run(loadPicture, clone_picture(picture), false);
}
}
} else {
// We loaded successfully. Now, make sure hash is up to date.
QtConcurrent::run(hashPicture, picture);
QtConcurrent::run(hashPicture, clone_picture(picture));
}
}

View file

@ -14,6 +14,7 @@ class ImageDownloader : public QObject {
Q_OBJECT;
public:
ImageDownloader(struct picture *picture);
~ImageDownloader();
void load(bool fromHash);
private:

View file

@ -1108,11 +1108,14 @@ QString fileFromHash(char *hash)
return localFilenameOf[QByteArray::fromHex(hash)];
}
// This needs to operate on a copy of picture as it frees it after finishing!
void updateHash(struct picture *picture) {
QByteArray hash = hashFile(fileFromHash(picture->hash));
learnHash(picture, hash);
picture_free(picture);
}
// This needs to operate on a copy of picture as it frees it after finishing!
void hashPicture(struct picture *picture)
{
char *oldHash = copy_string(picture->hash);
@ -1120,13 +1123,14 @@ void hashPicture(struct picture *picture)
if (!same_string(picture->hash, "") && !same_string(picture->hash, oldHash))
mark_divelist_changed((true));
free(oldHash);
picture_free(picture);
}
extern "C" void cache_picture(struct picture *picture)
{
QString filename = picture->filename;
if (!hashOf.contains(filename))
QtConcurrent::run(hashPicture, picture);
QtConcurrent::run(hashPicture, clone_picture(picture));
}
void learnImages(const QDir dir, int max_recursions)