change pointer returned by Serialize() to be smart

.github/workflows/docs.yml

@@ -29,9 +29,19 @@ jobs:
           run: |
             sudo apt-get update -y -qq
             sudo apt-get install -y --force-yes -qq build-essential python3-dev
-            pip install sphinx
-            pip install sphinx-rtd-theme
-            pip install sphinx-multiversion
+            python -m pip install --upgrade pip
+
+            # Pin sphinx to an older version until sphinx-multiversion is fixed
+            # see
+            # https://github.com/sphinx-contrib/multiversion/pull/202
+            python -m pip install "sphinx<8.2" sphinx-rtd-theme
+
+            # This should pull in the fixed version of multiversion when its available
+            python -m pip install "sphinx-multiversion @ git+https://github.com/sphinx-contrib/multiversion.git"
+
+            python -m pip show sphinx sphinx-multiversion
+            sphinx-build --version
+            sphinx-multiversion --version || true
         - name: build code
           run: |
             mkdir -p build

CHANGELOG.md

@@ -8,6 +8,7 @@
 ### Fixed (Repair bugs, etc)
 
 ### Changed (changing behavior/API/variables/...)
+- [[PR601]](https://github.com/lanl/singularity-eos/pull/601) Make Serialize() return a smart pointer
 
 ### Infrastructure (changes irrelevant to downstream codes)
 

doc/sphinx/src/using-eos.rst

@@ -81,11 +81,20 @@ The function
 fills the ``dst`` pointer with the memory required for serialization
 and returns the number of bytes written to ``dst``. The function
 
-.. cpp:function:: std::pair<std::size_t, char*> EOS::Serialize();
+.. cpp:function:: std::pair<std::size_t, std::shared_ptr<char[]>> EOS::Serialize();
 
-allocates a ``char*`` pointer to contain serialized data and fills
+allocates a ``std::shared_ptr<char[]>`` to contain serialized data and fills
 it.
 
+.. note::
+
+  Note that ``Serialize()`` uses a smart pointer while most other
+  ``singularity-eos`` machinery works with unmanaged pointers. This
+  means the pointer returned by ``Serialize()`` does not need to be
+  freed. However it is the responsiblity of the host code to manage
+  the memory for other pointers that the serialization machinery may
+  utilize or interact with.
+
 .. warning::
 
   Serialization and de-serialization may only be performed on objects
@@ -141,7 +150,9 @@ For example you might call ``DeSerialize`` as
   would call ``eos.Finalize()``. If the ``SharedMemSettings`` are
   utilized to request data be written to a shared memory pointer,
   however, you can free the ``src`` pointer, so long as you don't free
-  the shared memory pointer.
+  the shared memory pointer. This means you must manage these pointers
+  externally and not let them go out of scope, especially if you use
+  smart pointers.
 
 Putting everything together, a full sequence with MPI might look like this:
 

singularity-eos/base/serialization_utils.hpp

@@ -15,6 +15,7 @@
 #ifndef SINGULARITY_EOS_BASE_SERIALIZATION_UTILS_
 #define SINGULARITY_EOS_BASE_SERIALIZATION_UTILS_
 
+#include <memory>
 #include <numeric>
 #include <utility>
 #include <vector>
@@ -67,8 +68,8 @@ struct BulkSerializer {
   }
   auto Serialize() {
     std::size_t size = SerializedSizeInBytes();
-    char *dst = (char *)malloc(size);
-    std::size_t new_size = Serialize(dst);
+    std::shared_ptr<char[]> dst(new char[size]);
+    std::size_t new_size = Serialize(dst.get());
     PORTABLE_ALWAYS_REQUIRE(size == new_size, "Serialization failed!");
     return std::make_pair(size, dst);
   }

singularity-eos/eos/eos_base.hpp

@@ -17,6 +17,7 @@
 
 #include <cstring>
 #include <limits>
+#include <memory>
 #include <string>
 
 #include <ports-of-call/portability.hpp>
@@ -1052,8 +1053,8 @@ class EosBase {
   auto Serialize() {
     CRTP *pcrtp = static_cast<CRTP *>(this);
     std::size_t size = pcrtp->SerializedSizeInBytes();
-    char *dst = (char *)malloc(size);
-    std::size_t size_new = Serialize(dst);
+    std::shared_ptr<char[]> dst(new char[size]);
+    std::size_t size_new = Serialize(dst.get());
     PORTABLE_ALWAYS_REQUIRE(size_new == size, "Serialization failed!");
     return std::make_pair(size, dst);
   }

singularity-eos/eos/eos_variant.hpp

@@ -17,6 +17,7 @@
 #ifndef EOS_VARIANT_HPP
 #define EOS_VARIANT_HPP
 
+#include <memory>
 #include <set>
 #include <string>
 #include <vector>
@@ -1336,8 +1337,8 @@ class Variant {
   }
   auto Serialize() {
     std::size_t size = SerializedSizeInBytes();
-    char *dst = (char *)malloc(size);
-    std::size_t new_size = Serialize(dst);
+    std::shared_ptr<char[]> dst(new char[size]);
+    std::size_t new_size = Serialize(dst.get());
     PORTABLE_ALWAYS_REQUIRE(size == new_size, "Serialization failed!");
     return std::make_pair(size, dst);
   }

test/test_eos_helmholtz.cpp

@@ -47,7 +47,7 @@ SCENARIO("Helmholtz equation of state - Table interpolation (tgiven)", "[Helmhol
 
     Helmholtz host_eos_2;
     auto [size, data] = host_eos.Serialize();
-    auto read_size = host_eos_2.DeSerialize(data);
+    auto read_size = host_eos_2.DeSerialize(data.get());
     THEN("We can serialize!") {
       host_eos_2.CheckParams();
       REQUIRE(size == read_size);
@@ -158,6 +158,8 @@ SCENARIO("Helmholtz equation of state - Table interpolation (tgiven)", "[Helmhol
     REQUIRE(nwrong == 0);
     eos.Finalize();
     host_eos.Finalize();
+    eos_2.Finalize();
+    host_eos_2.Finalize();
   }
 }
 

test/test_eos_ideal.cpp

@@ -300,10 +300,10 @@ SCENARIO("Ideal gas serialization", "[IdealGas][Serialization]") {
 
       THEN("We can de-serialize new objects from them") {
         IdealGas new_bare;
-        new_bare.DeSerialize(data_bare);
+        new_bare.DeSerialize(data_bare.get());
 
         EOS new_variant;
-        new_variant.DeSerialize(data_var);
+        new_variant.DeSerialize(data_var.get());
 
         AND_THEN("The bare eos has the right Cv and Gruneisen params") {
           REQUIRE(new_bare.SpecificHeatFromDensityTemperature(1.0, 1.0) == Cv);
@@ -318,11 +318,10 @@ SCENARIO("Ideal gas serialization", "[IdealGas][Serialization]") {
           REQUIRE(new_variant.SpecificHeatFromDensityTemperature(1.0, 1.0) == Cv);
           REQUIRE(new_variant.GruneisenParamFromDensityTemperature(1.0, 1.0) == gm1);
         }
-      }
 
-      // cleanup
-      free(data_bare);
-      free(data_var);
+        new_bare.Finalize();
+        new_variant.Finalize();
+      }
     }
 
     // cleanup

test/test_eos_modifiers_minimal.cpp

@@ -206,7 +206,7 @@ SCENARIO("Serialization of modified EOSs preserves their properties",
 
       THEN("We can de-serialize the EOS") {
         singularity::VectorSerializer<EOS> deserializer;
-        deserializer.DeSerialize(data);
+        deserializer.DeSerialize(data.get());
         REQUIRE(deserializer.Size() == serializer.Size());
 
         AND_THEN("The de-serialized EOS still evaluates properly") {
@@ -216,8 +216,6 @@ SCENARIO("Serialization of modified EOSs preserves their properties",
                       temp_test, EPS));
         }
       }
-
-      free(data);
     }
 
     eos.Finalize();

test/test_eos_stellar_collapse.cpp

@@ -308,7 +308,7 @@ SCENARIO("Stellar Collapse EOS", "[StellarCollapse]") {
         REQUIRE(size > 0);
         THEN("We can de-serialize it into a new object") {
           StellarCollapse sc2;
-          std::size_t read_size = sc2.DeSerialize(data);
+          std::size_t read_size = sc2.DeSerialize(data.get());
           REQUIRE(read_size == size);
           REQUIRE(size > sizeof(StellarCollapse));
           sc2.CheckParams();
@@ -320,7 +320,7 @@ SCENARIO("Stellar Collapse EOS", "[StellarCollapse]") {
           }
           AND_THEN("We can de-serialize into two objects") {
             StellarCollapse sc3;
-            std::size_t read_size_2 = sc3.DeSerialize(data);
+            std::size_t read_size_2 = sc3.DeSerialize(data.get());
             REQUIRE(read_size_2 == size);
             sc3.CheckParams();
             AND_THEN("The two de-serialized objects use the same memory") {
@@ -335,7 +335,7 @@ SCENARIO("Stellar Collapse EOS", "[StellarCollapse]") {
             char *shared_data = (char *)malloc(shared_size);
             SharedMemSettings settings(shared_data, true);
             StellarCollapse sc3;
-            std::size_t read_size = sc3.DeSerialize(data, settings);
+            std::size_t read_size = sc3.DeSerialize(data.get(), settings);
             REQUIRE(read_size == size);
             REQUIRE(read_size > sizeof(StellarCollapse));
             sc3.CheckParams();
@@ -353,7 +353,7 @@ SCENARIO("Stellar Collapse EOS", "[StellarCollapse]") {
             AND_THEN("We can de-serialize from shared memory around one more object") {
               SharedMemSettings settings2(shared_data, false);
               StellarCollapse sc4;
-              std::size_t read_size_2 = sc4.DeSerialize(data, settings);
+              std::size_t read_size_2 = sc4.DeSerialize(data.get(), settings);
               REQUIRE(read_size_2 == size);
               REQUIRE(read_size_2 > sizeof(StellarCollapse));
               sc4.CheckParams();
@@ -367,7 +367,6 @@ SCENARIO("Stellar Collapse EOS", "[StellarCollapse]") {
             free(shared_data);
           }
         }
-        free(data);
       }
 
       WHEN("We serialize a variant that owns a StellarCollapse EOS") {
@@ -378,7 +377,7 @@ SCENARIO("Stellar Collapse EOS", "[StellarCollapse]") {
         REQUIRE(size > 0);
         THEN("We can de-serialize it into a new object") {
           EOS e2;
-          std::size_t read_size = e2.DeSerialize(data);
+          std::size_t read_size = e2.DeSerialize(data.get());
           REQUIRE(read_size == size);
           e2.CheckParams();
           AND_THEN("The two stellar collapse EOS's agree") {
@@ -394,12 +393,12 @@ SCENARIO("Stellar Collapse EOS", "[StellarCollapse]") {
             EOS e3, e4;
 
             std::size_t read_size_e3 =
-                e3.DeSerialize(data, SharedMemSettings(shared_data, true));
+                e3.DeSerialize(data.get(), SharedMemSettings(shared_data, true));
             REQUIRE(read_size_e3 == size);
             e3.CheckParams();
 
             std::size_t read_size_e4 =
-                e4.DeSerialize(data, SharedMemSettings(shared_data, false));
+                e4.DeSerialize(data.get(), SharedMemSettings(shared_data, false));
             REQUIRE(read_size_e4 == size);
             e4.CheckParams();
 
@@ -417,7 +416,6 @@ SCENARIO("Stellar Collapse EOS", "[StellarCollapse]") {
             free(shared_data);
           }
         }
-        free(data);
       }
       sc.Finalize();
     }

test/test_eos_tabulated.cpp

@@ -375,27 +375,27 @@ SCENARIO("SpinerEOS and EOSPAC Serialization",
         char *air_shared_data = (char *)malloc(air_shared_size);
 
         SpinerEOSDependsRhoT eos_rhoT;
-        std::size_t read_size_rhoT =
-            eos_rhoT.DeSerialize(rhoT_data, SharedMemSettings(rhoT_shared_data, true));
+        std::size_t read_size_rhoT = eos_rhoT.DeSerialize(
+            rhoT_data.get(), SharedMemSettings(rhoT_shared_data, true));
         REQUIRE(read_size_rhoT == rhoT_size);
         REQUIRE(RhoTTricks::DataBoxesPointToDifferentMemory(rhoT_orig, eos_rhoT));
 
         SpinerEOSDependsRhoSie eos_rhoSie;
         std::size_t read_size_rhoSie = eos_rhoSie.DeSerialize(
-            rhoSie_data, SharedMemSettings(rhoSie_shared_data, true));
+            rhoSie_data.get(), SharedMemSettings(rhoSie_shared_data, true));
         REQUIRE(read_size_rhoSie == rhoSie_size);
         REQUIRE(RhoSieTricks::DataBoxesPointToDifferentMemory(rhoSie_orig, eos_rhoSie));
 
         eospac_orig.Finalize();
         EOS eos_eospac = EOSPAC();
         std::size_t read_size_eospac = eos_eospac.DeSerialize(
-            eospac_data, SharedMemSettings(eospac_shared_data, true));
+            eospac_data.get(), SharedMemSettings(eospac_shared_data, true));
         REQUIRE(read_size_eospac == eospac_size);
 
         eospac_air.Finalize();
         EOS eos_air_2 = EOSPAC();
-        std::size_t read_size_air =
-            eos_air_2.DeSerialize(air_data, SharedMemSettings(air_shared_data, true));
+        std::size_t read_size_air = eos_air_2.DeSerialize(
+            air_data.get(), SharedMemSettings(air_shared_data, true));
         REQUIRE(read_size_air == air_size);
 
         AND_THEN("EOS lookups work") {
@@ -430,9 +430,6 @@ SCENARIO("SpinerEOS and EOSPAC Serialization",
         free(rhoSie_shared_data);
         free(eospac_shared_data);
       }
-      free(rhoT_data);
-      free(rhoSie_data);
-      free(eospac_data);
     }
 
     rhoT_orig.Finalize();