Simple Scan crashes result in preventable data loss

Absolutely in favour of the idea - i suggested it myself on Bug #843361. A temporary copy of everything scanned so far should be saved, and then if the session crashes, you can continue with what you were doing before by 'recovering it', or you can add new scan feeds to the document later.

It's a sorely needed feature and it makes a lot of bugs a lot more manageable because you obviously you then don't have to start scanning copies from the beginning of your enormous (sic) document again ;)

I'm working on a branch that implements autosaving. I'll let you know when it needs testing.

Hi Timo,

thanks for getting active! I have not tried your code yet, but from looking at your commits this looks great!
Ultimately Robert has to approve a possible merge, though :)

two comments from my point of view:

1- how does sqlite handle crashing? this patch will not improve the situation if one ends up with a corrupted sqlite database... does it do transactions or do you have something like fsync to force writeout ...

2- as far as i can tell the feature request from (amongst other reports) Bug #483392 -- namely: resuming a previous session -- could be trivially solved with the same underlying code if there was an option to gracefully exit simple-scan while leaving behind the sqlite-entry that will tigger "recovery" on the next startup. could you look into adding this?

Let me know if I can do something for you!

Best Regards
Michael

> 1- how does sqlite handle crashing? this patch will not improve the
> situation if one ends up with a corrupted sqlite database... does it do
> transactions or do you have something like fsync to force writeout ...

According to [1], Sqlite is extremely reliable in this regard. "All
changes within a single transaction in SQLite either occur completely
or not at all, even if the act of writing the change out to the disk
is interrupted by a program crash, an operating system crash, or a
power failure."

> 2- as far as i can tell the feature request from (amongst other reports)
> Bug #483392 -- namely: resuming a previous session -- could be trivially
> solved with the same underlying code if there was an option to
> gracefully exit simple-scan while leaving behind the sqlite-entry that
> will tigger "recovery" on the next startup. could you look into adding
> this?

You're right, this would be trivial. It just needs UI design. It
possibly makes sense to just have this as the default behaviour,
because it is easy enough to click the file -> new button. Or make it
default behaviour in case of unsaved changes.

[1] http://sqlite.org/transactional.html

For the record, here's my comment about my branch that I wrote to Robert earlier.

I'm still struggling with the last pieces. What I'm doing is scan a page, simulate a crash with ctrl+c, and then start a new instance. This should load the old page automatically. This is almost happening as it should. The pixel data is loaded correctly because I can export
it to pdf, but it is not shown on screen. I'm probably missing atrigger somewhere. Can you easily spot what I'm missing?

I used a Sqlite database to save all the data. I tried to work with libgda-4.0 first, but the vala bindings are too buggy to rely on. Now I'm using sqlite directly.

I had an IRC chat with Robert Ancell, here is the relevant transcript for everyone interested in the progress:

(23:40:07) Michael Nagel: aforementioned session recovery: https://bugs.launchpad.net/simple-scan/+bug/897469 did you have the chance to look at it and know why the gui does not properly refresh?
(23:40:27) Michael Nagel: how do you think about merging it soon?
(23:41:03) robert_ancell: ah, I replied to Timo directly, but I hadn't seen the bug
(23:41:13) robert_ancell: I'll try and merge it next week if it works well
(23:43:06) Michael Nagel: i obviously did not know this. i will post it to the bug, so the other affected people know whats going on behind the scenes...

Timo, could you resolve the issue with refreshing the displayed image? Do you need anything else?

I haven't been able to resolve the issue with refreshing yet. It would probably take someone familiar with the rendering code very little time to find out what's going on.

There's also one more thing. As it is now, the code uses a process id to know whether simple-scan is running and whether it should recover any pages. There could be problems if one uses two instances on two different computers sharing a home directory (via nfs, say). Then one instance will try to take ownership of the pages of the other. A possible solution would involve making simple-scan unique (using GtkApplication) and then using some sort of (dbus?) session id to tag the autosaves, instead of just the process id.

On the other hand, one could argue that even if two machines share the same home directory, they should have a local .cache directory. Then the problem disappears entirely.

I haven't researched this yet, and perhaps it is not the use case we are aiming for.

My latest commit fixes the refreshing issue.

Timo, could you create a formal merge request so Robert does not miss this again?
Thanks!

