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 774057 - Double close(fd) when executing thumbnailer causes crash
Double close(fd) when executing thumbnailer causes crash
Product: shotwell
Classification: Other
Component: video
Other Linux
: Normal normal
: ---
Assigned To: Shotwell Maintainers
Shotwell Maintainers
Depends on:
Reported: 2016-11-07 13:29 UTC by Damian Pietras
Modified: 2016-11-07 20:58 UTC
See Also:
GNOME target: ---
GNOME version: ---

Patch to fix double close() when running the thumbnailer (375 bytes, patch)
2016-11-07 13:29 UTC, Damian Pietras
committed Details | Review

Description Damian Pietras 2016-11-07 13:29:45 UTC
Created attachment 339252 [details] [review]
Patch to fix double close() when running the thumbnailer

A file descriptor used to communicate with the video thumbnailer process is closed twice. In case another thread allocates the same descriptor number in the meantime, the second close() actually closes a descriptor not owned by the thread. This causes crashes in most cases or unexpected behaviour.

See this strace fragment:

18003 close(20 <unfinished ...>
18003 <... close resumed> )             = 0
18003 wait4(18926,  <unfinished ...>
17980 open("/home/XXXXX/.local/share/shotwell/data/photo.db-journal", O_RDWR|O_CREAT|O_CLOEXEC, 0644 <unfinished ...>
18003 <... wait4 resumed> [{WIFSIGNALED(s) && WTERMSIG(s) == SIGPIPE}], 0, NULL) = 18926
18003 close(20 <unfinished ...>
17980 pwrite64(20, "w\225D\312", 4, 12820 <unfinished ...>
18003 <... close resumed> )             = 0
17980 <... pwrite64 resumed> )          = -1 EBADF (Bad file descriptor)

18003 is the thread that runs the thumbnailer. It closes FD 20 before wait() and after.
17980 is some unlucky thread that opened and modified the SQLite database in the meantime.

I'm attaching a patch that fixes this. The Posix.close() call is removed since the FD is closed automatically as the close flag is set to true earlier:

GLib.UnixInputStream unix_input = new GLib.UnixInputStream(child_stdout, true);

Please note I'm not a Vala or Glib programmer so double check before applying :)
Comment 1 Jens Georg 2016-11-07 13:49:02 UTC
Review of attachment 339252 [details] [review]: