From f1a7dd766e9eb4eefcf6eb3547494d81b4f2264f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Rainone?= <476650+arl@users.noreply.github.com> Date: Sat, 13 Jul 2019 15:05:39 +0200 Subject: [PATCH] all: Add comment to ensure correct atomics alignment (fixes #5813) Per the sync/atomic bug note: > On ARM, x86-32, and 32-bit MIPS, it is the caller's > responsibility to arrange for 64-bit alignment of 64-bit words > accessed atomically. The first word in a variable or in an > allocated struct, array, or slice can be relied upon to be > 64-bit aligned. All atomic accesses of 64-bit variables in syncthing code base are currently ok (i.e they are all 64-bit aligned). Generally, the bug is triggered because of incorrect alignement of struct fields. Free variables (declared in a function) are guaranteed to be 64-bit aligned by the Go compiler. To ensure the code remains correct upon further addition/removal of fields, which would change the currently correct alignment, I added the following comment where required: // atomic, must remain 64-bit aligned See https://golang.org/pkg/sync/atomic/#pkg-note-BUG. --- cmd/strelaysrv/status.go | 2 +- lib/protocol/counting.go | 8 ++++---- lib/scanner/walk.go | 2 +- lib/stun/stun.go | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/cmd/strelaysrv/status.go b/cmd/strelaysrv/status.go index f60522507..a9aaca8ae 100644 --- a/cmd/strelaysrv/status.go +++ b/cmd/strelaysrv/status.go @@ -86,9 +86,9 @@ func getStatus(w http.ResponseWriter, r *http.Request) { } type rateCalculator struct { + counter *int64 // atomic, must remain 64-bit aligned rates []int64 prev int64 - counter *int64 startTime time.Time } diff --git a/lib/protocol/counting.go b/lib/protocol/counting.go index d441ed311..2310e91f8 100644 --- a/lib/protocol/counting.go +++ b/lib/protocol/counting.go @@ -10,8 +10,8 @@ import ( type countingReader struct { io.Reader - tot int64 // bytes - last int64 // unix nanos + tot int64 // bytes (atomic, must remain 64-bit aligned) + last int64 // unix nanos (atomic, must remain 64-bit aligned) } var ( @@ -37,8 +37,8 @@ func (c *countingReader) Last() time.Time { type countingWriter struct { io.Writer - tot int64 // bytes - last int64 // unix nanos + tot int64 // bytes (atomic, must remain 64-bit aligned) + last int64 // unix nanos (atomic, must remain 64-bit aligned) } func (c *countingWriter) Write(bs []byte) (int, error) { diff --git a/lib/scanner/walk.go b/lib/scanner/walk.go index c89bbcd04..58d66c38c 100644 --- a/lib/scanner/walk.go +++ b/lib/scanner/walk.go @@ -537,7 +537,7 @@ func (w *walker) handleError(ctx context.Context, context, path string, err erro // A byteCounter gets bytes added to it via Update() and then provides the // Total() and one minute moving average Rate() in bytes per second. type byteCounter struct { - total int64 + total int64 // atomic, must remain 64-bit aligned metrics.EWMA stop chan struct{} } diff --git a/lib/stun/stun.go b/lib/stun/stun.go index f2b299ce9..16ae0bdda 100644 --- a/lib/stun/stun.go +++ b/lib/stun/stun.go @@ -39,7 +39,7 @@ const ( ) type writeTrackingPacketConn struct { - lastWrite int64 + lastWrite int64 // atomic, must remain 64-bit aligned net.PacketConn }