Remove borrowed types from FFI; fixed_size_list constructor accepts u32 (#8658)

FFI currently has two types of returned objects: ones are owned (inside
of which
there's an Arc), others are borrowed (because there's no Arc inside).

One example of the latter is a struct field name which returns a
borrowed
vx_string. We initially had a comment
caller shouldn't call the corresponding _free() function on these. This
is not
enough as caller can call a _clone() function on a borrowed vx_string
which is
UB because we try to increment a strong reference count on a pointer
which isn't
an Arc. From a callee's perspective, we can't distinguish owned and
borrowed
types, and this creates weird APIs like struct field dtype being owned
but name
being borrowed.

This PR removes all borrowed/owned distinction from FFI layer. Each
object now
returned from an FFI function is owned. It can be freely cloned, and
each must
be freed explicitly with _free() functions.

On another change, fixed size list constructor previously accepted a
size_t
(64-bit unsigned integer on most systems) but validated it against u32
because
that's our file format limitation for fixed size lists. This PR changes
the
signature so that now it accepts uint32_t as length parameter.

As the changes are significant enough to trigger bugs in our own tests,
this PR also re-enables FFI sanitizer tests in CI.

Signed-off-by: Mikhail Kot <mikhail@spiraldb.com>

Author: myrrc

.github/workflows/rust-instrumented.yml

@@ -194,7 +194,7 @@ jobs:
           - sanitizer: tsan
             sanitizer_flags: "-Zsanitizer=thread"
     name: "Rust/C++ FFI tests (${{ matrix.sanitizer }})"
-    timeout-minutes: 30
+    timeout-minutes: 5
     env:
       ASAN_OPTIONS: "symbolize=1:check_initialization_order=1:detect_leaks=1:leak_check_at_exit=1"
       LSAN_OPTIONS: "report_objects=1"
@@ -217,9 +217,6 @@ jobs:
           sccache: s3
       - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0  # v7
       - uses: ./.github/actions/setup-prebuild
-      - name: Install rustfilt
-        run: |
-          cargo install rustfilt
       - name: Install Rust nightly toolchain
         run: |
           rustup toolchain install $NIGHTLY_TOOLCHAIN
@@ -241,8 +238,7 @@ jobs:
       - name: Run tests
         run: |
           set -o pipefail
-#         re-enable once we can fix this hang
-#         ./vortex-ffi/build/test/vortex_ffi_test 2>&1 | rustfilt
+          ./vortex-ffi/build/test/vortex_ffi_test
       - name: Run examples
         run: |
           set -o pipefail
@@ -251,11 +247,11 @@ jobs:
           # error: Unable to walk dir: File system loop found
           rm -fr vortex-ffi/build/_deps/nanoarrow-src/python
 
-          ./vortex-ffi/build/examples/write_sample file.vortex 2>&1 | rustfilt
-          ./vortex-ffi/build/examples/write_sample file2.vortex 2>&1 | rustfilt
-          ./vortex-ffi/build/examples/dtype '*.vortex' 2>&1 | rustfilt
-          ./vortex-ffi/build/examples/scan '*.vortex' 2>&1 | rustfilt
-          ./vortex-ffi/build/examples/scan_to_arrow '*.vortex' 2>&1 | rustfilt
+          ./vortex-ffi/build/examples/write_sample file.vortex
+          ./vortex-ffi/build/examples/write_sample file2.vortex
+          ./vortex-ffi/build/examples/dtype '*.vortex'
+          ./vortex-ffi/build/examples/scan '*.vortex'
+          ./vortex-ffi/build/examples/scan_to_arrow '*.vortex'
 
   miri:
     name: "Rust tests (miri)"

vortex-ffi/README.md

@@ -1,4 +1,4 @@
-# Vortex C interface
+# Vortex C bindings
 
 ## Updating Headers
 
@@ -20,7 +20,7 @@ target_link_libraries(my_target, vortex_ffi_shared)
 # or target_link_libraries(my_target, vortex_ffi)
 ```
 
-## Running C examples:
+## Running C examples
 
 ```sh
 cmake -Bbuild -DBUILD_EXAMPLES=1
@@ -99,7 +99,7 @@ cargo +nightly build -Zbuild-std --target=<target triple> \
 2. Build tests with target triple:
 
 ```sh
-cmake -Bbuild -DWITH_ASAN=1 -DTARGET_TRIPLE=<target triple>
+cmake -Bbuild -DSANITIZER=asan -DTARGET_TRIPLE=<target triple>
 ```
 
 3. Run the tests (ctest doesn't output failures in detail):

vortex-ffi/cbindgen.toml

@@ -11,9 +11,11 @@ header = """
 #pragma once
 #include <stdint.h>
 
-//
 // THIS FILE IS AUTO-GENERATED, DO NOT MAKE EDITS DIRECTLY
-//
+
+// All operations return owned types which need to be freed by calling a
+// matching _free() function. This includes all arrays, data sources, scans,
+// errors, error messages, and other allocated objects.
 
 // https://arrow.apache.org/docs/format/CDataInterface.html#structure-definitions
 // If you want to use your own Arrow library like nanoarrow, define this macro