public inbox for cygwin@cygwin.com
 help / color / mirror / Atom feed
* sqlite3: bug with monotone
@ 2013-05-30  2:58 Yaakov (Cygwin/X)
  2013-05-30 11:17 ` Achim Gratz
  0 siblings, 1 reply; 34+ messages in thread
From: Yaakov (Cygwin/X) @ 2013-05-30  2:58 UTC (permalink / raw)
  To: cygwin

Warren,

I'm having a problem with my newly built monotone packages on both x86 
and x64:

mtn: warning: recoverable 'system' error while processing peer 
mtn://monotone.ca/monotone: 'error: sqlite error: SQL logic error or 
missing database'
mtn: error: processing failure while talking to peer 
'mtn://monotone.ca/monotone', disconnecting

This was working properly with my 3.7.15.2 x64 package, as well as with 
a self-built 3.7.17.  I don't know if it's a question of configuration 
options or a bug in that version.  Here's how I have built sqlite3:

http://cygwin-ports.git.sourceforge.net/git/gitweb.cgi?p=cygwin-ports/sqlite3;a=tree

Could you please provide an update for both arches?


Yaakov

--
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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: sqlite3: bug with monotone
  2013-05-30  2:58 sqlite3: bug with monotone Yaakov (Cygwin/X)
@ 2013-05-30 11:17 ` Achim Gratz
  2013-05-30 17:37   ` Warren Young
  2013-05-31  6:29   ` Yaakov (Cygwin/X)
  0 siblings, 2 replies; 34+ messages in thread
From: Achim Gratz @ 2013-05-30 11:17 UTC (permalink / raw)
  To: cygwin

