(arrow-go) branch main updated: fix: go's atomic operations require 64bit alignment in structs on ARM (#323)

[ze...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

zeroshade pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-go.git


The following commit(s) were added to refs/heads/main by this push:
     new a0206ec  fix: go's atomic operations require 64bit alignment in structs on ARM (#323)
a0206ec is described below

commit a0206ec757ff7bcb02cee332852ac3e2e7eeaa3a
Author: Chris Pahl <sa...@online.de>
AuthorDate: Fri Mar 21 19:30:33 2025 +0100

    fix: go's atomic operations require 64bit alignment in structs on ARM (#323)
    
    ### Rationale for this change
    
    I've tried to run a pqarrow based writer/reader on a RaspberryPi. This
    lead to an almost immediate crash however, due to an unaligned struct
    member in `ipc.Reader`.
    
    I did not check yet if there are other structs that might have similar
    issues. It would probably be nice to
    
    ### What changes are included in this PR?
    
    Swapping the struct field order is enough to fix the issue. See also the
    link in the diff.
    
    ### Are these changes tested?
    
    Yes, no additional unit tests due to hardware constraints, but my
    program runs through now.
    
    ### Are there any user-facing changes?
    
    Less crashing?
    
    ### Other note
    
    I noticed that this will also crash if I do not pass the `noasm` build
    tag, which is probably also a little unfortunate. This seems to be
    because the `funclist` in `parquet/internal/bmi/bmi.go` is empty. If you
    want I can add a default, so the funclist is always initialized with the
    fallback go implementation, unless overwritten.
---
 arrow/ipc/reader.go         | 15 +++++++--------
 parquet/internal/bmi/bmi.go | 10 ++++++++--
 2 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/arrow/ipc/reader.go b/arrow/ipc/reader.go
index 2a4f859..1934c71 100644
--- a/arrow/ipc/reader.go
+++ b/arrow/ipc/reader.go
@@ -39,7 +39,7 @@ type Reader struct {
 	r      MessageReader
 	schema *arrow.Schema
 
-	refCount int64
+	refCount atomic.Int64
 	rec      arrow.Record
 	err      error
 
@@ -70,13 +70,14 @@ func NewReaderFromMessageReader(r MessageReader, opts ...Option) (reader *Reader
 
 	rr := &Reader{
 		r:        r,
-		refCount: 1,
+		refCount: atomic.Int64{},
 		// types:    make(dictTypeMap),
 		memo:               dictutils.NewMemo(),
 		mem:                cfg.alloc,
 		ensureNativeEndian: cfg.ensureNativeEndian,
 		expectedSchema:     cfg.schema,
 	}
+	rr.refCount.Add(1)
 
 	if !cfg.noAutoSchema {
 		if err := rr.readSchema(cfg.schema); err != nil {
@@ -141,16 +142,16 @@ func (r *Reader) readSchema(schema *arrow.Schema) error {
 // Retain increases the reference count by 1.
 // Retain may be called simultaneously from multiple goroutines.
 func (r *Reader) Retain() {
-	atomic.AddInt64(&r.refCount, 1)
+	r.refCount.Add(1)
 }
 
 // Release decreases the reference count by 1.
 // When the reference count goes to zero, the memory is freed.
 // Release may be called simultaneously from multiple goroutines.
 func (r *Reader) Release() {
-	debug.Assert(atomic.LoadInt64(&r.refCount) > 0, "too many releases")
+	debug.Assert(r.refCount.Load() > 0, "too many releases")
 
-	if atomic.AddInt64(&r.refCount, -1) == 0 {
+	if r.refCount.Add(-1) == 0 {
 		if r.rec != nil {
 			r.rec.Release()
 			r.rec = nil
@@ -280,6 +281,4 @@ func (r *Reader) Read() (arrow.Record, error) {
 	return r.rec, nil
 }
 
-var (
-	_ array.RecordReader = (*Reader)(nil)
-)
+var _ array.RecordReader = (*Reader)(nil)
diff --git a/parquet/internal/bmi/bmi.go b/parquet/internal/bmi/bmi.go
index a12af3e..7139d6f 100644
--- a/parquet/internal/bmi/bmi.go
+++ b/parquet/internal/bmi/bmi.go
@@ -19,14 +19,20 @@
 // BMI2.
 package bmi
 
-import "math/bits"
+import (
+	"math/bits"
+)
 
 type funcs struct {
 	extractBits func(uint64, uint64) uint64
 	gtbitmap    func([]int16, int16) uint64
 }
 
-var funclist funcs
+// fallback until arch specific init() is called:
+var funclist = funcs{
+	extractBits: extractBitsGo,
+	gtbitmap:    greaterThanBitmapGo,
+}
 
 // ExtractBits performs a Parallel Bit extract as per the PEXT instruction for
 // x86/x86-64 cpus to use the second parameter as a mask to extract the bits from