Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

os: different os.File.WriteAt behavior with os.O_APPEND flag at osx and linux #30716

Closed
arturalbov opened this Issue Mar 10, 2019 · 19 comments

Comments

Projects
None yet
8 participants
@arturalbov
Copy link

arturalbov commented Mar 10, 2019

What version of Go are you using (go version)?

OSX: go version go1.12 darwin/amd64
Linux: go version go1.12 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env OSX output
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/aalbov/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/aalbov/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/29/22v2xtm10dl6b7kzv7r80xc40000gn/T/go-build111133557=/tmp/go-build -gno-record-gcc-switches -fno-common"

go env Linux output
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/hlb/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/hlb/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build481358956=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Execution of this code on linux and osx gives different output
https://play.golang.org/p/aMQRT8-8os_V

What did you expect to see?

With os.O_APPEND flag I suppose both outputs should be

[0 1 2 3 4 5 6 7 8 9]

What did you see instead?

Instead, on OSX output is:

[0 1 8 9 4 5 6 7]

and on Linux output is:

[0 1 2 3 4 5 6 7 8 9]

Btw, on playground output is the same as on OSX.

@arturalbov arturalbov changed the title Different os.File.WriteAt behaviour with os.O_APPEND flag at osx and linux Different os.File.WriteAt behavior with os.O_APPEND flag at osx and linux Mar 10, 2019

@cuonglm

This comment has been minimized.

Copy link
Contributor

cuonglm commented Mar 10, 2019

It's pwrite() on Linux, which is broken. Read BUGS:

POSIX requires that opening a file with the O_APPEND flag should have
no effect on the location at which pwrite() writes data. However, on
Linux, if a file is opened with O_APPEND, pwrite() appends data to
the end of the file, regardless of the value of offset.

@arturalbov

This comment has been minimized.

Copy link
Author

arturalbov commented Mar 10, 2019

@Gnouc Oh, I see. Thank you for answer! I'm closing this issue.

@arturalbov arturalbov closed this Mar 10, 2019

@cuonglm

This comment has been minimized.

Copy link
Contributor

cuonglm commented Mar 10, 2019

@arturalbov maybe should re-open for adding documentation.

cc @ianlancetaylor how do you think?

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

ianlancetaylor commented Mar 10, 2019

Yes, I think we should fix this one way or another. Ideally we should change the os package to make the different systems work consistently, though I don't know how feasible that is.

@andybons andybons changed the title Different os.File.WriteAt behavior with os.O_APPEND flag at osx and linux os: different os.File.WriteAt behavior with os.O_APPEND flag at osx and linux Mar 11, 2019

@andybons andybons added this to the Go1.13 milestone Mar 11, 2019

@gopherbot gopherbot removed the NeedsFix label Mar 11, 2019

@cuonglm

This comment has been minimized.

Copy link
Contributor

cuonglm commented Mar 11, 2019

Windows does the same thing with Linux
image_2019_03_11T06_21_21_022Z

@gopherbot

This comment has been minimized.

Copy link

gopherbot commented Mar 11, 2019

Change https://golang.org/cl/166578 mentions this issue: os: document WriteAt behavior when file opened with O_APPEND flag.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

ianlancetaylor commented Mar 12, 2019

@Gnouc Please cut and paste text, rather than providing an image of a screen shot. Thanks.

I want to make clear that I think that documenting this behavior is a last resort. It would be better to modify the os package to make it do the same thing on all platforms.

@cuonglm

This comment has been minimized.

Copy link
Contributor

cuonglm commented Mar 12, 2019

@ianlancetaylor

Please cut and paste text, rather than providing an image of a screen shot. Thanks.

Ah yes, just because it's not the code, and I have to ask my friend to run compiled program on his Windows, then he sent the result back to me.

I want to make clear that I think that documenting this behavior is a last resort. It would be better to modify the os package to make it do the same thing on all platforms.

Sure. I think about replace pwrite with lseek + write, not sure it's feasible.

@davecheney

This comment has been minimized.

Copy link
Contributor

davecheney commented Mar 12, 2019

@cuonglm

This comment has been minimized.

Copy link
Contributor

cuonglm commented Mar 12, 2019

@davecheney Seem we can’t, because O_APPEND takes effect on each write.

We can check if file is open with O_APPEND, then do seek + write, but it’s not atomic anymore

@davecheney

This comment has been minimized.

Copy link
Contributor

davecheney commented Mar 12, 2019

Ahh, right. Thanks for confirming.

@cuonglm

This comment has been minimized.

Copy link
Contributor

cuonglm commented Mar 16, 2019

@ianlancetaylor Seems pwritev does not affect by O_APPEND. But pwritev is not in syscall package. We could do it manually:

func Pwritev(fd int, iovec *syscall.Iovec, iovcnt int64, offset int64) (n int, err error) {
	r0, _, e1 := syscall.Syscall6(syscall.SYS_PWRITEV, uintptr(fd), uintptr(unsafe.Pointer(iovec)), uintptr(iovcnt), uintptr(offset), 0, 0)
	n = int(r0)
	if e1 != 0 {
		err = e1
	}
	return
}

How do you think? Implement it outside of syscall seems to be painful, I don't think it worth to try.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

ianlancetaylor commented Mar 18, 2019

Using pwritev is a interesting idea, although it seems that it was only added in Linux kernel version 2.6.30.

What can we do on Windows?

Another option might be to simply reject WriteAt if the file is opened with O_APPEND. The two uses are not obviously compatible.

@cuonglm

This comment has been minimized.

Copy link
Contributor

cuonglm commented Mar 18, 2019

What can we do on Windows?

I don't see Windows offer anything equivalent to pwritev to bypass O_APPEND/FILE_APPEND_DATA.

Another option might be to simply reject WriteAt if the file is opened with O_APPEND. The two uses are not obviously compatible.

does it break Go 1 compatibility promise? If not, I vote for it.

@davecheney

This comment has been minimized.

Copy link
Contributor

davecheney commented Mar 18, 2019

does it break Go 1 compatibility promise? If not, I vote for it.

Given that we made 12 point releases before anyone spotted this incompatibility the uses in the wild should be minimal, especially as anyone combining WriteAt with O_APPEND would corrupt their data, there is some evidence that there is little use of this combination in the wild.

@cuonglm

This comment has been minimized.

Copy link
Contributor

cuonglm commented Mar 20, 2019

I don't see Windows offer anything equivalent to pwritev to bypass O_APPEND/FILE_APPEND_DATA.

@alexbrainman do you have any idea?

@alexbrainman

This comment has been minimized.

Copy link
Member

alexbrainman commented Mar 20, 2019

@alexbrainman do you have any idea?

I don't. Not from the top of my head.

Alex

@cuonglm

This comment has been minimized.

Copy link
Contributor

cuonglm commented Mar 20, 2019

@ianlancetaylor Do you know any Windows experts to take a look at this issue?

Otherwise, I think rejecting WriteAt in append mode is ok. I just update the CL to implement that behavior, please take a look.

@bradfitz

This comment has been minimized.

Copy link
Member

bradfitz commented Mar 25, 2019

Another option might be to simply reject WriteAt if the file is opened with O_APPEND. The two uses are not obviously compatible.

@ianlancetaylor, you still fine with that option? That's what the CL (https://go-review.googlesource.com/c/go/+/166578) does now. Seems fine to me.

@gopherbot gopherbot closed this in c7f7f59 Mar 27, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.