After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 759403 - Shotwell crashes most of the times when clicking on "Open with external editor"
Shotwell crashes most of the times when clicking on "Open with external editor"
Status: RESOLVED FIXED
Product: shotwell
Classification: Other
Component: general
0.22.x
Other Linux
: High critical
: 0.24
Assigned To: Shotwell Maintainers
Shotwell Maintainers
https://bugs.debian.org/cgi-bin/bugre...
Depends on:
Blocks:
 
 
Reported: 2015-12-13 13:36 UTC by Joerg C. Frings-Fuerst
Modified: 2016-04-29 19:07 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Stacktrace from 0.22 (8.85 KB, text/plain)
2016-04-29 01:26 UTC, Kevin McBride
  Details
Relax assertion in Photo file monitor (1.52 KB, patch)
2016-04-29 19:06 UTC, Jens Georg
committed Details | Review

Description Joerg C. Frings-Fuerst 2015-12-13 13:36:18 UTC
Forwarded from Debian:

[quote]
Most of the times when I click on "Open with external editor" in the context menu of
a photo, shotwell crashes. The external editor (gimp) opens anyway with the photo.

Relevant output on the console:

----------------------
ERROR:src/Photo.c:19066:photo_on_editable_file_changed: assertion failed: (readers.editable != null && file.equal(readers.editable.get_file()))
Aborted
----------------------

thanks

Florian
[/quote]



CU
Jörg
Comment 1 Sebastien Bacher 2015-12-15 17:24:26 UTC
The issue has been reported on Ubuntu as well, https://bugs.launchpad.net/ubuntu/+source/shotwell/+bug/1511914

The code doesn't seem to deal fine with the inotify changes in glib 2.46
Comment 2 Jens Georg 2016-04-16 17:33:55 UTC
Confirmed. Happens with raw files only
Comment 3 Janne 2016-04-17 02:40:19 UTC
It happens with Jpeg files for me.
Comment 4 Jens Georg 2016-04-17 09:55:39 UTC
Yes, the difference is whether or not you have a _modified file already. Should be easy to fix.
Comment 5 Jens Georg 2016-04-28 21:35:12 UTC
So we receive the following events in error case:

L 20562 2016-04-28 22:51:01 [WRN] Photo.vala:4082: /home/jens/Pictures/2015/04/18/IMG_20150418_155443314_modified.jpg other: none G_FILE_MONITOR_EVENT_CHANGED /home/jens/Pictures/2015/04/18/IMG_20150418_155443314_modified.jpg
L 20562 2016-04-28 22:51:01 [WRN] Photo.vala:4082: /home/jens/Pictures/2015/04/18/IMG_20150418_155443314_modified.jpg other: none G_FILE_MONITOR_EVENT_CHANGES_DONE_HINT /home/jens/Pictures/2015/04/18/IMG_20150418_155443314_modified.jpg
L 20562 2016-04-28 22:51:01 [WRN] Photo.vala:4082: /home/jens/Pictures/2015/04/18/IMG_20150418_155443314_modified.jpg other: none G_FILE_MONITOR_EVENT_DELETED /home/jens/Pictures/2015/04/18/IMG_20150418_155443314_modified.jpg
L 20562 2016-04-28 22:51:01 [WRN] Photo.vala:4082: /home/jens/Pictures/2015/04/18/IMG_20150418_155443314_modified.jpg20562 other: none G_FILE_MONITOR_EVENT_DELETED /home/jens/Pictures/2015/04/18/IMG_20150418_155443314_modified.jpg

The idea of the monitor is to watch the file for changes by the external tool, but it seems we get the events for the file creation.

The DELETED events are odd, also the last event which gives some file with PID in the path
Comment 6 Jens Georg 2016-04-28 22:19:41 UTC
Actually, looking at the strace output, those events are correct. The temporary file is created by exiv2

