Fix memory leak in ffi test

[viirya]

Which issue does this PR close?

Closes #1872.

Rationale for this change

A few raw Arc pointers were not converted back to Arc structs so leading to leaks.

Verified by cargo +nightly miri test -p arrow -- --skip csv --skip ipc --skip json --skip util --skip compute --skip array --skip buffer --skip datatypes --skip bitmap --skip record_batch --skip tensor

running 15 tests                 
test ffi::tests::test_binary ... ok                                                   
test ffi::tests::test_bool ... ok                                                     
test ffi::tests::test_decimal_round_trip ... ok       
test ffi::tests::test_dictionary ... ok
test ffi::tests::test_duration ... ok
test ffi::tests::test_large_binary ... ok
test ffi::tests::test_large_list ... ok
test ffi::tests::test_large_string ... ok                                             
test ffi::tests::test_list ... ok                                                     
test ffi::tests::test_round_trip ... ok                                               
test ffi::tests::test_string ... ok
test ffi::tests::test_time32 ... ok                                                   
test ffi::tests::test_timestamp ... ok
test ffi_stream::tests::test_stream_round_trip_export ... ok
test ffi_stream::tests::test_stream_round_trip_import ... ok

test result: ok. 15 passed; 0 failed; 0 ignored; 0 measured; 1162 filtered out

   Doc-tests arrow

running 8 tests
test src/ffi_stream.rs - ffi_stream (line 27) ... ignored
test src/lib.rs - (line 69) ... ok
test src/lib.rs - (line 43) ... ok
test src/lib.rs - (line 28) ... ok
test src/lib.rs - (line 90) ... ok
test src/lib.rs - (line 126) ... ok
test src/lib.rs - (line 167) ... ok
test src/ffi.rs - ffi (line 27) ... ok

test result: ok. 7 passed; 0 failed; 1 ignored; 0 measured; 147 filtered out; finished in 1.03s

What changes are included in this PR?

Are there any user-facing changes?

[viirya]

cc @sunchao @jhorstmann

[jhorstmann]

Tried to reproduce the miri report and I see still some leaks related to test_round_trip in array::ffi::tests.

[codecov-commenter]

Codecov Report

Merging #1878 (5397126) into master (cedaf8a) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1878      +/-   ##
==========================================
- Coverage   83.46%   83.46%   -0.01%     
==========================================
  Files         201      201              
  Lines       57014    57016       +2     
==========================================
+ Hits        47586    47587       +1     
- Misses       9428     9429       +1     

Impacted Files	Coverage Δ
arrow/src/ffi.rs	86.97% <ø> (ø)
arrow/src/ffi_stream.rs	79.89% <100.00%> (+0.22%)	⬆️
parquet_derive/src/parquet_field.rs	65.98% <0.00%> (-0.23%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cedaf8a...5397126. Read the comment docs.

[viirya]

Tried to reproduce the miri report and I see still some leaks related to test_round_trip in array::ffi::tests.

How to reproduce it? I ran cargo +nightly miri test -p arrow -- --skip csv --skip ipc --skip json --skip util --skip compute --skip array --skip buffer --skip datatypes --skip bitmap --skip record_batch --skip tensor and got all tests passed without leaks.

[jhorstmann]

How to reproduce it?

I did run

cargo +nightly miri test -p arrow -- --skip csv --skip ipc --skip json

or

cargo +nightly miri test -p arrow -- --skip csv --skip ipc --skip json -- array::ffi::tests::test_u32

for one specific test. Only difference to your command line is fewer skip parameters :)

CI looks like it passed because of #1873

[viirya]

Oh, I skip array::xxx. There are a few ffi tests under array:: module too. Let me look at them. Thanks @jhorstmann

[viirya]

@jhorstmann Fixed them and verified by cargo +nightly miri test -p arrow -- --skip csv --skip ipc --skip json -- array::ffi::tests.

[jhorstmann]

Nice, can confirm that miri is happy with that change! I'm not that familiar with the ffi api, that ArrowArray::try_from_raw does not take ownership of the Arcs feels a bit asymmetric with the into_raw method, but this seems to be intentional from the discussion in #1449

[viirya]

Thanks @sunchao @jhorstmann