os: File.Sync() returns ENOTSUP on network mounts that support fsync

[iamcalledrob]

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

$ go version
go version go1.21.0 darwin/arm64

Does this issue reproduce with the latest release?

Yes

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

go env Output

$ go env
GO111MODULE=''
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/rob/Library/Caches/go-build'
GOENV='/Users/rob/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/rob/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/rob/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/opt/homebrew/Cellar/go/1.21.0/libexec'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/opt/homebrew/Cellar/go/1.21.0/libexec/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.21.0'
GCCGO='gccgo'
AR='ar'
CC='cc'
CXX='c++'
CGO_ENABLED='1'
GOMOD='/dev/null'
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 -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/9x/tzryjjm93zvcvhmn517n9t3h0000gn/T/go-build3328922198=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

1. Mount a SMB share on macOS that supports fsync (strict sync = yes, the default since Samba 4.7.0)
2. Open a file on that share
3. Call file.Sync()

What did you expect to see?

No error. The fsync instruction is passed down to the network share, which takes action (or not, depending on its configuration)

What did you see instead?

Error: ENOTSUP

It appears that a change was implemented in go1.12 for #26650 which switched File.Sync() to call fcntl(fd, F_FULLFSYNC) rather than fsync(fd). This was due to fsync not being implemented as expected on darwin.

On local volumes, this change appears to work correctly. However, F_FULLFSYNC appears to be unsupported on SMB volumes--and likely elsewhere too.

This means that the change as a result of #26650 leads to ENOTSUP being returned where a Sync would have succeeded before.

(I've verified via samba logging that calling unix.Fsync() on the fd results in the correct fsync instruction being sent to the server, so darwin's wonky fsync() behaviour seems to only apply to local volumes)

I'd bet that most Go developers are not testing file operations in SMB mounts, so are shipping software that would have worked in 1.11, but in 1.12+ will fail.

I've had to resort to a workaround:

func sync(f *os.File) error {
  err := f.Sync()

  // Ensure fsync is called if Sync is not supported. There are scenarios (SMB mounts, ...?) where fsync will succeed but Sync will fail due
  if (runtime.GOOS == "darwin" || runtime.GOOS == "ios") && errors.Is(err, syscall.ENOTSUP) {
    err = unix.Fsync(int(file.Fd()))
  }

  return err
}

[mauri870]

Thanks. It seems plausible to default to fsync if we get ENOTSUP in those platforms.

/cc @golang/darwin

[gopherbot]

Change https://go.dev/cl/543535 mentions this issue: internal/poll: change Fsync to fallback to syscall.FSync on darwin

[mauri870]

I found this post mentioning the issue:

https://macosx.com/threads/smb-samba-for-time-machine-backup.324958/

Although I don't have a SMB server to test this against, falling back to syscall.Fsync on ENOTSUP seems to be a fairly safe fix for the issue.

[iamcalledrob]

For testing purposes, spinning up Samba configured with:

vfs objects = full_audit
full_audit:prefix = %u|%I|%m|%S
full_audit:success = fsync fsync_recv
full_audit:failure = fsync fsync_recv
full_audit:priority = INFO

will show appropriate logging to verify that fsyncs are occurring as expected

[mauri870]

This is a bug in internal/poll.Fsync (os.File.Sync() as well) on darwin that only affect SMB mounts. There is no way around it AFAIK and it has been around circa 2018, after CL 130676 was merged.

Given that we entered a freeze, I want to check if the proposed fix at CL 543535 is worth submitting for 1.22 and if this bug deserves backports for 1.20 and 1.21.

cc @golang/release