(09:31:02) Michael Nagel: Simple Scan crashes result in preventable data loss https://bugs.launchpad.net/simple-scan/+bug/897469
(09:32:29) robert_ancell: yeah, I need to review that
(09:32:41) robert_ancell: that will be for the 3.6 release
(09:33:09) Michael Nagel: what is current?
(09:33:18) robert_ancell: 3.4
(09:33:29) Michael Nagel: 3.4 will be in precise, right?
(09:33:50) robert_ancell: yes
(09:34:10) Michael Nagel: what a pity
(09:36:38) robert_ancell: btw, I just noticed that branch doesn't have a merge request
(09:36:57) robert_ancell: so that's one reason why I haven't been noticing it
(09:37:27) Michael Nagel: like a formal merge request in launchpad?
(09:37:30) robert_ancell: yes
(09:37:43) Michael Nagel: ok, i will ask for one
(09:38:07) robert_ancell: so if you see anyone with a branch that wants a review to propose it for merging. If they know it still has problems, that's fine just note that in the merge request

Done. Timo

(19:22:19) Michael Nagel: Simple Scan crashes result in preventable data loss https://bugs.launchpad.net/simple-scan/+bug/897469
(19:22:50) Michael Nagel: is this going to be merged for the next release or are there open todos?
(19:24:54) robert_ancell: I need to review again, but now we are at the start of the next release cycle now is a good time to merge

PING! @Timo: how is this going?

Robert says:
(08:17:59) robert_ancell: it didn't work for me and I'm awaiting a response

I am willing to have another look at this.
To give me a head start, could you paste a series of commands (including downloading and building your version) that allow me to see the recovery in action?

Best Regards
Michael

I'm not currently working on this. I have no way to reproduce the
errors Robert is seeing and it works for me. Given the limited time I
can spend on this (having an unrelated job and all) it would be much
more efficient if someone experiencing the issue could try some
debugging on their machine.

Your testing would be more than welcome. Here's how to go about it:

sudo apt-get build-dep simple-scan
sudo apt-get install libsqlite3-dev
bzr branch lp:~tkluck/simple-scan/autosaves simple-scan-autosaves
cd simple-scan-autosaves
./autogen.sh && ./configure && make
src/simple-scan test

