code review 8203043: syscall: Use FILE_SHARE_DELETE flag when opening files ...

syscall: Use FILE_SHARE_DELETE flag when opening files on Windows

On Windows, a file opened without the FILE_SHARE_DELETE flag
can not be renamed or deleted until it's closed. This contrasts
with the semantics on POSIX systems (GNU/Linux, Mac OS X, etc).

Since there's no way to specify such flag in the Open() call,
this is necessary to ensure the same Go code behaves the same
on all platforms. Opening a file on Windows and then attempting
to rename it or delete it, from any Go thread or from an external
process, resulted in a Windows ERROR_SHARING_VIOLATION error.

This same type of issue happened for example in Erlang OTP's file
driver:

https://github.com/erlang/otp/commit/0e02f488971b32ff9ab88a3f0cb144fe5db161b2

[fdmanana, 2013-03-30 21:25:03]

On Windows, a file opened without the FILE_SHARE_FLAG can not
be renamed or deleted until it's closed. This contrasts with
the semantics on POSIX systems (GNU/Linux, Mac OS X, etc).

Since there's no way to specify such flag in the Open() call,
this is necessary to ensure the same Go code behaves the same
on all platforms. Opening a file on Windows and then attempting
to rename it or delete it, from any Go thread or from an external
process, resulted in a Windows ERROR_SHARING_VIOLATION error.

This same type of issue happened for example in Erlang OTP's file
driver:

https://github.com/erlang/otp/commit/0e02f488971b32ff9ab88a3f0cb144fe5db161b2

[fdmanana, 2013-03-30 21:51:19]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go

[bradfitz, 2013-03-31 03:23:25]

Interesting. Is this widely supported in Windows across versions? Add a
test?

Leaving for Windows people.
On Mar 30, 2013 8:19 PM, <fdmanana@gmail.com> wrote:

> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go
>
>
>
https://codereview.appspot.**com/8203043/<https://codereview.appspot.com/8203...
>
> --
>
> ---You received this message because you are subscribed to the Google
> Groups "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to
golang-dev+unsubscribe@**googlegroups.com<golang-dev%2Bunsubscribe@googlegrou...
> .
> For more options, visit
https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/o...
> .
>
>
>

[aram, 2013-03-31 10:00:23]

I am unconvinced this is the correct thing to do. To quote Rob here:
https://groups.google.com/d/msg/golang-dev/TlLZoZjJuCk/oQKXUkzGhoEJ

> package os's purpose is not to hide every detail, just to provide
> a consistent interface. The operating system may do different things
> with that interface, and often will.

[fdmanana, 2013-03-31 11:39:25]

On Sun, Mar 31, 2013 at 11:00 AM, Aram Hăvărneanu <aram@mgk.ro> wrote:

> I am unconvinced this is the correct thing to do. To quote Rob here:
> https://groups.google.com/d/msg/golang-dev/TlLZoZjJuCk/oQKXUkzGhoEJ
>
> > package os's purpose is not to hide every detail, just to provide
> > a consistent interface. The operating system may do different things
> > with that interface, and often will.
>

I generally agree with that. However here, there's no way (to my knowledge)
to be able to open a file with this flag, that is, the Open/OpenFile
functions do not allow the caller to specify any Windows specific
share-ability flags.

