Fix leaked host objects from moving out of unique_ptr::release()

cpp/REVIEW_GUIDELINES.md

@@ -42,6 +42,7 @@
 - CUDA stream/event leaks or improper cleanup
 - Missing RAII or proper cleanup, including in exception paths
 - Raw owning pointers instead of `std::unique_ptr`, `std::shared_ptr`, `std::reference_wrapper`
+- Moving from the raw pointer returned by `std::unique_ptr::release()` (e.g. `std::move(*ptr.release())`, `ptr.release()[0]`, `auto p = contents.data.release();`) leaks the host object; use `std::move(*ptr)` instead. (`table::release()` and `device_uvector::release()` return owning values and are fine.)
 
 ### API Breaking Changes
 - C++ API changes without proper deprecation warnings

cpp/benchmarks/common/generate_input.cu

@@ -523,11 +523,8 @@ std::unique_ptr<cudf::column> create_random_utf8_string_column(data_profile cons
                     cudf::create_null_mask(0, cudf::mask_state::UNALLOCATED)),
                   0};
 
-  return cudf::make_strings_column(num_rows,
-                                   std::move(offsets),
-                                   chars.release(),
-                                   null_count,
-                                   std::move(*result_bitmask.release()));
+  return cudf::make_strings_column(
+    num_rows, std::move(offsets), chars.release(), null_count, std::move(*result_bitmask));
 }
 
 // Forward declarations for create_rand_col_fn