Strace:
unlink("/home/jens/Pictures/2015/04/18/IMG_20150418_155443314_modified.jpg") = 0
rename("/home/jens/Pictures/2015/04/18/IMG_20150418_155443314_modified.jpg23253", "/home/jens/Pictures/2015/04/18/IMG_20150418_155443314_modified.jpg") = 0
unlink("/home/jens/Pictures/2015/04/18/IMG_20150418_155443314_modified.jpg23253") = -1 ENOENT (No such file or directory)

Backtrace for rename call:
  • #0 rename
    at ../sysdeps/unix/syscall-template.S line 84
  • #1 Exiv2::FileIo::transfer(Exiv2::BasicIo&)
  • #2 Exiv2::JpegBase::writeMetadata()
  • #3 0x00007ffff78f70d2 in
  • #4 gexiv2_metadata_save_file
  • #5 photo_metadata_write_to_file
    at /home/jens/Source/shotwell/src/photos/PhotoMetadata.vala line 152
  • #6 jfif_metadata_writer_real_write_metadata
    at /home/jens/Source/shotwell/src/photos/JfifSupport.vala line 143
  • #7 photo_export
    at /home/jens/Source/shotwell/src/Photo.vala line 3734

Somehow these events get "queued" and are sometimes
Comment 7 Kevin McBride 2016-04-29 01:26:50 UTC
Created attachment 326982 [details]
Stacktrace from 0.22

Not sure if the attached stacktrace will help, but my stacktrace shows something else is causing shotwell to crash.  (0.22 was used)

And it did not matter which file I chose.
Comment 8 Jens Georg 2016-04-29 07:15:31 UTC
> Created attachment 326982 [details]
> Stacktrace from 0.22
> 
> Not sure if the attached stacktrace will help, but my stacktrace shows
> something else is causing shotwell to crash.  (0.22 was used)
> 
> And it did not matter which file I chose.

No, it's the same issue. It doesn't matter which file, the only thing that matters is that there is no _modified.jpg created yet.

Apparently, with recent GLibs, the assertion is plain wrong because you can get an event for a file you are not monitoring if that file is renamed to the file you are watching. GLib will send you a DELETE event for the source file then if you do not use the WATCH_MOVES GFileMonitor flag.

Part of the issue is that exiv2 does a remove -> rename for windows compatibility reasons. On windows, you cannot rename to an existing file. This might trick glib into not coalescing the events.
Comment 9 Kevin McBride 2016-04-29 17:25:37 UTC
(In reply to Jens Georg from comment #8)
> > Created attachment 326982 [details]
> > Stacktrace from 0.22
> > 
> > Not sure if the attached stacktrace will help, but my stacktrace shows
> > something else is causing shotwell to crash.  (0.22 was used)
> > 
> > And it did not matter which file I chose.
> 
> No, it's the same issue. It doesn't matter which file, the only thing that
> matters is that there is no _modified.jpg created yet.
> 
> Apparently, with recent GLibs, the assertion is plain wrong because you can
> get an event for a file you are not monitoring if that file is renamed to
> the file you are watching. GLib will send you a DELETE event for the source
> file then if you do not use the WATCH_MOVES GFileMonitor flag.
> 
> Part of the issue is that exiv2 does a remove -> rename for windows
> compatibility reasons. On windows, you cannot rename to an existing file.
> This might trick glib into not coalescing the events.

I did a search on exiv2's bug tracker, but could not find a bug report that identifies this issue.  Do you think we should file a bug in exiv2's bug tracker?

http://dev.exiv2.org/projects/exiv2/issues
Comment 10 Jens Georg 2016-04-29 18:04:38 UTC
No, it's not an exiv2 bug. More an odd behavior of glib, if any.
Comment 11 Jens Georg 2016-04-29 19:06:06 UTC
Created attachment 327053 [details] [review]
Relax assertion in Photo file monitor

If the file is newly created, due to the way exiv2 does the metadata
writing (remove => rename due to windows limitations) and we do not ask
GLib for the MOVE event, we receive a DELETE event for the temp file
that exiv2 creats.

Signed-off-by: Jens Georg <mail@jensge.org>
Comment 12 Jens Georg 2016-04-29 19:07:16 UTC
Attachment 327053 [details] pushed as 1886512 - Relax assertion in Photo file monitor