Renaming or deleting an open file, is not an uncommon thing to do. For
example, Apache CouchDB and Couchbase do this during database/index
compaction (that's what led me to that Erlang VM patch as well).

Uploading a new patch set with a unit test.
Answering to the previous question from Brad, this flag exists in all
Windows version since (and including) Windows 2000.



-- 
Filipe David Manana,

"Reasonable men adapt themselves to the world.
 Unreasonable men adapt the world to themselves.
 That's why all progress depends on unreasonable men."

[fdmanana, 2013-03-31 11:42:29]

Hello golang-dev@googlegroups.com, bradfitz@golang.org, aram@mgk.ro (cc:
golang-dev@googlegroups.com),

Please take another look.

[fdmanana, 2013-03-31 11:45:52]

Hello golang-dev@googlegroups.com, bradfitz@golang.org, aram@mgk.ro (cc:
golang-dev@googlegroups.com),

Please take another look.

[aram, 2013-03-31 12:18:03]

> Renaming or deleting an open file, is not an uncommon thing to do.
> For example, Apache CouchDB and Couchbase do this during database/index
> compaction (that's what led me to that Erlang VM patch as well).

Just because something is useful (which nobody disputes) doesn't
mean it should be the default. It might be hard to obtain this
behavior now (requires using syscall), but with your patch it will
be hard to get the old behavior back. I'm also not sure this semantic
change is compatible with the Go 1 contract.

What about this idea, add syscall.FILE_SHARE_DELETE and change
syscall.Open so that you can pass it. This way you can use
os.OpenFile("foo", syscall.FILE_SHARE_DELETE, 0) if you need it.

[fdmanana, 2013-03-31 12:30:51]

On Sun, Mar 31, 2013 at 1:18 PM, Aram Hăvărneanu <aram@mgk.ro> wrote:
>
> Just because something is useful (which nobody disputes) doesn't
> mean it should be the default. It might be hard to obtain this
> behavior now (requires using syscall), but with your patch it will
> be hard to get the old behavior back. I'm also not sure this semantic
> change is compatible with the Go 1 contract.
>
> What about this idea, add syscall.FILE_SHARE_DELETE and change
> syscall.Open so that you can pass it. This way you can use
> os.OpenFile("foo", syscall.FILE_SHARE_DELETE, 0) if you need it.
>

Just doesn't make much sense to me.

The constant is already defined in the package syscall,
and unfortunately has same value (0x00000004) as other constants there,
such as FILE_APPEND_DATA for example.

Further, 2 more reasons:

1) Need to add logic such as this to programs:

if running on Windows then
    open file with this flag
else
    open file
end


2) What about the case where a program attempts to delete or rename a file
and that file was open by some other Go package that didn't pass that flag
to the OpenFile() call? Would need to change every such package out there
to add an optional file flags parameter/config.


Imho, the fact that no one reported this issue before is likely because no
one attempted to do this on Windows yet, or didn't bother investigating or
reporting the issue.






-- 
Filipe David Manana,

"Reasonable men adapt themselves to the world.
 Unreasonable men adapt the world to themselves.
 That's why all progress depends on unreasonable men."

[minux1, 2013-03-31 14:52:34]

On Sun, Mar 31, 2013 at 8:30 PM, Filipe David Manana <fdmanana@gmail.com>
wrote:
> Imho, the fact that no one reported this issue before is likely because no
> one attempted to do this on Windows yet, or didn't bother investigating or
> reporting the issue.
another reason: people using windows accept that not being able to rename or
remove an open file is the norm and has already get used to that fact.

We can't silently change the Go 1.0 program behavior here, perhaps people
actually depend on it.

[fdmanana, 2013-03-31 15:27:08]

On Sun, Mar 31, 2013 at 3:52 PM, minux <minux.ma@gmail.com> wrote:

> On Sun, Mar 31, 2013 at 8:30 PM, Filipe David Manana <fdmanana@gmail.com>
> wrote:
> > Imho, the fact that no one reported this issue before is likely because
> no
> > one attempted to do this on Windows yet, or didn't bother investigating
> or
> > reporting the issue.
> another reason: people using windows accept that not being able to rename
> or
> remove an open file is the norm and has already get used to that fact.
>
> We can't silently change the Go 1.0 program behavior here, perhaps people
> actually depend on it.
>

Then why does Open() (and OpenFile())  always, and unconditionally, pass
FILE_SHARE_READ and FILE_SHARE_WRITE to Window's CreateFile() function [1] ?

It's very inconsistent. People wanting to have full access to Windows
specific behaviour should then have the possibility to specify exactly
which share flags are passed to CreateFile(). Right now they just can't.

