AP_Terrain: allocate cache only in update method

This simplifies the check other parts of the system need to make.

This also avoids potential race conditions causing duplicate allocations
and leaks when multiple threads (e.g. scripting) call `height_amsl`.

Requires a slight hack to also allocate when unit tests force-enable the
library. This way it becomes active so the tests work.

libraries/AP_Terrain/AP_Terrain.cpp

@@ -114,7 +114,7 @@ AP_Terrain::AP_Terrain() :
  */
 bool AP_Terrain::height_amsl(const Location &loc, float &height, bool corrected)
 {
-    if (!allocate()) {
+    if (!active()) {
         return false;
     }
 
@@ -317,7 +317,7 @@ bool AP_Terrain::height_relative_home_equivalent(float terrain_altitude,
 */
 float AP_Terrain::lookahead(float bearing, float distance, float climb_ratio)
 {
-    if (!allocate() || grid_spacing <= 0) {
+    if (!active() || grid_spacing <= 0) {
         return 0;
     }
 
@@ -361,6 +361,11 @@ float AP_Terrain::lookahead(float bearing, float distance, float climb_ratio)
 void AP_Terrain::update(void)
 {
     if (!enable) { return; }
+
+    if (cache == nullptr && !memory_alloc_failed) {
+        allocate();
+    }
+
     // just schedule any needed disk IO
     schedule_disk_io();
 
@@ -398,7 +403,7 @@ void AP_Terrain::update(void)
     }
 
     // update capabilities and status
-    if (allocate()) {
+    if (active()) {
         if (!pos_valid) {
             // we don't know where we are
             system_status = TerrainStatusUnhealthy;
@@ -465,7 +470,7 @@ bool AP_Terrain::pre_arm_checks(char *failure_msg, uint8_t failure_msg_len) cons
 #if HAL_LOGGING_ENABLED
 void AP_Terrain::log_terrain_data()
 {
-    if (!allocate()) {
+    if (!active()) {
         return;
     }
     Location loc;
@@ -502,22 +507,15 @@ void AP_Terrain::log_terrain_data()
   allocate terrain cache. Making this dynamically allocated allows
   memory to be saved when terrain functionality is disabled
  */
-bool AP_Terrain::allocate(void)
+void AP_Terrain::allocate(void)
 {
-    if (enable == 0 || memory_alloc_failed) {
-        return false;
-    }
-    if (cache != nullptr) {
-        return true;
-    }
     cache = (struct grid_cache *)calloc(config_cache_size, sizeof(cache[0]));
     if (cache == nullptr) {
         GCS_SEND_TEXT(MAV_SEVERITY_CRITICAL, "Terrain: Allocation failed");
         memory_alloc_failed = true;
-        return false;
+        return;
     }
     cache_size = config_cache_size;
-    return true;
 }
 
 /*

libraries/AP_Terrain/AP_Terrain.h

@@ -100,7 +100,14 @@ public:
     void update(void);
 
     bool enabled() const { return enable; }
-    void set_enabled(bool _enable) { enable.set(_enable); }
+
+    // only called by unit tests!
+    void set_enabled(bool _enable) {
+        enable.set(_enable);
+        if (_enable && cache == nullptr && !memory_alloc_failed) {
+            allocate();
+        }
+    }
 
     // return status enum for health reporting
     enum TerrainStatus status(void) const { return system_status; }
@@ -211,8 +218,12 @@ public:
 #endif  // HAL_GCS_ENABLED
 
 private:
-    // allocate the terrain subsystem data
-    bool allocate(void);
+    // true if system is enabled and ready to go
+    bool active(void) {
+        return (enable && cache != nullptr);
+    }
+
+    void allocate(void);
 
     /*
       a grid block is a structure in a local file containing height

libraries/AP_Terrain/TerrainGCS.cpp

@@ -110,8 +110,7 @@ bool AP_Terrain::send_cache_request(GCS_MAVLINK &link)
  */
 void AP_Terrain::send_request(GCS_MAVLINK &link)
 {
-    if (!allocate()) {
-        // not enabled
+    if (!active()) {
         return;
     }
 

libraries/AP_Terrain/TerrainIO.cpp

@@ -62,7 +62,7 @@ void AP_Terrain::check_disk_write(void)
  */
 void AP_Terrain::schedule_disk_io(void)
 {
-    if (enable == 0 || !allocate() || diskless()) {
+    if (!active() || diskless()) {
         return;
     }