Mbox locking not NFS-safe
|Assignee:||Hiroyuki Yamamoto||% Done:|
I've been investigating bug #198 a bit and seems it seems worse: Sylpheed locking method is not NFS-safe.
If you look in libsylph/mbox.c:lock_mbox function, you can see there's a variety of locking methods implemented, but looking at inc.c:get_spool function only LOCK_FLOCK is attempted.
As detailed for example in https://www.spinnaker.de/linux/nfs-locking.html, the correct way would be to try LOCK_LOCKF then dotlocking if the former fails.
Note that current locking goes also against Debian policy 11.6 https://www.debian.org/doc/debian-policy/ch-customized-programs.html#s-mail-transport-agents wich requires MUAs to lock the mailbox in an NFS-safe way.
Thanks in advance,
#5 Updated by Hiroyuki Yamamoto almost 5 years ago
I have checked your patch.
1. Filename passed to lockfile_create() should be base + ".lock" ?
2. liblockfile just seems to create a lock file,
so lockf() (fcntl) also should be combined?
I'm not sure about the above, because I have never used liblockfile.
#7 Updated by Hiroyuki Yamamoto almost 5 years ago
I have created a patch loosely based on HAYASHI's one:
1. use lockf() rather than flock()
2. use liblockfile if 1. fails
Liblockfile replaces the current file locking implementation,
because it doesn't work because of permission.
lockfile_check() is omitted since g_timeout_add() doesn't work in this context.
Ricardo, can you test this patch and apply if it's okay?
#8 Updated by Ricardo Mones almost 5 years ago
Many thanks for the patch! As far as I can see it is correct: implements mbox locking in the recommended order, also giving priority to lockf (fnctl) over flock on systems which have both (like Debian systems).
Are you planning to do a new release soon?
Just asking to see if it's worth to port the patch to current Debian package or is simply better to wait a few days for the new release.
#12 Updated by Ricardo Mones almost 5 years ago
Hiroyuki Yamamoto wrote:
Applied at 3.5.0beta1.
(though I don't recommend to include beta versions in official distribution
repository, since they tend to contain incomplete implementation)
In Debian betas are uploaded to sid (unstable) in order to gain wider exposure and detect possible problems. They also migrate to the testing distribution according to the usual rules (time enough, no RC bugs). At the time of freeze it may happen a beta is the one in testing, therefore it may be released in the stable distribution.
By your comment I'm not sure if you would prefer another kind of management or not.
It's possible to upload betas just to experimental, and only final versions to unstable to ensure only final versions are distributed in stable. But this has also important drawbacks.
Just let me know. You can just mail me privately if you prefer or even on the mailing list if you want a public discussion.