Fix issue with gdal_viewshed with gcc 9.4 in debug mode (master only)

alg/viewshed/combiner.h

@@ -20,7 +20,8 @@
  * DEALINGS IN THE SOFTWARE.
  ****************************************************************************/
 
-#pragma once
+#ifndef VIEWSHED_COMBINER_H_INCLUDED
+#define VIEWSHED_COMBINER_H_INCLUDED
 
 #include "cumulative.h"
 #include "viewshed_types.h"
@@ -67,3 +68,5 @@ class Combiner
 
 }  // namespace viewshed
 }  // namespace gdal
+
+#endif

alg/viewshed/cumulative.cpp

@@ -44,6 +44,10 @@ Cumulative::Cumulative(const Options &opts) : m_opts(opts)
 {
 }
 
+/// Destructor
+///
+Cumulative::~Cumulative() = default;
+
 /// Compute the cumulative viewshed of a raster band.
 ///
 /// @param srcFilename  Source filename.

alg/viewshed/cumulative.h

@@ -20,7 +20,8 @@
  * DEALINGS IN THE SOFTWARE.
  ****************************************************************************/
 
-#pragma once
+#ifndef VIEWSHED_CUMULATIVE_H_INCLUDED
+#define VIEWSHED_CUMULATIVE_H_INCLUDED
 
 #include <atomic>
 #include <vector>
@@ -41,6 +42,11 @@ class Cumulative
 {
   public:
     CPL_DLL explicit Cumulative(const Options &opts);
+    // We define an explicit destructor, whose implementation is in libgdal,
+    // otherwise with gcc 9.4 of Ubuntu 20.04 in debug mode, this would need to
+    // redefinition of the NotifyQueue class in both libgdal and gdal_viewshed,
+    // leading to weird things related to mutex.
+    CPL_DLL ~Cumulative();
     CPL_DLL bool run(const std::string &srcFilename,
                      GDALProgressFunc pfnProgress = GDALDummyProgress,
                      void *pProgressArg = nullptr);
@@ -70,7 +76,12 @@ class Cumulative
     void rollupRasters();
     void scaleOutput();
     bool writeOutput(DatasetPtr pDstDS);
+
+    Cumulative(const Cumulative &) = delete;
+    Cumulative &operator=(const Cumulative &) = delete;
 };
 
 }  // namespace viewshed
 }  // namespace gdal
+
+#endif

alg/viewshed/notifyqueue.h

@@ -19,7 +19,11 @@
  * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
  * DEALINGS IN THE SOFTWARE.
  ****************************************************************************/
-#pragma once
+
+#ifndef VIEWSHED_NOTIFYQUEUE_H_INCLUDED
+#define VIEWSHED_NOTIFYQUEUE_H_INCLUDED
+
+#include "cpl_port.h"
 
 #include <condition_variable>
 #include <mutex>
@@ -134,3 +138,5 @@ template <class T> class NotifyQueue
 
 }  // namespace viewshed
 }  // namespace gdal
+
+#endif

alg/viewshed/progress.h

@@ -20,7 +20,8 @@
  * DEALINGS IN THE SOFTWARE.
  ****************************************************************************/
 
-#pragma once
+#ifndef VIEWSHED_PROGRESS_H_INCLUDED
+#define VIEWSHED_PROGRESS_H_INCLUDED
 
 #include <functional>
 #include <mutex>
@@ -54,3 +55,5 @@ class Progress
 
 }  // namespace viewshed
 }  // namespace gdal
+
+#endif

alg/viewshed/util.h

@@ -20,7 +20,8 @@
  * DEALINGS IN THE SOFTWARE.
  ****************************************************************************/
 
-#pragma once
+#ifndef VIEWSHED_UTIL_H_INCLUDED
+#define VIEWSHED_UTIL_H_INCLUDED
 
 #include "viewshed_types.h"
 
@@ -36,3 +37,5 @@ DatasetPtr createOutputDataset(GDALRasterBand &srcBand, const Options &opts,
 
 }  // namespace viewshed
 }  // namespace gdal
+
+#endif

alg/viewshed/viewshed.cpp

@@ -234,6 +234,12 @@ bool getTransforms(GDALRasterBand &band, double *pFwdTransform,
 
 }  // unnamed namespace
 
+Viewshed::Viewshed(const Options &opts) : oOpts{opts}
+{
+}
+
+Viewshed::~Viewshed() = default;
+
 /// Calculate the extent of the output raster in terms of the input raster and
 /// save the input raster extent.
 ///

alg/viewshed/viewshed.h

@@ -27,7 +27,8 @@
  * DEALINGS IN THE SOFTWARE.
  ****************************************************************************/
 
-#pragma once
+#ifndef VIEWSHED_H_INCLUDED
+#define VIEWSHED_H_INCLUDED
 
 #include <algorithm>
 #include <array>
