Rust protobuf: fix memory leaks

Our ASAN test runs have not had the heap checker enabled, so this has allowed a
few memory leaks to slip in. This CL fixes all of them so that we can turn on
the heap checker.

The first one takes place whenever we add an entry into a string-valued map
using the C++ kernel. The problem is that `InnerProtoString::into_raw()` gives
up ownership of the raw `std::string` pointer it holds, but then we never
delete that pointer. This CL fixes the problem by deleting the pointer in C++
right after we perform the map insertion. To simplify things, I created a
`MakeCleanup()` helper function that we always call in our map insertion
thunks, but it's a no-op in the cases where we don't need to free anything.

There were a couple similar memory leaks related to repeated field accessors in
the C++ kernel, and those were simple to fix just by adding the necessary
`delete` call.

Finally, there were two benign memory leaks in the upb kernel involving global
variables used for empty repeated fields and maps. It turned out that we did
not need to use `Box` at all here, so removing that simplified things and fixed
the leaks.

PiperOrigin-RevId: 652947042

rust/cpp_kernel/map.h

@@ -1,6 +1,30 @@
 #ifndef GOOGLE_PROTOBUF_RUST_CPP_KERNEL_MAP_H__
 #define GOOGLE_PROTOBUF_RUST_CPP_KERNEL_MAP_H__
 
+#include <memory>
+
+namespace google {
+namespace protobuf {
+namespace rust {
+
+// String and bytes values are passed across the FFI boundary as owned raw
+// pointers when we do map insertions. Unlike other types, they have to be
+// explicitly deleted. This MakeCleanup() helper does nothing by default, but
+// for std::string pointers it returns a std::unique_ptr to take ownership of
+// the raw pointer.
+template <typename T>
+int MakeCleanup(T value) {
+  return 0;
+}
+
+inline std::unique_ptr<std::string> MakeCleanup(std::string* value) {
+  return std::unique_ptr<std::string>(value);
+}
+
+}  // namespace rust
+}  // namespace protobuf
+}  // namespace google
+
 // Defines concrete thunks to access typed map methods from Rust.
 #define __PB_RUST_EXPOSE_SCALAR_MAP_METHODS(                                  \
     key_ty, rust_key_ty, ffi_key_ty, to_cpp_key, to_ffi_key, value_ty,        \
@@ -23,6 +47,8 @@
   }                                                                           \
   bool proto2_rust_thunk_Map_##rust_key_ty##_##rust_value_ty##_insert(        \
       google::protobuf::Map<key_ty, value_ty>* m, ffi_key_ty key, ffi_value_ty value) { \
+    auto cleanup = google::protobuf::rust::MakeCleanup(value);                          \
+    (void)cleanup;                                                            \
     auto iter_and_inserted = m->try_emplace(to_cpp_key, to_cpp_value);        \
     if (!iter_and_inserted.second) {                                          \
       iter_and_inserted.first->second = to_cpp_value;                         \

rust/cpp_kernel/repeated.cc

@@ -83,6 +83,7 @@ expose_repeated_field_methods(int64_t, i64);
       google::protobuf::RepeatedPtrField<std::string>* r, size_t index,        \
       std::string* val) {                                            \
     *r->Mutable(index) = std::move(*val);                            \
+    delete val;                                                      \
   }                                                                  \
   void proto2_rust_RepeatedField_##ty##_copy_from(                   \
       const google::protobuf::RepeatedPtrField<std::string>* src,              \

rust/upb.rs

@@ -467,18 +467,15 @@ pub fn empty_array<T: ?Sized + ProxiedInRepeated>() -> RepeatedView<'static, T>
     // TODO: Consider creating a static empty array in C.
 
     // Use `i32` for a shared empty repeated for all repeated types in the program.
-    static EMPTY_REPEATED_VIEW: OnceLock<RepeatedView<'static, i32>> = OnceLock::new();
+    static EMPTY_REPEATED_VIEW: OnceLock<Repeated<i32>> = OnceLock::new();
 
     // SAFETY:
     // - Because the repeated is never mutated, the repeated type is unused and
     //   therefore valid for `T`.
-    // - The view is leaked for `'static`.
     unsafe {
         RepeatedView::from_raw(
             Private,
-            EMPTY_REPEATED_VIEW
-                .get_or_init(|| Box::leak(Box::new(Repeated::new())).as_mut().into_view())
-                .as_raw(Private),
+            EMPTY_REPEATED_VIEW.get_or_init(Repeated::new).as_view().as_raw(Private),
         )
     }
 }
@@ -502,7 +499,7 @@ where
     //
     // If we used a larger key, then UPB would hash more bytes of the key than Rust
     // initialized.
-    static EMPTY_MAP_VIEW: OnceLock<MapView<'static, bool, bool>> = OnceLock::new();
+    static EMPTY_MAP_VIEW: OnceLock<Map<bool, bool>> = OnceLock::new();
 
     // SAFETY:
     // - The map is empty and never mutated.
@@ -511,14 +508,8 @@ where
     //   The map is empty, therefore it doesn't matter what hash is computed, but we
     //   have to use `bool` type as the smallest key possible (otherwise UPB would
     //   read more bytes than Rust allocated).
-    // - The view is leaked for `'static`.
     unsafe {
-        MapView::from_raw(
-            Private,
-            EMPTY_MAP_VIEW
-                .get_or_init(|| Box::leak(Box::new(Map::new())).as_mut().into_view())
-                .as_raw(Private),
-        )
+        MapView::from_raw(Private, EMPTY_MAP_VIEW.get_or_init(Map::new).as_view().as_raw(Private))
     }
 }
 

src/google/protobuf/compiler/rust/accessors/repeated_field.cc

@@ -232,7 +232,8 @@ void RepeatedField::InThunkCc(Context& ctx,
              [&] {
                ctx.Emit(
                    R"cc(
-                     $ContainerType$<$ElementType$>* $getter_mut_thunk$($QualifiedMsg$* msg) {
+                     $ContainerType$<$ElementType$>* $getter_mut_thunk$(
+                         $QualifiedMsg$* msg) {
                        return msg->mutable_$field$();
                      }
                      const $ContainerType$<$ElementType$>* $getter_thunk$(
@@ -243,6 +244,7 @@ void RepeatedField::InThunkCc(Context& ctx,
                          $QualifiedMsg$* msg,
                          $ContainerType$<$ElementType$>* value) {
                        *msg->mutable_$field$() = std::move(*value);
+                       delete value;
                      }
                    )cc");
              }}},