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 549595 - Silently removes main files while trying to save without lock
Silently removes main files while trying to save without lock
Status: VERIFIED FIXED
Product: GnuCash
Classification: Other
Component: Backend - XML
2.2.x
Other Linux
: Normal critical
: ---
Assigned To: Andreas Köhler
Andreas Köhler
Depends on:
Blocks: 552306
 
 
Reported: 2008-08-27 16:07 UTC by Micha Lenk
Modified: 2018-06-29 22:09 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Fixes link() returning ENOSYS when using over sshfs and makes gnc_int_link_or_make_backup() return FALSE on other link() errors (977 bytes, patch)
2008-08-28 18:19 UTC, Micha Lenk
none Details | Review

Description Micha Lenk 2008-08-27 16:07:54 UTC
Bas Wijnen <wijnen@debian.org> reported the following problem via the Debian Bug Tracker as Bug #496807 (http://bugs.debian.org/496807):

Package: gnucash
Version: 2.2.6-1
Severity: grave
Justification: causes data loss

I was trying to use gnucash over sshfs, to allow several machines to
handle the same file.  It complained that it was unable to get a lock,
and so couldn't prevent simultaneous writes.  This was no problem,
because there isn't another person working on them.  For the rest,
everything seemed to work fine.

However, when trying to reopen the file, I found that it had not been
written, but instead it was deleted.  So not only did I lose the work of
the session, but it actually deleted my previous work as well.

I tried creating a new file, which also complains about the lock file,
but seems to work fine otherwise, but it also doesn't create the file.

This would have been a "critical issue" (causes serious data loss),
except that gnucash writes loads of log and backup files, so normally
most of the work will not actually be lost.

As a quick fix, it would be acceptable to turn the lock warning into an
error.

Thanks,
Bas Wijnen
Comment 1 Micha Lenk 2008-08-28 18:19:23 UTC
Created attachment 117542 [details] [review]
Fixes link() returning ENOSYS when using over sshfs and makes gnc_int_link_or_make_backup() return FALSE on other link() errors

The attached patch fixes two problems at once as discussed in #gnucash as documented here: http://lists.gnucash.org/logs/2008/08/2008-08-28.html#T13:21:52

First, it handles the return code ENOSYS returned by link() in case the files are on a sshfs by failing-over to copy_file() function.

Second, it makes sure gnc_int_link_or_make_backup() returns FALSE in case copy_file() has not been called (i.e. link() returned an error code we could not gracefully recover by calling copy_file() instead) by saving the result of copy_file() to a separate variable copy_success initialized to FALSE, and *only* set to TRUE in case copy_file() has been called and succeeded. In case link() failed for whatever reason copy_success is a sane indicator for whether an error ocurred or not.
Comment 2 Boris Zbarsky 2008-09-15 03:02:03 UTC
Dosn't that change need to also be made to gnc_file_be_get_file_lock so that the "can't get lock" alert won't pop up every single time when there is no way to actually get the lock?
Comment 3 Boris Zbarsky 2008-09-15 03:14:16 UTC
See also similar issue in bug 552306.
Comment 4 Andreas Köhler 2008-09-15 12:36:31 UTC
(In reply to comment #2)
> Dosn't that change need to also be made to gnc_file_be_get_file_lock so that
> the "can't get lock" alert won't pop up every single time when there is no way
> to actually get the lock?

Well, saving an existing file works without that change, it just pops up a warning each time, just as you said.  But it is still impossible to save a new file onto an ssh file system.  So that change will be included, stay tuned :-)

Comment 5 Andreas Köhler 2008-09-15 13:19:39 UTC
So, finally (;-)) there is commit about that.  It would be nice if you both of you could verify that

http://svn.gnucash.org/trac/changeset/17524?format=diff&new=17524

works for you.
It combines Micha's and Boris' patch and adds ENOSYS to gnc_file_be_get_file_lock() as suggested in comment 2.


Regarding the patch status, rejected sounds hard, maybe I could have used obsolete (nothing newer existed anywhere) or needs-work (to be done by me, not the contributor).  Now it is definitely obsolete.  Or is it commited ;-)
Comment 6 Boris Zbarsky 2008-09-15 14:13:52 UTC
I can't build right this second, but it looks like the post-diff file there is identical to what I was testing locally, so should be good!  Thanks for getting that in.
Comment 7 Micha Lenk 2008-09-16 11:38:40 UTC
As far as I am involved the changes I wanted are included, so it should be fixed now, though I haven't built and checked it.
Comment 8 Andreas Köhler 2008-09-17 17:49:29 UTC
Applied to branches/2.2 as r17550 for inclusion in GnuCash 2.2.7.
Thanks a lot!
Comment 9 John Ralls 2018-06-29 22:09:16 UTC
GnuCash bug tracking has moved to a new Bugzilla host. This bug has been copied to https://bugs.gnucash.org/show_bug.cgi?id=549595. Please update any external references or bookmarks.