Yaakov (Cygwin/X <yselkowitz <at> users.sourceforge.net> writes:
> This was working properly with my 3.7.15.2 x64 package, as well as with 
> a self-built 3.7.17.  I don't know if it's a question of configuration 
> options or a bug in that version.  Here's how I have built sqlite3:

You are defining "SQLITE_OS_UNIX" which introduces known interoperability
problems with accesses from the windows side.  What happens with your
monotone problem when you drop that definition?  If monotone tries to use a
temporary table, this would perhaps explain the error.


Regards,
Achim.



--
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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: sqlite3: bug with monotone
  2013-05-30 11:17 ` Achim Gratz
@ 2013-05-30 17:37   ` Warren Young
  2013-05-31  0:56     ` Yaakov (Cygwin/X)
  2013-05-31  6:29   ` Yaakov (Cygwin/X)
  1 sibling, 1 reply; 34+ messages in thread
From: Warren Young @ 2013-05-30 17:37 UTC (permalink / raw)
  To: Cygwin-L

On 5/30/2013 04:27, Achim Gratz wrote:
> Yaakov (Cygwin/X <yselkowitz <at> users.sourceforge.net> writes:
>> This was working properly with my 3.7.15.2 x64 package, as well as with
>> a self-built 3.7.17.  I don't know if it's a question of configuration
>> options or a bug in that version.  Here's how I have built sqlite3:
>
> You are defining "SQLITE_OS_UNIX" which introduces known interoperability
> problems with accesses from the windows side.

Thoroughly explained here:

	http://stackoverflow.com/questions/11007024/

I don't see a way out of this trap, where either native Windows loses 
out to Cygwin or vice versa, until Cygwin provides some way to request 
mandatory locks on a per-process or per-subtree basis.

My only choice until then is which constituency to annoy.  Currently, 
I'm choosing to annoy the smaller of the two.

--
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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: sqlite3: bug with monotone
  2013-05-30 17:37   ` Warren Young
@ 2013-05-31  0:56     ` Yaakov (Cygwin/X)
  2013-05-31  1:50       ` Warren Young
  2013-05-31  9:22       ` sqlite3: bug with monotone Achim Gratz
  0 siblings, 2 replies; 34+ messages in thread
From: Yaakov (Cygwin/X) @ 2013-05-31  0:56 UTC (permalink / raw)
  To: cygwin

On 2013-05-30 12:02, Warren Young wrote:
> On 5/30/2013 04:27, Achim Gratz wrote:
>> Yaakov (Cygwin/X <yselkowitz <at> users.sourceforge.net> writes:
>>> This was working properly with my 3.7.15.2 x64 package, as well as with
>>> a self-built 3.7.17.  I don't know if it's a question of configuration
>>> options or a bug in that version.  Here's how I have built sqlite3:
>>
>> You are defining "SQLITE_OS_UNIX" which introduces known interoperability
>> problems with accesses from the windows side.

Yes, I can now confirm that (the lack of) SQLITE_OS_UNIX is the culprit.

> Thoroughly explained here:
>
>      http://stackoverflow.com/questions/11007024/
>
> I don't see a way out of this trap, where either native Windows loses
> out to Cygwin or vice versa, until Cygwin provides some way to request
> mandatory locks on a per-process or per-subtree basis.

So you *can't* have your cake and eat it too after all.  What a surprise.

If it's a choice between Cygwin programs functioning correctly (in this 
case, mtn clone), and allowing interoperability with Windows programs, 
there is NO QUESTION that the former MUST take priority.  Add a `mount 
-o mand' feature if you wish (or get someone who cares about this to do 
so), but DON'T BREAK Cygwin programs for the sake of those NOT USING Cygwin.

Please fix sqlite3 accordingly.


Yaakov


--
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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: sqlite3: bug with monotone
  2013-05-31  0:56     ` Yaakov (Cygwin/X)
@ 2013-05-31  1:50       ` Warren Young
  2013-05-31 15:00         ` Corinna Vinschen
  2013-05-31  9:22       ` sqlite3: bug with monotone Achim Gratz
  1 sibling, 1 reply; 34+ messages in thread
From: Warren Young @ 2013-05-31  1:50 UTC (permalink / raw)
  To: Cygwin-L

On 5/30/2013 16:36, Yaakov (Cygwin/X) wrote:
> DON'T BREAK Cygwin programs for the sake of those NOT USING Cygwin.

Where did you get the idea that my stance favors those not using Cygwin?

My choice favors those who want to use Cygwin *and* native Windows 
programs, together.  That's approximately 100% of all Cygwin users.

Did you miss the part in the referenced Stack Overflow post where I 
talked about how we had already tried building SQLite your way?  The 
post was a sort of damage control resulting from that experiment.

SQLite is one of the most widely used libraries, with hundreds of 
millions[1] of deployments.  This problem isn't just about Cygwin svn 
vs. TortoiseSVN.  It's also about Firefox, and Chrome, and Flash, and 
Skype, and Lightroom, and Dropbox, and...[2]

With so many native Windows programs using SQLite, it's virtually 
certain there are more bad interactions waiting to bite us if we make 
Cygwin SQLite incompatible with native SQLite again.

I see you're using Thunderbird.  Are you willing to give up the ability 
to use sqlite(1) on a live Thunderbird DB file?

> Please fix sqlite3 accordingly.

I don't see that switching Cygwin SQLite back to Unix mode is an option 
as long as it has a real potential of breaking half a billion other 
programs.

As I see it, the practical options are:

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.

2. You figure out which of the thousands of lines of code separating 
Unix mode from Cygwin mode in SQLite breaks monotone.  With any luck, 
it's not in the file locking code, so we can talk about selectively 
patching that piece of Unix mode in over the top of the stock Cygwin 
code.  The current package includes one such patch already.  (That patch 
makes it use the Unix mode /tmp path selector to get around a 
permissions issue.)

3. I start shipping two different versions of the libsqlite3 package, 
each built differently, and deploy the Unix mode version as 'test', 
perpetually.  (There isn't any more elegant way to ship two different 
versions of the same library, each of which could satisfy setup.exe's 
dependency checker, is there?)  Then individual users can choose which 
side of the fence they want the breakage to land on.

If you don't like those options, then I offer, in all seriousness:

4. You take over maintainership of the official Cygwin SQLite packages. 
  Then you can build it however you like.


[1] https://www.sqlite.org/mostdeployed.html
[2] https://www.sqlite.org/famous.html


--
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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: sqlite3: bug with monotone
  2013-05-30 11:17 ` Achim Gratz
  2013-05-30 17:37   ` Warren Young
@ 2013-05-31  6:29   ` Yaakov (Cygwin/X)
  2013-05-31 19:06     ` Warren Young
  1 sibling, 1 reply; 34+ messages in thread
From: Yaakov (Cygwin/X) @ 2013-05-31  6:29 UTC (permalink / raw)
  To: cygwin

On 2013-05-30 05:27, Achim Gratz wrote:
> Yaakov (Cygwin/X <yselkowitz <at> users.sourceforge.net> writes:
>> This was working properly with my 3.7.15.2 x64 package, as well as with
>> a self-built 3.7.17.  I don't know if it's a question of configuration
>> options or a bug in that version.  Here's how I have built sqlite3:
>
> You are defining "SQLITE_OS_UNIX" which introduces known interoperability
> problems with accesses from the windows side.  What happens with your
> monotone problem when you drop that definition?  If monotone tries to use a
> temporary table, this would perhaps explain the error.

Indeed, it does, thanks for the tip.  This got me looking at your post 
from a previous thread, where you mention the configuration option for 
this.  I can confirm that -DSQLITE_TEMP_STORE=2 is enough to fix 
monotone on its own.

I have updated my build accordingly, plus added two more configuration 
options, and a patch for the manpage, from Fedora.  Can *this* get into 
the distro package?


Yaakov


--
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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: sqlite3: bug with monotone
  2013-05-31  0:56     ` Yaakov (Cygwin/X)
  2013-05-31  1:50       ` Warren Young
@ 2013-05-31  9:22       ` Achim Gratz
  1 sibling, 0 replies; 34+ messages in thread
From: Achim Gratz @ 2013-05-31  9:22 UTC (permalink / raw)
  To: cygwin

Yaakov (Cygwin/X <yselkowitz <at> users.sourceforge.net> writes:
> Yes, I can now confirm that (the lack of) SQLITE_OS_UNIX is the culprit.
[...]
> If it's a choice between Cygwin programs functioning correctly (in this 
> case, mtn clone), and allowing interoperability with Windows programs, 
> there is NO QUESTION that the former MUST take priority.  Add a `mount 
> -o mand' feature if you wish (or get someone who cares about this to do 
> so), but DON'T BREAK Cygwin programs for the sake of those NOT USING Cygwin.

We've had this discussion before, please re-read the arguments made there if
you haven't done so already.

I've been running my own sqlite3 package for a while that treats Cygwin as
UNIX, but I don't use TortoiseSVN and other programs that would lead to
concurrent access from the Windows side (or from the Cygwin side, really). 
I do use (not often) temporary tables that are large and should not be held
in memory, so the even if that option was used to fix the temp table issue I
would still rather use my own package.

> Please fix sqlite3 accordingly.

I've looked into this situation a few times (and Warren has been doing that
more than just once, too).  Things aren't as clear cut as you seem to
believe.  For starters, upstream doesn't seem to give much thought about
Cygwin at all.  I've tried to find where to fix things and the situation is
that the test suite doesn't run at all when sqlite3 is compiled like
upstream wants (i.e. SQLITE_OS_WIN) and fails quite a few of its own
concurrency tests when compiled with SQLITE_OS_UNIX.  A few of these test
fails are simply of the sort that you are seeing with monotone (DB access
fails), but some of them look like database corruptions.  A "fixed" sqlite3
should not fail its own test suite I suppose.

I haven't had time yet to build sqlite3 and run the tests on a UNIX box to
get a baseline from which to work.  But whichever way you compile sqlite3 on
Cygwin, you can either not test it at all or you produce test failures, so
you can only chose which way to break it.  I agree that it would be a good
thing to fix this, but it takes a bit more work than just using a different
switch.


Regards,
Achim.



--
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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: sqlite3: bug with monotone
  2013-05-31  1:50       ` Warren Young
@ 2013-05-31 15:00         ` Corinna Vinschen
  2013-05-31 20:19           ` Warren Young
  0 siblings, 1 reply; 34+ messages in thread
From: Corinna Vinschen @ 2013-05-31 15:00 UTC (permalink / raw)
  To: cygwin

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?

Here's a proposal:

- Only add optional mandatory locking to fcntl and flock locks, not for
  lockf locks.

- 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).

- Using these flags, I'll resurrect the old pre-Cygwin 1.7 locking code
  which does NOT support F_GETLK.  I will try, but I can also not
  guarantee that a blocking, mandatory lock call will be interruptible
  by signals.

- For anything else, http://cygwin.com/acronyms/#PTC


Does that sound ok?


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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: sqlite3: bug with monotone
  2013-05-31  6:29   ` Yaakov (Cygwin/X)
@ 2013-05-31 19:06     ` Warren Young
  0 siblings, 0 replies; 34+ messages in thread
From: Warren Young @ 2013-05-31 19:06 UTC (permalink / raw)
  To: Cygwin-L

On 5/30/2013 22:23, Yaakov (Cygwin/X) wrote:
> I can confirm that -DSQLITE_TEMP_STORE=2 is enough to fix
> monotone on its own.
>
> I have updated my build accordingly, plus added two more configuration
> options, and a patch for the manpage, from Fedora.  Can *this* get into
> the distro package?

I don't see any serious problems from changing that build option.  It 
doubtless would prevent 32-bit Cygwin SQLite from accessing gigabyte+ 
DBs, but those needing to do such things can a) set the pragma to 
restore previous behavior; b) switch to Cygwin 64; or c) switch to 
native SQLite.