If the os package intended to allow for fine grained OS specific file
semantics, it should have had in the first place the possibility to allow
to specify these flags, rather than hardcode them. I believe the goal here,
by hardcoding the read and write share flags, was to provide POSIX alike
semantics to Windows programs.

If someone knows why those flags are hardcoded and if the share delete flag
was omitted intentionally  please let know. I'm very curious about the
rationale for this.

[1] -
https://code.google.com/p/go/source/browse/src/pkg/syscall/syscall_windows.go...


-- 
Filipe David Manana,

"Reasonable men adapt themselves to the world.
 Unreasonable men adapt the world to themselves.
 That's why all progress depends on unreasonable men."

[minux1, 2013-03-31 16:08:37]

cc: +alex.brainman, maybe he could tell you why FILE_SHARE_DELETE is not
used in syscall.Open.

On Sun, Mar 31, 2013 at 11:27 PM, Filipe David Manana
<fdmanana@gmail.com> wrote:
> On Sun, Mar 31, 2013 at 3:52 PM, minux <minux.ma@gmail.com> wrote:
>> On Sun, Mar 31, 2013 at 8:30 PM, Filipe David Manana <fdmanana@gmail.com>
>> wrote:
>> > Imho, the fact that no one reported this issue before is likely because
>> > no
>> > one attempted to do this on Windows yet, or didn't bother investigating
>> > or
>> > reporting the issue.
>> another reason: people using windows accept that not being able to rename
>> or
>> remove an open file is the norm and has already get used to that fact.
>>
>> We can't silently change the Go 1.0 program behavior here, perhaps people
>> actually depend on it.
> Then why does Open() (and OpenFile())  always, and unconditionally, pass
> FILE_SHARE_READ and FILE_SHARE_WRITE to Window's CreateFile() function [1] ?
> It's very inconsistent. People wanting to have full access to Windows
Even if they're inconsistent, unfortunately IMHO we can't change that now.
> specific behaviour should then have the possibility to specify exactly which
> share flags are passed to CreateFile(). Right now they just can't.
they can call syscall.CreateFile themselves and then os.NewFile.
> If the os package intended to allow for fine grained OS specific file
> semantics, it should have had in the first place the possibility to allow to
The os package doesn't intend to do that.
> specify these flags, rather than hardcode them. I believe the goal here, by
> hardcoding the read and write share flags, was to provide POSIX alike
> semantics to Windows programs.
because even with FILE_SHARE_DELETE, the semantics is still not like
that on Unix.
although you can delete the open file, but you can't create a new file with the
same name in that directory.

In fact, FILE_SHARE_DELETE is not used in Windows CRT, so almost
all windows programs behave the same as what Go behaves now, why
change that? If you have that specific requirement, you can always get
what you want by using syscall directly.

[aram, 2013-03-31 16:17:23]

> why does Open() (and OpenFile())  always, and unconditionally, pass
> FILE_SHARE_READ and FILE_SHARE_WRITE to Window's CreateFile()
> function

Because otherwise OpenFile would be useless. Note to people that
don't know Windows, FILE_SHARE_READ and FILE_SHARE_WRITE are not
like O_RDONLY or O_WRONLY, they are something else:
http://msdn.microsoft.com/en-us/library/windows/desktop/aa363858(v=vs.85).aspx

> If the os package intended to allow for fine grained OS specific file 
semantics

I don't think that's the purpose of the os package. If you need
fine-grained OS specific semantics there's syscall.

Please note that FILE_SHARE_DELETE still doesn't provide POSIX
semantics. More specifically, this doesn't work even with
FILE_SHARE_DELETE:

$ touch a
$ tail -f a &
[1] 86647
$ rm a
$ touch a
$ 

FILE_SHARE_DELETE also works pretty bad with SMB shares. The Windows
standard C runtime doesn't use it either.

[aram, 2013-03-31 16:18:31]