@@ -59,13 +60,10 @@ class Viewshed
      *
      * @param opts Options to use when calculating viewshed.
     */
-    CPL_DLL explicit Viewshed(const Options &opts)
-        : oOpts{opts}, oOutExtent{}, oCurExtent{}, poDstDS{}, pSrcBand{}
-    {
-    }
+    CPL_DLL explicit Viewshed(const Options &opts);
 
-    Viewshed(const Viewshed &) = delete;
-    Viewshed &operator=(const Viewshed &) = delete;
+    /** Destructor */
+    CPL_DLL ~Viewshed();
 
     CPL_DLL bool run(GDALRasterBandH hBand,
                      GDALProgressFunc pfnProgress = GDALDummyProgress,
@@ -83,10 +81,10 @@ class Viewshed
 
   private:
     Options oOpts;
-    Window oOutExtent;
-    Window oCurExtent;
-    DatasetPtr poDstDS;
-    GDALRasterBand *pSrcBand;
+    Window oOutExtent{};
+    Window oCurExtent{};
+    DatasetPtr poDstDS{};
+    GDALRasterBand *pSrcBand = nullptr;
 
     DatasetPtr execute(int nX, int nY, const std::string &outFilename);
     void setOutput(double &dfResult, double &dfCellVal, double dfZ);
@@ -96,7 +94,12 @@ class Viewshed
                                      std::vector<double> &thisLineVal);
     bool calcExtents(int nX, int nY,
                      const std::array<double, 6> &adfInvTransform);
+
+    Viewshed(const Viewshed &) = delete;
+    Viewshed &operator=(const Viewshed &) = delete;
 };
 
 }  // namespace viewshed
 }  // namespace gdal
+
+#endif

alg/viewshed/viewshed_types.h

@@ -19,7 +19,8 @@
  * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
  * DEALINGS IN THE SOFTWARE.
  ****************************************************************************/
-#pragma once
+#ifndef VIEWSHED_TYPES_H_INCLUDED
+#define VIEWSHED_TYPES_H_INCLUDED
 
 #include <algorithm>
 #include <string>
@@ -171,3 +172,5 @@ struct Window
 
 }  // namespace viewshed
 }  // namespace gdal
+
+#endif

port/cpl_worker_thread_pool.cpp

@@ -186,11 +186,8 @@ bool CPLWorkerThreadPool::SubmitJob(std::function<void()> task)
     if (static_cast<int>(aWT.size()) < m_nMaxThreads)
     {
         // CPLDebug("CPL", "Starting new thread...");
-        std::unique_ptr<CPLWorkerThread> wt(new CPLWorkerThread);
-        wt->pfnInitFunc = nullptr;
-        wt->pInitData = nullptr;
+        auto wt = std::make_unique<CPLWorkerThread>();
         wt->poTP = this;
-        wt->bMarkedAsWaiting = false;
         //ABELL - Why should this fail? And this is a *pool* thread, not necessarily
         //  tied to the submitted job. The submitted job still needs to run, even if
         //  this fails. If we can't create a thread, should the entire pool become invalid?
@@ -279,10 +276,7 @@ bool CPLWorkerThreadPool::SubmitJobs(CPLThreadFunc pfnFunc,
         if (static_cast<int>(aWT.size()) < m_nMaxThreads)
         {
             std::unique_ptr<CPLWorkerThread> wt(new CPLWorkerThread);
-            wt->pfnInitFunc = nullptr;
-            wt->pInitData = nullptr;
             wt->poTP = this;
-            wt->bMarkedAsWaiting = false;
             wt->hThread =
                 CPLCreateJoinableThread(WorkerThreadFunction, wt.get());
             if (wt->hThread == nullptr)
@@ -425,10 +419,6 @@ bool CPLWorkerThreadPool::Setup(int nThreads, CPLThreadFunc pfnInitFunc,
         wt->pfnInitFunc = pfnInitFunc;
         wt->pInitData = pasInitData ? pasInitData[i] : nullptr;
         wt->poTP = this;
-        {
-            std::lock_guard<std::mutex> oGuard(wt->m_mutex);
-            wt->bMarkedAsWaiting = false;
-        }
         wt->hThread = CPLCreateJoinableThread(WorkerThreadFunction, wt.get());
         if (wt->hThread == nullptr)
         {
@@ -529,10 +519,7 @@ CPLWorkerThreadPool::GetNextJob(CPLWorkerThread *psWorkerThread)
         std::unique_lock<std::mutex> oGuardThisThread(psWorkerThread->m_mutex);
         // coverity[uninit_use_in_call]
         oGuard.unlock();
-        while (psWorkerThread->bMarkedAsWaiting && eState != CPLWTS_STOP)
-        {
-            psWorkerThread->m_cv.wait(oGuardThisThread);
-        }
+        psWorkerThread->m_cv.wait(oGuardThisThread);
     }
 }