As for the rest of it, are you proposing that I adopt your Ports version 
of SQLite, replacing what I have?  Does yours include the Unix mode /tmp 
patch I mentioned?

--
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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: sqlite3: bug with monotone
  2013-05-31 15:00         ` Corinna Vinschen
@ 2013-05-31 20:19           ` Warren Young
  2013-05-31 21:11             ` Warren Young
  2013-06-01 10:57             ` Corinna Vinschen
  0 siblings, 2 replies; 34+ messages in thread
From: Warren Young @ 2013-05-31 20:19 UTC (permalink / raw)
  To: cygwin

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.

(Part of the reason I'm shaky on this is that it's actually a lot more 
complicated than that.  There are seven (?) locking strategies supported 
by the current code, only some of which are compiled in at any one time, 
depending on the platform.  On top of that, all of this runs through 
SQLite's VFS indirection layer, so tracing the calls is a bit like 
Choose Your Own Adventure.)

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.)

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

> - 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.

Four of the five uses in the code appear irrelevant to Cygwin.

The fifth, though, is that SQLite uses F_GETLK when it is in the path 
where it knows it will need to write to the DB "soon" but not 
immediately, so it attempts to get the current lock to see if it's 
currently unlocked before proceeding.  I'm not sure why it doesn't just 
blindly try the lock.

See unixCheckReservedLock() near line 24210 in the amalgam version of 
sqlite3.c.

> Does that sound ok?

Your previous proposal was to implement Linux's -omand mount option. 
There's a lot to recommend it.

For one thing, someone wanting Cygwin SQLite to behave as it currently 
does despite being built in Unix mode should be able to set this option 
on the Cygwin root and /cygdrive, no?

Those wanting a more nuanced approach can remount subtrees of their 
native filesystem with it, where they know they need it.

I suppose you could do like Linux here, and ignore the flags you've 
proposed adding when the subtree isn't marked as wanting mandatory 
locks.  If you did it that way, then these flags would operate more like 
the SysV file modes hack, being necessary to enable mandatory locks on 
that file but not sufficient.

--
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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: sqlite3: bug with monotone
  2013-05-31 20:19           ` Warren Young
@ 2013-05-31 21:11             ` Warren Young
  2013-06-01 10:58               ` Corinna Vinschen
  2013-06-01 10:57             ` Corinna Vinschen
  1 sibling, 1 reply; 34+ messages in thread
From: Warren Young @ 2013-05-31 21:11 UTC (permalink / raw)
  To: cygwin

On 5/31/2013 13:58, Warren Young wrote:
>
> The SQLite code prefers POSIX advisory locks, but it can fall back to
> BSD locks if it has to.

Just to clarify, when I say "POSIX locks" I always mean new style 
fcntl() locks.  There are no calls to lockf() in sqlite3.c.

> I'm not sure why it doesn't just
> blindly try the lock.

On reflection, I'm sure it has something to do with maintaining high 
concurrency.  If it knows its near-future DB file write is going to get 
blocked, it can choose to do something else while the existing DB lock 
holders finish.

By contrast, SQLite's flock() based locking is documented as being much 
more brute-force, resulting in much lower concurrency.

--
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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: sqlite3: bug with monotone
  2013-05-31 20:19           ` Warren Young
  2013-05-31 21:11             ` Warren Young
@ 2013-06-01 10:57             ` Corinna Vinschen
  2013-06-01 11:35               ` Corinna Vinschen
  2013-06-02 10:31               ` Corinna Vinschen
  1 sibling, 2 replies; 34+ messages in thread
From: Corinna Vinschen @ 2013-06-01 10:57 UTC (permalink / raw)
  To: cygwin

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: sqlite3: bug with monotone
  2013-05-31 21:11             ` Warren Young
@ 2013-06-01 10:58               ` Corinna Vinschen
  0 siblings, 0 replies; 34+ messages in thread
From: Corinna Vinschen @ 2013-06-01 10:58 UTC (permalink / raw)
  To: cygwin

On May 31 14:19, Warren Young wrote:
> On 5/31/2013 13:58, Warren Young wrote:
> >
> >The SQLite code prefers POSIX advisory locks, but it can fall back to
> >BSD locks if it has to.
> 
> Just to clarify, when I say "POSIX locks" I always mean new style
> fcntl() locks.  There are no calls to lockf() in sqlite3.c.
> 
> >I'm not sure why it doesn't just
> >blindly try the lock.
> 
> On reflection, I'm sure it has something to do with maintaining high
> concurrency.  If it knows its near-future DB file write is going to
> get blocked, it can choose to do something else while the existing
> DB lock holders finish.
> 
> By contrast, SQLite's flock() based locking is documented as being
> much more brute-force, resulting in much lower concurrency.

Makes sense, given that flock is not record but 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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: sqlite3: bug with monotone
  2013-06-01 10:57             ` Corinna Vinschen
@ 2013-06-01 11:35               ` Corinna Vinschen
  2013-06-01 12:57                 ` Corinna Vinschen
  2013-06-02 10:31               ` Corinna Vinschen
  1 sibling, 1 reply; 34+ messages in thread
From: Corinna Vinschen @ 2013-06-01 11:35 UTC (permalink / raw)
  To: cygwin

On Jun  1 12:57, Corinna Vinschen wrote:
> 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
                                    ^^^^^^^^^^^^^^^^^^^^^^

Make that "per-process/per-file object".

To clarify: The file object is the OS datastructure the handles refer
to.  A duplicated handle refers to the same file object, while a handle
to the same file created with CreateFile refers to another file object.

Duplicated handles within the same process share the locks.  Different
handles to the same file created with CreateFile don't share the locks.

Duplicated handles to the same file object in another process (via
inheritance or an explicit DuplicateHandle) don't share the locks.


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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: sqlite3: bug with monotone
  2013-06-01 11:35               ` Corinna Vinschen
