Re: sqlite3: bug with monotone

[Corinna Vinschen <corinna-cygwin at cygwin dot com>]

On May 31 13:58, Warren Young wrote:
> On 5/31/2013 03:22, Corinna Vinschen wrote:
> >On May 30 18:56, Warren Young wrote:
> >>1. We continue waiting for someone to to implement a per-process or
> >>per-subtree mandatory locking feature in Cygwin, so that "Unix mode"
> >>SQLite on Cygwin can be configured to cooperate with native SQLite.
> >
> >What kind of locking does sqlite use on Cygwin:
> >
> >[ ] POSIX fcntl locks
> >[ ] BSD flock locks
> >[ ] Old POSIX lockf locks?
> 
> The SQLite code prefers POSIX advisory locks, but it can fall back
> to BSD locks if it has to.
> 
> The docs and code comments talk about the choice between the two
> being dependent on the "file system", so that SQLite can use flock()
> where POSIX locks don't exist.  My understanding of the code is
> shaky, but it looks like it tries POSIX locks when opening the DB
> file and falls back to BSD locks if that fails.
> [...]
> If I had to guess, I'd say SQLite in Unix mode will use POSIX locks
> on Cygwin, provided they work as it expects.  If cygwin1.dll doesn't
> do what SQLite expects when it tries the lock, it may fall back to
> flock().
> 
> >Here's a proposal:
> >
> >- Only add optional mandatory locking to fcntl and flock locks, not for
> >   lockf locks.
> 
> Okay so far.
> 
> >- Add a flag F_MDLCK which can be or'ed to struct flock's l_type.
> >
> >- Add a flag LOCK_MD which can be or'ed to the 2nd parameter to flock(2).
> 
> I'm uneasy about this departure from SysV mandatory locking.
> (Nicely described in Linux's Documentation/mandatory.txt.)

You mean Documentation/filesystems/mandatory-locking.txt, I guess.

> I guess you're doing this because the setgid + g-x hack the SysV
> implementors chose can't work on NTFS?

I know how it works on Linux (or, fwiw, SysV), and I proposed this at
one point when we discussed sqlite a couple of months ago.  But there's
an outstanding drawback using this method.  Setting g+S,g-x permissions
works nicely on NTFS and NFS, and while it won't work via Samba or on
most other filesystems, I guess this we could live with.

The really crucial point is this: In contrast to Linux, you're using
mandatory locking *only* for interoperability with native tools using
mandatory locking on the same files.  Let's just take sqlite as example.
How do you make sure that the DB files have been created by the Cygwin
version of sqlite?  There's a 50% chance that the user used a native
Windows sqlite to create the files.  And those quite certainly don't
have the g+S,g-x permission setting.  How do you explain a user that
interoperability is broken, because the user "forgot" to set the correct
POSIX file permissions on the affected DB files?  From a user standpoint
this is really cumbersome, if not incomprehensible.  It's certainly not
a logical thing to do from ther native perspective.

I hate having to make this point, but that's what came up when thinking
about how to implement this.

Back to the Cygwin side.  Who's going to make the decision to use
advisory vs. mandatory locking?  The user or the application?  Ideally
both, I guess.  The idea to add a F_MDLCK flag obviously only allows the
application to switch to mandatory locking, but this is important.  For
sqlite you would like to use always mandatory locking for the reasons
you outlined, so the user shouldn't really have a say in the matter.

If F_MDLCK is not such a bright idea, maybe another fcntl is?  Something
along these lines:

    fd = open ("my.db", ...);
  #ifdef __CYGWIN__
    fcntl (fd, F_SETLK_MAND, 1);
  #endif

As for the user, we could add the g+S,g-x stuff additionally at one point,
but I'm rather reluctant to provide these means at all.  See below.

> >- Using these flags, I'll resurrect the old pre-Cygwin 1.7 locking code
> >   which does NOT support F_GETLK.
> 
> SQLite does use F_GETLK.

Here's the problem: Windows.

Windows does not provide any means to fetch information about existing
mandatory record locks on a file.  None at all.  The only way to implement
F_GETLK is this:

- Try to create a lock.
- If it worked, unlock it again (which makes the whole affair non-atomic).
- If it didn't work, *fake* a struct flock to return to the caller.
  What should it contain?!?  There's no information available to fill
  l_start, l_len, and l_pid at all, and filling l_type only works
  reliable if you requested F_GETLK/F_RDLCK.

I'll implement this nevertheless, but the results of F_GETLK are
sub-standard by definition.

> >Does that sound ok?
> 
> Your previous proposal was to implement Linux's -omand mount option.
> There's a lot to recommend it.

There's a lot to recommend not using mandatory locking at all, unless in
very limited circumstances where interoperability with native
applications using mandatory locking is required.  For one thing, this
doesn't occur very often, since mandatory record locking isn't used
a lot, not even on Windows.  But what's more important is that Windows
mandatory record locking works not as the user can usually expect from
fcntl or flock: Windows locks are per-process/per-handle.  Locks are not
inherited by child processes via handle inheritance.  This excludes
inheritance via execve on Cygwin!  That, and the inability to support
a *working* F_GETLK are serious enough to avoid and discourage using
mandatory file locking.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple