Description
We ran out of space on the partition where moin was installed. After no space was left someone (let's say user XXX) tried to edit a page which raised an exception. We immediately freed some space. All users except user XXX could continue working as normal. User XXX was not able to edit pages anymore because she always got the exception:
PageEditor.py:
838 revstr = f.read()
839 f.close()
840 rev = int(revstr)
841 if not was_deprecated:
842 if self.do_revision_backup or rev == 0:
ValueError
invalid literal for int():
* args = ('invalid literal for int(): ',)The problem was that editing of the page (after all space was gone) caused the file pages\XXX(2f)MoinEditorBackup\current become currupted (size 0) and therefore every edit/cancel/save whatever cycle caused moin to fail when trying to convert '' to an integer.
Having some sanity check at revstr = f.read() could avoid this hard to track down problems.
Installed version
FC5, Version: 1.5.8
Discussion
OK, we could of course avoid the backtrace, but as current is not just a lockfile, but authoritative data (it contains the revision number of the current revision) there is not much we can do without it, except maybe print a nicer error message like "this page is damaged".
For the special case of /MoinEditorBackup pages: this problem will go away in 1.6 (that kind of doing backups was replaced by a better method).
1.5.8 FIX: For Version 1.5.8 I have a quick and dirty solution to fix this problem:
- become root
- go to directory .../wiki/data/pages/USERNAME(2f)MoinEditorBackup/, where USERNAME is the User that can't modify pages
the file "current" should be empty (check with cat)
type ls revisions/ | tail -n 1 > current
The user should be able to edit pages again.
Alternatively, if the user does not need his moin editor backup anyway, simply delete the complete .../wiki/data/pages/USERNAME(2f)MoinEditorBackup/ directory.
Patch
I have worked on a patch that gives out a nicer error message for the user. It simply warns that the page cannot be edited and is "damaged". I think it could also be changed to guess what the current revision is, but i'm not sure i know enough about the moinmoin revision system to work that patch out.
The patch also solves the problem at its source, by atomically editing the current so that it is never corrupted.
1.6
1 diff -r 4d601d25eb5a MoinMoin/PageEditor.py
2 --- a/MoinMoin/PageEditor.py Mon Sep 17 13:42:09 2007 +0200
3 +++ b/MoinMoin/PageEditor.py Wed Sep 19 15:20:18 2007 -0400
4 @@ -946,6 +946,7 @@ Try a different name.""") % (wikiutil.es
5 revdir = os.path.join(pagedir, 'revisions')
6 cfn = os.path.join(pagedir, 'current')
7 clfn = os.path.join(pagedir, 'current-locked')
8 + cltfn = os.path.join(pagedir, 'current-locked.tmp')
9
10 # !!! these log objects MUST be created outside the locked area !!!
11
12 @@ -988,14 +989,33 @@ Try a different name.""") % (wikiutil.es
13 f = file(clfn)
14 revstr = f.read()
15 f.close()
16 - rev = int(revstr)
17 + try:
18 + rev = int(revstr)
19 + except ValueError, err:
20 + raise self.SaveError, _("Unabled to determine current page revision from the current page marker. The page %s is damaged and cannot be edited right now.") % self.page_name
21 +
22 if not was_deprecated:
23 if self.do_revision_backup or rev == 0:
24 rev += 1
25 revstr = '%08d' % rev
26 - f = file(clfn, 'w')
27 - f.write(revstr+'\n')
28 - f.close()
29 + # write the actual current marker to a tmpfile
30 + try:
31 + f = file(cltfn, 'w')
32 + f.write(revstr+'\n')
33 + f.close()
34 + except IOError, err:
35 + try:
36 + os.remove(cltfn)
37 + except:
38 + pass # we don't care for errors in the os.remove
39 + # throw a nicer exception
40 + if err.errno == errno.ENOSPC:
41 + raise self.SaveError, _("Cannot save page %s, no storage space left") % self.page_name
42 + else:
43 + raise self.SaveError, _("An unknown I/O error occured while saving page %s (errno=%d)") % (self.page_name, err.errno)
44 + # atomically put it in place (except on windows)
45 + else:
46 + filesys.rename(cltfn, clfn)
47
48 if not deleted:
49 # save to page file
50 diff -r 4d601d25eb5a MoinMoin/caching.py
51 --- a/MoinMoin/caching.py Mon Sep 17 13:42:09 2007 +0200
52 +++ b/MoinMoin/caching.py Wed Sep 19 15:20:18 2007 -0400
53 @@ -145,7 +145,7 @@ class CacheEntry:
54 self.wlock.release()
55 else:
56 self.request.log("Can't acquire write lock in %s" % self.lock_dir)
57 - except (pickle.PicklingError, IOError, ValueError), err:
58 + except (pickle.PicklingError, OSError, IOError, ValueError), err:
59 raise CacheError(str(err))
60
61 def remove(self):
1.5 backport
Untested 1.5 backport.
Note how it doesn't patch caching.py because it doesn't throw CacheErrors in 1.5.