@ 2013-06-01 12:57                 ` Corinna Vinschen
  0 siblings, 0 replies; 34+ messages in thread
From: Corinna Vinschen @ 2013-06-01 12:57 UTC (permalink / raw)
  To: cygwin

On Jun  1 13:35, Corinna Vinschen wrote:
> On Jun  1 12:57, Corinna Vinschen wrote:
> > 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
>                                     ^^^^^^^^^^^^^^^^^^^^^^
> 
> Make that "per-process/per-file object".
> 
> To clarify: The file object is the OS datastructure the handles refer
> to.  A duplicated handle refers to the same file object, while a handle
> to the same file created with CreateFile refers to another file object.
> 
> Duplicated handles within the same process share the locks.  Different
> handles to the same file created with CreateFile don't share the locks.
> 
> Duplicated handles to the same file object in another process (via
> inheritance or an explicit DuplicateHandle) don't share the locks.

Oh and, while I'm at it, here's another inconsistency:

fcntl locks, when owned, can be overwritten with another lock type
(F_RDLCK/F_WRLCK) without having to unlock the existing lock first.
fcntl locks can be locked and unlocked byte per byte on an as needed
basis.  Locks are split and merged as you lock along.

This doesn't work at all with Windows mandatory locks.  The owning
handle can overwrite read locks with other read locks, but they are not
merged.  Rather they add up and each of them has to be explicitly
unlocked to get out of the way.  The owning handle can't overwrite a
read lock with a write lock at all.  The owning handle can't overwrite a
write lock with an identical write lock.  Locked regions can't be
unlocked partially, they have to be unlocked exactly the same offset and
length as they have been locked.  Therefore, there's no atomic way to
convert arbitrary regions from one lock type to another, unless it's
guaranteed that the lock/unlock calls are guarded in a cooperative way.


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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: sqlite3: bug with monotone
  2013-06-01 10:57             ` Corinna Vinschen
  2013-06-01 11:35               ` Corinna Vinschen
@ 2013-06-02 10:31               ` Corinna Vinschen
  2013-06-02 11:24                 ` Corinna Vinschen
                                   ` (2 more replies)
  1 sibling, 3 replies; 34+ messages in thread
From: Corinna Vinschen @ 2013-06-02 10:31 UTC (permalink / raw)
  To: cygwin

Hi Warren,

On Jun  1 12:57, Corinna Vinschen wrote:
> 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.
> [...]
> 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. [...]

I just applied a patch to implement mandatory locking.  It also supports
F_GETLK, with limited usability due to Windows restrictions, as I
explained in other mail.  It does NOT yet support BSD flock locking,
only POSIX fcntl locking.

I dropped the F_MDLCK idea. Instead I implemented a specific fcntl code
to switch to mandatory locking on a file descriptor:

  fcntl (fd, F_LCK_MANDATORY, 1);

The name F_SETLK_MAND didn't seem right.  Anyway, afterwards, you can
use the usual locking fcntls, but with Windows mandatory locking
semantics.

I didn't add a way for the user to switch on mandatory locking for now,
and I don't intend to do that for 1.7.19.  Hope that helps, nevertheless.

I'm just about to generate a 2013-06-02 developer snapshot for 32 bit,
a 64 bit DLL will follow tomorrow.

Please give the 32 bit snapshot a try ASAP.  I intend to release 1.7.19
very soon, probably tomorrow or Tuesday.


Thanks,
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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: sqlite3: bug with monotone
  2013-06-02 10:31               ` Corinna Vinschen
@ 2013-06-02 11:24                 ` Corinna Vinschen
  2013-06-03  8:03                 ` Achim Gratz
  2013-06-03 17:36                 ` Warren Young
  2 siblings, 0 replies; 34+ messages in thread
From: Corinna Vinschen @ 2013-06-02 11:24 UTC (permalink / raw)
  To: cygwin

On Jun  2 12:31, Corinna Vinschen wrote:
> [...]
>   fcntl (fd, F_LCK_MANDATORY, 1);
> [...]
> I'm just about to generate a 2013-06-02 developer snapshot for 32 bit,
> a 64 bit DLL will follow tomorrow.

64 bit test release 1.7.19-9 uploaded.

> Please give the 32 bit snapshot a try ASAP.  I intend to release 1.7.19
> very soon, probably tomorrow or Tuesday.


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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: sqlite3: bug with monotone
  2013-06-02 10:31               ` Corinna Vinschen
  2013-06-02 11:24                 ` Corinna Vinschen
@ 2013-06-03  8:03                 ` Achim Gratz
  2013-06-03  8:42                   ` Corinna Vinschen
  2013-06-03 17:36                 ` Warren Young
  2 siblings, 1 reply; 34+ messages in thread
From: Achim Gratz @ 2013-06-03  8:03 UTC (permalink / raw)
  To: cygwin

Corinna Vinschen <corinna-cygwin <at> cygwin.com> writes:
> Please give the 32 bit snapshot a try ASAP.  I intend to release 1.7.19
> very soon, probably tomorrow or Tuesday.

Have it installed now, but I can't give it much testing before the end of
the week.

BTW, on the webpage with the snapshots the 'colspan="3"' should be removed
in the table cells for the file names.


Regards,
Achim.


--
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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: sqlite3: bug with monotone
  2013-06-03  8:03                 ` Achim Gratz
@ 2013-06-03  8:42                   ` Corinna Vinschen
  2013-06-03  9:54                     ` Achim Gratz
  0 siblings, 1 reply; 34+ messages in thread
From: Corinna Vinschen @ 2013-06-03  8:42 UTC (permalink / raw)
  To: cygwin

On Jun  3 08:03, Achim Gratz wrote:
> Corinna Vinschen <corinna-cygwin <at> cygwin.com> writes:
> > Please give the 32 bit snapshot a try ASAP.  I intend to release 1.7.19
> > very soon, probably tomorrow or Tuesday.
> 
> Have it installed now, but I can't give it much testing before the end of
> the week.
> 
> BTW, on the webpage with the snapshots the 'colspan="3"' should be removed
> in the table cells for the file names.

