Skip to content

Commit

Permalink
GH-33864: [Go] Don't directly coerce cgo.Handle to unsafe.Pointer (#3…
Browse files Browse the repository at this point in the history
…3865)

### Rationale for this change

Converting cgo.Handle to unsafe.Pointer is not allowed as per the docs. Doing so appears to cause occasional panics at runtime, possibly because something corrupts or collects the handle unexpectedly.

### What changes are included in this PR?

Heap-allocate the Handle to preserve it.

### Are these changes tested?

We were unable to reproduce this outside of an integration test.

### Are there any user-facing changes?

No user-facing changes.
* Closes: #33864

Authored-by: David Li <li.davidm96@gmail.com>
Signed-off-by: Matt Topol <zotthewizard@gmail.com>
  • Loading branch information
lidavidm committed Jan 25, 2023
1 parent ff4227b commit 6ae9821
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 8 deletions.
2 changes: 1 addition & 1 deletion go/arrow/cdata/cdata_exports.go
Expand Up @@ -396,7 +396,7 @@ func exportArray(arr arrow.Array, out *CArrowArray, outSchema *CArrowSchema) {

arr.Data().Retain()
h := cgo.NewHandle(arr.Data())
out.private_data = unsafe.Pointer(&h)
out.private_data = createHandle(h)
out.release = (*[0]byte)(C.goReleaseArray)
switch arr := arr.(type) {
case array.ListLike:
Expand Down
28 changes: 22 additions & 6 deletions go/arrow/cdata/exports.go
Expand Up @@ -70,6 +70,22 @@ func releaseExportedSchema(schema *CArrowSchema) {
C.free(unsafe.Pointer(schema.children))
}

// apache/arrow#33864: allocate a new cgo.Handle and store its address
// in a heap-allocated uintptr_t.
func createHandle(hndl cgo.Handle) unsafe.Pointer {
// uintptr_t* hptr = malloc(sizeof(uintptr_t));
hptr := (*C.uintptr_t)(C.malloc(C.sizeof_uintptr_t))
// *hptr = (uintptr)hndl;
*hptr = C.uintptr_t(uintptr(hndl))
return unsafe.Pointer(hptr)
}

func getHandle(ptr unsafe.Pointer) cgo.Handle {
// uintptr_t* hptr = (uintptr_t*)ptr;
hptr := (*C.uintptr_t)(ptr)
return cgo.Handle((uintptr)(*hptr))
}

//export releaseExportedArray
func releaseExportedArray(arr *CArrowArray) {
if C.ArrowArrayIsReleased(arr) == 1 {
Expand Down Expand Up @@ -100,35 +116,35 @@ func releaseExportedArray(arr *CArrowArray) {
C.free(unsafe.Pointer(arr.children))
}

h := *(*cgo.Handle)(arr.private_data)
h := getHandle(arr.private_data)
h.Value().(arrow.ArrayData).Release()
h.Delete()
}

//export streamGetSchema
func streamGetSchema(handle *CArrowArrayStream, out *CArrowSchema) C.int {
h := *(*cgo.Handle)(handle.private_data)
h := getHandle(handle.private_data)
rdr := h.Value().(cRecordReader)
return C.int(rdr.getSchema(out))
}

//export streamGetNext
func streamGetNext(handle *CArrowArrayStream, out *CArrowArray) C.int {
h := *(*cgo.Handle)(handle.private_data)
h := getHandle(handle.private_data)
rdr := h.Value().(cRecordReader)
return C.int(rdr.next(out))
}

//export streamGetError
func streamGetError(handle *CArrowArrayStream) *C.cchar_t {
h := *(*cgo.Handle)(handle.private_data)
h := getHandle(handle.private_data)
rdr := h.Value().(cRecordReader)
return rdr.getLastError()
}

//export streamRelease
func streamRelease(handle *CArrowArrayStream) {
h := *(*cgo.Handle)(handle.private_data)
h := getHandle(handle.private_data)
h.Value().(cRecordReader).release()
h.Delete()
handle.release = nil
Expand All @@ -141,5 +157,5 @@ func exportStream(rdr array.RecordReader, out *CArrowArrayStream) {
out.get_last_error = (*[0]byte)(C.streamGetError)
out.release = (*[0]byte)(C.streamRelease)
h := cgo.NewHandle(cRecordReader{rdr: rdr, err: nil})
out.private_data = unsafe.Pointer(&h)
out.private_data = createHandle(h)
}
2 changes: 1 addition & 1 deletion go/go.mod
Expand Up @@ -28,6 +28,7 @@ require (
github.com/google/flatbuffers v2.0.8+incompatible
github.com/klauspost/asmfmt v1.3.2
github.com/klauspost/compress v1.15.9
github.com/klauspost/cpuid/v2 v2.0.9
github.com/minio/asm2plan9s v0.0.0-20200509001527-cdd76441f9d8
github.com/minio/c2goasm v0.0.0-20190812172519-36a3d3bbc4f3
github.com/pierrec/lz4/v4 v4.1.15
Expand All @@ -49,7 +50,6 @@ require (
github.com/golang/protobuf v1.5.2 // indirect
github.com/google/uuid v1.3.0 // indirect
github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 // indirect
github.com/klauspost/cpuid/v2 v2.0.9 // indirect
github.com/kr/pretty v0.3.0 // indirect
github.com/mattn/go-isatty v0.0.16 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
Expand Down

0 comments on commit 6ae9821

Please sign in to comment.