(now click "scan", wait for the scan to finish, and interrupt the
process using ctrl+C in the terminal

src/simple-scan test

(now you should see the scanned document reappear)

I've just pushed a commit with verbose logging output. It would be very helpful if someone for whom my patch doesn't work could build that version, follow the above instructions for testing the autosaves, and post the logfile here.

You should use

    src/simple-scan --debug test

to enable the verbose debugging output, and the resulting logfile is

~/.cache/simple-scan/simple-scan.log

i am trying this right now.

i can build and run simple-scan trunk, with your branch i get:

nailor@needle:*mple-scan-autosaves$ ./autogen.sh --prefix=`pwd`/install
/usr/bin/gnome-autogen.sh
checking for autoconf >= 2.53...
  testing autoconf2.50... not found.
  testing autoconf... found 2.68
checking for automake >= 1.7...
  testing automake-1.11... found 1.11.3
checking for intltool >= 0.30...
  testing intltoolize... found 0.50.2
checking for pkg-config >= 0.14.0...
  testing pkg-config... found 0.26
checking for gnome-doc-utils >= 0.4.2...
  testing gnome-doc-prepare... not found.
***Error***: You must have gnome-doc-utils >= 0.4.2 installed
  to build simple_scan. Download the appropriate package for
  from your distribution or get the source tarball at
    http://ftp.gnome.org/pub/GNOME/sources/gnome-doc-utils/

this can be worked around with:
sudo apt-get install gnome-doc-utils

as far as i can tell the recovery works just fine for completely scanned pages.
partially scanned pages do not recover (they show up as a white page).
imho this is a huge improvement over the current situation.

robert: does it always fail or just sometimes? i could not reproduce your problem.

timo: the database in $HOME/.cache/simple-scan/autosaves/autosaves.db seems to be growing over time. i think sometimes the database might not be cleared properly...

Thanks for testing, Michael!

>timo: the database in $HOME/.cache/simple-scan/autosaves/autosaves.db seems to be growing over time. i think
> sometimes the database might not be cleared properly...
There isn't necessarily a problem with clearing the database, because I know that sqlite doesnt' immediately return the space it doesn't use anymore to the filesystem. So the file may seem to grow at first, but I think that sqlite will re-use that space.

You can look inside the database using

    sqlite3 ~/.cache/simple-scan/autosaves/autosaves.db

and see if the database is indeed still populated with autosaves (I can't imagine, because then they would be recovered on the next run), or that it is emtpy.

you are right:

nailor@needle:~$ ls -lah ~/.cache/simple-scan/autosaves/autosaves.db
-rw-r--r-- 1 nailor nailor 113M Aug 14 17:53 /home/nailor/.cache/simple-scan/autosaves/autosaves.db

nailor@needle:~$ sqlite3 ~/.cache/simple-scan/autosaves/autosaves.db
SQLite version 3.7.9 2011-11-01 00:52:41
Enter ".help" for instructions
Enter SQL statements terminated with a ";"
sqlite> select count(*) from pages;
0
sqlite> .quit

nailor@needle:~$ ls -lah ~/.cache/simple-scan/autosaves/autosaves.db
-rw-r--r-- 1 nailor nailor 113M Aug 14 17:53 /home/nailor/.cache/simple-scan/autosaves/autosaves.db

nailor@needle:~$ sqlite3 ~/.cache/simple-scan/autosaves/autosaves.db
SQLite version 3.7.9 2011-11-01 00:52:41
Enter ".help" for instructions
Enter SQL statements terminated with a ";"
sqlite> VACUUM;
sqlite> .quit

nailor@needle:~$ ls -lah ~/.cache/simple-scan/autosaves/autosaves.db
-rw-r--r-- 1 nailor nailor 2,0K Aug 14 19:12 /home/nailor/.cache/simple-scan/autosaves/autosaves.db

maybe simple scan should vacuum the db occasionally...

robert, whats your general comment on this (i.e. the pull request)?

Fuhfuhfuhfuhfuhfuh! +1 for this bug please. Just lost a load of work, not happy.

I'd just like to add that I don't mind if there's no UI support for recovering after a crash, I'd be happy with a well known folder I could find temporary copies in after a crash.

Hello, can anyone tell me if / when this fix will be available from the repos, and for which distributions? As this is a data loss bug I think it should go into the 12.04 LTS release as well as 12.10. Thanks

Compacted the above to a one-liner for convenience:

  sudo apt-get build-dep simple-scan && sudo apt-get install libsqlite3-dev bzr gnome-doc-utils && bzr branch lp:~tkluck/simple-scan/autosaves simple-scan-autosaves.bzr && cd simple-scan-autosaves.bzr/ && ./autogen.sh && ./configure && make && src/simple-scan --debug test

This resulted for me in:

... snip ...
make[1]: Leaving directory `/home/tim/repo/simple-scan-autosaves.bzr'

** (simple-scan:12856): CRITICAL **: ui.vala:1271: Unable to load UI /usr/local/share/simple-scan/simple-scan.ui: Failed to open file '/usr/local/share/simple-scan/simple-scan.ui': No such file or directory

:-(

Looks like that only works if you've already installed the package. I used checkinstall http://www.linuxjournal.com/content/using-checkinstall-build-packages-source to install but keeping within the package manager. Then it worked, and the ctrl-c test worked, showing me the doc I had already scanned. Thanks. (I've learnt my lesson for randomly installing from source on my main pc)

For the record I changed the following, as I didn't want a name clash, and the default version was invalid:
tim@atom:~/repo/simple-scan-autosaves.bzr$ sudo checkinstall
...
This package will be built according to these values:
...
2 - Name: [ simple-scan-autosave ]
3 - Version: [ 1.0.0 ]
...

Running, killing and re-running the app shown below for the record:

tim@atom:~/repo/simple-scan-autosaves.bzr$ simple-scan

** (simple-scan:25776): WARNING **: scanner.vala:1024: Unable to disable compression, please file a bug
^C
tim@atom:~/repo/simple-scan-autosaves.bzr$ simple-scan

I've attached the deb file just in case it'll help anyone else.

That build hangs when I hit crop. Bad day, getting worse. Screenshot of corrupted file on re-opening added. And tomboy just deleted all my notes. Argh.

Tim: thanks for sharing this. My branch has been merged already, so it is
better to do your testing on the master branch. Just use
    $ bzr clone lp:simple-scan

and install it like you're doing, and you should have autosaves and any
fixes that we may add to it later. My personal development branch is now
unmaintained.

Looks like LTS is too old to build the main branch.

./autogen.sh
...
configure: error: Vala 0.16.0 not found.

valac --version
Vala 0.14.2

Seeing as I can't build this, can you offer any clues if/when the patched version will make it into the Ubuntu repos? I can't figure it out from what information I can find on launchpad. If you could link to any public info on how to find out such info I'd appreciate it.

P.S. I got "The command 'bzr clone' has been deprecated in bzr 2.4. Please use 'bzr branch' instead." in my output.

You could grab vala 0.16 by adding ppa:vala-team/ppa to your software sources.

I don't want to get you into "dependency hell" so if it turns out that
there are more packages that are too ancient in Precise, at some point
you may have to content yourself with simple-scan's version in the
repositories, or with my unmaintained branch. That's just the known
downside of LTS releases.

Good luck!

PS you're right about bzr clone, you should use bzr branch instead.

please see the README.md file for instructions how to build on ubuntu 12.04 lts

Thanks, the instructions in the readme worked. I now have 3.6.0 built from source, packaged and running (I'm liking checkinstall).

I can report that the crop function works as it normally does in this build, no hanging. The autosave function also seems to be working reliably now. I did manage to get it to fail to load one of the scans to start with but I can't reproduce it after playing with ctrl-c for a bit.

Good work, and many thanks folks. Given the grumbling on the software centre reviews about the crashes and the lifetime of LTS (2017) perhaps this is worth a backport to the LTS release, or is this the wrong place to mention that?

refs:
* LTS release https://wiki.ubuntu.com/Releases
* App comments https://apps.ubuntu.com/cat/applications/precise/simple-scan/

This ticket is the wrong place to discuss backports.
The bugtracker is a good place to discuss backports -- please open a new ticket.
Robert Ancell (the main developer) would love to see backports, but someone has to prepare them and make sure they work, are tested and can be accepted into ubuntu. I do not really know what the requirements are there. But lets discuss those things in the other ticket to be created.

Hi, Thanks for the info. I've read up on the SRU process for getting this fix included in the LTS repo, and before that can be started it is apparently a prerequisite that this bug be marked as "Fix Released", whereas it is currently "Fix Committed".

I can't work out from the info on this project whether the bugfix is in simple-scan's latest release so don't know if that can be satisfied.

Also required is the following (quote):

* An explanation of the bug on users and justification for backporting the fix to the stable release. This may be preceded with an [Impact] header, but this is not required. In addition, it is helpful, but not required, to include an explanation of how the upload fixes this bug.
 * A [Test Case] section with detailed instructions how to reproduce the bug. These should allow someone who is not familiar with the affected package to reproduce the bug and verify that the updated package fixes the problem.
* A [Regression Potential] section with a discussion of how regressions are most likely to manifest as a result of this change. It is assumed that any SRU candidate patch is well-tested before upload and has a low overall risk of regression, but it's important to make the effort to think about what could happen in the event of a regression. This both shows the SRU team that the risks have been considered, and provides guidance to testers in regression-testing the SRU.

This information is from: https://wiki.ubuntu.com/StableReleaseUpdates#Procedure

I don't think it's my place to make these changes, so perhaps someone from the project could look into this?

Once that is done I think I can continue the SRU process as it looks to me like this bug satisfies the SRU requirements based on this example: "Bugs which may, under realistic circumstances, directly cause a loss of user data"

Many Thanks

---
* Fix Committed: a developer has committed his/her fix to the project's codebase.
* Fix Released: a new version of the software, featuring the bug fix, has been released.
ref: https://wiki.kubuntu.org/Bugs/WorkFlow

The fix has been merged here: http://bazaar.launchpad.net/~simple-scan-team/simple-scan/trunk/revision/602
and unfortunately there has not been a new release since.

Also, may I suggest to open a new ticket to keep this discussion focused?

Fixed in 3.7.1

Great to know this has been implemented!

Is there a way for Ubuntu 12.04 LTS users to get v3.7.1?

you can build it yourself from source.
https://github.com/mnagel/simple-scan/blob/master/README.md#building

this should work quite smoothly. if you have any problems with building, please let me know.

As others have commented before, would be great to have a pre-compiled binary package :-)

Having said that, the build process failed for me - the configure script can't find sqlite3 and I'm not sure how to fix that.

Please see below for details:

Following https://github.com/mnagel/simple-scan/blob/master/README.md#building

1. bzr branch lp:simple-scan simple-scan && cd simple-scan
bzr was not installed, sudo apt-get install bzr - OK

2. sudo apt-get build-dep simple-scan
This brings in ~150MB of stuff (is all of that necessary to build simple-scan or is there a lighter selection of packages we could get away with?) - works OK.

3. sudo apt-get install valac-0.16 vala-0.16
Brings in another 25MB of stuff - works OK

4. sudo update-alternatives --config valac
Works fine, vala-0.14 was default, selected vala-0.16

5. ./autogen.sh

alex@bigboy:~/ss/simple-scan$ ./autogen.sh
/usr/bin/gnome-autogen.sh
checking for autoconf >= 2.53...
  testing autoconf2.50... not found.
  testing autoconf... found 2.68
checking for automake >= 1.7...
  testing automake-1.11... found 1.11.3
checking for intltool >= 0.30...
  testing intltoolize... found 0.50.2
checking for pkg-config >= 0.14.0...
  testing pkg-config... found 0.26
checking for gnome-common >= 2.3.0...
  testing gnome-doc-common... found 3.1.0
Checking for required M4 macros...
Checking for forbidden M4 macros...
**Warning**: I am going to run `configure' with no arguments.
If you wish to pass any to it, please specify them on the
`./autogen.sh' command line.

Processing ./configure.ac
Running intltoolize...
Running gnome-doc-common...
Running aclocal-1.11...
Running autoconf...
Running automake-1.11...
configure.ac:10: installing `./compile'
configure.ac:4: installing `./install-sh'
configure.ac:4: installing `./missing'
src/Makefile.am: installing `./depcomp'
Running ./configure --enable-maintainer-mode ...
checking for a BSD-compatible install... /usr/bin/install -c
checking whether build environment is sane... yes
checking for a thread-safe mkdir -p... /bin/mkdir -p
checking for gawk... no
checking for mawk... mawk
checking whether make sets $(MAKE)... yes
checking whether make supports nested variables... yes
checking whether to enable maintainer-specific portions of Makefiles... yes
checking for valac... /usr/bin/valac
checking /usr/bin/valac is at least version 0.16.0... yes
checking for style of include used by make... GNU
checking for gcc... gcc
checking whether the C compiler works... yes
checking for C compiler default output file name... a.out
checking for suffix of executables...
checking whether we are cross compiling... no
checking for suffix of object files... o
checking whether we are using the GNU C compiler... yes
checking whether gcc accepts -g... yes
checking for gcc option to accept ISO C89... none needed
checking dependency style of gcc... gcc3
checking whether gcc and cc understand -c and -o together... yes
checking for pkg-config... /usr/bin/pkg-config
checking pkg-config is at least version 0.16... yes
checking for SIMPLE_SCAN... no
configure: error: Package requirements (
    glib-2.0 >= 2.32
    gtk+-3.0
    gmodule-export-2.0
    gthread-2.0
    zlib
    cairo
    gd...

please run
sudo apt-get install libsqlite3-dev
and try again

i will update the instructions soonish.
i will have a look about what can be done about binary packages / a ppa.

Thank you, this worked as expected.

To sum up, for my system (vanilla Ubuntu 12.04.02) the series of steps to install the latest simple scan appears to be:

1. sudo apt-get install bzr libsqlite3-dev valac-0.16 vala-0.16
2. bzr branch lp:simple-scan simple-scan && cd simple-scan
3. sudo apt-get build-dep simple-scan
4. sudo update-alternatives --config valac # select vala-0.16
5. ./autogen.sh
6. make
7. sudo make install

Then launch simple-scan as usual and check Help -> About (which now for me says 3.8.0)

A little clean-up after the build process:

8. sudo apt-get remove bzr libsqlite3-dev valac-0.16 vala-0.16
9. sudo apt-get autoremove
10. cd .. && rm -rf simple-scan

build instructions updated:
http://bazaar.launchpad.net/~simple-scan-team/simple-scan/trunk/revision/611
https://github.com/mnagel/simple-scan/commit/9593feb0b6561a9cb6d92b97671492892283fe43

discussion regarding binary packages should be moved here:
Bug #897027

please see the following regression:

crop temporarily hangs the app since simple-scan uses sqlite
Simple Scan Bugs Bug #1121169