I'm not a web designer, so, why?


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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: sqlite3: bug with monotone
  2013-06-03  8:42                   ` Corinna Vinschen
@ 2013-06-03  9:54                     ` Achim Gratz
  2013-06-03 10:12                       ` Corinna Vinschen
  0 siblings, 1 reply; 34+ messages in thread
From: Achim Gratz @ 2013-06-03  9:54 UTC (permalink / raw)
  To: cygwin

Corinna Vinschen <corinna-cygwin <at> cygwin.com> writes:
> > BTW, on the webpage with the snapshots the 'colspan="3"' should be
> > removed in the table cells for the file names.
> 
> I'm not a web designer, so, why?

The cell with the snapshot date specifies 'colspan="4"' and the other rows
have four columns, so the intention is clearly that it should span across
the whole table.  So trying to span three non-existing columns in the
filename cells causes the date cell to span only two columns effectively and
leaves the last two columns empty (since no cell is defined for those).

If you want to test it first: In Firefox, Shift-F2 gives you a command line
where you can remove all colspan definitions via "pagemod remove attribute
colspan td".  Then use the Inspector (Ctrl-Shift-I) to re-add 'colspan="4"'
to one of the date cells.


Regards,
Achim.




--
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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: sqlite3: bug with monotone
  2013-06-03  9:54                     ` Achim Gratz
@ 2013-06-03 10:12                       ` Corinna Vinschen
  2013-06-03 10:22                         ` Achim Gratz
  0 siblings, 1 reply; 34+ messages in thread
From: Corinna Vinschen @ 2013-06-03 10:12 UTC (permalink / raw)
  To: cygwin

On Jun  3 09:53, Achim Gratz wrote:
> Corinna Vinschen <corinna-cygwin <at> cygwin.com> writes:
> > > BTW, on the webpage with the snapshots the 'colspan="3"' should be
> > > removed in the table cells for the file names.
> > 
> > I'm not a web designer, so, why?
> 
> The cell with the snapshot date specifies 'colspan="4"' and the other rows
> have four columns, so the intention is clearly that it should span across
> the whole table.  So trying to span three non-existing columns in the
> filename cells causes the date cell to span only two columns effectively and
> leaves the last two columns empty (since no cell is defined for those).
> 
> If you want to test it first: In Firefox, Shift-F2 gives you a command line
> where you can remove all colspan definitions via "pagemod remove attribute
> colspan td".  Then use the Inspector (Ctrl-Shift-I) to re-add 'colspan="4"'
> to one of the date cells.

The colspan=3 is for the cell containing the filesize.  There's no
colspan for the cell with the filename, afaics.  Changing the colspan
for the filesize cell to 4 looks weird nad incorrect.


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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: sqlite3: bug with monotone
  2013-06-03 10:12                       ` Corinna Vinschen
@ 2013-06-03 10:22                         ` Achim Gratz
  2013-06-03 10:32                           ` Corinna Vinschen
  0 siblings, 1 reply; 34+ messages in thread
From: Achim Gratz @ 2013-06-03 10:22 UTC (permalink / raw)
  To: cygwin

Corinna Vinschen <corinna-cygwin <at> cygwin.com> writes:
> The colspan=3 is for the cell containing the filesize.

Yes, and it should be eliminated.  I guess formerly the header line with the
snapshot date had multiple cells, but now it has just one.

> There's no colspan for the cell with the filename, afaics.

No, sorry - I've just remembered incorrectly.

> Changing the colspan
> for the filesize cell to 4 looks weird nad incorrect.

Yes, it should have no colspan at all.


Regards,
Achim.


--
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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: sqlite3: bug with monotone
  2013-06-03 10:22                         ` Achim Gratz
@ 2013-06-03 10:32                           ` Corinna Vinschen
  2013-06-03 10:36                             ` Corinna Vinschen
  2013-06-03 10:38                             ` Achim Gratz
  0 siblings, 2 replies; 34+ messages in thread
From: Corinna Vinschen @ 2013-06-03 10:32 UTC (permalink / raw)
  To: cygwin

On Jun  3 10:21, Achim Gratz wrote:
> Corinna Vinschen <corinna-cygwin <at> cygwin.com> writes:
> > The colspan=3 is for the cell containing the filesize.
> 
> Yes, and it should be eliminated.  I guess formerly the header line with the
> snapshot date had multiple cells, but now it has just one.
> 
> > There's no colspan for the cell with the filename, afaics.
> 
> No, sorry - I've just remembered incorrectly.
> 
> > Changing the colspan
> > for the filesize cell to 4 looks weird nad incorrect.
> 
> Yes, it should have no colspan at all.

The result of removing colspan is a more narrow table, which doesn't
match the width of the headline.  Afaics, a colspan=6 for the daily
headline containing date/changelog/diff would look better, wouldn't it?


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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: sqlite3: bug with monotone
  2013-06-03 10:32                           ` Corinna Vinschen
@ 2013-06-03 10:36                             ` Corinna Vinschen
  2013-06-03 10:49                               ` Achim Gratz
  2013-06-03 10:38                             ` Achim Gratz
  1 sibling, 1 reply; 34+ messages in thread
From: Corinna Vinschen @ 2013-06-03 10:36 UTC (permalink / raw)
  To: cygwin

On Jun  3 12:32, Corinna Vinschen wrote:
> On Jun  3 10:21, Achim Gratz wrote:
> > Corinna Vinschen <corinna-cygwin <at> cygwin.com> writes:
> > > The colspan=3 is for the cell containing the filesize.
> > 
> > Yes, and it should be eliminated.  I guess formerly the header line with the
> > snapshot date had multiple cells, but now it has just one.
> > 
> > > There's no colspan for the cell with the filename, afaics.
> > 
> > No, sorry - I've just remembered incorrectly.
> > 
> > > Changing the colspan
> > > for the filesize cell to 4 looks weird nad incorrect.
> > 
> > Yes, it should have no colspan at all.
> 
> The result of removing colspan is a more narrow table, which doesn't
> match the width of the headline.  Afaics, a colspan=6 for the daily
> headline containing date/changelog/diff would look better, wouldn't it?

I changed the snapshots page for testing, btw.  Have a look, please.


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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: sqlite3: bug with monotone
  2013-06-03 10:32                           ` Corinna Vinschen
  2013-06-03 10:36                             ` Corinna Vinschen
@ 2013-06-03 10:38                             ` Achim Gratz
  2013-06-03 11:00                               ` Corinna Vinschen
  1 sibling, 1 reply; 34+ messages in thread
From: Achim Gratz @ 2013-06-03 10:38 UTC (permalink / raw)
  To: cygwin

Corinna Vinschen <corinna-cygwin <at> cygwin.com> writes:
> The result of removing colspan is a more narrow table, which doesn't
> match the width of the headline.  Afaics, a colspan=6 for the daily
> headline containing date/changelog/diff would look better, wouldn't it?

It seems we aren't looking at the same table... I'm talking about what I see
when loading

http://cygwin.com/snapshots/

Is there some postprocessing done before it goes live on the server?  That
would perhaps explain the apparent mismatch.


Regards,
Achim.



--
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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: sqlite3: bug with monotone
  2013-06-03 10:36                             ` Corinna Vinschen
