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 :)