Improve robustness when dealing with malformed F-Spot databases
Submitted by Lucas Beeler
Link to original bug (#718138)
Description
---- Reported by lucas@yorba.org 2011-12-14 12:38:00 -0800 ----
Original Redmine bug id: 4487
Original URL: http://redmine.yorba.org/issues/4487
Searchable id: yorba-bug-4487
Original author: Lucas Beeler
Original description:
I had this exchange with Dougie Nisbet in response to a message he posted to the mailing list:
Dougie wrote:
I suspect the problem is probably connected
with my f-spot database, which is almost
certainly corrupt, or at least, full of
inconsistencies. For example, during shotwell
import I can see from the log file that many
image files that are in the f-spot database
are physically missing on the disk, and in many
cases the tags in f-spot's database will be
different to the physical tags in the file.
I responded:
Alas, Shotwell's F-Spot import subsystem isn't
robust to errors in the source database, so a
malformed F-Spot database could cause bad things
to happen. This is certainly something I'd like
to fix, but like many things in Shotwell, it's
a question of limited developer time...
We've had other reports from users of F-Spot import failing on large F-Spot databases (e.g., 10,000 photos or more) that had been used for years, whereas other users have reported that they've imported 30,000 photos from F-Spot without a hitch. On occasion, both Clinton and I have asked for F-Spot databases from users and have seen what appear to be data inconsistencies and constraint violations (or rather, they should be constraint violations -- it appears that constraints weren't properly imposed on some table columns). Anyway, it seems apparent that for some users, using F-Spot over long periods of time with large libraries can cause database problems that can cause the Shotwell F-Spot import code to either crash or assert failure. We should look into this sometime, just because it's clear that quite a few of these databases are present in the wild.
---- Additional Comments From shotwell-maint@gnome.bugs 2013-05-01 11:38:00 -0700 ----
History
Comment 1
Updated by Bruno Girin almost 2 years ago
I'm happy to look into it once I'm done with 3614 but I would definitely need some example F-Spot databases that trigger crashes. The F-Spot table structure, and consequently the code that talks to the F-Spot DB directly, is relatively straightforward. So fixing crashes that result from constraint violations shouldn't be too difficult when looking at real life examples.
Comment 2
Updated by Lucas Beeler almost 2 years ago
A mailing list post and reply recently recorded a specific instance of F-Spot database malformation that we should deal with:
Hi Kirk,
It seems that you've burned by Shotwell 4487 (http://redmine.yorba.org/issues/4487). It looks like what's happened here is that you've got a situation where your F-Spot database is malformed in a very specific way: there's an F-Spot database field that holds a string that represents the path to the actual photo file on disk (e.g., "/home/kirk/Pictures/DSCF001.JPEG") and in one case in your database, that string is NULL. In an ideal world, this would never happen, because even if F-Spot can't find photo file on disk, it should still record its last-known path, so the path field in F-Spot's database would never be NULL. But, of course, we don't live in an ideal world... ;-)
That said, Shotwell should never crash, no matter how malformed your F-Spot database is. You'll be happy to know that Bruno Girin, one of our top Shotwell contributors, is working on this problem and we hope to have it fixed in the next major release of Shotwell (0.12).
Cheers,
Lucas
On Sun, Jan 1, 2012 at 6:08 AM, Kirk Bressler kbressl@gmail.com wrote:
(shotwell:6826): GLib-GIO-CRITICAL **: g_file_get_path: assertion
`G_IS_FILE (file)' failed
(shotwell:6826): GLib-GIO-CRITICAL **: g_file_get_parent: assertion
`G_IS_FILE (file)' failed
(shotwell:6826): GLib-GIO-CRITICAL **: g_file_get_path: assertion
`G_IS_FILE (file)' failed
(shotwell:6826): GLib-GIO-CRITICAL **: g_file_new_for_path: assertion `path
!= NULL' failed
(shotwell:6826): GLib-GIO-CRITICAL **: g_file_get_basename: assertion
`G_IS_FILE (file)' failed
(shotwell:6826): GLib-GIO-CRITICAL **: g_file_get_child: assertion
`G_IS_FILE (file)' failed
(shotwell:6826): GLib-GIO-CRITICAL **: g_file_get_path: assertion
`G_IS_FILE (file)' failed
(shotwell:6826): GLib-CRITICAL **: g_utf8_collate: assertion `str1 != NULL'
failed
ETC......
Shotwell mailing list
http://lists.yorba.org/cgi-bin/mailman/listinfo/shotwell
Comment 3
Updated by Bruno Girin almost 2 years ago
OK, I see, it is most likely an old database (around F-Spot 0.4) that got migrated from a version where the file path was in the photos table to one where the file path was in the photo_versions table. It's quite easy to fix on top of the ticket #3614 (closed) code. I can either fix it as part of #3614 (closed) or provide a separate patch once #3614 (closed) has been merged. Either way, it can definitely be fixed in time for 0.12.
Comment 4
Updated by Bruno Girin almost 2 years ago
- Assignee set to Bruno Girin
Comment 5
Updated by Adam Dingle almost 2 years ago
- Priority changed from Normal to High
- Target version set to 0.12
Comment 6
Updated by Bruno Girin almost 2 years ago
- File 0001-Added-handling-for-null-file-names-and-missing-files.patch added
- Status changed from Open to Review
You will find attached a patch that deals with the file issues described in this ticket:
- Null values in file paths and file names are handled: if the photo_versions row has null values, fall back to the corresponding photos row; if both have null values, ignore the entry completely;
- File paths stored with URI encoding are decoded;
- Files are checked for existence and the photo entry is ignored if the file doesn't exist on disk (note that this is handled first in the F-Spot plugin code and validated in the plugin host itself to add a level of safety when building additional plugins in the future).
Note that I tested on manufactured data so this needs testing on real databases where issues have been seen in the past to make sure edge cases are handled.
Comment 7
Updated by Adam Dingle almost 2 years ago
Lucas, a reminder: please review this outstanding patch. Thanks!
Comment 8
Updated by Lucas Beeler almost 2 years ago
- Category set to 4
- Status changed from Review to 5
- Resolution set to fixed
Fixed in 21cc3f2f. Thanks Bruno!
Comment 9
Updated by Charles Lindsay 7 months ago
- Status changed from 5 to Fixed
--- Bug imported by chaz@yorba.org 2013-11-25 21:55 UTC ---
This bug was previously known as bug 4487 at http://redmine.yorba.org/show_bug.cgi?id=4487 Imported an attachment (id=262213)
Unknown Component Using default product and component set in Parameters Unknown milestone "unknown in product shotwell. Setting to default milestone for this product, "---". Setting qa contact to the default for this product. This bug either had no qa contact or an invalid one.
Version: 0.12
Resolution: RESOLVED FIXED