@ 2013-06-03 10:49                               ` Achim Gratz
  0 siblings, 0 replies; 34+ messages in thread
From: Achim Gratz @ 2013-06-03 10:49 UTC (permalink / raw)
  To: cygwin

Corinna Vinschen <corinna-cygwin <at> cygwin.com> writes:
> > The result of removing colspan is a more narrow table, which doesn't
> > match the width of the headline.  Afaics, a colspan=6 for the daily
> > headline containing date/changelog/diff would look better, wouldn't it?

OK, got it - disregard my other mail.  The table is the same width in my
browser, but the filesize cell gets extra padding to the left when it spans
columns.

> I changed the snapshots page for testing, btw.  Have a look, please.

Looks good now, thanks for taking care of it.


Regards,
Achim.


--
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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: sqlite3: bug with monotone
  2013-06-03 10:38                             ` Achim Gratz
@ 2013-06-03 11:00                               ` Corinna Vinschen
  0 siblings, 0 replies; 34+ messages in thread
From: Corinna Vinschen @ 2013-06-03 11:00 UTC (permalink / raw)
  To: cygwin

On Jun  3 10:38, Achim Gratz wrote:
> Corinna Vinschen <corinna-cygwin <at> cygwin.com> writes:
> > The result of removing colspan is a more narrow table, which doesn't
> > match the width of the headline.  Afaics, a colspan=6 for the daily
> > headline containing date/changelog/diff would look better, wouldn't it?
> 
> It seems we aren't looking at the same table... I'm talking about what I see
> when loading
> 
> http://cygwin.com/snapshots/

This is what I see as well.

> Is there some postprocessing done before it goes live on the server?

No.  The page gets created in the background and then index.html gets
replaced with the newly generated one, that's all.

> That
> would perhaps explain the apparent mismatch.

What mismatch?  Here's what I see after applying the colspan=6 change.

  http://cygwin.de/snapshots-screenshot.png


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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: sqlite3: bug with monotone
  2013-06-02 10:31               ` Corinna Vinschen
  2013-06-02 11:24                 ` Corinna Vinschen
  2013-06-03  8:03                 ` Achim Gratz
@ 2013-06-03 17:36                 ` Warren Young
  2013-06-04  8:48                   ` Corinna Vinschen
  2 siblings, 1 reply; 34+ messages in thread
From: Warren Young @ 2013-06-03 17:36 UTC (permalink / raw)
  To: cygwin

On 6/2/2013 04:31, Corinna Vinschen wrote:
>
> I just applied a patch to implement mandatory locking.

Thank you!

> It also supports
> F_GETLK, with limited usability due to Windows restrictions, as I
> explained in other mail.

It is what it is.

A haiku:

     Yesterday it worked.
     Today it is not working.
     Windows is like that.

> I dropped the F_MDLCK idea. Instead I implemented a specific fcntl code
> to switch to mandatory locking on a file descriptor:
>
>    fcntl (fd, F_LCK_MANDATORY, 1);

Could you add an O_MAND open(2) option as well to turn on the same 
feature?  This will avoid a race condition.

I apologize for not thinking of this earlier.  It just occurred to me as 
I was working on the SQLite patch to make use of this new feature.

> I didn't add a way for the user to switch on mandatory locking for now,
> and I don't intend to do that for 1.7.19.  Hope that helps, nevertheless.

I *think* it's sufficient.  I may think differently later. :)

My current plan is that Cygwin SQLite will build in "Unix mode" now, but 
with the build flag Yaakov requested above, and mandatory locking 
enabled unless an environment variable is set.  This will let those in 
Achim's camp use the official Cygwin SQLite but ask it to run in pure 
POSIX mode.

A test build will appear later today.

> Please give the 32 bit snapshot a try ASAP.

As I see it, the test will proceed in several stages:

0. I patch SQLite and run it against both .18 and your snapshot, and 
strace it to verify that fcntl(fd, 0x99, 1) is called and returns 0 with 
the snapshot and returns -1 with errno == EINVAL for .18.  Both count as 
"success" so we don't have to require .19.

1. I upload it, and Achim and Yaakov try it and see if this new build 
satisfies their use cases.

2. I promote test to curr -- waiting for .19 to hit the mirrors if Achim 
and Yaakov get back to me before that -- and wait for screams from the 
users.

3. A month or two hence, I'll tell upstream about the patch, and hope 
they adopt it.

--
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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: sqlite3: bug with monotone
  2013-06-03 17:36                 ` Warren Young
@ 2013-06-04  8:48                   ` Corinna Vinschen
  2013-06-05 16:15                     ` Mandatory file locking semantics (was: sqlite3: bug with monotone) Warren Young
  0 siblings, 1 reply; 34+ messages in thread
From: Corinna Vinschen @ 2013-06-04  8:48 UTC (permalink / raw)
  To: cygwin

On Jun  3 11:36, Warren Young wrote:
> On 6/2/2013 04:31, Corinna Vinschen wrote:
> >I dropped the F_MDLCK idea. Instead I implemented a specific fcntl code
> >to switch to mandatory locking on a file descriptor:
> >
> >   fcntl (fd, F_LCK_MANDATORY, 1);
> 
> Could you add an O_MAND open(2) option as well to turn on the same
> feature?  This will avoid a race condition.

If you call F_LCK_MANDATORY right after open in the opening routine,
there won't be (much of) a race.  I know what you mean (it's the
O_CLOEXEC dilemma) but is it really that important in terms of locking?

> As I see it, the test will proceed in several stages:
> 
> 0. I patch SQLite and run it against both .18 and your snapshot, and
> strace it to verify that fcntl(fd, 0x99, 1) is called and returns 0
> with the snapshot and returns -1 with errno == EINVAL for .18.  Both
> count as "success" so we don't have to require .19.
> 
> 1. I upload it, and Achim and Yaakov try it and see if this new
> build satisfies their use cases.

Well, we will have more than one round of testing, apparently...(*)


Corinna


(*) http://cygwin.com/ml/cygwin/2013-06/msg00042.html

-- 
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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: Mandatory file locking semantics (was: sqlite3: bug with monotone)
  2013-06-04  8:48                   ` Corinna Vinschen
