Bug 163602 - Race condition during image download from Camera/ USB device: image corruption and/or loss (patch attached)
Summary: Race condition during image download from Camera/ USB device: image corruptio...
Status: RESOLVED FIXED
Alias: None
Product: digikam
Classification: Applications
Component: Import-UMS (show other bugs)
Version: unspecified
Platform: openSUSE Linux
: NOR critical
Target Milestone: ---
Assignee: Digikam Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-06-09 13:36 UTC by Chris Drexler
Modified: 2017-08-16 09:27 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In: 0.9.5


Attachments
patch for removing race condition in cameraGUI for downloading images (1.22 KB, patch)
2008-06-09 13:37 UTC, Chris Drexler
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Drexler 2008-06-09 13:36:25 UTC
Version:           0.9.3, but still in latest SVN (using KDE 3.5.9)
Installed from:    SuSE RPMs
Compiler:          gcc version 4.2.1 (SUSE Linux)
 
OS:                Linux

CameraGUI does a download of images by copying it to local file within the destination directory and manipulating this one before renaming it to the final name. 
A race condition exists because the download and the renaming is done asynchronously and the same temp filename is used for all downloaded images (".digikam-camera-tmp1-%1" whereas %1 is replaced by the PID).

What happens is that in the best case, the renaming is happening while the next download is running, therefore an error happens (access denied) and the rename fails. This is shown by a "X" in the thumbnail view of the CameraGui. At least the problem is visible.

In the not-so-good case it can happen, that one ends up with two times the same image on the local disk, because somehow (why exactly is not clear to me) the second downloaded image is renamed twice (I didn't go into details of how this can happen). This is worse than case one, because in the thumbnail view the images show up as correctly downloaded, but on disk you find duplicates of certain images while others are missing.

The solution for me was to generate temporary filenames that depend on the filename that is actually downloaded. As you can see in the provided patch, the changes are minimal and very local. Only cameracontroller.cpp needs to be changed!

BTW: This problem showed up when I upgraded a $1.99 USB 1.1 card reader to a faster 2.0 version after using digiKam already for many years!

Please feel free to contact me with any questions that you might have regarding this!

digiKam is a really cool piece of software and I'm happy to be able to contribute to making it even better!
Comment 1 Chris Drexler 2008-06-09 13:37:34 UTC
Created attachment 25224 [details]
patch for removing race condition in cameraGUI for downloading images
Comment 2 Roger Larsson 2008-08-19 08:22:34 UTC
I can confirm this, both from Compact flash and DVD.
This is a new bug for me - recently upgraded to SuSE 11.0 (64 bit) and even more recently to dual core.
Severity 'critical' since it can cause loss of data.
Comment 3 caulier.gilles 2008-08-19 08:43:47 UTC
Roger,

The patch from this report is already applied to svn since a long time (I don't know why it have not been closed). digiKam 0.9.4 include it. Please update to this version.

Gilles Caulier
Comment 4 Roger Larsson 2008-08-19 21:10:27 UTC
digicam 0.9.4 was released 2008-07-16, and since the release notes does not contain information about this critical fix - SuSE for one has not picked it up, they still offer 0.9.3-70.1

There is a real chance of data loss with this bug!
Let me describe what happens for me.

In the download window I note red X on several images.
This file names are not imported properly to digikam.
Marking them and downloading again succeeds.

But what is worse is that it was not really that image that is missing.
It overwrote the one before!
And you have seemingly correct file names but you lost one file (it will be gone when deleting that image from the memory card) instead you got a duplicate.

This can not be seen in the download applet - but check the files in digikam main window.
Comment 5 Roger Larsson 2008-08-19 22:10:14 UTC
I have reviewed the patch/change.

The race was between 'CameraCommand::gp_download' and 'CameraEvent::gp_downloaded'

Normally the messages/events arrive
'CameraCommand::gp_download'
   emits a gp_downloaded("file-PID")
'CameraEvent::gp_downloaded(temp)'

'CameraCommand::gp_download'
   emits a gp_downloaded("file-PID")
'CameraEvent::gp_downloaded(temp)'

And the temp parameter will be the same but it wont matter
since processing keeps the sequence.

If the message queue becomes
'CameraCommand::gp_download'
   emits a gp_downloaded("file-PID")
'CameraCommand::gp_download'
   emits a gp_downloaded("file-PID")
'CameraEvent::gp_downloaded(temp)'
'CameraEvent::gp_downloaded(temp)'

But in this case the second gp_download will overwrite the same file as used earlier. Final processing, 'gp_downloaded', will be done with the wrong file contents. And the last 'gp_downloaded' will fail since the file has either been renamed or removed.
=> this matches the observed behavior.

The patch looks OK to me.