BugFix: DeviceAgentVector's ownership could be passed out of scope.

DeviceAgentVector's root shared_ptr is now owned by the CUDAAgent.

This has a tiny optimisation benefit of reducing unnecessary syncs, if consecutive host fns use the same device vector.

Closes #522

include/flamegpu/gpu/CUDAAgent.h

@@ -288,6 +288,22 @@ class CUDAAgent : public AgentInterface {
      * @param hostapi HostAPI object, this is used to provide cub temp storage
      */
     void assignIDs(HostAPI &hostapi);
+    /**
+     * Used to allow HostAgentAPI to store a persistent DeviceAgentVector
+     * @param state_name Agent state to affect
+     * @param d_vec The DeviceAgentVector to be stored
+     */
+    void setPopulationVec(const std::string& state_name, const std::shared_ptr<DeviceAgentVector_impl>& d_vec);
+    /**
+     * Used to allow HostAgentAPI to retrieve a stored DeviceAgentVector
+     * @param state_name Agent state to affect
+     */
+    std::shared_ptr<DeviceAgentVector_impl> getPopulationVec(const std::string& state_name);
+    /**
+     * Used to allow HostAgentAPI to clear the stored DeviceAgentVector
+     * Any changes will be synchronised first
+     */
+    void resetPopulationVecs();
 
  private:
     /**
@@ -356,6 +372,10 @@ class CUDAAgent : public AgentInterface {
      * Mutex for writing to newBuffs
      */
     std::mutex newBuffsMutex;
+    /**
+     * Nullptr until getPopulationData() is called, after which it holds the return value
+     */
+    std::map<std::string, std::shared_ptr<DeviceAgentVector_impl>> population_dvec;
 };
 
 }  // namespace flamegpu

include/flamegpu/runtime/HostAgentAPI.cuh