@ 2013-06-05 16:15                     ` Warren Young
  2013-06-05 17:02                       ` Corinna Vinschen
  0 siblings, 1 reply; 34+ messages in thread
From: Warren Young @ 2013-06-05 16:15 UTC (permalink / raw)
  To: cygwin

On 6/4/2013 02:48, Corinna Vinschen wrote:
> On Jun  3 11:36, Warren Young wrote:
>> Could you add an O_MAND open(2) option as well to turn on the same
>> feature?  This will avoid a race condition.
>
> If you call F_LCK_MANDATORY right after open in the opening routine,
> there won't be (much of) a race.  I know what you mean (it's the
> O_CLOEXEC dilemma)

As long as everyone plays by the rules, there actually isn't a race.

For this style of locking to work at all, every program involved has to 
enable the mandatory locking flag before doing any I/O on the file.  In 
that sense, locking is already atomic.

Adding O_MAND changes the call pattern from:

    fd = open(..., O_RDWR);
    fcntl(fd, F_LCK_MANDATORY, 1);
    fcntl(fd, F_SETLK, &lock);

to:

    fd = open(..., O_RDWR | O_MAND);
    fcntl(fd, F_SETLK, &lock);

Since there are two required calls due to the POSIX design, there's 
still a hole here, where an uncooperative program could call open(2) for 
writing on the same file without acquiring the lock.  To fix *that* race 
condition, you'd need an extension to open(2) that lets it acquire the 
lock on open.  That just isn't in the cards.

Or am I missing something?  Does mandatory locking on Linux or SysV 
somehow avoid the "uncooperative program" race?




That having been said, I still think O_MAND works better.

Cygwin's mandatory locking is effectively a flag on the file that, once 
set, stays with the file descriptor as long as it is open.

You can later unset the flag, but under what condition would that ever 
be the right thing?  That effectively tries to mix locking semantics on 
the same file descriptor!

The F_LCK_MANDATORY design also allows you to do arbitrary I/O between 
open() and setting the flag, another thing which can never be correct. 
It violates the rule laid out above that allows two Cygwin programs to 
cooperatively enable mandatory file locking "atomically".

If a Cygwin program wants mandatory locking, it should be enabled 
continuously from open() to close().



If those weren't enough reasons, it now feels weird that you use fcntl() 
to set the locking flag even when you're going to use flock() or lockf() 
to do the actual locking.

--
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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: Mandatory file locking semantics (was: sqlite3: bug with monotone)
  2013-06-05 16:15                     ` Mandatory file locking semantics (was: sqlite3: bug with monotone) Warren Young
@ 2013-06-05 17:02                       ` Corinna Vinschen
  2013-06-05 17:19                         ` Mandatory file locking semantics Warren Young
  0 siblings, 1 reply; 34+ messages in thread
From: Corinna Vinschen @ 2013-06-05 17:02 UTC (permalink / raw)
  To: cygwin

On Jun  5 10:15, Warren Young wrote:
> On 6/4/2013 02:48, Corinna Vinschen wrote:
> >On Jun  3 11:36, Warren Young wrote:
> >>Could you add an O_MAND open(2) option as well to turn on the same
> >>feature?  This will avoid a race condition.
> >
> >If you call F_LCK_MANDATORY right after open in the opening routine,
> >there won't be (much of) a race.  I know what you mean (it's the
> >O_CLOEXEC dilemma)
> 
> As long as everyone plays by the rules, there actually isn't a race.
> 
> For this style of locking to work at all, every program involved has
> to enable the mandatory locking flag before doing any I/O on the
> file.  In that sense, locking is already atomic.
> 
> Adding O_MAND changes the call pattern from:
> 
>    fd = open(..., O_RDWR);
>    fcntl(fd, F_LCK_MANDATORY, 1);
>    fcntl(fd, F_SETLK, &lock);
> 
> to:
> 
>    fd = open(..., O_RDWR | O_MAND);
>    fcntl(fd, F_SETLK, &lock);
> 
> Since there are two required calls due to the POSIX design, there's
> still a hole here, where an uncooperative program could call open(2)
> for writing on the same file without acquiring the lock.  To fix
> *that* race condition, you'd need an extension to open(2) that lets
> it acquire the lock on open.  That just isn't in the cards.

Advisory locking is *based* on cooperative behaviour.  That means,
the assumption is that all applications accessing certain files
behave like they should and not write to a file before asking for the
lock.  In your case, there's the sqlite lib.  If every application
accessing an sqlite db uses the sqlite lib, there's no reason to
assume that there's an uncooperative application.

> Or am I missing something?  Does mandatory locking on Linux or SysV
> somehow avoid the "uncooperative program" race?

Not at all.  Did you read Section 0, "Why you should avoid mandatory
locking" in Linux Documentation/filesystems/mandatory-locking.txt file?
It's all about races.

> That having been said, I still think O_MAND works better.
> [...]

Again, to be clear: Mandatory locking is not for the every day use of a
Cygwin application.  I provided it for the sake of a *very* limited
scenario, where Cygwin applications have to cooperate with native Windows
applications using record locking.  The burden to use it correctly is
on the application developer.


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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: Mandatory file locking semantics
  2013-06-05 17:02                       ` Corinna Vinschen
@ 2013-06-05 17:19                         ` Warren Young
  2013-06-05 18:15                           ` Corinna Vinschen
  0 siblings, 1 reply; 34+ messages in thread
From: Warren Young @ 2013-06-05 17:19 UTC (permalink / raw)
  To: cygwin

On 6/5/2013 11:02, Corinna Vinschen wrote:
> The burden to use it correctly is
> on the application developer.

