Merge branch 'sh-ensure-bytes-remaining-64-bit-aligned' into 'master'

catfile: Ensure structs are properly aligned in memory for 32-bit CPUs

See merge request gitlab-org/gitaly!4139

2 files changed, 20 insertions, 14 deletions

diff --git a/internal/git/catfile/object_reader.go b/internal/git/catfile/object_reader.go
index 9d12291cc..7020adcbe 100644
--- a/internal/git/catfile/object_reader.go
+++ b/internal/git/catfile/object_reader.go
@@ -155,22 +155,25 @@ func (o *objectReader) ObjectQueue(ctx context.Context) (ObjectQueue, func(), er
 
 // Object represents data returned by `git cat-file --batch`
 type Object struct {
-	// ObjectInfo represents main information about object
-	ObjectInfo
-
-	// dataReader is reader which has all the object data.
-	dataReader io.LimitedReader
-
-	// bytesLeft tracks the number of bytes which are left to be read. While this duplicates the
+	// bytesRemaining tracks the number of bytes which are left to be read. While this duplicates the
 	// information tracked in dataReader.N, this cannot be helped given that we need to make
 	// access to this information atomic so there's no race between updating it and checking the
 	// process for dirtiness. While we could use locking instead of atomics, we'd have to lock
 	// during the whole read duration -- and thus it'd become impossible to check for dirtiness
 	// at the same time.
+	//
+	// We list the atomic fields first to ensure they are 64-bit and 32-bit aligned:
+	// https://pkg.go.dev/sync/atomic#pkg-note-BUG
 	bytesRemaining int64
 
 	// closed determines whether the object is closed for reading.
 	closed int32
+
+	// ObjectInfo represents main information about object
+	ObjectInfo
+
+	// dataReader is reader which has all the object data.
+	dataReader io.LimitedReader
 }
 
 // isDirty determines whether the object is still dirty, that is whether there are still unconsumed
diff --git a/internal/git/catfile/request_queue.go b/internal/git/catfile/request_queue.go
index 2b43f8a3c..5b99d1e9e 100644
--- a/internal/git/catfile/request_queue.go
+++ b/internal/git/catfile/request_queue.go
@@ -22,21 +22,24 @@ const (
 )
 
 type requestQueue struct {
-	// isObjectQueue is set to `true` when this is a request queue which can be used for reading
-	// objects. If set to `false`, then this can only be used to read object info.
-	isObjectQueue bool
-
-	stdout *bufio.Reader
-	stdin  *bufio.Writer
-
 	// outstandingRequests is the number of requests which have been queued up. Gets incremented
 	// on request, and decremented when starting to read an object (not when that object has
 	// been fully consumed).
+	//
+	// We list the atomic fields first to ensure they are 64-bit and 32-bit aligned:
+	// https://pkg.go.dev/sync/atomic#pkg-note-BUG
 	outstandingRequests int64
 
 	// closed indicates whether the queue is closed for additional requests.
 	closed int32
 
+	// isObjectQueue is set to `true` when this is a request queue which can be used for reading
+	// objects. If set to `false`, then this can only be used to read object info.
+	isObjectQueue bool
+
+	stdout *bufio.Reader
+	stdin  *bufio.Writer
+
 	// currentObject is the currently read object.
 	currentObject     *Object
 	currentObjectLock sync.Mutex