@@ -73,15 +73,8 @@ class HostAgentAPI {
         : api(_api)
         , agent(_agent)
         , stateName(_stateName)
-        , population(nullptr)
         , agentOffsets(_agentOffsets)
         , newAgentData(_newAgentData) { }
-    /**
-     * Destructor
-     *
-     * Ensures any changes to agent data or births are synchronised prior to the host function returning.
-     */
-    ~HostAgentAPI();
     /**
      * Copy constructor
      * Not actually sure this is required
@@ -90,7 +83,6 @@ class HostAgentAPI {
         : api(other.api)
         , agent(other.agent)
         , stateName(other.stateName)
-        , population(nullptr)  // Never copy DeviceAgentVector
         , agentOffsets(other.agentOffsets)
         , newAgentData(other.newAgentData)
     { }
@@ -271,10 +263,6 @@ class HostAgentAPI {
      * Agent state being accessed
      */
     const std::string stateName;
-    /**
-     * Nullptr until getPopulationData() is called, after which it holds the return value
-     */
-    std::shared_ptr<DeviceAgentVector_impl> population;
     /**
      * Holds offsets for accessing newAgentData
      * @see newAgent()
@@ -298,6 +286,7 @@ InT HostAgentAPI::sum(const std::string &variable) const {
 template<typename InT, typename OutT>
 OutT HostAgentAPI::sum(const std::string &variable) const {
     static_assert(sizeof(InT) <= sizeof(OutT), "Template arg OutT should not be of a smaller size than InT");
+    std::shared_ptr<DeviceAgentVector_impl> population = agent.getPopulationVec(stateName);
     if (population) {
         // If the user has a DeviceAgentVector out, sync changes
         population->syncChanges();
@@ -333,6 +322,7 @@ OutT HostAgentAPI::sum(const std::string &variable) const {
 }
 template<typename InT>
 InT HostAgentAPI::min(const std::string &variable) const {
+    std::shared_ptr<DeviceAgentVector_impl> population = agent.getPopulationVec(stateName);
     if (population) {
         // If the user has a DeviceAgentVector out, sync changes
         population->syncChanges();
@@ -368,6 +358,7 @@ InT HostAgentAPI::min(const std::string &variable) const {
 }
 template<typename InT>
 InT HostAgentAPI::max(const std::string &variable) const {
+    std::shared_ptr<DeviceAgentVector_impl> population = agent.getPopulationVec(stateName);
     if (population) {
         // If the user has a DeviceAgentVector out, sync changes
         population->syncChanges();
@@ -403,6 +394,7 @@ InT HostAgentAPI::max(const std::string &variable) const {
 }
 template<typename InT>
 unsigned int HostAgentAPI::count(const std::string &variable, const InT &value) {
+    std::shared_ptr<DeviceAgentVector_impl> population = agent.getPopulationVec(stateName);
     if (population) {
         // If the user has a DeviceAgentVector out, sync changes
         population->syncChanges();
@@ -430,6 +422,7 @@ std::vector<unsigned int> HostAgentAPI::histogramEven(const std::string &variabl
 }
 template<typename InT, typename OutT>
 std::vector<OutT> HostAgentAPI::histogramEven(const std::string &variable, const unsigned int &histogramBins, const InT &lowerBound, const InT &upperBound) const {
+    std::shared_ptr<DeviceAgentVector_impl> population = agent.getPopulationVec(stateName);
     if (population) {
         // If the user has a DeviceAgentVector out, sync changes
         population->syncChanges();
@@ -471,6 +464,7 @@ std::vector<OutT> HostAgentAPI::histogramEven(const std::string &variable, const
 }
 template<typename InT, typename reductionOperatorT>
 InT HostAgentAPI::reduce(const std::string &variable, reductionOperatorT /*reductionOperator*/, const InT &init) const {
+    std::shared_ptr<DeviceAgentVector_impl> population = agent.getPopulationVec(stateName);
     if (population) {
         // If the user has a DeviceAgentVector out, sync changes
         population->syncChanges();
@@ -508,6 +502,7 @@ InT HostAgentAPI::reduce(const std::string &variable, reductionOperatorT /*reduc
 }
 template<typename InT, typename OutT, typename transformOperatorT, typename reductionOperatorT>
 OutT HostAgentAPI::transformReduce(const std::string &variable, transformOperatorT /*transformOperator*/, reductionOperatorT /*reductionOperator*/, const OutT &init) const {
+    std::shared_ptr<DeviceAgentVector_impl> population = agent.getPopulationVec(stateName);
     if (population) {
         // If the user has a DeviceAgentVector out, sync changes
         population->syncChanges();
@@ -533,6 +528,7 @@ OutT HostAgentAPI::transformReduce(const std::string &variable, transformOperato
 
 template<typename VarT>
 void HostAgentAPI::sort(const std::string &variable, Order order, int beginBit, int endBit) {
+    std::shared_ptr<DeviceAgentVector_impl> population = agent.getPopulationVec(stateName);
     if (population) {
         // If the user has a DeviceAgentVector out, sync changes
         population->syncChanges();
@@ -595,6 +591,7 @@ void HostAgentAPI::sort(const std::string &variable, Order order, int beginBit,
 
 template<typename Var1T, typename Var2T>
 void HostAgentAPI::sort(const std::string &variable1, Order order1, const std::string &variable2, Order order2) {
+    std::shared_ptr<DeviceAgentVector_impl> population = agent.getPopulationVec(stateName);
     if (population) {
         // If the user has a DeviceAgentVector out, sync changes
         population->syncChanges();

include/flamegpu/sim/AgentInterface.h

@@ -2,12 +2,14 @@
 #define INCLUDE_FLAMEGPU_SIM_AGENTINTERFACE_H_
 
 #include <string>
+#include <memory>
 
 #include "flamegpu/model/ModelData.h"
 #include "flamegpu/defines.h"
 
-namespace flamegpu {
 
+namespace flamegpu {
+class DeviceAgentVector_impl;
 /**
  * Base-class (interface) for classes like CUDAAgent, which provide access to agent data
  */
@@ -23,6 +25,28 @@ class AgentInterface {
      * @return An ID that can be assigned to an agent that wil be stored within this Agent collection
      */
     virtual id_t nextID(unsigned int count) = 0;
+    /**
+     * Used to allow HostAgentAPI to store a persistent DeviceAgentVector
+     * @param state_name Agent state to affect
+     * @param d_vec The DeviceAgentVector to be stored
+     *
+     * @note The presence of this inside AgentInterface is questionable, and should be made more generic in future if HostSimulation is created
+     */
+    virtual void setPopulationVec(const std::string &state_name, const std::shared_ptr<DeviceAgentVector_impl>& d_vec) = 0;
+    /**
+     * Used to allow HostAgentAPI to retrieve a stored DeviceAgentVector
+     * @param state_name Agent state to affect
+     *
+     * @note The presence of this inside AgentInterface is questionable, and should be made more generic in future if HostSimulation is created
+     */
+    virtual std::shared_ptr<DeviceAgentVector_impl> getPopulationVec(const std::string& state_name) = 0;
+    /**
+     * Used to allow HostAgentAPI to clear the stored DeviceAgentVector
+     * Any changes will be synchronised first
+     *
+     * @note The presence of this inside AgentInterface is questionable, and should be made more generic in future if HostSimulation is created
+     */
+    virtual void resetPopulationVecs() = 0;
 };
 
 }  // namespace flamegpu

src/flamegpu/gpu/CUDAAgent.cu

@@ -39,6 +39,7 @@ using std::experimental::filesystem::v1::path;
 #include "flamegpu/gpu/CUDAScatter.cuh"
 #include "flamegpu/util/detail/compute_capability.cuh"
 #include "flamegpu/util/nvtx.h"
+#include "flamegpu/pop/DeviceAgentVector_impl.h"
 
 namespace flamegpu {
 
@@ -718,4 +719,23 @@ void CUDAAgent::assignIDs(HostAPI& hostapi) {
     fat_agent->assignIDs(hostapi);
 }
 
+void CUDAAgent::setPopulationVec(const std::string& state_name, const std::shared_ptr<DeviceAgentVector_impl>& d_vec) {
+    population_dvec[state_name] = d_vec;
+}
+std::shared_ptr<DeviceAgentVector_impl> CUDAAgent::getPopulationVec(const std::string& state_name) {
+    auto find = population_dvec.find(state_name);
+    if (find != population_dvec.end())
+        return find->second;
+    return nullptr;
+}
+void CUDAAgent::resetPopulationVecs() {
+    for (auto &vec : population_dvec) {
+        if (vec.second) {
+            vec.second->syncChanges();
+            vec.second.reset();
+        }
+    }
+    population_dvec.clear();
+}
+
 }  // namespace flamegpu

src/flamegpu/gpu/CUDASimulation.cu

@@ -249,6 +249,10 @@ void CUDASimulation::initFunctions() {
     }
     // Check if host agent creation was used in init functions
     if (model->initFunctions.size() || model->initFunctionCallbacks.size()) {
+        // Sync any device vectors, before performing host agent creation
+        for (auto& ca : agent_map) {
+            ca.second->resetPopulationVecs();
+        }
         processHostAgentCreation(0);
     }
 
@@ -1039,6 +1043,10 @@ void CUDASimulation::layerHostFunctions(const std::shared_ptr<LayerData>& layer,
     }
     // If we have host layer functions, we might have host agent creation
     if (layer->host_functions.size() || (layer->host_functions_callbacks.size())) {
+        // Sync any device vectors, before performing host agent creation
+        for (auto& ca : agent_map) {
+            ca.second->resetPopulationVecs();
+        }
         // @todo - What is the most appropriate stream to use here?
         processHostAgentCreation(0);
     }
@@ -1058,6 +1066,10 @@ void CUDASimulation::stepStepFunctions() {
     }
     // If we have step functions, we might have host agent creation
     if (model->stepFunctions.size() || model->stepFunctionCallbacks.size()) {
+        // Sync any device vectors, before performing host agent creation
+        for (auto &ca : agent_map) {
+            ca.second->resetPopulationVecs();
+        }
         processHostAgentCreation(0);
     }
 }

src/flamegpu/runtime/HostAgentAPI.cu

@@ -3,13 +3,6 @@
 
 namespace flamegpu {
 
-HostAgentAPI::~HostAgentAPI() {
-    if (population) {
-        population->syncChanges();
-        population.reset();
-    }
-}
-
 HostNewAgentAPI HostAgentAPI::newAgent() {
     // Create the agent in our backing data structure
     newAgentData.emplace_back(NewAgentStorage(agentOffsets, agent.nextID(1)));
@@ -18,9 +11,10 @@ HostNewAgentAPI HostAgentAPI::newAgent() {
 }
 
 unsigned HostAgentAPI::count() {
-    if (population) {
+    std::shared_ptr<DeviceAgentVector_impl> d_vec = agent.getPopulationVec(stateName);
+    if (d_vec) {
         // If the user has a DeviceAgentVector out, use that instead
-        return population->size();
+        return d_vec->size();
     }
     return agent.getStateSize(stateName);
 }
@@ -51,10 +45,13 @@ void HostAgentAPI::sortBuffer(void *dest, void*src, unsigned int *position, cons
 
 DeviceAgentVector HostAgentAPI::getPopulationData() {
     // Create and return a new AgentVector
-    if (!population) {
-        population = std::make_shared<DeviceAgentVector_impl>(static_cast<CUDAAgent&>(agent), stateName, agentOffsets, newAgentData, api.scatter, api.streamId, api.stream);
+    std::shared_ptr<DeviceAgentVector_impl> d_vec = agent.getPopulationVec(stateName);
+
+    if (!d_vec) {
+        d_vec = std::make_shared<DeviceAgentVector_impl>(static_cast<CUDAAgent&>(agent), stateName, agentOffsets, newAgentData, api.scatter, api.streamId, api.stream);
+        agent.setPopulationVec(stateName, d_vec);
     }
-    return *population;
+    return *d_vec;
 }
 
 }  // namespace flamegpu

tests/test_cases/pop/test_device_agent_vector.cu

@@ -15,8 +15,8 @@ namespace DeviceAgentVectorTest {
     const std::string AGENT_NAME = "agent";
 
 FLAMEGPU_STEP_FUNCTION(SetGet) {
-    HostAgentAPI agent = FLAMEGPU->agent(AGENT_NAME);
-    DeviceAgentVector av = agent.getPopulationData();
+    // Accessing DeviceAgentVector like this would previously lead to an access violation (Issue #522, PR #751)
+    DeviceAgentVector av = FLAMEGPU->agent(AGENT_NAME).getPopulationData();
     for (AgentVector::Agent ai : av) {
         ai.setVariable<int>("int", ai.getVariable<int>("int") + 12);
     }