Heh, Minux beat me to it, but I'm amazed by the similarities of our answers.

[fdmanana, 2013-03-31 16:24:43]

On Sun, Mar 31, 2013 at 5:08 PM, minux <minux.ma@gmail.com> wrote:

> cc: +alex.brainman, maybe he could tell you why FILE_SHARE_DELETE is not
> used in syscall.Open.
>
> On Sun, Mar 31, 2013 at 11:27 PM, Filipe David Manana
> <fdmanana@gmail.com> wrote:
> > On Sun, Mar 31, 2013 at 3:52 PM, minux <minux.ma@gmail.com> wrote:
> >> On Sun, Mar 31, 2013 at 8:30 PM, Filipe David Manana <
> fdmanana@gmail.com>
> >> wrote:
> >> > Imho, the fact that no one reported this issue before is likely
> because
> >> > no
> >> > one attempted to do this on Windows yet, or didn't bother
> investigating
> >> > or
> >> > reporting the issue.
> >> another reason: people using windows accept that not being able to
> rename
> >> or
> >> remove an open file is the norm and has already get used to that fact.
> >>
> >> We can't silently change the Go 1.0 program behavior here, perhaps
> people
> >> actually depend on it.
> > Then why does Open() (and OpenFile())  always, and unconditionally, pass
> > FILE_SHARE_READ and FILE_SHARE_WRITE to Window's CreateFile() function
> [1] ?
> > It's very inconsistent. People wanting to have full access to Windows
> Even if they're inconsistent, unfortunately IMHO we can't change that now.
> > specific behaviour should then have the possibility to specify exactly
> which
> > share flags are passed to CreateFile(). Right now they just can't.
> they can call syscall.CreateFile themselves and then os.NewFile.
> > If the os package intended to allow for fine grained OS specific file
> > semantics, it should have had in the first place the possibility to
> allow to
> The os package doesn't intend to do that.
> > specify these flags, rather than hardcode them. I believe the goal here,
> by
> > hardcoding the read and write share flags, was to provide POSIX alike
> > semantics to Windows programs.
> because even with FILE_SHARE_DELETE, the semantics is still not like
> that on Unix.
> although you can delete the open file, but you can't create a new file
> with the
> same name in that directory.
>

Correct. That's why it's common to rename first the file (to some uuid for
e.g.) and then delete it - works if one opened the file with the share
delete flag (we do this rename + delete in couchdb for that reason).


>
> In fact, FILE_SHARE_DELETE is not used in Windows CRT, so almost
> all windows programs behave the same as what Go behaves now, why
> change that? If you have that specific requirement, you can always get
> what you want by using syscall directly.
>

I understand your point, but to me it still seems confusing why not using
all share flags by default. Imho it makes it much closer to POSIX semantics
(basically to every other platform supported other than Windows).



-- 
Filipe David Manana,

"Reasonable men adapt themselves to the world.
 Unreasonable men adapt the world to themselves.
 That's why all progress depends on unreasonable men."

[brainman, 2013-04-01 03:24:06]

On 2013/03/31 16:08:37, minux wrote:
> cc: +alex.brainman, maybe he could tell you why FILE_SHARE_DELETE is not
> used in syscall.Open.
> 

I am not Raymond Chen, so do not take my ramblings seriously. :-)

I do not remember who wrote that piece of code. But, I am from old school, and
that is how most programs I have seen behave.

I am not arguing that new behavior is not usable sometimes. But for "common
case" on Windows, it looks unusual to me. Cannot you open file with CreateFile
and pass file handle to os.NewFile to do what you want?

Alex

PS: I do not see what is wrong with FILE_APPEND_DATA = 4. ???

[gobot, 2013-12-20 16:21:40]

Replacing golang-dev with golang-codereviews.

[bradfitz, 2014-01-16 19:32:17]

R=close

Sounds like there was consensus against this. Closing to clean up the dashboard.
Please re-open if people disagree.