GNU bug report logs - #13807
The lock for 'DIR/FILE' should always be 'DIR/.#FILE'.

Previous Next

Package: emacs;

Reported by: Paul Eggert <eggert <at> cs.ucla.edu>

Date: Sun, 24 Feb 2013 22:51:01 UTC

Severity: wishlist

Tags: patch

Done: Paul Eggert <eggert <at> cs.ucla.edu>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 13807 in the body.
You can then email your comments to 13807 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#13807; Package emacs. (Sun, 24 Feb 2013 22:51:01 GMT) Full text and rfc822 format available.

Acknowledgement sent to Paul Eggert <eggert <at> cs.ucla.edu>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Sun, 24 Feb 2013 22:51:01 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: bug-gnu-emacs <at> gnu.org
Subject: The lock for 'DIR/FILE' should always be 'DIR/.#FILE'.
Date: Sun, 24 Feb 2013 14:48:53 -0800
[Message part 1 (text/plain, inline)]
Tags: patch

Attached is a proposed cleanup patch, following up on the thread in
<http://lists.gnu.org/archive/html/emacs-devel/2013-02/msg00531.html>.
[filelock.txt (text/plain, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#13807; Package emacs. (Mon, 25 Feb 2013 20:00:02 GMT) Full text and rfc822 format available.

Message #8 received at 13807 <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 13807 <at> debbugs.gnu.org
Subject: Re: bug#13807: The lock for 'DIR/FILE' should always be 'DIR/.#FILE'.
Date: Mon, 25 Feb 2013 14:57:43 -0500
Paul Eggert wrote:

> Attached is a proposed cleanup patch, following up on the thread in
> <http://lists.gnu.org/archive/html/emacs-devel/2013-02/msg00531.html>.

AFAICS, all that thread says is "we should remove this because it never
worked", when in fact it did work just fine until recently.

So the motivation for this seems to be entirely as given in the NEWS
(which is not really where it belongs IMO) for this change. Where it
just says "this was more trouble than it was worth and led to race
conditions of its own". No-one ever reported any (non-theoretical)
problems with it in practice, AFAIK.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#13807; Package emacs. (Mon, 25 Feb 2013 23:43:01 GMT) Full text and rfc822 format available.

Message #11 received at 13807 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Glenn Morris <rgm <at> gnu.org>
Cc: 13807 <at> debbugs.gnu.org
Subject: Re: bug#13807: The lock for 'DIR/FILE' should always be 'DIR/.#FILE'.
Date: Mon, 25 Feb 2013 15:40:11 -0800
[Message part 1 (text/plain, inline)]
On 02/25/13 11:57, Glenn Morris wrote:
>> <http://lists.gnu.org/archive/html/emacs-devel/2013-02/msg00531.html>.
> 
> AFAICS, all that thread says is "we should remove this because it never
> worked", when in fact it did work just fine until recently.

Yes, the discussion should have been clearer.  This patch was prompted
by a code inspection after fixing the bug mentioned in that thread; the patch
does not fix the bug (the bug's already fixed).  I tried to clarify
this in the revised patch (attached).

> So the motivation for this seems to be entirely as given in the NEWS
> (which is not really where it belongs IMO)

OK, I moved the motivation out of NEWS and into the ChangeLog entry.

> No-one ever reported any (non-theoretical)
> problems with it in practice, AFAIK.

The problem is more likely to happen with today's changes to the
MS-Windows side.  And even if the problem was less likely, it's still
a race condition that should get fixed -- the point of that lock
file is to avoid races, after all.

[filelock.txt (text/plain, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#13807; Package emacs. (Tue, 26 Feb 2013 22:22:01 GMT) Full text and rfc822 format available.

Message #14 received at 13807 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: 13807 <at> debbugs.gnu.org
Subject: updated version to avoid MS-Windows vs non-MS-Windows clashes
Date: Tue, 26 Feb 2013 14:19:52 -0800
[Message part 1 (text/plain, inline)]
Attached is an updated version of the patch, which avoids
several of the problems mentioned, by using a different lock file name
on MS-Windows.  Non-MS-Windows uses .#FILE symlinks as before;
MS-Windows uses .#-FILE regular files.  This avoids clashes
between the two approaches.  It also means MS-Windows ignores
non-MS-Windows locks and vice versa, but given all the
inherent incompatibilities involved this may be the best that
we can do reliably.
[filelock.txt (text/plain, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#13807; Package emacs. (Wed, 27 Feb 2013 18:52:02 GMT) Full text and rfc822 format available.

Message #17 received at 13807 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 13807 <at> debbugs.gnu.org
Subject: Re: bug#13807: updated version to avoid MS-Windows vs
	non-MS-Windows	clashes
Date: Wed, 27 Feb 2013 20:49:42 +0200
> Date: Tue, 26 Feb 2013 14:19:52 -0800
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> 
> Attached is an updated version of the patch, which avoids
> several of the problems mentioned, by using a different lock file name
> on MS-Windows.  Non-MS-Windows uses .#FILE symlinks as before;
> MS-Windows uses .#-FILE regular files.  This avoids clashes
> between the two approaches.  It also means MS-Windows ignores
> non-MS-Windows locks and vice versa, but given all the
> inherent incompatibilities involved this may be the best that
> we can do reliably.

I think I found a better solution (committed as trunk revision
111888).  Instead of using 'emacs_open', which just calls 'open', I
now use '_sopen', which allows to specify sharing restrictions for
accessing the same file via other file descriptors, both by the same
Emacs which is creating the lock file, and by other processes.  The
sharing flag I used denies any kind of access to the same file until
the file descriptor returned by _sopen is closed.  This makes the
whole open/write/close sequence in create_lock_file atomic, as far as
other potential readers or writers of the file are concerned -- they
will not be able to access the file until its contents is ready.

I tested this with one NFS-mounted volume that can be accessed from
both Windows and a GNU/Linux box.  The results were that when Emacs
running on GNU/Linux had the file locked, Emacs running on Windows
would refrain from locking it, and vice versa when the file was locked
by Emacs running on Windows and a GNU/Linux Emacs would try to lock
it.  This is IMO better than ignoring the locks from the other
platform, because the latter means each Emacs thinks it has an
exclusive lock on the file, while in fact there are several lockers.

I can make a similar change in read_lock_data, but I don't think it's
needed there, since 'symlink' on Posix hosts is atomic, and thus Emacs
on Windows will always either see the symlink in its final state or
not at all.

So I think with this change, there's no longer a need to use different
file names for the lock files on Windows and on Posix hosts.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#13807; Package emacs. (Sat, 02 Mar 2013 20:44:01 GMT) Full text and rfc822 format available.

Message #20 received at 13807 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 13807 <at> debbugs.gnu.org
Subject: Re: bug#13807: updated version to avoid MS-Windows vs non-MS-Windows
	clashes
Date: Sat, 02 Mar 2013 12:43:05 -0800
On 02/27/2013 10:49 AM, Eli Zaretskii wrote:

> The results were that when Emacs running on GNU/Linux had the file
> locked, Emacs running on Windows would refrain from locking it, and
> vice versa when the file was locked by Emacs running on Windows and
> a GNU/Linux Emacs would try to lock it.

Unfortunately when (for example) the GNU/Linux Emacs refrained from
locking the file, what it was actually doing was using its own
separate lock file, whose name it got in a buggy way.  That is, when
locking FILE it discovered a regular file .#FILE (the MS-Windows lock
file) and then decided to use a symlink .#FILE.0, thus ignoring the
MS-Windows lock.

The process of guessing a lock file name by appending ".0" is
obviously flaky, as it's prone to races.  The recent MS-Windows
changes have made the races more likely, but they were present even
before the changes.  Emacs should not guess the lock file name.

For now, I've installed the patch as trunk bzr 111918, as it fixes these
races.  This patch causes Emacs to use a different lock file name
.#-FILE for MS-Windows than the usual lock file .#FILE for GNU/Linux,
which is not good, but Emacs was using different lock files anyway,
and a virtue of the patch is that any problems in the MS/Windows
implementation won't get in the way of GNU/Linux users on a networked
file system.

I will look into adjusting Emacs so that it uses the same lock file
name .#FILE for both GNU/Linux and MS-Windows, which would be nicer
than the patched Emacs.  This will take some more thought, though.





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#13807; Package emacs. (Sat, 02 Mar 2013 21:18:02 GMT) Full text and rfc822 format available.

Message #23 received at 13807 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 13807 <at> debbugs.gnu.org
Subject: Re: bug#13807: updated version to avoid MS-Windows vs non-MS-Windows
	clashes
Date: Sat, 02 Mar 2013 23:17:17 +0200
> Date: Sat, 02 Mar 2013 12:43:05 -0800
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> CC: 13807 <at> debbugs.gnu.org
> 
> On 02/27/2013 10:49 AM, Eli Zaretskii wrote:
> 
> > The results were that when Emacs running on GNU/Linux had the file
> > locked, Emacs running on Windows would refrain from locking it, and
> > vice versa when the file was locked by Emacs running on Windows and
> > a GNU/Linux Emacs would try to lock it.
> 
> Unfortunately when (for example) the GNU/Linux Emacs refrained from
> locking the file, what it was actually doing was using its own
> separate lock file, whose name it got in a buggy way.  That is, when
> locking FILE it discovered a regular file .#FILE (the MS-Windows lock
> file) and then decided to use a symlink .#FILE.0, thus ignoring the
> MS-Windows lock.

That's not what happened in my testing.  There, there was no lock file
named .#FILE.0 or anything like that.  Can you describe your testing
in more details, and what versions of NFS and Windows did you use?

> The process of guessing a lock file name by appending ".0" is
> obviously flaky, as it's prone to races.  The recent MS-Windows
> changes have made the races more likely, but they were present even
> before the changes.  Emacs should not guess the lock file name.

That's a different issue, though.

> For now, I've installed the patch as trunk bzr 111918, as it fixes these
> races.  This patch causes Emacs to use a different lock file name
> .#-FILE for MS-Windows than the usual lock file .#FILE for GNU/Linux,
> which is not good, but Emacs was using different lock files anyway,
> and a virtue of the patch is that any problems in the MS/Windows
> implementation won't get in the way of GNU/Linux users on a networked
> file system.

I wish you didn't install the .#-FILE part.  It is no longer
justified.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#13807; Package emacs. (Sat, 02 Mar 2013 22:38:01 GMT) Full text and rfc822 format available.

Message #26 received at 13807 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 13807 <at> debbugs.gnu.org
Subject: Re: bug#13807: updated version to avoid MS-Windows vs non-MS-Windows
	clashes
Date: Sat, 02 Mar 2013 14:37:29 -0800
[Message part 1 (text/plain, inline)]
On 03/02/2013 01:17 PM, Eli Zaretskii wrote:
> Can you describe your testing
> in more details, and what versions of NFS and Windows did you use?

Sure, I created a MS-Windows style lock file .#FILE by hand, then
edited FILE with a GNU/Linux Emacs.  I didn't build the MS-Windows
Emacs, and didn't need to use NFS to reproduce the problem.

There's another issue: a GNU/Linux Emacs might be using an
MS-Windows file system that does not support symbolic links.  Such
an Emacs should use a regular-file lock, just as MS-Windows Emacs does,
which means that the code to create regular-file locks should be implementable
in POSIXish primitives.  Also, locking should work even if the Emacs
instance that created a lock uses a symlink whereas the Emacs instance
trying to get the lock would use a regular file, or vice versa.

It'll take some thinking to get all this to work well.  I've written
a first cut for this and have attached it.  I have not tested this
on MS-Windows at all, and haven't tested it as much as I'd like on
GNU/Linux, but it should give a feel for the sort of changes that
need to be made.  Unfortunately the patch is a bit complicated,
but to some extent this is inherent in such a complicated area.
[filelock2.txt (text/plain, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#13807; Package emacs. (Sun, 03 Mar 2013 16:41:01 GMT) Full text and rfc822 format available.

Message #29 received at 13807 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>, Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 13807 <at> debbugs.gnu.org
Subject: Re: bug#13807: updated version to avoid MS-Windows vs non-MS-Windows
	clashes
Date: Sun, 03 Mar 2013 18:39:55 +0200
> Date: Sat, 02 Mar 2013 14:37:29 -0800
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> CC: 13807 <at> debbugs.gnu.org
> 
> On 03/02/2013 01:17 PM, Eli Zaretskii wrote:
> > Can you describe your testing
> > in more details, and what versions of NFS and Windows did you use?
> 
> Sure, I created a MS-Windows style lock file .#FILE by hand, then
> edited FILE with a GNU/Linux Emacs.

But this is not a faithful reproduction of what happens when .#FILE is
created by Emacs running on Windows.  Because of the way Emacs on
Windows creates/opens this file, it is effectively inaccessible to any
other process, even _after_ the file is written and its handle closed.
My testing indicates that 'readdir' does return the file's name, but
every other system call I tried, including even 'lstat', fails with
EPERM.  (I don't know whether all NFS servers behave that way.)  And
that caused Emacs on GNU/Linux to refrain from locking the file.

In addition, with the changes you installed, I think even if .#FILE
_can_ be accessed, Emacs on a Posix host will refrain from locking it,
because 'readlink' will fail for it, right?  There are no more
.#FILE.0 alternative names.

So I still don't see the reason for using different names on Windows
and Posix hosts.

> There's another issue: a GNU/Linux Emacs might be using an
> MS-Windows file system that does not support symbolic links.  Such
> an Emacs should use a regular-file lock, just as MS-Windows Emacs does,
> which means that the code to create regular-file locks should be implementable
> in POSIXish primitives.  Also, locking should work even if the Emacs
> instance that created a lock uses a symlink whereas the Emacs instance
> trying to get the lock would use a regular file, or vice versa.

But these issues are unrelated to the MS-Windows build of Emacs.  They
existed since about forever, and we never cared.  Why is it suddenly
so important that this feature works with 110% reliability, something
it never did?  Am I the only one who thinks we are way past the point
of diminishing returns here1?  Stefan?  Anyone?

> It'll take some thinking to get all this to work well.  I've written
> a first cut for this and have attached it.  I have not tested this
> on MS-Windows at all, and haven't tested it as much as I'd like on
> GNU/Linux, but it should give a feel for the sort of changes that
> need to be made.  Unfortunately the patch is a bit complicated,
> but to some extent this is inherent in such a complicated area.

I think we are wasting our time and energy here.  Nothing terrible
will happen if locking silently fails from time to time, as it always
did.  If we want, we could even optionally make these failures be
announced, by looking at the value lock_file returns, which we
currently ignore.

But if we do go in the direction you suggest, I think we will need to
provide two completely separate implementations for create_lock_file,
one for Posix, the other for MS-Windows.  This is because the
primitives you used in your suggested patch have problems on Windows:
'rename' is not atomic when it needs to delete the target (this is not
supported by the MS 'rename', so we emulate this in w32.c:sys_rename),
and hard links are only supported on NTFS, so editing files on FAT32
volumes (flash drives are normally formatted this way) will be unable
to lock files.  By contrast, using '_sopen', like I did in the current
code, really does make creation of the lock file atomic, and works
with any filesystem supported by Windows, including NFS-mounted
volumes.

So, unless there's a way to do on Posix platforms what '_sopen' does
on Windows (perhaps some file-locking feature? I don't know enough
about that), I think having 2 completely separate implementations will
be cleaner and more maintainable.  (Assuming, that is, that we really
do want to make file locking so bulletproof.)




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#13807; Package emacs. (Sun, 03 Mar 2013 23:57:02 GMT) Full text and rfc822 format available.

Message #32 received at 13807 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 13807 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#13807: updated version to avoid MS-Windows vs non-MS-Windows
	clashes
Date: Sun, 03 Mar 2013 15:56:03 -0800
[Message part 1 (text/plain, inline)]
On 03/03/2013 08:39 AM, Eli Zaretskii wrote:

> the primitives you used in your suggested patch have problems on Windows:
> 'rename' is not atomic when it needs to delete the target ...
> and hard links are only supported on NTFS

Thanks for mentioning these problems.  The latter issue
affects GNU/Linux, too, since it also can mount FAT32 file
systems.  Attached is a revised patch that tries to address
these problems.

> My testing indicates that 'readdir' does return the file's name, but
> every other system call I tried, including even 'lstat', fails with
> EPERM.  (I don't know whether all NFS servers behave that way.)

They don't.  Traditional NFS servers are stateless, and do
not have a state where a file exists and its parent
directory is accessible but you cannot stat the file.  I'd
even call that behavior a bug: file servers with that
behavior will cause problems with many GNU programs,
including Emacs.  It would not be wise for Emacs to rely on
or encourage that behavior.

> with the changes you installed, I think even if .#FILE
> _can_ be accessed, Emacs on a Posix host will refrain from
> locking it, because 'readlink' will fail for it, right?

That's correct: the existence of a regular-file .#FILE
prevents Emacs from locking FILE, and Emacs goes ahead and
accesses FILE without bothering to lock it.  The assumption
is that .#FILE was created for some other reason and should
not be messed with.

> these issues are unrelated to the MS-Windows build of Emacs.

I don't see why they're unrelated.  If an MS-Windows Emacs
uses regular files for locks, these files get in the way of
GNU/Linux Emacs lock files, and that makes these issues pop up.

> They existed since about forever, and we never cared.
> Why is it suddenly so important that this feature works

It's important because the MS-Windows code was recently
changed to create lock files, the existence of which could
break GNU/Linux Emacs's locking.

> with 110% reliability, something it never did?

Even with the attached patch, lock files are not 100%
reliable.  The main point of my patches have been to prevent
lock files from being significantly less reliable than they
already were.  If we easily can make them more
reliable, so much the better, but that's not the main goal.

[filelock2.txt (text/plain, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#13807; Package emacs. (Mon, 04 Mar 2013 16:53:02 GMT) Full text and rfc822 format available.

Message #35 received at 13807 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 13807 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca
Subject: Re: bug#13807: updated version to avoid MS-Windows vs non-MS-Windows
	clashes
Date: Mon, 04 Mar 2013 18:50:32 +0200
> Date: Sun, 03 Mar 2013 15:56:03 -0800
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> CC: Stefan Monnier <monnier <at> iro.umontreal.ca>, 13807 <at> debbugs.gnu.org
> 
> > My testing indicates that 'readdir' does return the file's name, but
> > every other system call I tried, including even 'lstat', fails with
> > EPERM.  (I don't know whether all NFS servers behave that way.)
> 
> They don't.  Traditional NFS servers are stateless, and do
> not have a state where a file exists and its parent
> directory is accessible but you cannot stat the file.  I'd
> even call that behavior a bug: file servers with that
> behavior will cause problems with many GNU programs,
> including Emacs.  It would not be wise for Emacs to rely on
> or encourage that behavior.

Like it or not, it's out there, and others might bump into it.

> > these issues are unrelated to the MS-Windows build of Emacs.
> 
> I don't see why they're unrelated.  If an MS-Windows Emacs
> uses regular files for locks, these files get in the way of
> GNU/Linux Emacs lock files, and that makes these issues pop up.
> 
> > They existed since about forever, and we never cared.
> > Why is it suddenly so important that this feature works
> 
> It's important because the MS-Windows code was recently
> changed to create lock files, the existence of which could
> break GNU/Linux Emacs's locking.

I meant what you wrote here:

> There's another issue: a GNU/Linux Emacs might be using an
> MS-Windows file system that does not support symbolic links.  Such
> an Emacs should use a regular-file lock, just as MS-Windows Emacs does,
> which means that the code to create regular-file locks should be implementable
> in POSIXish primitives.  Also, locking should work even if the Emacs
> instance that created a lock uses a symlink whereas the Emacs instance
> trying to get the lock would use a regular file, or vice versa.

These issues are unrelated to whether Emacs on Windows does or doesn't
lock files.  They existed before, as did the issue with FAT32 volumes
being used from Posix hosts.

And I think you exaggerate the probability of having Emacs running on
Windows to access via NFS files shared with Posix systems.  In my
experience, this is quite rare (as is having people use Emacs on
Windows in general).

> +      if (fd < 0)
> +	err = errno;
> +      else
> +	{
> +	  ptrdiff_t lock_info_len = strlen (lock_info_str);
> +	  err = 0;
> +	  if (emacs_write (fd, lock_info_str, lock_info_len) != lock_info_len
> +	      || fchmod (fd, S_IRUSR | S_IRGRP | S_IROTH) != 0)
> +	    err = errno;

This will need a no-op emulation of fchmod for Windows (since a file
created here will be world-writable anyway).  Other than that, I don't
see any problems with your changes (but I didn't try to build with
them, I only read them).

Thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#13807; Package emacs. (Tue, 05 Mar 2013 02:27:02 GMT) Full text and rfc822 format available.

Message #38 received at 13807 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 13807 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca
Subject: Re: bug#13807: updated version to avoid MS-Windows vs non-MS-Windows
	clashes
Date: Mon, 04 Mar 2013 18:25:32 -0800
[Message part 1 (text/plain, inline)]
On 03/04/13 08:50, Eli Zaretskii wrote:

> it's out there, and others might bump into it.

OK.  Luckily, with the latest proposed patch, Emacs users will be less
likely to bump into the NFS problem, since Emacs won't attempt to create
lock files that exploit the issue.

> These issues are unrelated to whether Emacs on Windows does or doesn't
> lock files.  They existed before, as did the issue with FAT32 volumes
> being used from Posix hosts.

Yes, quite right.  Sorry I misunderstood you.

> I think you exaggerate the probability of having Emacs running on
> Windows to access via NFS files shared with Posix systems.

Possibly.  It depends on local practice.  Many locations don't use NFS
at all.  Around here we use NFS heavily.

> This will need a no-op emulation of fchmod for Windows (since a file
> created here will be world-writable anyway).

OK, thanks.  Also, older POSIXish hosts that lack mkstemp won't need
the fchmod either.  I added the following to try to address these two points.
Revised complete patch attached, relative to trunk bzr 111938.

=== modified file 'src/filelock.c'
--- src/filelock.c	2013-03-04 19:27:39 +0000
+++ src/filelock.c	2013-03-04 19:36:45 +0000
@@ -407,15 +407,21 @@
       USE_SAFE_ALLOCA;
       char *nonce = SAFE_ALLOCA (lfdirlen + sizeof nonce_base);
       int fd;
+      bool need_fchmod;
+      mode_t world_readable = S_IRUSR | S_IRGRP | S_IROTH;
       memcpy (nonce, lfname, lfdirlen);
       strcpy (nonce + lfdirlen, nonce_base);
 
 #if HAVE_MKSTEMP
+      /* Prefer mkstemp if available, as it avoids a race between
+	 mktemp and emacs_open.  */
       fd = mkstemp (nonce);
+      need_fchmod = 1;
 #else
       mktemp (nonce);
       fd = emacs_open (nonce, O_WRONLY | O_CREAT | O_EXCL | O_BINARY,
-		       S_IRUSR | S_IWUSR);
+		       world_readable);
+      need_fchmod = 0;
 #endif
 
       if (fd < 0)
@@ -425,7 +431,7 @@
 	  ptrdiff_t lock_info_len = strlen (lock_info_str);
 	  err = 0;
 	  if (emacs_write (fd, lock_info_str, lock_info_len) != lock_info_len
-	      || fchmod (fd, S_IRUSR | S_IRGRP | S_IROTH) != 0)
+	      || (need_fchmod && fchmod (fd, world_readable) != 0))
 	    err = errno;
 	  if (emacs_close (fd) != 0)
 	    err = errno;

=== modified file 'src/w32.c'
--- src/w32.c	2013-03-03 23:12:54 +0000
+++ src/w32.c	2013-03-05 01:42:12 +0000
@@ -3416,6 +3416,12 @@
 }
 
 int
+fchmod (int fd, mode_t mode)
+{
+  return 0;
+}
+
+int
 sys_rename_replace (const char *oldname, const char *newname, BOOL force)
 {
   BOOL result;

=== modified file 'src/w32.h'
--- src/w32.h	2013-03-03 23:12:54 +0000
+++ src/w32.h	2013-03-05 01:42:12 +0000
@@ -186,6 +186,7 @@
 extern void srandom (int);
 extern int random (void);
 
+extern int fchmod (int, mode_t);
 extern int sys_rename_replace (char const *, char const *, BOOL);
 extern int sys_pipe (int *);
 


[filelock2.txt (text/plain, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#13807; Package emacs. (Tue, 05 Mar 2013 18:39:02 GMT) Full text and rfc822 format available.

Message #41 received at 13807 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 13807 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca
Subject: Re: bug#13807: updated version to avoid MS-Windows vs non-MS-Windows
	clashes
Date: Tue, 05 Mar 2013 20:38:02 +0200
> Date: Mon, 04 Mar 2013 18:25:32 -0800
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> CC: monnier <at> iro.umontreal.ca, 13807 <at> debbugs.gnu.org
> 
> > This will need a no-op emulation of fchmod for Windows (since a file
> > created here will be world-writable anyway).
> 
> OK, thanks.  Also, older POSIXish hosts that lack mkstemp won't need
> the fchmod either.  I added the following to try to address these two points.
> Revised complete patch attached, relative to trunk bzr 111938.

Thanks, I have 2 more nits.

> +  while ((nbytes = readlinkat (AT_FDCWD, lfname, lfinfo, MAX_LFINFO + 1)) < 0
> +	 && errno == EINVAL)
>      {
> -      lfinfo[nbytes] = '\0';
> -      return build_string (lfinfo);
> +      int fd = emacs_open (lfname, O_RDONLY | O_BINARY | O_NOFOLLOW, 0);
> +      if (0 <= fd)
> +	{
> +	  ptrdiff_t read_bytes = emacs_read (fd, lfinfo, MAX_LFINFO + 1);
> +	  int read_errno = errno;
> +	  if (emacs_close (fd) != 0)
> +	    return -1;
> +	  errno = read_errno;
> +	  return read_bytes;
> +	}
> +
> +      if (errno != ELOOP)
> +	return -1;

We will need to define away O_NOFOLLOW and ELOOP, to get this to
compile on Windows.  I think the right place for the former is
nt/inc/unistd.h, near O_NOCTTY, and for the latter nt/inc/ms-w32.h,
where ENOTSUP is defined.

Other than that, I think this is OK.  Thanks.





Reply sent to Paul Eggert <eggert <at> cs.ucla.edu>:
You have taken responsibility. (Tue, 05 Mar 2013 22:40:01 GMT) Full text and rfc822 format available.

Notification sent to Paul Eggert <eggert <at> cs.ucla.edu>:
bug acknowledged by developer. (Tue, 05 Mar 2013 22:40:01 GMT) Full text and rfc822 format available.

Message #46 received at 13807-done <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 13807-done <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca
Subject: Re: bug#13807: updated version to avoid MS-Windows vs non-MS-Windows
	clashes
Date: Tue, 05 Mar 2013 14:38:44 -0800
On 03/05/13 10:38, Eli Zaretskii wrote:
> We will need to define away O_NOFOLLOW and ELOOP, to get this to
> compile on Windows.  I think the right place for the former is
> nt/inc/unistd.h, near O_NOCTTY, and for the latter nt/inc/ms-w32.h,
> where ENOTSUP is defined.

Thanks, I did the former, but for the latter it's possible ELOOP
won't be defined on a (very old) POSIXish host, so I put a conditional
definition for it into filelock.c itself, which should work for
MS-Windows as well.  Installed as trunk bzr 111948 and marking
this as done.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Wed, 03 Apr 2013 11:24:02 GMT) Full text and rfc822 format available.

This bug report was last modified 11 years and 35 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.