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

x/sys/unix: missing some AIX syscalls, notably Flock #64669

Open
mohammed90 opened this issue Dec 12, 2023 · 11 comments
Open

x/sys/unix: missing some AIX syscalls, notably Flock #64669

mohammed90 opened this issue Dec 12, 2023 · 11 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FeatureRequest NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. OS-AIX
Milestone

Comments

@mohammed90
Copy link

Go version

1.21.5

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

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/runner/.cache/go-build'
GOENV='/home/runner/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/runner/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/runner/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/opt/hostedtoolcache/go/1.21.5/x64'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/opt/hostedtoolcache/go/1.21.5/x64/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.21.5'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/home/runner/work/caddy/caddy/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build498393660=/tmp/go-build -gno-record-gcc-switches'

What did you do?

Trying to (cross-)compile Caddy for AIX with the following command

GOOS=AIX GOARCH=ppc64 go build -trimpath -o caddy

What did you expect to see?

Successful compilation and build

What did you see instead?

The compilation fails with errors about unix.Flock not existing:

$ GOOS=aix GOARCH=ppc64 go build
# github.com/dgraph-io/badger
../../../../go/pkg/mod/github.com/dgraph-io/badger@v1.6.2/dir_unix.go:63:13: undefined: unix.Flock
# github.com/dgraph-io/badger/v2
../../../../go/pkg/mod/github.com/dgraph-io/badger/v2@v2.2007.4/dir_unix.go:62:13: undefined: unix.Flock

The error points to the absence of Flock syscall for AIX in the golang.org/x/sys/unix package. I searched the respective directory within the package, and indeed the function is missing. AIX documentation documents the syscall Flock exists (see: flock.h File, and lockfx, lockf, flock, or lockf64 Subroutine). I understand the syscalls definitions in the package are generated, so whatever source or script used to generate them is resulting in the gap.

As of now, we're only aware of the Flock function missing because it directly impacts us (see caddyserver/caddy#5970), but others may be missing and subject to verification.

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Dec 12, 2023
@gopherbot gopherbot added this to the Unreleased milestone Dec 12, 2023
@prattmic
Copy link
Member

cc @golang/aix

@prattmic prattmic added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Dec 12, 2023
@laboger
Copy link
Contributor

laboger commented Dec 12, 2023

I see these CLs related to the removal of the Flock syscall on AIX:
https://go-review.googlesource.com/152397
https://go-review.googlesource.com/153938
Apparently AIX does not support the Flock syscall, and before these CLs went in it was emulated using FcntlFlock.

@bcmills
Copy link
Contributor

bcmills commented Dec 12, 2023

AIX documentation documents the syscall Flock exists (see: flock.h File, and lockfx, lockf, flock, or lockf64 Subroutine).

Note that per https://www.ibm.com/docs/en/aix/7.3?topic=l-lockfx-lockf-flock-lockf64-subroutine (emphasis added):

The flock subroutine locks and unlocks entire files. This is a limited interface maintained for BSD compatibility, although its behavior differs from BSD in a few subtle ways. To apply a shared lock, the file must be opened for reading. To apply an exclusive lock, the file must be opened for writing.

Unfortunately, there isn't enough detail in that documentation for me to tell whether the “subtle differences” are just the restrictions on the file mode, or other behaviors as well. (For example, there may be other differences regarding the behavior of locks across exec, reentrant locking by other threads or file descriptors in the same process, and so on.)

With more documentation it might be possible to provide a usable AIX flock wrapper. In the meantime, I suggest using FcntlFlock instead. That is the approach taken by cmd/go/internal/lockedfile, and it has been fairly robust in practice once we worked out some non-trivial kinks like #32817.

There is an open proposal to export the lockedfile package as a publicly-importable package:

See previously:

@laboger
Copy link
Contributor

laboger commented Dec 12, 2023

@ayappanec Can you provide some input on this issue?

@ayappanec
Copy link

Let me check with AIX core team if we can get more details about this flock subroutine.

@bmarwell
Copy link

@ayappanec have you heard back from the AIX team?

@ayappanec
Copy link

I haven't heard anything from them yet, as most of them are on vacation.
I myself had a look at the flock implementation. It is almost same as other bsd implementations, like the one here --> https://github.com/NetBSD/src/blob/trunk/tools/compat/flock.c
The one difference in AIX is EWOULDBLOCK can also happen for the errno EACCES.

@mohammed90
Copy link
Author

Sorry for the tag, @bcmills, but are the details shared by @ayappanec enough to re-instate a wrapper of flock? It'll basically be re-copying the body of Flock from this CL (where it's deleted) back into syscall_aix.go. I can send the CL if yes.

@bcmills
Copy link
Contributor

bcmills commented Jan 8, 2024

@ayappanec, note that other BSD implementations generally have a proper flock(2) implementation that is separate from fcntl, not just a libc wrapper. (For example, NetBSD appears to have a separate sys_flock system call.)

I don't know where that NetBSD fcntl compatibility wrapper is actually used, but it isn't close enough to be used in heavily multi-threaded programs (like most Go programs tend to be). fcntl locks are per inode×process, rather than per file descriptor, so they require extreme care to use in a multi-threaded program that may open multiple descriptors for the same file.

@mohammed90
Copy link
Author

Do the same concerns apply to zOS implementation where it's also a wrapper around FcntlFlock?

https://github.com/golang/sys/blob/master/unix%2Fsyscall_zos_s390x.go#L1276-L1307

@bcmills
Copy link
Contributor

bcmills commented Jan 9, 2024

Do the same concerns apply to zOS implementation where it's also a wrapper around FcntlFlock?

Yes, they do. That wrapper probably should not have been added without an explicit proposal, especially given that it was added well after #24684 and #29084 were decided (CC @billotosyr, @tklauser, @ianlancetaylor).

Either way, note that zos is not a port supported by the Go project — it doesn't meet the requirements of even a secondary port per https://go.dev/wiki/PortingPolicy.

@seankhliao seankhliao changed the title x/sys/unix: Missing some AIX syscalls, notably Flock x/sys/unix: missing some AIX syscalls, notably Flock Jan 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FeatureRequest NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. OS-AIX
Projects
Development

No branches or pull requests

9 participants