@@ -632,7 +629,7 @@ std::unique_ptr<cudf::column> create_random_column(data_profile const& profile,
                   0};
 
   return std::make_unique<cudf::column>(
-    dtype, num_rows, data.release(), std::move(*result_bitmask.release()), null_count);
+    dtype, num_rows, data.release(), std::move(*result_bitmask), null_count);
 }
 
 /**
@@ -724,7 +721,7 @@ std::unique_ptr<cudf::column> create_random_column<cudf::struct_view>(data_profi
       current_child += children_to_adopt.size();
 
       *current_parent = cudf::make_structs_column(
-        num_rows, std::move(children_to_adopt), null_count, std::move(*null_mask.release()));
+        num_rows, std::move(children_to_adopt), null_count, std::move(*null_mask));
     }
 
     if (lvl == 1) {
@@ -812,7 +809,7 @@ std::unique_ptr<cudf::column> create_random_column<cudf::list_view>(data_profile
                                           std::move(offsets_column),
                                           std::move(current_child_column),
                                           null_count,
-                                          std::move(*null_mask.release()));
+                                          std::move(*null_mask));
 
     if (auto const cv = list_column->view();
         cudf::has_nonempty_nulls(cv, cudf::get_default_stream())) {
@@ -844,7 +841,7 @@ std::unique_ptr<cudf::column> create_distinct_rows_column(data_profile const& pr
     auto null_mask = valid_dist(engine, num_rows);
     auto [result_bitmask, null_count] =
       cudf::bools_to_mask(cudf::device_span<bool const>(null_mask), cudf::get_default_stream());
-    col->set_null_mask(std::move(*result_bitmask.release()), null_count);
+    col->set_null_mask(std::move(*result_bitmask), null_count);
   }
 
   return col;

cpp/benchmarks/io/text/multibyte_split.cpp

@@ -74,7 +74,7 @@ static cudf::string_scalar create_random_input(int32_t num_chars,
 
   // extract the chars from the returned strings column.
   auto input_column_contents = input_column->release();
-  auto chars_buffer          = input_column_contents.data.release();
+  auto chars_buffer          = std::move(input_column_contents.data);
 
   // turn the chars in to a string scalar.
   return cudf::string_scalar(std::move(*chars_buffer));

cpp/src/copying/copy_range.cu

@@ -184,7 +184,7 @@ std::unique_ptr<cudf::column> out_of_place_copy_range_dispatch::operator()<cudf:
   auto indices_column = std::make_unique<cudf::column>(
     new_indices.type(),
     new_indices.size(),
-    std::move(*(target_indices->release().data.release())),
+    std::move(*(target_indices->release().data)),
     cudf::create_null_mask(0, cudf::mask_state::UNALLOCATED, stream, mr),
     0);
 
@@ -194,7 +194,7 @@ std::unique_ptr<cudf::column> out_of_place_copy_range_dispatch::operator()<cudf:
   // create column with keys_column and indices_column
   return make_dictionary_column(std::move(keys_column),
                                 std::move(indices_column),
-                                std::move(*(target_contents.null_mask.release())),
+                                std::move(*(target_contents.null_mask)),
                                 null_count);
 }
 

cpp/src/dictionary/detail/concatenate.cu

@@ -289,7 +289,7 @@ std::unique_ptr<column> concatenate(host_span<column_view const> columns,
   // finally, frankenstein that dictionary column together
   return make_dictionary_column(std::move(keys_column),
                                 std::move(indices_column),
-                                std::move(*(contents.null_mask.release())),
+                                std::move(*(contents.null_mask)),
                                 null_count);
 }
 

cpp/src/dictionary/dictionary_factories.cu

@@ -141,7 +141,7 @@ std::unique_ptr<column> make_dictionary_column(std::unique_ptr<column> keys,
     if (new_type == indices_type || cudf::size_of(new_type) == cudf::size_of(indices_type)) {
       return std::make_unique<column>(new_type,
                                       indices_size,
-                                      std::move(*(contents.data.release())),
+                                      std::move(*(contents.data)),
                                       cudf::create_null_mask(0, cudf::mask_state::UNALLOCATED),
                                       0);
     }
@@ -150,10 +150,8 @@ std::unique_ptr<column> make_dictionary_column(std::unique_ptr<column> keys,
     return cudf::detail::cast(cast_view, new_type, stream, mr);
   }();
 
-  return make_dictionary_column(std::move(keys),
-                                std::move(indices_column),
-                                std::move(*(contents.null_mask.release())),
-                                null_count);
+  return make_dictionary_column(
+    std::move(keys), std::move(indices_column), std::move(*(contents.null_mask)), null_count);
 }
 
 }  // namespace cudf

cpp/src/interop/from_arrow_host_strings.cu

@@ -62,7 +62,7 @@ std::unique_ptr<column> from_arrow_string(
                              std::move(offsets_column),
                              std::move(chars),
                              null_count,
-                             std::move(*mask.release()));
+                             std::move(*mask));
 }
 
 constexpr int stringview_vector_idx = 1;

cpp/src/io/json/write_json.cu

@@ -383,7 +383,7 @@ std::unique_ptr<column> struct_to_strings(table_view const& strings_columns,
     num_rows,
     std::make_unique<cudf::column>(
       std::move(row_string_offsets), cudf::create_null_mask(0, cudf::mask_state::UNALLOCATED), 0),
-    std::move(chars_data.release()[0]),
+    std::move(*chars_data),
     0,
     cudf::create_null_mask(0, cudf::mask_state::UNALLOCATED));
 }
@@ -546,7 +546,7 @@ std::unique_ptr<column> join_list_of_strings(lists_column_view const& lists_stri
     num_lists,
     std::make_unique<cudf::column>(
       std::move(row_string_offsets), cudf::create_null_mask(0, cudf::mask_state::UNALLOCATED), 0),
-    std::move(chars_data.release()[0]),
+    std::move(*chars_data),
     lists_strings.null_count(),
     cudf::detail::copy_bitmask(lists_strings.parent(), stream, mr));
 }

cpp/src/io/utilities/column_buffer.cpp

@@ -210,7 +210,7 @@ std::unique_ptr<column> make_column(column_buffer_base<string_policy>& buffer,
           auto col_content      = string_col->release();
 
           // convert to uint8 column, strings are currently stored as int8
-          auto data      = col_content.data.release();
+          auto data      = std::move(col_content.data);
           auto char_size = data->size();
 
           CUDF_EXPECTS(char_size < static_cast<std::size_t>(std::numeric_limits<size_type>::max()),

cpp/src/lists/contains.cu

@@ -256,7 +256,7 @@ std::unique_ptr<column> to_contains(std::unique_ptr<column>&& key_positions,
 
   auto const null_count                             = key_positions->null_count();
   [[maybe_unused]] auto [data, null_mask, children] = key_positions->release();
-  result->set_null_mask(std::move(*null_mask.release()), null_count);
+  result->set_null_mask(std::move(*null_mask), null_count);
 
   return result;
 }

cpp/src/reductions/collect_ops.cu

@@ -38,10 +38,10 @@ std::unique_ptr<scalar> collect_list(column_view const& col,
                                      rmm::device_async_resource_ref mr)
 {
   if (need_handle_nulls(col, null_handling)) {
-    auto d_view             = column_device_view::create(col, stream);
-    auto filter             = cudf::detail::validity_accessor(*d_view);
-    auto null_purged_table  = cudf::detail::copy_if(table_view{{col}}, filter, stream, mr);
-    column* null_purged_col = null_purged_table->release().front().release();
+    auto d_view            = column_device_view::create(col, stream);
+    auto filter            = cudf::detail::validity_accessor(*d_view);
+    auto null_purged_table = cudf::detail::copy_if(table_view{{col}}, filter, stream, mr);
+    auto null_purged_col   = std::move(null_purged_table->release().front());
     null_purged_col->set_null_mask(
       cudf::create_null_mask(0, cudf::mask_state::UNALLOCATED, stream, mr), 0);
     return std::make_unique<list_scalar>(std::move(*null_purged_col), true, stream, mr);

cpp/src/reductions/histogram.cu

@@ -85,8 +85,7 @@ auto gather_histogram(table_view const& input,
                         stream,
                         mr);
 
-  return std::make_unique<cudf::list_scalar>(
-    std::move(*output_structs.release()), true, stream, mr);
+  return std::make_unique<cudf::list_scalar>(std::move(*output_structs), true, stream, mr);
 }
 
 }  // namespace

cpp/src/reductions/segmented/simple.cuh

@@ -241,8 +241,8 @@ std::unique_ptr<column> fixed_point_segmented_reduction(
 
   return std::make_unique<column>(data_type{type_to_id<InputType>(), scale},
                                   size,
-                                  std::move(*(contents.data.release())),
-                                  std::move(*(contents.null_mask.release())),
+                                  std::move(*(contents.data)),
+                                  std::move(*(contents.null_mask)),
                                   null_count);
 }
 

cpp/src/strings/replace/multi.cu

@@ -417,7 +417,7 @@ std::unique_ptr<column> replace_character_parallel(strings_column_view const& in
   // build the strings columns from the chars and offsets
   return make_strings_column(strings_count,
                              std::move(offsets),
-                             std::move(chars_data.release()[0]),
+                             std::move(*chars_data),
                              input.null_count(),
                              cudf::detail::copy_bitmask(input.parent(), stream, mr));
 }

cpp/src/strings/replace/replace.cu

@@ -358,7 +358,7 @@ std::unique_ptr<column> replace_character_parallel(strings_column_view const& in
   // build the strings columns from the chars and offsets
   return make_strings_column(strings_count,
                              std::move(offsets),
-                             std::move(chars_data.release()[0]),
+                             std::move(*chars_data),
                              input.null_count(),
                              cudf::detail::copy_bitmask(input.parent(), stream, mr));
 }

cpp/src/transform/nans_to_nulls.cu

@@ -88,11 +88,8 @@ std::unique_ptr<column> column_nans_to_nulls(column_view const& input,
     cudf::type_dispatcher(input.type(), dispatch_nan_to_null{}, input, stream, mr);
 
   auto data = cudf::type_dispatcher(input.type(), copy_float_data_fn{input, stream, mr});
-  return std::make_unique<column>(input.type(),
-                                  input.size(),
-                                  std::move(*data.release()),
-                                  std::move(*null_mask.release()),
-                                  null_count);
+  return std::make_unique<column>(
+    input.type(), input.size(), std::move(*data), std::move(*null_mask), null_count);
 }
 }  // namespace detail
 

java/src/main/native/src/ColumnViewJni.cu

@@ -136,7 +136,7 @@ void post_process_list_overlap(cudf::column_view const& lhs,
       overlap_result->set_null_mask(std::move(null_mask), null_count);
     } else {
       // Just set the output nullmask as the new nullmask.
-      overlap_result->set_null_mask(std::move(*new_null_mask.release()), new_null_count);
+      overlap_result->set_null_mask(std::move(*new_null_mask), new_null_count);
     }
   }
 }