That's always true, for all APIs.

If a design change can make it more likely that application developers 
will use it correctly, shouldn't the design be changed?

It's not like anyone is actually depending on this yet.  Granted, it's 
now present in a release version of Cygwin, but you declared it 
preliminary.  I don't see that anyone can complain if the design changes 
before it's declared stable.

What does the fcntl(F_LCK_MANDATORY) design have to recommend it, other 
than "it already exists"?

Understand, I'm not rejecting your gift to the community.  If this is 
all I can have, I'm glad to have it.  This may be your job, but you 
aren't my employee, so I don't feel any expectation that you should do 
what I want.  I'm just asking nicely.

--
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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: Mandatory file locking semantics
  2013-06-05 17:19                         ` Mandatory file locking semantics Warren Young
@ 2013-06-05 18:15                           ` Corinna Vinschen
  2013-06-05 19:33                             ` Warren Young
  0 siblings, 1 reply; 34+ messages in thread
From: Corinna Vinschen @ 2013-06-05 18:15 UTC (permalink / raw)
  To: cygwin

On Jun  5 11:18, Warren Young wrote:
> On 6/5/2013 11:02, Corinna Vinschen wrote:
> >The burden to use it correctly is
> >on the application developer.
> 
> That's always true, for all APIs.
> 
> If a design change can make it more likely that application
> developers will use it correctly, shouldn't the design be changed?

The fcntl commands are int, and they never have to be or'ed together.
It's no problem to add yet another fcntl command.  They won't collide
with POSIX or Linux commands, there are a lot of commands to express
in an int.

Open flags are or'ed together and mode_t is also an int.  This means,
there's only room for 32 (actually 33) open flags, and POSIX and Linux
are adding open flags as they go along.  Linux already uses 19 of the 32
bits.  I'm not overly enthusiastic to add a non-POSIX, non-Linux flag
for the sake of something which will be used only in very rare
circumstances, if at all.

> It's not like anyone is actually depending on this yet.  Granted,
> it's now present in a release version of Cygwin, but you declared it
> preliminary.  I don't see that anyone can complain if the design
> changes before it's declared stable.

Actually, adding this stuff to Cygwin took almost four days of my
life as it is today.  And while doing something entirely different, I
was thinking about ways to hammer a more POSIXy (or BSDy) behaviour into
these dreaded Windows locks.  Needless to say that I always came up
empty handed, given the dethpicable(*) shortcomings of Windows locks.

An application using POSIX or BSD lock calls, expecting POSIX or BSD
behaviour, will be in for a surprise, and there's just no way around
that, unless the application *knows* how Windows locks work, and choses
an accommodating code path.

So far it's not even clear if you really can use it in sqlite.  Using
fcntl locks apparently does not work, does the flock code path?

Given all that, I'd appreciate if you'd first test if the whole effort
was even worth it.  If it works, we can talk about other options.  And
if it turns out that the whole exercise was useless, I'm more inclined
to rip out the entire mandatory locking code again.

Does that make sense, at least a bit?


Corinna


(*) Think Daffy Duck.


-- 
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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: Mandatory file locking semantics
  2013-06-05 18:15                           ` Corinna Vinschen
@ 2013-06-05 19:33                             ` Warren Young
  0 siblings, 0 replies; 34+ messages in thread
From: Warren Young @ 2013-06-05 19:33 UTC (permalink / raw)
  To: cygwin

On 6/5/2013 12:15, Corinna Vinschen wrote:
>
> Actually, adding this stuff to Cygwin took almost four days of my
> life as it is today.

Understood.  Likewise, because I am not full-time on Cygwin SQLite, it's 
taken similar amounts of time from me, which is why I'm ignoring your 
xidepend complaints right now. :)

> So far it's not even clear if you really can use it in sqlite.

That's definitely a possibility.  I'll be posting my thoughts on that in 
another part of the thread.

I hope that even if this doesn't end up helping SQLite, though, that the 
code is still useful enough to stay in the DLL.  I can't be the only one 
who has ever had such interop problems.

> Given all that, I'd appreciate if you'd first test if the whole effort
> was even worth it.

Okay.  -2 packages are built now.  Announcement elsewhere.

> Does that make sense, at least a bit?

Thank you for taking the time to explain.

--
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

^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2013-06-05 19:33 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-30  2:58 sqlite3: bug with monotone Yaakov (Cygwin/X)
2013-05-30 11:17 ` Achim Gratz
2013-05-30 17:37   ` Warren Young
2013-05-31  0:56     ` Yaakov (Cygwin/X)
2013-05-31  1:50       ` Warren Young
2013-05-31 15:00         ` Corinna Vinschen
2013-05-31 20:19           ` Warren Young
2013-05-31 21:11             ` Warren Young
2013-06-01 10:58               ` Corinna Vinschen
2013-06-01 10:57             ` Corinna Vinschen
2013-06-01 11:35               ` Corinna Vinschen
2013-06-01 12:57                 ` Corinna Vinschen
2013-06-02 10:31               ` Corinna Vinschen
2013-06-02 11:24                 ` Corinna Vinschen
2013-06-03  8:03                 ` Achim Gratz
2013-06-03  8:42                   ` Corinna Vinschen
2013-06-03  9:54                     ` Achim Gratz
2013-06-03 10:12                       ` Corinna Vinschen
2013-06-03 10:22                         ` Achim Gratz
2013-06-03 10:32                           ` Corinna Vinschen
2013-06-03 10:36                             ` Corinna Vinschen
2013-06-03 10:49                               ` Achim Gratz
2013-06-03 10:38                             ` Achim Gratz
2013-06-03 11:00                               ` Corinna Vinschen
2013-06-03 17:36                 ` Warren Young
2013-06-04  8:48                   ` Corinna Vinschen
2013-06-05 16:15                     ` Mandatory file locking semantics (was: sqlite3: bug with monotone) Warren Young
2013-06-05 17:02                       ` Corinna Vinschen
2013-06-05 17:19                         ` Mandatory file locking semantics Warren Young
2013-06-05 18:15                           ` Corinna Vinschen
2013-06-05 19:33                             ` Warren Young
2013-05-31  9:22       ` sqlite3: bug with monotone Achim Gratz
2013-05-31  6:29   ` Yaakov (Cygwin/X)
2013-05-31 19:06     ` Warren Young

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).