new version: 2020.3.3

Fixes crashes where a request times-out, but then is completed by
UDN sometime afterwards, with a free-d object. Have the DNS::Client own
requests, and be able to retrieve the udns_query to cancel them, in 
the timeout case.

Fixes a couple of Sentry reports.

CMakeLists.txt

@@ -49,7 +49,7 @@ set(CMAKE_CXX_STANDARD 11)
 set(CMAKE_CXX_STANDARD_REQUIRED YES)
 
 # read 'version' file into a variable (stripping any newlines or spaces)
-file(READ version versionFile)
+file(READ simgear-version versionFile)
 string(STRIP ${versionFile} SIMGEAR_VERSION)
 
 project(SimGear VERSION ${SIMGEAR_VERSION} LANGUAGES C CXX)
@@ -278,7 +278,14 @@ else()
     endif()
 endif(SIMGEAR_HEADLESS)
 
-find_package(ZLIB 1.2.4 REQUIRED)
+if(${CMAKE_SYSTEM_NAME} MATCHES "OpenBSD")
+    # As of 2020-08-01, OpenBSD's system zlib is slightly old, but it's usable
+    # with a workaround in simgear/io/iostreams/gzfstream.cxx.
+    find_package(ZLIB 1.2.3 REQUIRED)
+else()
+    find_package(ZLIB 1.2.4 REQUIRED)
+endif()
+
 find_package(CURL REQUIRED)
 
 if (SYSTEM_EXPAT)
@@ -433,6 +440,10 @@ if(CMAKE_COMPILER_IS_GNUCXX)
     check_cxx_source_compiles(
        "int main() { unsigned mValue; return __sync_add_and_fetch(&mValue, 1); }"
         GCC_ATOMIC_BUILTINS_FOUND)
+
+      # override CMake default RelWithDebInfo flags.
+    set(CMAKE_CXX_FLAGS_RELWITHDEBINFO "-O3 -g -DNDEBUG")
+    set(CMAKE_C_FLAGS_RELWITHDEBINFO  "-O3 -g -DNDEBUG")
 endif(CMAKE_COMPILER_IS_GNUCXX)
 
 if (CLANG)

simgear-version

@@ -0,0 +1 @@
+2020.3.3

simgear/CMakeLists.txt

@@ -167,7 +167,8 @@ target_link_libraries(SimGearCore PRIVATE
     ${CMAKE_THREAD_LIBS_INIT}
     ${COCOA_LIBRARY}
     ${CURL_LIBRARIES}
-    ${WINSOCK_LIBRARY})
+    ${WINSOCK_LIBRARY}
+    ${SHLWAPI_LIBRARY})
 
 if(SYSTEM_EXPAT)
     target_link_libraries(SimGearCore PRIVATE ${EXPAT_LIBRARIES})

simgear/canvas/elements/CanvasImage.cxx

@@ -850,7 +850,7 @@ namespace canvas
 void fillRow(GLubyte* row, GLuint pixel, GLuint width, GLuint pixelBytes)
 {
   GLubyte* dst = row;
-  for (int x = 0; x < width; ++x) {
+  for (GLuint x = 0; x < width; ++x) {
     memcpy(dst, &pixel, pixelBytes);
     dst += pixelBytes;
   }

simgear/canvas/layout/BoxLayout.cxx

@@ -73,7 +73,7 @@ namespace canvas
                               int stretch,
                               uint8_t alignment )
   {
-    ItemData item_data = {0};
+    ItemData item_data = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
     item_data.layout_item = item;
     item_data.stretch = std::max(0, stretch);
 

simgear/debug/BufferedLogCallback.hxx

@@ -26,7 +26,7 @@
 #include <vector>
 #include <memory> // for std::unique_ptr
 
-#include <simgear/debug/logstream.hxx>
+#include <simgear/debug/LogCallback.hxx>
 
 namespace simgear
 {

simgear/debug/CMakeLists.txt

@@ -1,7 +1,10 @@
 
 include (SimGearComponent)
 
-set(HEADERS debug_types.h logstream.hxx BufferedLogCallback.hxx OsgIoCapture.hxx)
-set(SOURCES logstream.cxx BufferedLogCallback.cxx)
+set(HEADERS debug_types.h 
+    logstream.hxx BufferedLogCallback.hxx OsgIoCapture.hxx
+    LogCallback.hxx LogEntry.hxx)
+set(SOURCES logstream.cxx BufferedLogCallback.cxx
+    LogCallback.cxx LogEntry.cxx)
 
 simgear_component(debug debug "${SOURCES}" "${HEADERS}")

simgear/debug/LogCallback.cxx

@@ -0,0 +1,116 @@
+// This library is free software; you can redistribute it and/or
+// modify it under the terms of the GNU Library General Public
+// License as published by the Free Software Foundation; either
+// version 2 of the License, or (at your option) any later version.
+//
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+// Library General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program; if not, write to the Free Software
+// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+//
+
+#include <simgear_config.h>
+
+#include "LogCallback.hxx"
+
+using namespace simgear;
+
+LogCallback::LogCallback(sgDebugClass c, sgDebugPriority p) : m_class(c),
+                                                              m_priority(p)
+{
+}
+
+void LogCallback::operator()(sgDebugClass c, sgDebugPriority p,
+                             const char* file, int line, const std::string& aMessage)
+{
+    // override me
+}
+
+bool LogCallback::doProcessEntry(const LogEntry& e)
+{
+    return false;
+}
+
+void LogCallback::processEntry(const LogEntry& e)
+{
+    if (doProcessEntry(e))
+        return; // derived class used the new API
+
+    // call the old API
+    (*this)(e.debugClass, e.debugPriority, e.file, e.line, e.message);
+}
+
+
+bool LogCallback::shouldLog(sgDebugClass c, sgDebugPriority p) const
+{
+    if ((c & m_class) != 0 && p >= m_priority)
+        return true;
+    if (c == SG_OSG) // always have OSG logging as it OSG logging is configured separately.
+        return true;
+    return false;
+}
+
+void LogCallback::setLogLevels(sgDebugClass c, sgDebugPriority p)
+{
+    m_priority = p;
+    m_class = c;
+}
+const char* LogCallback::debugPriorityToString(sgDebugPriority p)
+{
+    switch (p) {
+    case SG_DEV_ALERT:
+    case SG_ALERT:
+        return "ALRT";
+    case SG_BULK: return "BULK";
+    case SG_DEBUG: return "DBUG";
+    case SG_MANDATORY_INFO:
+    case SG_INFO:
+        return "INFO";
+    case SG_POPUP: return "POPU";
+    case SG_DEV_WARN:
+    case SG_WARN:
+        return "WARN";
+
+    default: return "UNKN";
+    }
+}
+
+const char* LogCallback::debugClassToString(sgDebugClass c)
+{
+    switch (c) {
+    case SG_NONE: return "none";
+    case SG_TERRAIN: return "terrain";
+    case SG_ASTRO: return "astro";
+    case SG_FLIGHT: return "flight";
+    case SG_INPUT: return "input";
+    case SG_GL: return "opengl";
+    case SG_VIEW: return "view";
+    case SG_COCKPIT: return "cockpit";
+    case SG_GENERAL: return "general";
+    case SG_MATH: return "math";
+    case SG_EVENT: return "event";
+    case SG_AIRCRAFT: return "aircraft";
+    case SG_AUTOPILOT: return "autopilot";
+    case SG_IO: return "io";
+    case SG_CLIPPER: return "clipper";
+    case SG_NETWORK: return "network";
+    case SG_ATC: return "atc";
+    case SG_NASAL: return "nasal";
+    case SG_INSTR: return "instruments";
+    case SG_SYSTEMS: return "systems";
+    case SG_AI: return "ai";
+    case SG_ENVIRONMENT: return "environment";
+    case SG_SOUND: return "sound";
+    case SG_NAVAID: return "navaid";
+    case SG_GUI: return "gui";
+    case SG_TERRASYNC: return "terrasync";
+    case SG_PARTICLES: return "particles";
+    case SG_HEADLESS: return "headless";
+    case SG_OSG: return "OSG";
+    default: return "unknown";
+    }
+}

simgear/debug/LogCallback.hxx

@@ -0,0 +1,56 @@
+// This library is free software; you can redistribute it and/or
+// modify it under the terms of the GNU Library General Public
+// License as published by the Free Software Foundation; either
+// version 2 of the License, or (at your option) any later version.
+//
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+// Library General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program; if not, write to the Free Software
+// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+//
+
+#pragma once
+
+#include <string>
+
+#include "LogEntry.hxx"
+#include "debug_types.h"
+
+namespace simgear {
+
+class LogCallback
+{
+public:
+    virtual ~LogCallback() = default;
+
+    // newer API: return true if you handled the message, otherwise
+    // the old API will be called
+    virtual bool doProcessEntry(const LogEntry& e);
+
+    // old API, kept for compatability
+    virtual void operator()(sgDebugClass c, sgDebugPriority p,
+                            const char* file, int line, const std::string& aMessage);
+
+    void setLogLevels(sgDebugClass c, sgDebugPriority p);
+
+    void processEntry(const LogEntry& e);
+
+protected:
+    LogCallback(sgDebugClass c, sgDebugPriority p);
+
+    bool shouldLog(sgDebugClass c, sgDebugPriority p) const;
+
+    static const char* debugClassToString(sgDebugClass c);
+    static const char* debugPriorityToString(sgDebugPriority p);
+
+private:
+    sgDebugClass m_class;
+    sgDebugPriority m_priority;
+};
+
+
+} // namespace simgear

simgear/debug/LogEntry.cxx

@@ -0,0 +1,45 @@
+
+// This library is free software; you can redistribute it and/or
+// modify it under the terms of the GNU Library General Public
+// License as published by the Free Software Foundation; either
+// version 2 of the License, or (at your option) any later version.
+//
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+// Library General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program; if not, write to the Free Software
+// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+//
+
+#include <simgear_config.h>
+
+#include "LogEntry.hxx"
+
+#include <cstring> // for strdup
+
+namespace simgear {
+
+LogEntry::~LogEntry()
+{
+    if (freeFilename) {
+        free(const_cast<char*>(file));
+    }
+}
+
+LogEntry::LogEntry(const LogEntry& c) : debugClass(c.debugClass),
+                                        debugPriority(c.debugPriority),
+                                        originalPriority(c.originalPriority),
+                                        file(c.file),
+                                        line(c.line),
+                                        message(c.message),
+                                        freeFilename(c.freeFilename)
+{
+    if (c.freeFilename) {
+        file = strdup(c.file);
+    }
+}
+
+} // namespace simgear

simgear/debug/LogEntry.hxx

@@ -0,0 +1,54 @@
+// This library is free software; you can redistribute it and/or
+// modify it under the terms of the GNU Library General Public
+// License as published by the Free Software Foundation; either
+// version 2 of the License, or (at your option) any later version.
+//
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+// Library General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program; if not, write to the Free Software
+// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+//
+
+#pragma once
+
+#include <string>
+
+#include "debug_types.h"
+
+namespace simgear {
+/**
+ * storage of a single log entry. This is used to pass log entries from
+ * the various threads to the logging thread, and also to store the startup
+ * entries
+ */
+class LogEntry
+{
+public:
+    LogEntry(sgDebugClass c, sgDebugPriority p,
+             sgDebugPriority op,
+             const char* f, int l, const std::string& msg) : debugClass(c), debugPriority(p), originalPriority(op),
+                                                             file(f), line(l),
+                                                             message(msg)
+    {
+    }
+
+    LogEntry(const LogEntry& c);
+    LogEntry& operator=(const LogEntry& c) = delete;
+
+    ~LogEntry();
+
+    const sgDebugClass debugClass;
+    const sgDebugPriority debugPriority;
+    const sgDebugPriority originalPriority;
+    const char* file;
+    const int line;
+    const std::string message;
+
+    bool freeFilename = false; ///< if true, we own, and therefore need to free, the memory pointed to by 'file'
+};
+
+} // namespace simgear

simgear/debug/OsgIoCapture.hxx

@@ -2,8 +2,7 @@
 
 #include <osg/Notify>
 
-using namespace osg;
-
+#include <simgear/debug/logstream.hxx>
 
 /**
 * merge OSG output into our logging system, so it gets recorded to file,

simgear/debug/debug_types.h

@@ -1,3 +1,5 @@
+#pragma once
+
 /** \file debug_types.h
  *  Define the various logging classes and priorities
  */
@@ -52,16 +54,17 @@ typedef enum {
  * appended, or the priority Nasal reports to compiled code will change.
  */
 typedef enum {
-    SG_BULK = 1,       // For frequent messages
-    SG_DEBUG,          // Less frequent debug type messages
-    SG_INFO,           // Informatory messages
-    SG_WARN,           // Possible impending problem
-    SG_ALERT,          // Very possible impending problem
-    SG_POPUP,          // Severe enough to alert using a pop-up window
+    SG_BULK = 1, // For frequent messages
+    SG_DEBUG,    // Less frequent debug type messages
+    SG_INFO,     // Informatory messages
+    SG_WARN,     // Possible impending problem
+    SG_ALERT,    // Very possible impending problem
+    SG_POPUP,    // Severe enough to alert using a pop-up window
     // SG_EXIT,        // Problem (no core)
     // SG_ABORT        // Abandon ship (core)
 
-    SG_DEV_WARN,       // Warning for developers, translated to other priority
-    SG_DEV_ALERT       // Alert for developers, translated
-} sgDebugPriority;
+    SG_DEV_WARN,  // Warning for developers, translated to other priority
+    SG_DEV_ALERT, // Alert for developers, translated
 
+    SG_MANDATORY_INFO // information, but should always be shown
+} sgDebugPriority;

simgear/debug/logstream.cxx

@@ -35,6 +35,7 @@
 #include <simgear/threads/SGThread.hxx>
 #include <simgear/threads/SGQueue.hxx>
 
+#include "LogCallback.hxx"
 #include <simgear/io/iostreams/sgstream.hxx>
 #include <simgear/misc/sg_path.hxx>
 #include <simgear/misc/strutils.hxx>
@@ -47,86 +48,9 @@
     #include <io.h>
 #endif
 
-
-
 //////////////////////////////////////////////////////////////////////////////
 
-namespace simgear
-{
 
-LogCallback::LogCallback(sgDebugClass c, sgDebugPriority p) :
-	m_class(c),
-	m_priority(p)
-{
-}
-
-bool LogCallback::shouldLog(sgDebugClass c, sgDebugPriority p) const
-{
-
-    if ((c & m_class) != 0 && p >= m_priority)
-        return true;
-    if (c == SG_OSG) // always have OSG logging as it OSG logging is configured separately.
-        return true;
-    return false;
-}
-
-void LogCallback::setLogLevels( sgDebugClass c, sgDebugPriority p )
-{
-	m_priority = p;
-	m_class = c;
-}
-const char* LogCallback::debugPriorityToString(sgDebugPriority p)
-{
-    switch (p) {
-    case SG_ALERT: return "ALRT";
-    case SG_BULK:  return "BULK";
-    case SG_DEBUG: return "DBUG";
-    case SG_INFO:  return "INFO";
-    case SG_POPUP: return "POPU";
-    case SG_WARN:  return "WARN";
-    default: return "UNKN";
-    }
-}
-
-const char* LogCallback::debugClassToString(sgDebugClass c)
-{
-    switch (c) {
-        case SG_NONE:       return "none";
-        case SG_TERRAIN:    return "terrain";
-        case SG_ASTRO:      return "astro";
-        case SG_FLIGHT:     return "flight";
-        case SG_INPUT:      return "input";
-        case SG_GL:         return "opengl";
-        case SG_VIEW:       return "view";
-        case SG_COCKPIT:    return "cockpit";
-        case SG_GENERAL:    return "general";
-        case SG_MATH:       return "math";
-        case SG_EVENT:      return "event";
-        case SG_AIRCRAFT:   return "aircraft";
-        case SG_AUTOPILOT:  return "autopilot";
-        case SG_IO:         return "io";
-        case SG_CLIPPER:    return "clipper";
-        case SG_NETWORK:    return "network";
-        case SG_ATC:        return "atc";
-        case SG_NASAL:      return "nasal";
-        case SG_INSTR:      return "instruments";
-        case SG_SYSTEMS:    return "systems";
-        case SG_AI:         return "ai";
-        case SG_ENVIRONMENT:return "environment";
-        case SG_SOUND:      return "sound";
-        case SG_NAVAID:     return "navaid";
-        case SG_GUI:        return "gui";
-        case SG_TERRASYNC:  return "terrasync";
-        case SG_PARTICLES:  return "particles";
-        case SG_HEADLESS:   return "headless";
-        case SG_OSG:        return "OSG";
-        default:            return "unknown";
-    }
-}
-
-} // of namespace simgear
-
-//////////////////////////////////////////////////////////////////////////////
 
 class FileLogCallback : public simgear::LogCallback
 {
@@ -139,8 +63,8 @@ public:
         logTimer.stamp();
     }
 
-    virtual void operator()(sgDebugClass c, sgDebugPriority p,
-        const char* file, int line, const std::string& message)
+    void operator()(sgDebugClass c, sgDebugPriority p,
+                    const char* file, int line, const std::string& message) override
     {
         if (!shouldLog(c, p)) return;
 
@@ -196,8 +120,8 @@ public:
     }
 #endif
 
-    virtual void operator()(sgDebugClass c, sgDebugPriority p,
-        const char* file, int line, const std::string& aMessage)
+    void operator()(sgDebugClass c, sgDebugPriority p,
+                    const char* file, int line, const std::string& aMessage) override
     {
         if (!shouldLog(c, p)) return;
         //fprintf(stderr, "%s\n", aMessage.c_str());
@@ -226,8 +150,8 @@ public:
     {
     }
 
-    virtual void operator()(sgDebugClass c, sgDebugPriority p,
-        const char* file, int line, const std::string& aMessage)
+    void operator()(sgDebugClass c, sgDebugPriority p,
+                    const char* file, int line, const std::string& aMessage) override
     {
         if (!shouldLog(c, p)) return;
 
@@ -242,28 +166,6 @@ public:
 class logstream::LogStreamPrivate : public SGThread
 {
 private:
-    /**
-     * storage of a single log entry. This is used to pass log entries from
-     * the various threads to the logging thread, and also to store the startup
-     * entries
-     */
-    class LogEntry
-    {
-    public:
-        LogEntry(sgDebugClass c, sgDebugPriority p,
-            const char* f, int l, const std::string& msg) :
-            debugClass(c), debugPriority(p), file(f), line(l),
-                message(msg)
-        {
-        }
-
-        const sgDebugClass debugClass;
-        const sgDebugPriority debugPriority;
-        const char* file;
-        const int line;
-        const std::string message;
-    };
-
     /**
      * RAII object to pause the logging thread if it's running, and restart it.
      * used to safely make configuration changes.
@@ -401,13 +303,20 @@ public:
     ~LogStreamPrivate()
     {
         removeCallbacks();
+
+        // house-keeping, avoid leak warnings if we exit before disabling
+        // startup logging
+        {
+            std::lock_guard<std::mutex> g(m_lock);
+            clearStartupEntriesLocked();
+        }
     }
 
     std::mutex m_lock;
-    SGBlockingQueue<LogEntry> m_entries;
+    SGBlockingQueue<simgear::LogEntry> m_entries;
 
     // log entries posted during startup
-    std::vector<LogEntry> m_startupEntries;
+    std::vector<simgear::LogEntry> m_startupEntries;
     bool m_startupLogging = false;
 
     typedef std::vector<simgear::LogCallback*> CallbackVec;
@@ -430,6 +339,8 @@ public:
     // test suite mode.
     bool m_testMode = false;
 
+    std::vector<std::string> _popupMessages;
+
     void startLog()
     {
         std::lock_guard<std::mutex> g(m_lock);
@@ -447,14 +358,19 @@ public:
         {
             std::lock_guard<std::mutex> g(m_lock);
             m_startupLogging = on;
-            m_startupEntries.clear();
+            clearStartupEntriesLocked();
         }
     }
 
-    virtual void run()
+    void clearStartupEntriesLocked()
+    {
+        m_startupEntries.clear();
+    }
+
+    void run() override
     {
         while (1) {
-            LogEntry entry(m_entries.pop());
+            simgear::LogEntry entry(m_entries.pop());
             // special marker entry detected, terminate the thread since we are
             // making a configuration change or quitting the app
             if ((entry.debugClass == SG_NONE) && entry.file && !strcmp(entry.file, "done")) {
@@ -470,8 +386,7 @@ public:
             }
             // submit to each installed callback in turn
             for (simgear::LogCallback* cb : m_callbacks) {
-                (*cb)(entry.debugClass, entry.debugPriority,
-                    entry.file, entry.line, entry.message);
+                cb->processEntry(entry);
             }
         } // of main thread loop
     }
@@ -486,7 +401,7 @@ public:
 
             // log a special marker value, which will cause the thread to wakeup,
             // and then exit
-            log(SG_NONE, SG_ALERT, "done", -1, "");
+            log(SG_NONE, SG_ALERT, "done", -1, "", false);
         }
         join();
 
@@ -501,9 +416,8 @@ public:
 
         // we clear startup entries not using this, so always safe to run
         // this code, container will simply be empty
-        for (auto entry : m_startupEntries) {
-            (*cb)(entry.debugClass, entry.debugPriority,
-               entry.file, entry.line, entry.message);
+        for (const auto& entry : m_startupEntries) {
+            cb->processEntry(entry);
         }
     }
 
@@ -531,9 +445,9 @@ public:
         PauseThread pause(this);
         m_logPriority = p;
         m_logClass = c;
-                for (auto cb : m_consoleCallbacks) {
-	        cb->setLogLevels(c, p);
-		}
+        for (auto cb : m_consoleCallbacks) {
+            cb->setLogLevels(c, p);
+        }
     }
 
     bool would_log( sgDebugClass c, sgDebugPriority p ) const
@@ -551,14 +465,17 @@ public:
     }
 
     void log( sgDebugClass c, sgDebugPriority p,
-            const char* fileName, int line, const std::string& msg)
+            const char* fileName, int line, const std::string& msg,
+             bool freeFilename)
     {
-        p = translatePriority(p);
+        auto tp = translatePriority(p);
         if (!m_fileLine) {
             /* This prevents output of file:line in StderrLogCallback. */
             line = -line;
         }
-        LogEntry entry(c, p, fileName, line, msg);
+
+        simgear::LogEntry entry(c, tp, p, fileName, line, msg);
+        entry.freeFilename = freeFilename;
         m_entries.push(entry);
     }
 
@@ -589,8 +506,8 @@ logstream::logstream()
 
 logstream::~logstream()
 {
-    popup_msgs.clear();
     d->stop();
+    d.reset();
 }
 
 void
@@ -625,7 +542,14 @@ void
 logstream::log( sgDebugClass c, sgDebugPriority p,
         const char* fileName, int line, const std::string& msg)
 {
-    d->log(c, p, fileName, line, msg);
+    d->log(c, p, fileName, line, msg, false);
+}
+
+void
+logstream::logCopyingFilename( sgDebugClass c, sgDebugPriority p,
+         const char* fileName, int line, const std::string& msg)
+{
+    d->log(c, p, strdup(fileName), line, msg, true);
 }
 
 
@@ -686,17 +610,18 @@ void logstream::hexdump(sgDebugClass c, sgDebugPriority p, const char* fileName,
 void
 logstream::popup( const std::string& msg)
 {
-    popup_msgs.push_back(msg);
+    std::lock_guard<std::mutex> g(d->m_lock);
+    d->_popupMessages.push_back(msg);
 }
 
 std::string
 logstream::get_popup()
 {
-    std::string rv = "";
-    if (!popup_msgs.empty())
-    {
-        rv = popup_msgs.front();
-        popup_msgs.erase(popup_msgs.begin());
+    std::string rv;
+    std::lock_guard<std::mutex> g(d->m_lock);
+    if (!d->_popupMessages.empty()) {
+        rv = d->_popupMessages.front();
+        d->_popupMessages.erase(d->_popupMessages.begin());
     }
     return rv;
 }
@@ -704,7 +629,8 @@ logstream::get_popup()
 bool
 logstream::has_popup()
 {
-    return (popup_msgs.size() > 0) ? true : false;
+    std::lock_guard<std::mutex> g(d->m_lock);
+    return !d->_popupMessages.empty();
 }
 
 bool
@@ -737,6 +663,16 @@ logstream::set_log_classes( sgDebugClass c)
     d->setLogLevels(c, d->m_logPriority);
 }
 
+sgDebugPriority logstream::priorityFromString(const std::string& s)
+{
+    if (s == "bulk") return SG_BULK;
+    if (s == "debug") return SG_DEBUG;
+    if (s == "info") return SG_INFO;
+    if (s == "warn") return SG_WARN;
+    if (s == "alert") return SG_ALERT;
+
+    throw std::invalid_argument("Couldn't parse log prioirty:" + s);
+}
 
 logstream&
 sglog()

simgear/debug/logstream.hxx

@@ -37,27 +37,8 @@ class SGPath;
       
 namespace simgear
 {
-     
-class LogCallback
-{
-public:
-    virtual ~LogCallback() {}
-    virtual void operator()(sgDebugClass c, sgDebugPriority p, 
-        const char* file, int line, const std::string& aMessage) = 0;
-
-	void setLogLevels(sgDebugClass c, sgDebugPriority p);
-protected:
-	LogCallback(sgDebugClass c, sgDebugPriority p);
-
-	bool shouldLog(sgDebugClass c, sgDebugPriority p) const;
-
-    static const char* debugClassToString(sgDebugClass c);
-    static const char* debugPriorityToString(sgDebugPriority p);
-private:
-	sgDebugClass m_class;
-	sgDebugPriority m_priority;
-};
 
+class LogCallback;
 /**
  * Helper force a console on platforms where it might optional, when
  * we need to show a console. This basically means Windows at the
@@ -105,6 +86,11 @@ public:
     
     sgDebugPriority get_log_priority() const;
 
+    /**
+        @brief convert a string value to a log prioirty.
+        throws std::invalid_argument if the string is not valid
+     */
+    static sgDebugPriority priorityFromString(const std::string& s);
     /**
      * set developer mode on/off. In developer mode, SG_DEV_WARN messags
      * are treated as warnings. In normal (non-developer) mode they are
@@ -124,6 +110,14 @@ public:
     void log( sgDebugClass c, sgDebugPriority p,
             const char* fileName, int line, const std::string& msg);
 
+    // overload of above, which can transfer ownership of the file-name.
+    // this is unecesary overhead when logging from C++, since __FILE__ points
+    // to constant data, but it's needed when the filename is Nasal data (for
+    // example) since during shutdown the filename is freed by Nasal GC
+    // asynchronously with the logging thread.
+    void logCopyingFilename( sgDebugClass c, sgDebugPriority p,
+             const char* fileName, int line, const std::string& msg);
+    
     /**
     * output formatted hex dump of memory block
     */
@@ -185,8 +179,6 @@ private:
     // constructor
     logstream();
 
-    std::vector<std::string> popup_msgs;
-
     class LogStreamPrivate;
 
     std::unique_ptr<LogStreamPrivate> d;

simgear/environment/metar.cxx

@@ -631,11 +631,15 @@ bool SGMetar::scanWind()
 	int dir;
 	if (!strncmp(m, "VRB", 3))
 		m += 3, dir = -1;
+	else if (!strncmp(m, "///", 3))	// direction not measurable
+		m += 3, dir = -1;
 	else if (!scanNumber(&m, &dir, 3))
 		return false;
 
 	int i;
-	if (!scanNumber(&m, &i, 2, 3))
+	if (!strncmp(m, "//", 2))	// speed not measurable
+		m += 2, i = -1;
+	else if (!scanNumber(&m, &i, 2, 3))
 		return false;
 	double speed = i;
 
@@ -655,6 +659,8 @@ bool SGMetar::scanWind()
 		m += 3, factor = SG_KMH_TO_MPS;
 	else if (!strncmp(m, "MPS", 3))
 		m += 3, factor = 1.0;
+	else if (!strncmp(m, " ", 1))		// default to Knots
+		factor = SG_KT_TO_MPS;
 	else
 		return false;
 	if (!scanBoundary(&m))
@@ -952,6 +958,8 @@ bool SGMetar::scanWeather()
 		weather += string(a->text) + " ";
 		if (!strcmp(a->id, "RA"))
 			_rain = w.intensity;
+		else if (!strcmp(a->id, "DZ"))
+			_rain = LIGHT;
 		else if (!strcmp(a->id, "HA"))
 			_hail = w.intensity;
 		else if (!strcmp(a->id, "SN"))

simgear/environment/test_metar.cxx

@@ -76,12 +76,29 @@ void test_sensor_failure_cloud()
     SG_CHECK_EQUAL_EP2(m1.getPressure_hPa(), 1025, TEST_EPSILON);
 }
 
+void test_sensor_failure_wind()
+{
+    SGMetar m1("2020/10/23 16:55 LIVD 231655Z /////KT 9999 OVC025 10/08 Q1020 RMK OVC VIS MIN 9999 BLU");
+    SG_CHECK_EQUAL(m1.getWindDir(), -1);
+    SG_CHECK_EQUAL_EP2(m1.getWindSpeed_kt(), -1, TEST_EPSILON);
+}
+
+void test_wind_unit_not_specified()
+{
+    SGMetar m1("2020/10/23 11:58 KLSV 231158Z 05010G14 10SM CLR 16/M04 A2992 RMK SLPNO WND DATA ESTMD ALSTG/SLP ESTMD 10320 20124 5//// $");
+    SG_CHECK_EQUAL(m1.getWindDir(), 50);
+    SG_CHECK_EQUAL_EP2(m1.getWindSpeed_kt(), 10.0, TEST_EPSILON);
+    SG_CHECK_EQUAL_EP2(m1.getGustSpeed_kt(), 14.0, TEST_EPSILON);
+}
+
 int main(int argc, char* argv[])
 {
     try {
         test_basic();
         test_sensor_failure_weather();
         test_sensor_failure_cloud();
+        test_sensor_failure_wind();
+        test_wind_unit_not_specified();
     } catch (sg_exception& e) {
         cerr << "got exception:" << e.getMessage() << endl;
         return -1;

simgear/io/CMakeLists.txt

@@ -35,10 +35,12 @@ set(SOURCES
     sg_socket.cxx
     sg_socket_udp.cxx
     HTTPClient.cxx
+    HTTPTestApi_private.hxx
     HTTPFileRequest.cxx
     HTTPMemoryRequest.cxx
     HTTPRequest.cxx
     HTTPRepository.cxx
+    HTTPRepository_private.hxx
     untar.cxx
     )
 
@@ -81,6 +83,7 @@ add_test(binobj ${EXECUTABLE_OUTPUT_PATH}/test_binobj)
 
 add_executable(test_repository test_repository.cxx)
 target_link_libraries(test_repository ${TEST_LIBS})
+target_compile_definitions(test_repository PUBLIC BUILDING_TESTSUITE)
 add_test(http_repository ${EXECUTABLE_OUTPUT_PATH}/test_repository)
 
 add_executable(test_untar test_untar.cxx)

simgear/io/DNSClient.cxx

@@ -61,6 +61,10 @@ public:
 
     struct dns_ctx * ctx;
     static size_t instanceCounter;
+
+    using RequestVec = std::vector<Request_ptr>;
+
+    RequestVec _activeRequests;
 };
 
 size_t Client::ClientPrivate::instanceCounter = 0;
@@ -78,6 +82,11 @@ Request::~Request()
 {
 }
 
+void Request::cancel()
+{
+    _cancelled = true;
+}
+
 bool Request::isTimeout() const
 {
     return (time(NULL) - _start) > _timeout_secs;
@@ -114,18 +123,20 @@ static void dnscbSRV(struct dns_ctx *ctx, struct dns_rr_srv *result, void *data)
 {
     SRVRequest * r = static_cast<SRVRequest*>(data);
     if (result) {
-        r->cname = result->dnssrv_cname;
-        r->qname = result->dnssrv_qname;
-        r->ttl = result->dnssrv_ttl;
-        for (int i = 0; i < result->dnssrv_nrr; i++) {
-            SRVRequest::SRV_ptr srv(new SRVRequest::SRV);
-            r->entries.push_back(srv);
-            srv->priority = result->dnssrv_srv[i].priority;
-            srv->weight = result->dnssrv_srv[i].weight;
-            srv->port = result->dnssrv_srv[i].port;
-            srv->target = result->dnssrv_srv[i].name;
+        if (!r->isCancelled()) {
+            r->cname = result->dnssrv_cname;
+            r->qname = result->dnssrv_qname;
+            r->ttl = result->dnssrv_ttl;
+            for (int i = 0; i < result->dnssrv_nrr; i++) {
+                SRVRequest::SRV_ptr srv(new SRVRequest::SRV);
+                r->entries.push_back(srv);
+                srv->priority = result->dnssrv_srv[i].priority;
+                srv->weight = result->dnssrv_srv[i].weight;
+                srv->port = result->dnssrv_srv[i].port;
+                srv->target = result->dnssrv_srv[i].name;
+            }
+            std::sort(r->entries.begin(), r->entries.end(), sortSRV);
         }
-        std::sort( r->entries.begin(), r->entries.end(), sortSRV );
         free(result);
     }
     r->setComplete();
@@ -134,11 +145,16 @@ static void dnscbSRV(struct dns_ctx *ctx, struct dns_rr_srv *result, void *data)
 void SRVRequest::submit( Client * client )
 {
     // if service is defined, pass service and protocol
-    if (!dns_submit_srv(client->d->ctx, getDn().c_str(), _service.empty() ? NULL : _service.c_str(), _service.empty() ? NULL : _protocol.c_str(), 0, dnscbSRV, this )) {
+    auto q = dns_submit_srv(client->d->ctx, getDn().c_str(), _service.empty() ? NULL : _service.c_str(),
+                            _service.empty() ? NULL : _protocol.c_str(),
+                            0, dnscbSRV, this);
+
+    if (!q) {
         SG_LOG(SG_IO, SG_ALERT, "Can't submit dns request for " << getDn());
         return;
     }
     _start = time(NULL);
+    _query = q;
 }
 
 TXTRequest::TXTRequest( const std::string & dn ) :
@@ -151,17 +167,24 @@ static void dnscbTXT(struct dns_ctx *ctx, struct dns_rr_txt *result, void *data)
 {
     TXTRequest * r = static_cast<TXTRequest*>(data);
     if (result) {
-        r->cname = result->dnstxt_cname;
-        r->qname = result->dnstxt_qname;
-        r->ttl = result->dnstxt_ttl;
-        for (int i = 0; i < result->dnstxt_nrr; i++) {
-          //TODO: interprete the .len field of dnstxt_txt?
-          string txt = string((char*)result->dnstxt_txt[i].txt);
-          r->entries.push_back( txt );
-          string_list tokens = simgear::strutils::split( txt, "=", 1 );
-          if( tokens.size() == 2 ) {
-            r->attributes[tokens[0]] = tokens[1];
-          }
+        if (!r->isCancelled()) {
+            r->cname = result->dnstxt_cname;
+            r->qname = result->dnstxt_qname;
+            r->ttl = result->dnstxt_ttl;
+            for (int i = 0; i < result->dnstxt_nrr; i++) {
+                //TODO: interprete the .len field of dnstxt_txt?
+                auto rawTxt = reinterpret_cast<char*>(result->dnstxt_txt[i].txt);
+                if (!rawTxt) {
+                    continue;
+                }
+
+                const string txt{rawTxt};
+                r->entries.push_back(txt);
+                string_list tokens = simgear::strutils::split(txt, "=", 1);
+                if (tokens.size() == 2) {
+                    r->attributes[tokens[0]] = tokens[1];
+                }
+            }
         }
         free(result);
     }
@@ -171,11 +194,13 @@ static void dnscbTXT(struct dns_ctx *ctx, struct dns_rr_txt *result, void *data)
 void TXTRequest::submit( Client * client )
 {
     // protocol and service an already encoded in DN so pass in NULL for both
-    if (!dns_submit_txt(client->d->ctx, getDn().c_str(), DNS_C_IN, 0, dnscbTXT, this )) {
+    auto q = dns_submit_txt(client->d->ctx, getDn().c_str(), DNS_C_IN, 0, dnscbTXT, this);
+    if (!q) {
         SG_LOG(SG_IO, SG_ALERT, "Can't submit dns request for " << getDn());
         return;
     }
     _start = time(NULL);
+    _query = q;
 }
 
 
@@ -190,27 +215,29 @@ static void dnscbNAPTR(struct dns_ctx *ctx, struct dns_rr_naptr *result, void *d
 {
     NAPTRRequest * r = static_cast<NAPTRRequest*>(data);
     if (result) {
-        r->cname = result->dnsnaptr_cname;
-        r->qname = result->dnsnaptr_qname;
-        r->ttl = result->dnsnaptr_ttl;
-        for (int i = 0; i < result->dnsnaptr_nrr; i++) {
-            if( !r->qservice.empty() && r->qservice != result->dnsnaptr_naptr[i].service )
-                continue;
+        if (!r->isCancelled()) {
+            r->cname = result->dnsnaptr_cname;
+            r->qname = result->dnsnaptr_qname;
+            r->ttl = result->dnsnaptr_ttl;
+            for (int i = 0; i < result->dnsnaptr_nrr; i++) {
+                if (!r->qservice.empty() && r->qservice != result->dnsnaptr_naptr[i].service)
+                    continue;
 
-            //TODO: case ignore and result flags may have more than one flag
-            if( !r->qflags.empty() && r->qflags != result->dnsnaptr_naptr[i].flags )
-                continue;
+                //TODO: case ignore and result flags may have more than one flag
+                if (!r->qflags.empty() && r->qflags != result->dnsnaptr_naptr[i].flags)
+                    continue;
 
-            NAPTRRequest::NAPTR_ptr naptr(new NAPTRRequest::NAPTR);
-            r->entries.push_back(naptr);
-            naptr->order = result->dnsnaptr_naptr[i].order;
-            naptr->preference = result->dnsnaptr_naptr[i].preference;
-            naptr->flags = result->dnsnaptr_naptr[i].flags;
-            naptr->service = result->dnsnaptr_naptr[i].service;
-            naptr->regexp = result->dnsnaptr_naptr[i].regexp;
-            naptr->replacement = result->dnsnaptr_naptr[i].replacement;
+                NAPTRRequest::NAPTR_ptr naptr(new NAPTRRequest::NAPTR);
+                r->entries.push_back(naptr);
+                naptr->order = result->dnsnaptr_naptr[i].order;
+                naptr->preference = result->dnsnaptr_naptr[i].preference;
+                naptr->flags = result->dnsnaptr_naptr[i].flags;
+                naptr->service = result->dnsnaptr_naptr[i].service;
+                naptr->regexp = result->dnsnaptr_naptr[i].regexp;
+                naptr->replacement = result->dnsnaptr_naptr[i].replacement;
+            }
+            std::sort(r->entries.begin(), r->entries.end(), sortNAPTR);
         }
-        std::sort( r->entries.begin(), r->entries.end(), sortNAPTR );
         free(result);
     }
     r->setComplete();
@@ -218,11 +245,13 @@ static void dnscbNAPTR(struct dns_ctx *ctx, struct dns_rr_naptr *result, void *d
 
 void NAPTRRequest::submit( Client * client )
 {
-    if (!dns_submit_naptr(client->d->ctx, getDn().c_str(), 0, dnscbNAPTR, this )) {
+    auto q = dns_submit_naptr(client->d->ctx, getDn().c_str(), 0, dnscbNAPTR, this);
+    if (!q) {
         SG_LOG(SG_IO, SG_ALERT, "Can't submit dns request for " << getDn());
         return;
     }
     _start = time(NULL);
+    _query = q;
 }
 
 
@@ -237,6 +266,7 @@ Client::Client() :
 
 void Client::makeRequest(const Request_ptr& r)
 {
+    d->_activeRequests.push_back(r);
     r->submit(this);
 }
 
@@ -247,6 +277,19 @@ void Client::update(int waitTimeout)
         return;
 
     dns_ioevent(d->ctx, now);
+
+    // drop our owning ref to completed requests,
+    // and cancel any which timed out
+    auto it = std::remove_if(d->_activeRequests.begin(), d->_activeRequests.end(),
+                             [this](const Request_ptr& r) {
+                                 if (r->isTimeout()) {
+                                     dns_cancel(d->ctx, reinterpret_cast<struct dns_query*>(r->_query));
+                                     return true;
+                                 }
+
+                                 return r->isComplete();
+                             });
+    d->_activeRequests.erase(it, d->_activeRequests.end());
 }
 
 } // of namespace DNS

simgear/io/DNSClient.hxx

@@ -40,28 +40,38 @@ namespace DNS
 {
 
 class Client;
+
+using UDNSQueryPtr = void*;
+
 class Request : public SGReferenced
 {
 public:
     Request( const std::string & dn );
     virtual ~Request();
-    std::string getDn() const { return _dn; }
+    const std::string& getDn() const { return _dn; }
     int getType() const { return _type; }
     bool isComplete() const { return _complete; }
     bool isTimeout() const;
     void setComplete( bool b = true ) { _complete = b; }
+    bool isCancelled() const { return _cancelled; }
 
     virtual void submit( Client * client) = 0;
 
+    void cancel();
+
     std::string cname;
     std::string qname;
     unsigned ttl;
 protected:
+    friend class Client;
+
+    UDNSQueryPtr _query = nullptr;
     std::string _dn;
     int _type;
     bool _complete;
     time_t _timeout_secs;
     time_t _start;
+    bool _cancelled = false;
 };
 typedef SGSharedPtr<Request> Request_ptr;
 
@@ -69,7 +79,7 @@ class NAPTRRequest : public Request
 {
 public:
     NAPTRRequest( const std::string & dn );
-    virtual void submit( Client * client );
+    void submit(Client* client) override;
 
     struct NAPTR : SGReferenced {
         int order;
@@ -92,7 +102,7 @@ class SRVRequest : public Request
 public:
     SRVRequest( const std::string & dn );
     SRVRequest( const std::string & dn, const string & service, const string & protocol );
-    virtual void submit( Client * client );
+    void submit(Client* client) override;
 
     struct SRV : SGReferenced {
       int priority;
@@ -112,7 +122,7 @@ class TXTRequest : public Request
 {
 public:
     TXTRequest( const std::string & dn );
-    virtual void submit( Client * client );
+    void submit(Client* client) override;
 
     typedef std::vector<string> TXT_list;
     typedef std::map<std::string,std::string> TXT_Attribute_map;

simgear/io/HTTPClient.cxx

@@ -37,7 +37,6 @@
 
 #include <simgear/simgear_config.h>
 
-#include <curl/multi.h>
 
 #include <simgear/io/sg_netChat.hxx>
 
@@ -47,6 +46,9 @@
 #include <simgear/timing/timestamp.hxx>
 #include <simgear/structure/exception.hxx>
 
+#include "HTTPClient_private.hxx"
+#include "HTTPTestApi_private.hxx"
+
 #if defined( HAVE_VERSION_H ) && HAVE_VERSION_H
 #include "version.h"
 #else
@@ -64,48 +66,20 @@ namespace HTTP
 extern const int DEFAULT_HTTP_PORT = 80;
 const char* CONTENT_TYPE_URL_ENCODED = "application/x-www-form-urlencoded";
 
-class Connection;
-typedef std::multimap<std::string, Connection*> ConnectionDict;
-typedef std::list<Request_ptr> RequestList;
-
-class Client::ClientPrivate
-{
-public:
-    CURLM* curlMulti;
-
-    void createCurlMulti()
-    {
-        curlMulti = curl_multi_init();
-        // see https://curl.haxx.se/libcurl/c/CURLMOPT_PIPELINING.html
-        // we request HTTP 1.1 pipelining
-        curl_multi_setopt(curlMulti, CURLMOPT_PIPELINING, 1 /* aka CURLPIPE_HTTP1 */);
+void Client::ClientPrivate::createCurlMulti() {
+  curlMulti = curl_multi_init();
+  // see https://curl.haxx.se/libcurl/c/CURLMOPT_PIPELINING.html
+  // we request HTTP 1.1 pipelining
+  curl_multi_setopt(curlMulti, CURLMOPT_PIPELINING, 1 /* aka CURLPIPE_HTTP1 */);
 #if (LIBCURL_VERSION_MINOR >= 30)
-        curl_multi_setopt(curlMulti, CURLMOPT_MAX_TOTAL_CONNECTIONS, (long) maxConnections);
-        curl_multi_setopt(curlMulti, CURLMOPT_MAX_PIPELINE_LENGTH,
-                          (long) maxPipelineDepth);
-        curl_multi_setopt(curlMulti, CURLMOPT_MAX_HOST_CONNECTIONS,
-                          (long) maxHostConnections);
+  curl_multi_setopt(curlMulti, CURLMOPT_MAX_TOTAL_CONNECTIONS,
+                    (long)maxConnections);
+  curl_multi_setopt(curlMulti, CURLMOPT_MAX_PIPELINE_LENGTH,
+                    (long)maxPipelineDepth);
+  curl_multi_setopt(curlMulti, CURLMOPT_MAX_HOST_CONNECTIONS,
+                    (long)maxHostConnections);
 #endif
-    }
-
-    typedef std::map<Request_ptr, CURL*> RequestCurlMap;
-    RequestCurlMap requests;
-
-    std::string userAgent;
-    std::string proxy;
-    int proxyPort;
-    std::string proxyAuth;
-    unsigned int maxConnections;
-    unsigned int maxHostConnections;
-    unsigned int maxPipelineDepth;
-
-    RequestList pendingRequests;
-
-    SGTimeStamp timeTransferSample;
-    unsigned int bytesTransferred;
-    unsigned int lastTransferRate;
-    uint64_t totalBytesDownloaded;
-};
+}
 
 Client::Client() :
     d(new ClientPrivate)
@@ -120,6 +94,8 @@ Client::Client() :
     d->maxPipelineDepth = 5;
     setUserAgent("SimGear-" SG_STRINGIZE(SIMGEAR_VERSION));
 
+    d->tlsCertificatePath = SGPath::fromEnv("SIMGEAR_TLS_CERT_PATH");
+
     static bool didInitCurlGlobal = false;
     static std::mutex initMutex;
 
@@ -161,6 +137,14 @@ void Client::setMaxPipelineDepth(unsigned int depth)
 #endif
 }
 
+void Client::reset()
+{
+    curl_multi_cleanup(d->curlMulti);
+    d.reset(new ClientPrivate);
+    d->tlsCertificatePath = SGPath::fromEnv("SIMGEAR_TLS_CERT_PATH");
+    d->createCurlMulti();
+}
+
 void Client::update(int waitTimeout)
 {
     if (d->requests.empty()) {
@@ -171,32 +155,22 @@ void Client::update(int waitTimeout)
     }
 
     int remainingActive, messagesInQueue;
-#if defined(SG_MAC)
-    // Mac 10.8 libCurl lacks this, let's keep compat for now
-    fd_set curlReadFDs, curlWriteFDs, curlErrorFDs;
-    int maxFD;
-    curl_multi_fdset(d->curlMulti,
-                     &curlReadFDs,
-                     &curlWriteFDs,
-                     &curlErrorFDs,
-                     &maxFD);
-
-    struct timeval timeout;
-    long t;
-
-    curl_multi_timeout(d->curlMulti, &t);
-    if ((t < 0) || (t > waitTimeout)) {
-        t = waitTimeout;
+    int numFds;
+    CURLMcode mc = curl_multi_wait(d->curlMulti, NULL, 0, waitTimeout, &numFds);
+    if (mc != CURLM_OK) {
+        SG_LOG(SG_IO, SG_WARN, "curl_multi_wait failed:" << curl_multi_strerror(mc));
+        return;
     }
 
-    timeout.tv_sec = t / 1000;
-    timeout.tv_usec = (t % 1000) * 1000;
-    ::select(maxFD, &curlReadFDs, &curlWriteFDs, &curlErrorFDs, &timeout);
-#else
-    int numFds;
-    curl_multi_wait(d->curlMulti, NULL, 0, waitTimeout, &numFds);
-#endif
-    curl_multi_perform(d->curlMulti, &remainingActive);
+    mc = curl_multi_perform(d->curlMulti, &remainingActive);
+    if (mc == CURLM_CALL_MULTI_PERFORM) {
+        // we could loop here, but don't want to get blocked
+        // also this shouldn't  ocurr in any modern libCurl
+        curl_multi_perform(d->curlMulti, &remainingActive);
+    } else if (mc != CURLM_OK) {
+        SG_LOG(SG_IO, SG_WARN, "curl_multi_perform failed:" << curl_multi_strerror(mc));
+        return;
+    }
 
     CURLMsg* msg;
     while ((msg = curl_multi_info_read(d->curlMulti, &messagesInQueue))) {
@@ -221,12 +195,23 @@ void Client::update(int waitTimeout)
           assert(it->second == e);
           d->requests.erase(it);
 
-        if (msg->data.result == 0) {
-          req->responseComplete();
-        } else {
-          SG_LOG(SG_IO, SG_WARN, "CURL Result:" << msg->data.result << " " << curl_easy_strerror(msg->data.result));
-          req->setFailure(msg->data.result, curl_easy_strerror(msg->data.result));
-        }
+          bool doProcess = true;
+          if (d->testsuiteResponseDoneCallback) {
+            doProcess =
+                !d->testsuiteResponseDoneCallback(msg->data.result, req);
+          }
+
+          if (doProcess) {
+            if (msg->data.result == 0) {
+              req->responseComplete();
+            } else {
+              SG_LOG(SG_IO, SG_WARN,
+                     "CURL Result:" << msg->data.result << " "
+                                    << curl_easy_strerror(msg->data.result));
+              req->setFailure(msg->data.result,
+                              curl_easy_strerror(msg->data.result));
+            }
+          }
 
         curl_multi_remove_handle(d->curlMulti, e);
         curl_easy_cleanup(e);
@@ -285,6 +270,11 @@ void Client::makeRequest(const Request_ptr& r)
 
     curl_easy_setopt(curlRequest, CURLOPT_FOLLOWLOCATION, 1);
 
+    if (!d->tlsCertificatePath.isNull()) {
+        const auto utf8 = d->tlsCertificatePath.utf8Str();
+        curl_easy_setopt(curlRequest, CURLOPT_CAINFO, utf8.c_str());
+    }
+
     if (!d->proxy.empty()) {
       curl_easy_setopt(curlRequest, CURLOPT_PROXY, d->proxy.c_str());
       curl_easy_setopt(curlRequest, CURLOPT_PROXYPORT, d->proxyPort);
@@ -552,6 +542,17 @@ void Client::clearAllConnections()
     d->createCurlMulti();
 }
 
+/////////////////////////////////////////////////////////////////////
+
+void TestApi::setResponseDoneCallback(Client *cl, ResponseDoneCallback cb) {
+  cl->d->testsuiteResponseDoneCallback = cb;
+}
+
+void TestApi::markRequestAsFailed(Request_ptr req, int curlCode,
+                                  const std::string &message) {
+  req->setFailure(curlCode, message);
+}
+
 } // of namespace HTTP
 
 } // of namespace simgear

simgear/io/HTTPClient.hxx

@@ -24,7 +24,8 @@
 #ifndef SG_HTTP_CLIENT_HXX
 #define SG_HTTP_CLIENT_HXX
 
-#include <memory> // for std::unique_ptr
+#include <functional>
+#include <memory>   // for std::unique_ptr
 #include <stdint.h> // for uint_64t
 
 #include <simgear/io/HTTPFileRequest.hxx>
@@ -47,6 +48,8 @@ public:
 
     void update(int waitTimeout = 0);
 
+    void reset();
+
     void makeRequest(const Request_ptr& r);
 
     void cancelRequest(const Request_ptr& r, std::string reason = std::string());
@@ -123,6 +126,7 @@ private:
 
     friend class Connection;
     friend class Request;
+    friend class TestApi;
 
     class ClientPrivate;
     std::unique_ptr<ClientPrivate> d;

simgear/io/HTTPClient_private.hxx

@@ -0,0 +1,68 @@
+// This library is free software; you can redistribute it and/or
+// modify it under the terms of the GNU Library General Public
+// License as published by the Free Software Foundation; either
+// version 2 of the License, or (at your option) any later version.
+//
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+// Library General Public License for more details.
+//
+// You should have received a copy of the GNU Library General Public
+// License along with this library; if not, write to the Free Software
+// Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301, USA
+
+#pragma once
+
+#include <list>
+#include <map>
+
+#include "HTTPClient.hxx"
+#include "HTTPRequest.hxx"
+
+#include <simgear/timing/timestamp.hxx>
+
+#include <curl/multi.h>
+
+namespace simgear {
+namespace HTTP {
+
+typedef std::list<Request_ptr> RequestList;
+
+using ResponseDoneCallback =
+    std::function<bool(int curlResult, Request_ptr req)>;
+
+class Client::ClientPrivate {
+public:
+  CURLM *curlMulti;
+
+  void createCurlMulti();
+
+  typedef std::map<Request_ptr, CURL *> RequestCurlMap;
+  RequestCurlMap requests;
+
+  std::string userAgent;
+  std::string proxy;
+  int proxyPort;
+  std::string proxyAuth;
+  unsigned int maxConnections;
+  unsigned int maxHostConnections;
+  unsigned int maxPipelineDepth;
+
+  RequestList pendingRequests;
+
+  SGTimeStamp timeTransferSample;
+  unsigned int bytesTransferred;
+  unsigned int lastTransferRate;
+  uint64_t totalBytesDownloaded;
+
+  SGPath tlsCertificatePath;
+
+  // only used by unit-tests / test-api, but
+  // only costs us a pointe here to declare it.
+  ResponseDoneCallback testsuiteResponseDoneCallback;
+};
+
+} // namespace HTTP
+
+} // namespace simgear

simgear/io/HTTPRepository.hxx

@@ -20,6 +20,7 @@
 #ifndef SG_IO_HTTP_REPOSITORY_HXX
 #define SG_IO_HTTP_REPOSITORY_HXX
 
+#include <functional>
 #include <memory>
 
 #include <simgear/misc/sg_path.hxx>
@@ -32,49 +33,83 @@ class HTTPRepoPrivate;
 class HTTPRepository
 {
 public:
-    enum ResultCode {
-        REPO_NO_ERROR = 0,
-        REPO_ERROR_NOT_FOUND,
-        REPO_ERROR_SOCKET,
-        SVN_ERROR_XML,
-        SVN_ERROR_TXDELTA,
-        REPO_ERROR_IO,
-        REPO_ERROR_CHECKSUM,
-        REPO_ERROR_FILE_NOT_FOUND,
-        REPO_ERROR_HTTP,
-        REPO_ERROR_CANCELLED,
-        REPO_PARTIAL_UPDATE
+  enum ResultCode {
+    REPO_NO_ERROR = 0,
+    REPO_ERROR_NOT_FOUND,
+    REPO_ERROR_SOCKET,
+    SVN_ERROR_XML,
+    SVN_ERROR_TXDELTA,
+    REPO_ERROR_IO,
+    REPO_ERROR_CHECKSUM,
+    REPO_ERROR_FILE_NOT_FOUND,
+    REPO_ERROR_HTTP,
+    REPO_ERROR_CANCELLED,
+    REPO_PARTIAL_UPDATE ///< repository is working, but file-level failures
+                        ///< occurred
+  };
+
+  HTTPRepository(const SGPath &root, HTTP::Client *cl);
+  virtual ~HTTPRepository();
+
+  virtual SGPath fsBase() const;
+
+  virtual void setBaseUrl(const std::string &url);
+  virtual std::string baseUrl() const;
+
+  virtual HTTP::Client *http() const;
+
+  virtual void update();
+
+  virtual bool isDoingSync() const;
+
+  /**
+        @brief call this periodically to progress non-network tasks
+     */
+  void process();
+
+  virtual ResultCode failure() const;
+
+  virtual size_t bytesToDownload() const;
+
+  virtual size_t bytesDownloaded() const;
+
+  /**
+   * optionally provide the location of an installer copy of this
+   * repository. When a file is missing it will be copied from this tree.
+   */
+  void setInstalledCopyPath(const SGPath &copyPath);
+
+  static std::string resultCodeAsString(ResultCode code);
+
+  enum class SyncAction { Add, Update, Delete, UpToDate };
+
+  enum EntryType { FileType, DirectoryType, TarballType };
+
+  struct SyncItem {
+    const std::string directory; // relative path in the repository
+    const EntryType type;
+    const std::string filename;
+    const SyncAction action;
+    const SGPath pathOnDisk; // path the entry does / will have
+  };
+
+    using SyncPredicate = std::function<bool(const SyncItem &item)>;
+
+    void setFilter(SyncPredicate sp);
+
+    struct Failure {
+      SGPath path;
+      ResultCode error;
     };
 
-    HTTPRepository(const SGPath& root, HTTP::Client* cl);
-    virtual ~HTTPRepository();
-
-    virtual SGPath fsBase() const;
-
-    virtual void setBaseUrl(const std::string& url);
-    virtual std::string baseUrl() const;
-
-    virtual HTTP::Client* http() const;
-
-    virtual void update();
-
-    virtual bool isDoingSync() const;
-
-    virtual ResultCode failure() const;
-
-    virtual size_t bytesToDownload() const;
-
-    virtual size_t bytesDownloaded() const;
+    using FailureVec = std::vector<Failure>;
 
     /**
-     * optionally provide the location of an installer copy of this
-     * repository. When a file is missing it will be copied from this tree.
+     * @brief return file-level failures
      */
-    void setInstalledCopyPath(const SGPath& copyPath);
-    
-    static std::string resultCodeAsString(ResultCode code);
+    FailureVec failures() const;
 
-private:
+  private:
     bool isBare() const;
 
     std::unique_ptr<HTTPRepoPrivate> _d;

simgear/io/HTTPRepository_private.hxx

@@ -0,0 +1,126 @@
+// HTTPRepository.cxx -- plain HTTP TerraSync remote client
+//
+// Copyright (C) 20126  James Turner <zakalawe@mac.com>
+//
+// This program is free software; you can redistribute it and/or
+// modify it under the terms of the GNU General Public License as
+// published by the Free Software Foundation; either version 2 of the
+// License, or (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful, but
+// WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+// General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program; if not, write to the Free Software
+// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301,
+// USA.
+
+#pragma once
+
+#include <deque>
+#include <functional>
+#include <memory>
+#include <string>
+#include <unordered_map>
+
+#include <simgear/io/HTTPClient.hxx>
+#include <simgear/misc/sg_path.hxx>
+
+#include "HTTPRepository.hxx"
+
+namespace simgear {
+
+class HTTPDirectory;
+using HTTPDirectory_ptr = std::unique_ptr<HTTPDirectory>;
+
+class HTTPRepoGetRequest : public HTTP::Request {
+public:
+  HTTPRepoGetRequest(HTTPDirectory *d, const std::string &u)
+      : HTTP::Request(u), _directory(d) {}
+
+  virtual void cancel();
+
+  size_t contentSize() const { return _contentSize; }
+
+  void setContentSize(size_t sz) { _contentSize = sz; }
+
+protected:
+  HTTPDirectory *_directory;
+  size_t _contentSize = 0;
+};
+
+using RepoRequestPtr = SGSharedPtr<HTTPRepoGetRequest>;
+
+class HTTPRepoPrivate {
+public:
+
+
+  HTTPRepository::FailureVec failures;
+  int maxPermittedFailures = 16;
+
+  HTTPRepoPrivate(HTTPRepository *parent)
+      : p(parent), isUpdating(false), status(HTTPRepository::REPO_NO_ERROR),
+        totalDownloaded(0) {
+    ;
+  }
+
+  ~HTTPRepoPrivate();
+
+  HTTPRepository *p; // link back to outer
+  HTTP::Client *http;
+  std::string baseUrl;
+  SGPath basePath;
+  bool isUpdating;
+  HTTPRepository::ResultCode status;
+  HTTPDirectory_ptr rootDir;
+  size_t totalDownloaded;
+  HTTPRepository::SyncPredicate syncPredicate;
+
+  HTTP::Request_ptr updateFile(HTTPDirectory *dir, const std::string &name,
+                               size_t sz);
+  HTTP::Request_ptr updateDir(HTTPDirectory *dir, const std::string &hash,
+                              size_t sz);
+
+  void failedToGetRootIndex(HTTPRepository::ResultCode st);
+  void failedToUpdateChild(const SGPath &relativePath,
+                           HTTPRepository::ResultCode fileStatus);
+
+  void updatedChildSuccessfully(const SGPath &relativePath);
+
+  void checkForComplete();
+
+  typedef std::vector<RepoRequestPtr> RequestVector;
+  RequestVector queuedRequests, activeRequests;
+
+  void makeRequest(RepoRequestPtr req);
+
+  enum class RequestFinish { Done, Retry };
+
+  void finishedRequest(const RepoRequestPtr &req, RequestFinish retryRequest);
+
+  HTTPDirectory *getOrCreateDirectory(const std::string &path);
+  bool deleteDirectory(const std::string &relPath, const SGPath &absPath);
+
+
+  typedef std::vector<HTTPDirectory_ptr> DirectoryVector;
+  DirectoryVector directories;
+
+  void scheduleUpdateOfChildren(HTTPDirectory *dir);
+
+  SGPath installedCopyPath;
+
+  int countDirtyHashCaches() const;
+  void flushHashCaches();
+
+  enum ProcessResult { ProcessContinue, ProcessDone, ProcessFailed };
+
+  using RepoProcessTask = std::function<ProcessResult(HTTPRepoPrivate *repo)>;
+
+  void addTask(RepoProcessTask task);
+
+  std::deque<RepoProcessTask> pendingTasks;
+};
+
+} // namespace simgear

simgear/io/HTTPRequest.cxx

@@ -56,6 +56,15 @@ Request::~Request()
 
 }
 
+void Request::prepareForRetry() {
+  setReadyState(UNSENT);
+  _willClose = false;
+  _connectionCloseHeader = false;
+  _responseStatus = 0;
+  _responseLength = 0;
+  _receivedBodyBytes = 0;
+}
+
 //------------------------------------------------------------------------------
 Request* Request::done(const Callback& cb)
 {
@@ -193,13 +202,16 @@ void Request::onDone()
 //------------------------------------------------------------------------------
 void Request::onFail()
 {
-  SG_LOG
-  (
-    SG_IO,
-    SG_INFO,
-    "request failed:" << url() << " : "
-                      << responseCode() << "/" << responseReason()
-  );
+  // log if we FAIELD< but not if we CANCELLED
+  if (_ready_state == FAILED) {
+    SG_LOG
+    (
+      SG_IO,
+      SG_INFO,
+      "request failed:" << url() << " : "
+                        << responseCode() << "/" << responseReason()
+    );
+  }
 }
 
 //------------------------------------------------------------------------------
@@ -344,12 +356,17 @@ void Request::setSuccess(int code)
 //------------------------------------------------------------------------------
 void Request::setFailure(int code, const std::string& reason)
 {
+  // we use -1 for cancellation, don't be noisy in that case
+  if (code >= 0) { 
     SG_LOG(SG_IO, SG_WARN, "HTTP request: set failure:" << code << " reason " << reason);
+  }
+
   _responseStatus = code;
   _responseReason = reason;
 
-  if( !isComplete() )
-    setReadyState(FAILED);
+  if( !isComplete() ) {
+    setReadyState(code < 0 ? CANCELLED : FAILED);
+  }
 }
 
 //------------------------------------------------------------------------------
@@ -373,6 +390,12 @@ void Request::setReadyState(ReadyState state)
 
     _cb_fail(this);
   }
+  else if (state == CANCELLED )
+  {
+    onFail(); // do this for compatability
+    onAlways();
+    _cb_fail(this);
+  }
   else
     return;
 
@@ -402,7 +425,7 @@ bool Request::serverSupportsPipelining() const
 //------------------------------------------------------------------------------
 bool Request::isComplete() const
 {
-  return _ready_state == DONE || _ready_state == FAILED;
+  return _ready_state == DONE || _ready_state == FAILED || _ready_state == CANCELLED;
 }
 
 //------------------------------------------------------------------------------

simgear/io/HTTPRequest.hxx

@@ -54,7 +54,8 @@ public:
       HEADERS_RECEIVED,
       LOADING,
       DONE,
-      FAILED
+      FAILED,
+      CANCELLED
     };
 
     virtual ~Request();
@@ -207,7 +208,9 @@ public:
      */
     bool serverSupportsPipelining() const;
 
-protected:
+    virtual void prepareForRetry();
+
+  protected:
     Request(const std::string& url, const std::string method = "GET");
 
     virtual void requestStart();
@@ -221,12 +224,14 @@ protected:
     virtual void onFail();
     virtual void onAlways();
 
-    void setFailure(int code, const std::string& reason);
     void setSuccess(int code);
-private:
+    void setFailure(int code, const std::string &reason);
+
+  private:
     friend class Client;
     friend class Connection;
     friend class ContentDecoder;
+    friend class TestApi;
 
     Request(const Request&); // = delete;
     Request& operator=(const Request&); // = delete;

simgear/io/HTTPTestApi_private.hxx

@@ -0,0 +1,45 @@
+// This library is free software; you can redistribute it and/or
+// modify it under the terms of the GNU Library General Public
+// License as published by the Free Software Foundation; either
+// version 2 of the License, or (at your option) any later version.
+//
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+// Library General Public License for more details.
+//
+// You should have received a copy of the GNU Library General Public
+// License along with this library; if not, write to the Free Software
+// Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301, USA
+
+#pragma once
+
+#include <functional>
+
+#include "HTTPRequest.hxx"
+
+namespace simgear {
+namespace HTTP {
+
+class Client;
+
+using ResponseDoneCallback =
+    std::function<bool(int curlResult, Request_ptr req)>;
+
+/**
+ * @brief this API is for unit-testing HTTP code.
+ * Don't use it for anything else. It's for unit-testing.
+ */
+class TestApi {
+public:
+  // alow test suite to manipulate requests to simulate network errors;
+  // without this, it's hard to provoke certain failures in a loop-back
+  // network sitation.
+  static void setResponseDoneCallback(Client *cl, ResponseDoneCallback cb);
+
+  static void markRequestAsFailed(Request_ptr req, int curlCode,
+                                  const std::string &message);
+};
+
+} // namespace HTTP
+} // namespace simgear

simgear/io/iostreams/gzfstream.cxx

@@ -185,6 +185,9 @@ gzfilebuf::setcompressionstrategy( int comp_strategy )
 
 z_off_t
 gzfilebuf::approxOffset() {
+    #ifdef __OpenBSD__
+    z_off_t res = 0;
+    #else
     z_off_t res = gzoffset(file);
 
     if (res == -1) {
@@ -201,7 +204,7 @@ gzfilebuf::approxOffset() {
         SG_LOG( SG_GENERAL, SG_ALERT, errMsg );
         throw sg_io_exception(errMsg);
     }
-
+    #endif
     return res;
 }
 

simgear/io/sg_binobj.cxx

@@ -42,6 +42,7 @@
 
 #include <simgear/bucket/newbucket.hxx>
 #include <simgear/misc/sg_path.hxx>
+#include <simgear/misc/strutils.hxx>
 #include <simgear/math/SGGeometry.hxx>
 #include <simgear/structure/exception.hxx>
 
@@ -563,6 +564,12 @@ bool SGBinObject::read_bin( const SGPath& file ) {
     // read headers
     unsigned int header;
     sgReadUInt( fp, &header );
+
+    if (sgReadError()) {
+        gzclose(fp);
+        throw sg_io_exception("Unable to read BTG header: " + simgear::strutils::error_string(errno), sg_location(file));
+    }
+
     if ( ((header & 0xFF000000) >> 24) == 'S' &&
          ((header & 0x00FF0000) >> 16) == 'G' ) {
 

simgear/io/sg_file.cxx

@@ -71,21 +71,31 @@ SGFile::~SGFile() {
 std::string SGFile::computeHash()
 {
     if (!file_name.exists())
-        return std::string();
+        return {};
+        
     simgear::sha1nfo info;
     sha1_init(&info);
-    char* buf = static_cast<char*>(malloc(1024 * 1024));
+    
+    // unique_ptr with custom deleter for exception safety
+    const int bufSize = 1024 * 1024;
+    std::unique_ptr<char, std::function<void(char*)>> buf{static_cast<char*>(malloc(bufSize)), 
+        [](char* p) { free(p); }};
+
+    if (!buf) {
+        SG_LOG(SG_IO, SG_ALERT, "Failed to malloc buffer for SHA1 check");
+    }
+
     size_t readLen;
     SGBinaryFile f(file_name);
     if (!f.open(SG_IO_IN)) {
-        throw sg_io_exception("Couldn't open file for compute hash", file_name);
+        SG_LOG(SG_IO, SG_ALERT, "SGFile::computeHash: Failed to open " << file_name);
+        return {};
     }
-    while ((readLen = f.read(buf, 1024 * 1024)) > 0) {
-        sha1_write(&info, buf, readLen);
+    while ((readLen = f.read(buf.get(), bufSize)) > 0) {
+        sha1_write(&info, buf.get(), readLen);
     }
 
     f.close();
-    free(buf);
     std::string hashBytes((char*)sha1_result(&info), HASH_LENGTH);
     return simgear::strutils::encodeHex(hashBytes);
 }

simgear/io/test_repository.cxx

@@ -1,16 +1,18 @@
 #include <cassert>
 #include <cstdlib>
+#include <errno.h>
+#include <fcntl.h>
+#include <functional>
 #include <iostream>
 #include <map>
 #include <sstream>
-#include <errno.h>
-#include <fcntl.h>
 
 #include <simgear/simgear_config.h>
 
-#include "test_HTTP.hxx"
-#include "HTTPRepository.hxx"
 #include "HTTPClient.hxx"
+#include "HTTPRepository.hxx"
+#include "HTTPTestApi_private.hxx"
+#include "test_HTTP.hxx"
 
 #include <simgear/misc/strutils.hxx>
 #include <simgear/misc/sg_hash.hxx>
@@ -25,6 +27,8 @@
 
 using namespace simgear;
 
+using TestApi = simgear::HTTP::TestApi;
+
 std::string dataForFile(const std::string& parentName, const std::string& name, int revision)
 {
     std::ostringstream os;
@@ -45,6 +49,9 @@ std::string hashForData(const std::string& d)
     return strutils::encodeHex(sha1_result(&info), HASH_LENGTH);
 }
 
+class TestRepoEntry;
+using AccessCallback = std::function<void(TestRepoEntry &entry)>;
+
 class TestRepoEntry
 {
 public:
@@ -70,7 +77,8 @@ public:
     int requestCount;
     bool getWillFail;
     bool returnCorruptData;
-    std::unique_ptr<SGCallback> accessCallback;
+
+    AccessCallback accessCallback;
 
     void clearRequestCounts();
 
@@ -270,8 +278,8 @@ public:
                 return;
             }
 
-            if (entry->accessCallback.get()) {
-                (*entry->accessCallback)();
+            if (entry->accessCallback) {
+              entry->accessCallback(*entry);
             }
 
             if (entry->getWillFail) {
@@ -282,20 +290,29 @@ public:
             entry->requestCount++;
 
             std::string content;
+            bool closeSocket = false;
+            size_t contentSize = 0;
+
             if (entry->returnCorruptData) {
                 content = dataForFile("!$£$!" + entry->parent->name,
                                       "corrupt_" + entry->name,
                                       entry->revision);
+                contentSize = content.size();
             } else {
-                content = entry->data();
+              content = entry->data();
+              contentSize = content.size();
             }
 
             std::stringstream d;
             d << "HTTP/1.1 " << 200 << " " << reasonForCode(200) << "\r\n";
-            d << "Content-Length:" << content.size() << "\r\n";
+            d << "Content-Length:" << contentSize << "\r\n";
             d << "\r\n"; // final CRLF to terminate the headers
             d << content;
             push(d.str().c_str());
+
+            if (closeSocket) {
+              closeWhenDone();
+            }
         } else {
             sendErrorResponse(404, false, "");
         }
@@ -392,6 +409,7 @@ void waitForUpdateComplete(HTTP::Client* cl, HTTPRepository* repo)
         cl->update();
         testServer.poll();
 
+        repo->process();
         if (!repo->isDoingSync()) {
             return;
         }
@@ -401,6 +419,16 @@ void waitForUpdateComplete(HTTP::Client* cl, HTTPRepository* repo)
     std::cerr << "timed out" << std::endl;
 }
 
+void runForTime(HTTP::Client *cl, HTTPRepository *repo, int msec = 15) {
+  SGTimeStamp start(SGTimeStamp::now());
+  while (start.elapsedMSec() < msec) {
+    cl->update();
+    testServer.poll();
+    repo->process();
+    SGTimeStamp::sleepForMSec(1);
+  }
+}
+
 void testBasicClone(HTTP::Client* cl)
 {
     std::unique_ptr<HTTPRepository> repo;
@@ -618,9 +646,19 @@ void testAbandonCorruptFiles(HTTP::Client* cl)
     repo->setBaseUrl("http://localhost:2000/repo");
     repo->update();
     waitForUpdateComplete(cl, repo.get());
-    if (repo->failure() != HTTPRepository::REPO_ERROR_CHECKSUM) {
-        std::cerr << "Got failure state:" << repo->failure() << std::endl;
-        throw sg_exception("Bad result from corrupt files test");
+    if (repo->failure() != HTTPRepository::REPO_PARTIAL_UPDATE) {
+      std::cerr << "Got failure state:" << repo->failure() << std::endl;
+      throw sg_exception("Bad result from corrupt files test");
+    }
+
+    auto failedFiles = repo->failures();
+    if (failedFiles.size() != 1) {
+      throw sg_exception("Bad result from corrupt files test");
+    }
+
+    if (failedFiles.front().path.utf8Str() != "dirB/subdirG/fileBGA") {
+      throw sg_exception("Bad path from corrupt files test:" +
+                         failedFiles.front().path.utf8Str());
     }
 
     repo.reset();
@@ -657,15 +695,21 @@ void testServerModifyDuringSync(HTTP::Client* cl)
     repo.reset(new HTTPRepository(p, cl));
     repo->setBaseUrl("http://localhost:2000/repo");
 
-    global_repo->findEntry("dirA/fileAA")->accessCallback.reset(make_callback(&modifyBTree));
+    global_repo->findEntry("dirA/fileAA")->accessCallback =
+        [](const TestRepoEntry &r) {
+          std::cout << "Modifying sub-tree" << std::endl;
+          global_repo->findEntry("dirB/subdirA/fileBAC")->revision++;
+          global_repo->defineFile("dirB/subdirZ/fileBZA");
+          global_repo->findEntry("dirB/subdirB/fileBBB")->revision++;
+        };
 
     repo->update();
     waitForUpdateComplete(cl, repo.get());
 
-    global_repo->findEntry("dirA/fileAA")->accessCallback.reset();
+    global_repo->findEntry("dirA/fileAA")->accessCallback = AccessCallback{};
 
-    if (repo->failure() != HTTPRepository::REPO_ERROR_CHECKSUM) {
-        throw sg_exception("Bad result from modify during sync test");
+    if (repo->failure() != HTTPRepository::REPO_PARTIAL_UPDATE) {
+      throw sg_exception("Bad result from modify during sync test");
     }
 
     std::cout << "Passed test modify server during sync" << std::endl;
@@ -755,6 +799,103 @@ void testCopyInstalledChildren(HTTP::Client* cl)
     std::cout << "passed Copy installed children" << std::endl;
 }
 
+void testRetryAfterSocketFailure(HTTP::Client *cl) {
+  global_repo->clearRequestCounts();
+  global_repo->clearFailFlags();
+
+  std::unique_ptr<HTTPRepository> repo;
+  SGPath p(simgear::Dir::current().path());
+  p.append("http_repo_retry_after_socket_fail");
+  simgear::Dir pd(p);
+  if (pd.exists()) {
+    pd.removeChildren();
+  }
+
+  repo.reset(new HTTPRepository(p, cl));
+  repo->setBaseUrl("http://localhost:2000/repo");
+
+  int aaFailsRemaining = 2;
+  int subdirBAFailsRemaining = 2;
+  TestApi::setResponseDoneCallback(
+      cl, [&aaFailsRemaining, &subdirBAFailsRemaining](int curlResult,
+                                                       HTTP::Request_ptr req) {
+        if (req->url() == "http://localhost:2000/repo/dirA/fileAA") {
+          if (aaFailsRemaining == 0)
+            return false;
+
+          --aaFailsRemaining;
+          TestApi::markRequestAsFailed(req, 56, "Simulated socket failure");
+          return true;
+        } else if (req->url() ==
+                   "http://localhost:2000/repo/dirB/subdirA/.dirindex") {
+          if (subdirBAFailsRemaining == 0)
+            return false;
+
+          --subdirBAFailsRemaining;
+          TestApi::markRequestAsFailed(req, 56, "Simulated socket failure");
+          return true;
+        } else {
+          return false;
+        }
+      });
+
+  repo->update();
+  waitForUpdateComplete(cl, repo.get());
+
+  if (repo->failure() != HTTPRepository::REPO_NO_ERROR) {
+    throw sg_exception("Bad result from retry socket failure test");
+  }
+
+  verifyFileState(p, "dirA/fileAA");
+  verifyFileState(p, "dirB/subdirA/fileBAA");
+  verifyFileState(p, "dirB/subdirA/fileBAC");
+
+  verifyRequestCount("dirA/fileAA", 3);
+  verifyRequestCount("dirB/subdirA", 3);
+  verifyRequestCount("dirB/subdirA/fileBAC", 1);
+}
+
+void testPersistentSocketFailure(HTTP::Client *cl) {
+  global_repo->clearRequestCounts();
+  global_repo->clearFailFlags();
+
+  std::unique_ptr<HTTPRepository> repo;
+  SGPath p(simgear::Dir::current().path());
+  p.append("http_repo_persistent_socket_fail");
+  simgear::Dir pd(p);
+  if (pd.exists()) {
+    pd.removeChildren();
+  }
+
+  repo.reset(new HTTPRepository(p, cl));
+  repo->setBaseUrl("http://localhost:2000/repo");
+
+  TestApi::setResponseDoneCallback(
+      cl, [](int curlResult, HTTP::Request_ptr req) {
+        const auto url = req->url();
+        if (url.find("http://localhost:2000/repo/dirB") == 0) {
+          TestApi::markRequestAsFailed(req, 56, "Simulated socket failure");
+          return true;
+        }
+
+        return false;
+      });
+
+  repo->update();
+  waitForUpdateComplete(cl, repo.get());
+
+  if (repo->failure() != HTTPRepository::REPO_PARTIAL_UPDATE) {
+    throw sg_exception("Bad result from retry socket failure test");
+  }
+
+  verifyFileState(p, "dirA/fileAA");
+  verifyRequestCount("dirA/fileAA", 1);
+
+  verifyRequestCount("dirD/fileDA", 1);
+  verifyRequestCount("dirD/subdirDA/fileDAA", 1);
+  verifyRequestCount("dirD/subdirDB/fileDBA", 1);
+}
+
 int main(int argc, char* argv[])
 {
   sglog().setLogLevels( SG_ALL, SG_INFO );
@@ -800,6 +941,8 @@ int main(int argc, char* argv[])
     cl.clearAllConnections();
 
     testCopyInstalledChildren(&cl);
+    testRetryAfterSocketFailure(&cl);
+    testPersistentSocketFailure(&cl);
 
     std::cout << "all tests passed ok" << std::endl;
     return 0;

simgear/io/untar.cxx

@@ -31,6 +31,8 @@
 #include <simgear/sg_inlines.h>
 #include <simgear/io/sg_file.hxx>
 #include <simgear/misc/sg_dir.hxx>
+#include <simgear/misc/strutils.hxx>
+
 #include <simgear/io/iostreams/sgstream.hxx>
 #include <simgear/debug/logstream.hxx>
 #include <simgear/package/unzip.h>
@@ -592,7 +594,7 @@ public:
 
 		outFile.open(path, std::ios::binary | std::ios::trunc | std::ios::out);
 		if (outFile.fail()) {
-			throw sg_io_exception("failed to open output file for writing", path);
+			throw sg_io_exception("failed to open output file for writing:" + strutils::error_string(errno), path);
 		}
 
 		while (!eof) {

simgear/misc/ResourceManager.cxx

@@ -49,6 +49,11 @@ ResourceManager* ResourceManager::instance()
     return static_manager;
 }    
 
+bool ResourceManager::haveInstance()
+{
+    return static_manager != nullptr;
+}
+
 ResourceManager::~ResourceManager()
 {
     assert(this == static_manager);
@@ -56,6 +61,15 @@ ResourceManager::~ResourceManager()
     std::for_each(_providers.begin(), _providers.end(), 
         [](ResourceProvider* p) { delete p; });
 }
+
+void ResourceManager::reset()
+{
+    if (static_manager) {
+        delete static_manager;
+        static_manager = nullptr;
+    }
+}
+
 /**
  * trivial provider using a fixed base path
  */
@@ -107,6 +121,8 @@ void ResourceManager::removeProvider(ResourceProvider* aProvider)
         SG_LOG(SG_GENERAL, SG_DEV_ALERT, "unknown provider doing remove");
         return;
     }
+
+    _providers.erase(it);
 }
 
 SGPath ResourceManager::findPath(const std::string& aResource, SGPath aContext)

simgear/misc/ResourceManager.hxx

@@ -45,8 +45,12 @@ public:
       PRIORITY_HIGH = 1000
     } Priority;
 
-    static ResourceManager* instance();    
-    
+    static ResourceManager* instance();
+
+    static bool haveInstance();
+
+    static void reset();
+
     /**
      * add a simple fixed resource location, to resolve against
      */

simgear/misc/sg_path.cxx

@@ -44,6 +44,7 @@
 #if defined(SG_WINDOWS)
 #  include <direct.h>
 #  include <sys/utime.h>
+#  include <Shlwapi.h>
 #endif
 
 #include "sg_path.hxx"
@@ -194,7 +195,8 @@ SGPath::SGPath(PermissionChecker validator)
     _permission_checker(validator),
     _cached(false),
     _rwCached(false),
-    _cacheEnabled(true)
+    _cacheEnabled(true),
+    _existsCached(false)
 {
 }
 
@@ -205,7 +207,8 @@ SGPath::SGPath( const std::string& p, PermissionChecker validator )
     _permission_checker(validator),
     _cached(false),
     _rwCached(false),
-    _cacheEnabled(true)
+    _cacheEnabled(true),
+    _existsCached(false)
 {
     fix();
 }
@@ -230,7 +233,8 @@ SGPath::SGPath( const SGPath& p,
     _permission_checker(validator),
     _cached(false),
     _rwCached(false),
-    _cacheEnabled(p._cacheEnabled)
+    _cacheEnabled(p._cacheEnabled),
+    _existsCached(false)
 {
     append(r);
     fix();
@@ -510,6 +514,19 @@ void SGPath::checkAccess() const
 
 bool SGPath::exists() const
 {
+#if defined(SG_WINDOWS)
+  // optimisation: _wstat is slow, eg for TerraSync
+  if (!_cached && !_existsCached) {
+      std::wstring w(wstr());
+    if ((path.length() > 1) && (path.back() == '/')) {
+	    w.pop_back();
+    }
+
+    _existsCached = true;
+    _exists = PathFileExistsW(w.c_str());
+    return _exists;
+  }
+#endif
   validate();
   return _exists;
 }

simgear/misc/sg_path.hxx

@@ -356,6 +356,7 @@ private:
     mutable bool _exists : 1;
     mutable bool _isDir : 1;
     mutable bool _isFile : 1;
+    mutable bool _existsCached : 1; ///< only used on Windows
     mutable time_t _modTime;
     mutable size_t _size;
 };

simgear/misc/strutils.cxx

@@ -267,6 +267,38 @@ namespace simgear {
 	    return do_strip( s, BOTHSTRIP );
 	}
 
+    string makeStringSafeForPropertyName(const std::string& str)
+    {
+        // This function replaces all characters in 'str' that are not
+        // alphanumeric or '-'.
+        // TODO: make the function multibyte safe.
+        string res = str;
+
+        int index = 0;
+        for (char& c : res) {
+            if (!std::isalpha(c) && !std::isdigit(c) && c != '-') {
+                switch (c) {
+                case ' ':
+                case '\t':
+                case '\n':
+                case '\r':
+                case '_':
+                case '.':
+                case '/':
+                case '\\':
+                    res[index] = '-';
+                    break;
+                default:
+                    res[index] = '_';
+                    SG_LOG(SG_GENERAL, SG_WARN, "makeStringSafeForPropertyName: Modified '" << str << "' to '" << res << "'");
+                }
+            }
+            index++;
+        }
+
+        return res;
+    }
+
         void
         stripTrailingNewlines_inplace(string& s)
         {

simgear/misc/strutils.hxx

@@ -75,7 +75,9 @@ namespace simgear {
 	std::string rstrip( const std::string& s );
 	std::string strip( const std::string& s );
 
-	/**
+    std::string makeStringSafeForPropertyName(const std::string& str);
+
+    /**
          * Return a new string with any trailing \\r and \\n characters removed.
          * Typically useful to clean a CR-terminated line obtained from
          * std::getline() which, upon reading CRLF (\\r\\n), discards the Line

simgear/misc/strutils_test.cxx

@@ -737,6 +737,11 @@ void testDecodeHex()
     SG_VERIFY(decoded == data1);
 }
 
+void test_makeStringSafeForPropertyName()
+{
+    SG_CHECK_EQUAL(strutils::makeStringSafeForPropertyName(" ABC/01234\t:\\\"_*$"), "-ABC-01234-_-_-__");
+}
+
 int main(int argc, char* argv[])
 {
     test_strip();
@@ -761,6 +766,7 @@ int main(int argc, char* argv[])
     test_formatGeod();
     test_iequals();
     testDecodeHex();
-    
+    test_makeStringSafeForPropertyName();
+
     return EXIT_SUCCESS;
 }

simgear/nasal/code.c

@@ -36,7 +36,7 @@ void naRuntimeError(naContext c, const char* fmt, ...)
 
 void naRethrowError(naContext subc)
 {
-    strncpy(subc->callParent->error, subc->error, sizeof(subc->error));
+    strncpy(subc->callParent->error, subc->error, sizeof(subc->callParent->error));
     subc->callParent->dieArg = subc->dieArg;
     longjmp(subc->callParent->jumpHandle, 1);
 }

simgear/nasal/cppbind/NasalContext.cxx

@@ -21,6 +21,7 @@
 #include "NasalString.hxx"
 
 #include <cassert>
+#include <stdexcept> // for std::runtime_error
 
 namespace nasal
 {

simgear/nasal/iolib.c

@@ -7,7 +7,7 @@
 #include "iolib.h"
 
 static void ghostDestroy(void* g);
-naGhostType naIOGhostType = { ghostDestroy, "iofile" };
+naGhostType naIOGhostType = { ghostDestroy, "iofile", NULL, NULL };
 
 static struct naIOGhost* ioghost(naRef r)
 {

simgear/nasal/lib.c

@@ -472,7 +472,7 @@ static naRef f_sprintf(naContext c, naRef me, int argc, naRef* args)
         } else {
             arg = naNumValue(arg);
             if(naIsNil(arg))
-                fout = dosprintf(fstr, "nil");
+                fout = dosprintf("nil");
             else if(t=='d' || t=='i' || t=='c')
                 fout = dosprintf(fstr, (int)naNumValue(arg).num);
             else if(t=='o' || t=='u' || t=='x' || t=='X')

simgear/nasal/threadlib.c

@@ -9,10 +9,10 @@
 #include "code.h"
 
 static void lockDestroy(void* lock) { naFreeLock(lock); }
-static naGhostType LockType = { lockDestroy };
+static naGhostType LockType = { lockDestroy, NULL, NULL, NULL };
 
 static void semDestroy(void* sem) { naFreeSem(sem); }
-static naGhostType SemType = { semDestroy };
+static naGhostType SemType = { semDestroy, NULL, NULL, NULL };
 
 typedef struct {
     naContext ctx;

simgear/package/Catalog.cxx

@@ -595,13 +595,17 @@ void Catalog::setUserEnabled(bool b)
 
     m_userEnabled = b;
     SGPath disableMarkerFile = installRoot() / "_disabled_";
-    if (m_userEnabled) {
-        sg_ofstream of(disableMarkerFile);
-        of << "1\n"; // touch the file
+
+    if (m_userEnabled == false) {
+        sg_ofstream of(disableMarkerFile, std::ios::trunc | std::ios::out);
+        of << "1" << std::flush; // touch the file
+        of.close();
     } else {
-        bool ok = disableMarkerFile.remove();
-        if (!ok) {
-            SG_LOG(SG_GENERAL, SG_ALERT, "Failed to remove catalog-disable marker file:" << disableMarkerFile);
+        if (disableMarkerFile.exists()) {
+            const bool ok = disableMarkerFile.remove();
+            if (!ok) {
+                SG_LOG(SG_IO, SG_WARN, "Failed to remove catalog-disable marker file:" << disableMarkerFile);
+            }
         }
     }
 
@@ -639,16 +643,53 @@ void Catalog::processAlternate(SGPropertyNode_ptr alt)
         return;
     }
 
-    // we have an alternate ID, and it's differnt from our ID, so let's
+    // we have an alternate ID, and it's different from our ID, so let's
     // define a new catalog
     if (!altId.empty()) {
-        SG_LOG(SG_GENERAL, SG_INFO, "Adding new catalog:" << altId << " as version alternate for " << id());
-        // new catalog being added
-        createFromUrl(root(), altUrl);
-
-        // and we can go idle now
+      // don't auto-re-add Catalogs the user has explicilty rmeoved, that would
+      // suck
+      const auto removedByUser = root()->explicitlyRemovedCatalogs();
+      auto it = std::find(removedByUser.begin(), removedByUser.end(), altId);
+      if (it != removedByUser.end()) {
         changeStatus(Delegate::FAIL_VERSION);
         return;
+      }
+
+      SG_LOG(SG_GENERAL, SG_WARN,
+             "Adding new catalog:" << altId << " as version alternate for "
+                                   << id());
+      // new catalog being added
+      auto newCat = createFromUrl(root(), altUrl);
+
+      bool didRun = false;
+      newCat->m_migratedFrom = this;
+
+      auto migratePackagesCb = [didRun](Catalog *c) mutable {
+        // removing callbacks is awkward, so use this
+        // flag to only run once. (and hence, we need to be mutable)
+        if (didRun)
+          return;
+
+        if (c->status() == Delegate::STATUS_REFRESHED) {
+          didRun = true;
+
+          string_list existing;
+          for (const auto &pack : c->migratedFrom()->installedPackages()) {
+            existing.push_back(pack->id());
+          }
+
+          const int count = c->markPackagesForInstallation(existing);
+          SG_LOG(
+              SG_GENERAL, SG_INFO,
+              "Marked " << count
+                        << " packages from previous catalog for installation");
+        }
+      };
+
+      newCat->addStatusCallback(migratePackagesCb);
+      // and we can go idle now
+      changeStatus(Delegate::FAIL_VERSION);
+      return;
     }
 
     SG_LOG(SG_GENERAL, SG_INFO, "Migrating catalog " << id() << " to new URL:" << altUrl);
@@ -657,6 +698,26 @@ void Catalog::processAlternate(SGPropertyNode_ptr alt)
     root()->makeHTTPRequest(dl);
 }
 
+int Catalog::markPackagesForInstallation(const string_list &packageIds) {
+  int result = 0;
+
+  for (const auto &id : packageIds) {
+    auto ourPkg = getPackageById(id);
+    if (!ourPkg)
+      continue;
+
+    auto existing = ourPkg->existingInstall();
+    if (!existing) {
+      ourPkg->markForInstall();
+      ++result;
+    }
+  } // of outer package ID candidates iteration
+
+  return result;
+}
+
+CatalogRef Catalog::migratedFrom() const { return m_migratedFrom; }
+
 } // of namespace pkg
 
 } // of namespace simgear

simgear/package/Catalog.hxx

@@ -23,6 +23,7 @@
 #include <map>
 
 #include <simgear/misc/sg_path.hxx>
+#include <simgear/misc/strutils.hxx>
 #include <simgear/props/props.hxx>
 
 #include <simgear/structure/SGReferenced.hxx>
@@ -93,7 +94,7 @@ public:
 
     /**
      * retrieve all the packages in the catalog which are installed
-     * and have a pendig update
+     * and have a pending update
      */
     PackageList packagesNeedingUpdate() const;
 
@@ -151,7 +152,32 @@ public:
     
     bool isUserEnabled() const;
     void setUserEnabled(bool b);
-private:
+
+    /**
+     * Given a list of package IDs, mark all which exist in this package,
+     * for installation. ANy packahe IDs not present in this catalog,
+     * will be ignored.
+     *
+     * @result The number for packages newly marked for installation.
+     */
+    int markPackagesForInstallation(const string_list &packageIds);
+
+    /**
+     * When a catalog is added due to migration, this will contain the
+     * Catalog which triggered the add. Usually this will be a catalog
+     * corresponding to an earlier version.
+     *
+     * Note it's only valid at the time, the migration actually took place;
+     * when the new catalog is loaded from disk, this value will return
+     * null.
+     *
+     * This is intended to allow Uis to show a 'catalog was migrated'
+     * feedback, when they see a catalog refresh, which has a non-null
+     * value of this method.
+     */
+    CatalogRef migratedFrom() const;
+
+  private:
     Catalog(Root* aRoot);
 
     class Downloader;
@@ -197,6 +223,8 @@ private:
     PackageWeakMap m_variantDict;
 
     function_list<Callback> m_statusCallbacks;
+
+    CatalogRef m_migratedFrom;
 };
 
 } // of namespace pkg

simgear/package/CatalogTest.cxx

@@ -827,7 +827,10 @@ void testVersionMigrateToId(HTTP::Client* cl)
         
         it = std::find(enabledCats.begin(), enabledCats.end(), altCat);
         SG_VERIFY(it != enabledCats.end());
-        
+
+        SG_CHECK_EQUAL(altCat->packagesNeedingUpdate().size(),
+                       1); // should be the 737
+
         // install a parallel package from the new catalog
         pkg::PackageRef p2 = root->getPackageById("org.flightgear.test.catalog-alt.b737-NG");
         SG_CHECK_EQUAL(p2->id(), "b737-NG");
@@ -840,8 +843,8 @@ void testVersionMigrateToId(HTTP::Client* cl)
         pkg::PackageRef p3 = root->getPackageById("b737-NG");
         SG_CHECK_EQUAL(p2, p3);
     }
-    
-    // test that re-init-ing doesn't mirgate again
+
+    // test that re-init-ing doesn't migrate again
     {
         pkg::RootRef root(new pkg::Root(rootPath, "7.5"));
         root->setHTTPClient(cl);
@@ -1184,6 +1187,128 @@ void testMirrorsFailure(HTTP::Client* cl)
     
 }
 
+void testMigrateInstalled(HTTP::Client *cl) {
+  SGPath rootPath(simgear::Dir::current().path());
+  rootPath.append("pkg_migrate_installed");
+  simgear::Dir pd(rootPath);
+  pd.removeChildren();
+
+  pkg::RootRef root(new pkg::Root(rootPath, "8.1.2"));
+  root->setHTTPClient(cl);
+
+  pkg::CatalogRef oldCatalog, newCatalog;
+
+  {
+    oldCatalog = pkg::Catalog::createFromUrl(
+        root.ptr(), "http://localhost:2000/catalogTest1/catalog.xml");
+    waitForUpdateComplete(cl, root);
+
+    pkg::PackageRef p1 =
+        root->getPackageById("org.flightgear.test.catalog1.b747-400");
+    p1->install();
+    auto p2 = root->getPackageById("org.flightgear.test.catalog1.c172p");
+    p2->install();
+    auto p3 = root->getPackageById("org.flightgear.test.catalog1.b737-NG");
+    p3->install();
+    waitForUpdateComplete(cl, root);
+  }
+
+  {
+    newCatalog = pkg::Catalog::createFromUrl(
+        root.ptr(), "http://localhost:2000/catalogTest2/catalog.xml");
+    waitForUpdateComplete(cl, root);
+
+    string_list existing;
+    for (const auto &pack : oldCatalog->installedPackages()) {
+      existing.push_back(pack->id());
+    }
+
+    SG_CHECK_EQUAL(4, existing.size());
+
+    int result = newCatalog->markPackagesForInstallation(existing);
+    SG_CHECK_EQUAL(2, result);
+    SG_CHECK_EQUAL(2, newCatalog->packagesNeedingUpdate().size());
+
+    auto p1 = root->getPackageById("org.flightgear.test.catalog2.b737-NG");
+    auto ins = p1->existingInstall();
+    SG_CHECK_EQUAL(0, ins->revsion());
+  }
+
+  {
+    root->scheduleAllUpdates();
+    waitForUpdateComplete(cl, root);
+
+    SG_CHECK_EQUAL(0, newCatalog->packagesNeedingUpdate().size());
+
+    auto p1 = root->getPackageById("org.flightgear.test.catalog2.b737-NG");
+    auto ins = p1->existingInstall();
+    SG_CHECK_EQUAL(ins->revsion(), p1->revision());
+  }
+}
+
+void testDontMigrateRemoved(HTTP::Client *cl) {
+  global_catalogVersion = 2; // version which has migration info
+  SGPath rootPath(simgear::Dir::current().path());
+  rootPath.append("cat_dont_migrate_id");
+  simgear::Dir pd(rootPath);
+  pd.removeChildren();
+
+  // install and mnaully remove the alt catalog
+
+  {
+    pkg::RootRef root(new pkg::Root(rootPath, "8.1.2"));
+    root->setHTTPClient(cl);
+
+    pkg::CatalogRef c = pkg::Catalog::createFromUrl(
+        root.ptr(), "http://localhost:2000/catalogTest1/catalog-alt.xml");
+    waitForUpdateComplete(cl, root);
+
+    root->removeCatalogById("org.flightgear.test.catalog-alt");
+  }
+
+  // install the migration catalog
+  {
+    pkg::RootRef root(new pkg::Root(rootPath, "8.1.2"));
+    root->setHTTPClient(cl);
+
+    pkg::CatalogRef c = pkg::Catalog::createFromUrl(
+        root.ptr(), "http://localhost:2000/catalogTest1/catalog.xml");
+    waitForUpdateComplete(cl, root);
+    SG_VERIFY(c->isEnabled());
+  }
+
+  // change version to an alternate one
+  {
+    pkg::RootRef root(new pkg::Root(rootPath, "7.5"));
+
+    auto removed = root->explicitlyRemovedCatalogs();
+    auto j = std::find(removed.begin(), removed.end(),
+                       "org.flightgear.test.catalog-alt");
+    SG_VERIFY(j != removed.end());
+
+    root->setHTTPClient(cl);
+
+    // this would tirgger migration, but we blocked it
+    root->refresh(true);
+    waitForUpdateComplete(cl, root);
+
+    pkg::CatalogRef cat = root->getCatalogById("org.flightgear.test.catalog1");
+    SG_VERIFY(!cat->isEnabled());
+    SG_CHECK_EQUAL(cat->status(), pkg::Delegate::FAIL_VERSION);
+    SG_CHECK_EQUAL(cat->id(), "org.flightgear.test.catalog1");
+    SG_CHECK_EQUAL(cat->url(),
+                   "http://localhost:2000/catalogTest1/catalog.xml");
+
+    auto enabledCats = root->catalogs();
+    auto it = std::find(enabledCats.begin(), enabledCats.end(), cat);
+    SG_VERIFY(it == enabledCats.end());
+
+    // check the new catalog
+    auto altCat = root->getCatalogById("org.flightgear.test.catalog-alt");
+    SG_VERIFY(altCat.get() == nullptr);
+  }
+}
+
 int main(int argc, char* argv[])
 {
     sglog().setLogLevels( SG_ALL, SG_WARN );
@@ -1228,7 +1353,11 @@ int main(int argc, char* argv[])
     testInstallBadPackage(&cl);
     
     testMirrorsFailure(&cl);
-    
+
+    testMigrateInstalled(&cl);
+
+    testDontMigrateRemoved(&cl);
+
     cerr << "Successfully passed all tests!" << endl;
     return EXIT_SUCCESS;
 }

simgear/package/Package.cxx

@@ -34,7 +34,7 @@ namespace simgear {
 namespace pkg {
 
 Package::Package(const SGPropertyNode* aProps, CatalogRef aCatalog) :
-    m_catalog(aCatalog)
+    m_catalog(aCatalog.get())
 {
     initWithProps(aProps);
 }
@@ -198,6 +198,11 @@ InstallRef Package::install()
 {
     InstallRef ins = existingInstall();
     if (ins) {
+      // if there's updates, treat this as a 'start update' request
+      if (ins->hasUpdate()) {
+        m_catalog->root()->scheduleToUpdate(ins);
+      }
+
         return ins;
     }
 
@@ -210,13 +215,39 @@ InstallRef Package::install()
     return ins;
 }
 
+InstallRef Package::markForInstall() {
+  InstallRef ins = existingInstall();
+  if (ins) {
+    return ins;
+  }
+
+  const auto pd = pathOnDisk();
+
+  Dir dir(pd);
+  if (!dir.create(0700)) {
+    SG_LOG(SG_IO, SG_ALERT,
+           "Package::markForInstall: couldn't create directory at:" << pd);
+    return {};
+  }
+
+  ins = new Install{this, pd};
+  _install_cb(this, ins); // not sure if we should trigger the callback for this
+
+  // repeat for dependencies to be kind
+  for (auto dep : dependencies()) {
+    dep->markForInstall();
+  }
+
+  return ins;
+}
+
 InstallRef Package::existingInstall(const InstallCallback& cb) const
 {
     InstallRef install;
     try {
         install = m_catalog->root()->existingInstallForPackage(const_cast<Package*>(this));
     } catch (std::exception& ) {
-        return InstallRef();
+      return {};
     }
 
   if( cb )
@@ -235,6 +266,11 @@ std::string Package::id() const
     return m_id;
 }
 
+CatalogRef Package::catalog() const
+{
+    return {m_catalog};
+}
+
 std::string Package::qualifiedId() const
 {
     return m_catalog->id() + "." + id();

simgear/package/Package.hxx

@@ -61,6 +61,13 @@ public:
     InstallRef
     existingInstall(const InstallCallback& cb = InstallCallback()) const;
 
+    /**
+     * Mark this package for installation, but don't actually start the
+     * download process. This creates the on-disk placeholder, so
+     * the package will appear an eededing to be updated.
+     */
+    InstallRef markForInstall();
+
     bool isInstalled() const;
 
     /**
@@ -130,8 +137,7 @@ public:
 
     size_t fileSizeBytes() const;
 
-    CatalogRef catalog() const
-        { return m_catalog; }
+    CatalogRef catalog() const;
 
     bool matches(const SGPropertyNode* aFilter) const;
 
@@ -236,7 +242,7 @@ private:
     SGPropertyNode_ptr m_props;
     std::string m_id;
     string_set m_tags;
-    CatalogRef m_catalog;
+    Catalog* m_catalog = nullptr; // non-owning ref, explicitly
     string_list m_variants;
 
     mutable function_list<InstallCallback> _install_cb;

simgear/package/Root.cxx

@@ -283,6 +283,32 @@ public:
         fireDataForThumbnail(url, reinterpret_cast<const uint8_t*>(bytes.data()), bytes.size());
     }
 
+    void writeRemovedCatalogsFile() const {
+      SGPath p = path / "RemovedCatalogs";
+      sg_ofstream stream(p, std::ios::out | std::ios::trunc | std::ios::binary);
+      for (const auto &cid : manuallyRemovedCatalogs) {
+        stream << cid << "\n";
+      }
+      stream.close();
+    }
+
+    void loadRemovedCatalogsFile() {
+      manuallyRemovedCatalogs.clear();
+      SGPath p = path / "RemovedCatalogs";
+      if (!p.exists())
+        return;
+
+      sg_ifstream stream(p, std::ios::in);
+      while (!stream.eof()) {
+        std::string line;
+        std::getline(stream, line);
+        const auto trimmed = strutils::strip(line);
+        if (!trimmed.empty()) {
+          manuallyRemovedCatalogs.push_back(trimmed);
+        }
+      } // of lines iteration
+    }
+
     DelegateVec delegates;
 
     SGPath path;
@@ -312,6 +338,9 @@ public:
 
     typedef std::map<PackageRef, InstallRef> InstallCache;
     InstallCache m_installs;
+
+    /// persistent list of catalogs the user has manually removed
+    string_list manuallyRemovedCatalogs;
 };
 
 
@@ -400,6 +429,8 @@ Root::Root(const SGPath& aPath, const std::string& aVersion) :
         thumbsCacheDir.create(0755);
     }
 
+    d->loadRemovedCatalogsFile();
+
     for (SGPath c : dir.children(Dir::TYPE_DIR | Dir::NO_DOT_OR_DOTDOT)) {
         // note this will set the catalog status, which will insert into
         // disabled catalogs automatically if necesary
@@ -621,6 +652,13 @@ void Root::scheduleToUpdate(InstallRef aInstall)
     }
 }
 
+void Root::scheduleAllUpdates() {
+  auto toBeUpdated = packagesNeedingUpdate(); // make a copy
+  for (const auto &u : toBeUpdated) {
+    scheduleToUpdate(u->existingInstall());
+  }
+}
+
 bool Root::isInstallQueued(InstallRef aInstall) const
 {
     auto it = std::find(d->updateDeque.begin(), d->updateDeque.end(), aInstall);
@@ -684,20 +722,17 @@ void Root::catalogRefreshStatus(CatalogRef aCat, Delegate::StatusCode aReason)
     auto catIt = d->catalogs.find(aCat->id());
     d->fireRefreshStatus(aCat, aReason);
 
-    if (aReason == Delegate::STATUS_IN_PROGRESS) {
-        d->refreshing.insert(aCat);
-    } else {
-        d->refreshing.erase(aCat);
-    }
-
-    if ((aReason == Delegate::STATUS_REFRESHED) && (catIt == d->catalogs.end())) {
+    if (aCat->isUserEnabled() &&
+        (aReason == Delegate::STATUS_REFRESHED) && 
+        (catIt == d->catalogs.end())) 
+    {
         assert(!aCat->id().empty());
         d->catalogs.insert(catIt, CatalogDict::value_type(aCat->id(), aCat));
 
-        // catalog might have been previously disabled, let's remove in that case
+    // catalog might have been previously disabled, let's remove in that case
         auto j = std::find(d->disabledCatalogs.begin(),
-                           d->disabledCatalogs.end(),
-                           aCat);
+                        d->disabledCatalogs.end(),
+                        aCat);
         if (j != d->disabledCatalogs.end()) {
             SG_LOG(SG_GENERAL, SG_INFO, "re-enabling disabled catalog:" << aCat->id());
             d->disabledCatalogs.erase(j);
@@ -705,7 +740,7 @@ void Root::catalogRefreshStatus(CatalogRef aCat, Delegate::StatusCode aReason)
     }
 
     if (!aCat->isEnabled()) {
-        // catalog has errors, disable it
+        // catalog has errors or was disabled by user, disable it
         auto j = std::find(d->disabledCatalogs.begin(),
                            d->disabledCatalogs.end(),
                            aCat);
@@ -720,6 +755,17 @@ void Root::catalogRefreshStatus(CatalogRef aCat, Delegate::StatusCode aReason)
         }
     } // of catalog is disabled
 
+    // remove from refreshing /after/ checking for enable / disabled, since for
+    // new catalogs, the reference in d->refreshing might be our /only/
+    // reference to the catalog. Once the refresh is done (either failed or
+    // succeeded) the Catalog will be in either d->catalogs or
+    // d->disabledCatalogs
+    if (aReason == Delegate::STATUS_IN_PROGRESS) {
+      d->refreshing.insert(aCat);
+    } else {
+      d->refreshing.erase(aCat);
+    }
+
     if (d->refreshing.empty()) {
         d->fireRefreshStatus(CatalogRef(), Delegate::STATUS_REFRESHED);
         d->firePackagesChanged();
@@ -780,6 +826,9 @@ bool Root::removeCatalogById(const std::string& aId)
             << "failed to remove directory");
     }
 
+    d->manuallyRemovedCatalogs.push_back(aId);
+    d->writeRemovedCatalogsFile();
+
     // notify that a catalog is being removed
     d->firePackagesChanged();
 
@@ -851,6 +900,10 @@ void Root::unregisterInstall(InstallRef ins)
     d->fireFinishUninstall(ins->package());
 }
 
+string_list Root::explicitlyRemovedCatalogs() const {
+  return d->manuallyRemovedCatalogs;
+}
+
 } // of namespace pkg
 
 } // of namespace simgear

simgear/package/Root.hxx

@@ -155,7 +155,22 @@ public:
     void requestThumbnailData(const std::string& aUrl);
     
     bool isInstallQueued(InstallRef aInstall) const;
-private:
+
+    /**
+     * Mark all 'to be updated' packages for update now
+     */
+    void scheduleAllUpdates();
+
+    /**
+     * @brief list of catalog IDs, the user has explicitly removed via
+     * removeCatalogById(). This is important to allow the user to opt-out
+     * of migrated packages.
+     *
+     * This information is stored in a helper file, in the root directory
+     */
+    string_list explicitlyRemovedCatalogs() const;
+
+  private:
     friend class Install;
     friend class Catalog;    
     friend class Package;

simgear/package/catalogTest2/catalog.xml

@@ -0,0 +1,86 @@
+<?xml version="1.0"?>
+
+<PropertyList>
+    <id>org.flightgear.test.catalog2</id>
+    <description>Second test catalog</description>
+    <url>http://localhost:2000/catalogTest2/catalog.xml</url>
+    <catalog-version>4</catalog-version>
+
+    <version>8.1.*</version>
+    <version>8.0.0</version>
+    <version>8.2.0</version>
+
+    <package>
+        <id>alpha</id>
+        <name>Alpha package</name>
+        <revision type="int">8</revision>
+        <file-size-bytes type="int">593</file-size-bytes>
+
+        <md5>a469c4b837f0521db48616cfe65ac1ea</md5>
+        <url>http://localhost:2000/catalogTest1/alpha.zip</url>
+
+        <dir>alpha</dir>
+
+    </package>
+
+
+    <package>
+        <id>b737-NG</id>
+        <name>Boeing 737 NG</name>
+        <dir>b737NG</dir>
+        <description>A popular twin-engined narrow body jet</description>
+        <revision type="int">111</revision>
+        <file-size-bytes type="int">860</file-size-bytes>
+
+        <tag>boeing</tag>
+        <tag>jet</tag>
+        <tag>ifr</tag>
+
+      <!-- not within a localized element -->
+          <de>
+            <description>German description of B737NG XYZ</description>
+          </de>
+          <fr>
+            <description>French description of B737NG</description>
+          </fr>
+
+        <rating>
+          <FDM type="int">5</FDM>
+          <systems type="int">5</systems>
+          <model type="int">4</model>
+          <cockpit type="int">4</cockpit>
+        </rating>
+
+        <md5>a94ca5704f305b90767f40617d194ed6</md5>
+        <url>http://localhost:2000/mirrorA/b737.tar.gz</url>
+        <url>http://localhost:2000/mirrorB/b737.tar.gz</url>
+        <url>http://localhost:2000/mirrorC/b737.tar.gz</url>
+      </package>
+
+    <package>
+        <id>b747-400</id>
+        <name>Boeing 747-400</name>
+        <dir>b744</dir>
+        <description>A popular four-engined wide-body jet</description>
+        <revision type="int">111</revision>
+        <file-size-bytes type="int">860</file-size-bytes>
+
+        <tag>boeing</tag>
+        <tag>jet</tag>
+        <tag>ifr</tag>
+
+  
+
+        <rating>
+          <FDM type="int">5</FDM>
+          <systems type="int">5</systems>
+          <model type="int">4</model>
+          <cockpit type="int">4</cockpit>
+        </rating>
+
+        <md5>4d3f7417d74f811aa20ccc4f35673d20</md5>
+        <!-- this URL will sometimes fail, on purpose -->
+        <url>http://localhost:2000/catalogTest1/b747.tar.gz</url>
+      </package>
+
+</PropertyList>

simgear/props/AtomicChangeListener.cxx

@@ -59,6 +59,12 @@ void AtomicChangeListener::fireChangeListeners()
     listeners.clear();
 }
 
+void AtomicChangeListener::clearPendingChanges()
+{
+    auto& listeners = ListenerListSingleton::instance()->listeners;
+    listeners.clear();
+}
+
 void AtomicChangeListener::valueChangedImplementation()
 {
     if (!_dirty) {

simgear/props/AtomicChangeListener.hxx

@@ -52,7 +52,17 @@ public:
     bool isDirty() { return _dirty; }
     bool isValid() { return _valid; }
     virtual void unregister_property(SGPropertyNode* node) override;
+
     static void fireChangeListeners();
+
+    /**
+     * @brief Ensure we've deleted any pending changes. 
+     * 
+     * This is important in shutdown and reset, to avoid holding
+     * property listeners around after the property tree is destroyed
+     */
+    static void clearPendingChanges();
+
 private:
     virtual void valueChangedImplementation() override;
     virtual void valuesChanged();

simgear/props/props.hxx

@@ -2197,11 +2197,12 @@ public:
         _property->addChangeListener(this,initial);
     }
 
-	SGPropertyChangeCallback(const SGPropertyChangeCallback<T>& other) :
-		_obj(other._obj), _callback(other._callback), _property(other._property)
-	{
-		 _property->addChangeListener(this,false);
-	}
+    SGPropertyChangeCallback(const SGPropertyChangeCallback<T>& other)
+        : SGPropertyChangeListener(other),
+        _obj(other._obj), _callback(other._callback), _property(other._property)
+    {
+        _property->addChangeListener(this,false);
+    }
 
     virtual ~SGPropertyChangeCallback()
     {

simgear/scene/material/matmodel.cxx

@@ -28,7 +28,7 @@
 #include <simgear/compiler.h>
 
 #include <map>
-
+#include <mutex>
 
 #include <osg/AlphaFunc>
 #include <osg/Group>
@@ -103,37 +103,39 @@ SGMatModel::get_model_count( SGPropertyNode *prop_root )
 inline void
 SGMatModel::load_models( SGPropertyNode *prop_root )
 {
-				// Load model only on demand
-  if (!_models_loaded) {
-    for (unsigned int i = 0; i < _paths.size(); i++) {
-      osg::Node *entity = SGModelLib::loadModel(_paths[i], prop_root);
-      if (entity != 0) {
-        // FIXME: this stuff can be handled
-        // in the XML wrapper as well (at least,
-        // the billboarding should be handled
-        // there).
-        
-        if (_heading_type == HEADING_BILLBOARD) {
-          // if the model is a billboard, it is likely :
-          // 1. a branch with only leaves,
-          // 2. a tree or a non rectangular shape faked by transparency
-          // We add alpha clamp then
-          osg::StateSet* stateSet = entity->getOrCreateStateSet();
-          osg::AlphaFunc* alphaFunc =
-            new osg::AlphaFunc(osg::AlphaFunc::GREATER, 0.01f);
-          stateSet->setAttributeAndModes(alphaFunc,
-                                         osg::StateAttribute::OVERRIDE);
-          stateSet->setRenderingHint(osg::StateSet::TRANSPARENT_BIN);
-        } 
-        
-        _models.push_back(entity);
-        
-      } else {
-        SG_LOG(SG_INPUT, SG_ALERT, "Failed to load object " << _paths[i]);
-        // Ensure the vector contains something, otherwise get_random_model below fails
-        _models.push_back(new osg::Node());
-      }
-    }
+    std::lock_guard<std::mutex> g(_loadMutex);
+
+    // Load model only on demand
+    if (!_models_loaded) {
+        for (unsigned int i = 0; i < _paths.size(); i++) {
+            osg::Node* entity = SGModelLib::loadModel(_paths[i], prop_root);
+            if (entity != 0) {
+                // FIXME: this stuff can be handled
+                // in the XML wrapper as well (at least,
+                // the billboarding should be handled
+                // there).
+
+                if (_heading_type == HEADING_BILLBOARD) {
+                    // if the model is a billboard, it is likely :
+                    // 1. a branch with only leaves,
+                    // 2. a tree or a non rectangular shape faked by transparency
+                    // We add alpha clamp then
+                    osg::StateSet* stateSet = entity->getOrCreateStateSet();
+                    osg::AlphaFunc* alphaFunc =
+                        new osg::AlphaFunc(osg::AlphaFunc::GREATER, 0.01f);
+                    stateSet->setAttributeAndModes(alphaFunc,
+                                                   osg::StateAttribute::OVERRIDE);
+                    stateSet->setRenderingHint(osg::StateSet::TRANSPARENT_BIN);
+                }
+
+                _models.push_back(entity);
+
+            } else {
+                SG_LOG(SG_INPUT, SG_ALERT, "Failed to load object " << _paths[i]);
+                // Ensure the vector contains something, otherwise get_random_model below fails
+                _models.push_back(new osg::Node());
+            }
+        }
   }
   _models_loaded = true;
 }

simgear/scene/material/matmodel.hxx

@@ -149,6 +149,8 @@ private:
     double _spacing_m;
     double _range_m;
     HeadingType _heading_type;
+
+    std::mutex _loadMutex;
 };
 
 

simgear/scene/material/mipmap.cxx

@@ -82,13 +82,13 @@ void _writeColor(GLenum pixelFormat, T* data, float scale, osg::Vec4 value)
     switch(pixelFormat)
     {
         case(GL_DEPTH_COMPONENT):   //intentionally fall through and execute the code for GL_LUMINANCE
-        case(GL_LUMINANCE):         { *data++ = value.r()*scale; break; }
-        case(GL_ALPHA):             { *data++ = value.a()*scale; break; }
-        case(GL_LUMINANCE_ALPHA):   { *data++ = value.r()*scale; *data++ = value.a()*scale; break; }
-        case(GL_RGB):               { *data++ = value.r()*scale; *data++ = value.g()*scale; *data++ = value.b()*scale; break; }
-        case(GL_RGBA):              { *data++ = value.r()*scale; *data++ = value.g()*scale; *data++ = value.b()*scale; *data++ = value.a()*scale; break; }
-        case(GL_BGR):               { *data++ = value.b()*scale; *data++ = value.g()*scale; *data++ = value.r()*scale; break; }
-        case(GL_BGRA):              { *data++ = value.b()*scale; *data++ = value.g()*scale; *data++ = value.r()*scale; *data++ = value.a()*scale; break; }
+        case(GL_LUMINANCE):         { *data = value.r()*scale; break; }
+        case(GL_ALPHA):             { *data = value.a()*scale; break; }
+        case(GL_LUMINANCE_ALPHA):   { *data++ = value.r()*scale; *data = value.a()*scale; break; }
+        case(GL_RGB):               { *data++ = value.r()*scale; *data++ = value.g()*scale; *data = value.b()*scale; break; }
+        case(GL_RGBA):              { *data++ = value.r()*scale; *data++ = value.g()*scale; *data++ = value.b()*scale; *data = value.a()*scale; break; }
+        case(GL_BGR):               { *data++ = value.b()*scale; *data++ = value.g()*scale; *data = value.r()*scale; break; }
+        case(GL_BGRA):              { *data++ = value.b()*scale; *data++ = value.g()*scale; *data++ = value.r()*scale; *data = value.a()*scale; break; }
     }
 }
 
@@ -265,7 +265,13 @@ osg::Image* computeMipmap( osg::Image* image, MipMapTuple attrs )
 {
     bool computeMipmap = false;
     unsigned int nbComponents = osg::Image::computeNumComponents( image->getPixelFormat() );
-    if ( std::get<0>(attrs) != AUTOMATIC &&
+    int s = image->s();
+    int t = image->t();
+    if ( (s & (s - 1)) || (t & (t - 1)) ) // power of two test
+    {
+        SG_LOG(SG_IO, SG_DEV_ALERT, "mipmapping: texture size not a power-of-two: " + image->getFileName());
+    }
+    else if ( std::get<0>(attrs) != AUTOMATIC &&
         ( std::get<1>(attrs) != AUTOMATIC || nbComponents < 2 ) &&
         ( std::get<2>(attrs) != AUTOMATIC || nbComponents < 3 ) &&
         ( std::get<3>(attrs) != AUTOMATIC || nbComponents < 4 ) )
@@ -283,9 +289,7 @@ osg::Image* computeMipmap( osg::Image* image, MipMapTuple attrs )
     if ( computeMipmap )
     {
         osg::ref_ptr<osg::Image> mipmaps = new osg::Image();
-        int s = image->s(),
-            t = image->t(),
-            r = image->r();
+        int r = image->r();
         int nb = osg::Image::computeNumberOfMipmapLevels(s, t, r);
         osg::Image::MipmapDataType mipmapOffsets;
         unsigned int offset = 0;

simgear/scene/model/ModelRegistry.cxx

@@ -359,6 +359,8 @@ ModelRegistry::readImage(const string& fileName,
                 if (res.validImage()) {
                     osg::ref_ptr<osg::Image> srcImage = res.getImage();
                     int width = srcImage->s();
+                    //int packing = srcImage->getPacking();
+                    //printf("packing %d format %x pixel size %d InternalTextureFormat %x\n", packing, srcImage->getPixelFormat(), srcImage->getPixelSizeInBits(), srcImage->getInternalTextureFormat() );
                     bool transparent = srcImage->isImageTranslucent();
                     bool isNormalMap = false;
                     bool isEffect = false;
@@ -367,6 +369,11 @@ ModelRegistry::readImage(const string& fileName,
                     */
                     bool can_compress = (transparent && compress_transparent) || (!transparent && compress_solid);
 
+                    if (srcImage->getPixelSizeInBits() <= 16) {
+                        SG_LOG(SG_IO, SG_INFO, "Ignoring " + absFileName + " for inclusion into the texture cache because pixel density too low at " << srcImage->getPixelSizeInBits() << " bits per pixek");
+                        can_compress = false;
+                    }
+
                     int height = srcImage->t();
 
                     // use the new file origin to determine any special processing

simgear/scene/model/SGPickAnimation.cxx

@@ -136,11 +136,19 @@ osg::Vec2d eventToWindowCoords(const osgGA::GUIEventAdapter& ea)
      
    virtual void update(double dt, int keyModState)
    {
-       if (!_condition || _condition->test()) {
-           SG_UNUSED(keyModState);
-           if (!_repeatable)
-               return;
+       SG_UNUSED(keyModState);
+       if (_condition && !_condition->test()) {
+           return;
+       }
 
+       if (!_repeatable)
+           return;
+
+       const bool zeroInterval = (_repeatInterval <= 0.0);
+       if (zeroInterval) {
+           // fire once per frame
+           fireBindingList(_bindingsDown);
+       } else {
            _repeatTime += dt;
            while (_repeatInterval < _repeatTime) {
                _repeatTime -= _repeatInterval;
@@ -678,12 +686,18 @@ public:
         if (_hasDragged) {
             return;
         }
-        
-        _repeatTime += dt;
-        while (_repeatInterval < _repeatTime) {
-            _repeatTime -= _repeatInterval;
+
+        const bool zeroInterval = (_repeatInterval <= 0.0);
+        if (zeroInterval) {
+            // fire once per frame
             fire(keyModState & osgGA::GUIEventAdapter::MODKEY_SHIFT, _direction);
-        } // of repeat iteration
+        } else {
+            _repeatTime += dt;
+            while (_repeatInterval < _repeatTime) {
+                _repeatTime -= _repeatInterval;
+                fire(keyModState & osgGA::GUIEventAdapter::MODKEY_SHIFT, _direction);
+            } // of repeat iteration
+        }
     }
 
     bool hover( const osg::Vec2d& windowPos, const Info& ) override

simgear/scene/model/SGReaderWriterXML.cxx

@@ -490,7 +490,8 @@ sgLoad3DModel_internal(const SGPath& path,
         }
     }
 
-    if (GlobalParticleCallback::getEnabled()){//dbOptions->getPluginStringData("SimGear::PARTICLESYSTEM") != "OFF") {
+    auto particlesManager = ParticlesGlobalManager::instance();
+    if (particlesManager->isEnabled()) { //dbOptions->getPluginStringData("SimGear::PARTICLESYSTEM") != "OFF") {
         std::vector<SGPropertyNode_ptr> particle_nodes;
         particle_nodes = props->getChildren("particlesystem");
         for (unsigned i = 0; i < particle_nodes.size(); ++i) {
@@ -503,9 +504,9 @@ sgLoad3DModel_internal(const SGPath& path,
 
                 options2->setDatabasePath(texturepath.utf8Str());
             }
-            group->addChild(Particles::appendParticles(particle_nodes[i],
-                                                       prop_root,
-                                                       options2.get()));
+            group->addChild(particlesManager->appendParticles(particle_nodes[i],
+                                                              prop_root,
+                                                              options2.get()));
         }
     }
 

simgear/scene/model/particles.cxx

@@ -17,9 +17,12 @@
 // Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 //
 
-#ifdef HAVE_CONFIG_H
-#  include <simgear_config.h>
-#endif
+#include <simgear_config.h>
+
+#include "particles.hxx"
+
+
+#include <mutex>
 
 #include <simgear/misc/sg_path.hxx>
 #include <simgear/props/props.hxx>
@@ -42,53 +45,122 @@
 #include <osg/MatrixTransform>
 #include <osg/Node>
 
-#include "particles.hxx"
+
+#include <simgear/scene/model/animation.hxx>
 
 namespace simgear
 {
-void GlobalParticleCallback::operator()(osg::Node* node, osg::NodeVisitor* nv)
-{
-    enabled = !enabledNode || enabledNode->getBoolValue();
-    if (!enabled)
-        return;
-    SGQuatd q
-        = SGQuatd::fromLonLatDeg(modelRoot->getFloatValue("/position/longitude-deg",0),
-                                 modelRoot->getFloatValue("/position/latitude-deg",0));
-    osg::Matrix om(toOsg(q));
-    osg::Vec3 v(0,0,9.81);
-    gravity = om.preMult(v);
-    // NOTE: THIS WIND COMPUTATION DOESN'T SEEM TO AFFECT PARTICLES
-    const osg::Vec3& zUpWind = Particles::getWindVector();
-    osg::Vec3 w(zUpWind.y(), zUpWind.x(), -zUpWind.z());
-    wind = om.preMult(w);
 
-    // SG_LOG(SG_PARTICLES, SG_ALERT,
-    //        "wind vector:" << w[0] << "," <<w[1] << "," << w[2]);
+class ParticlesGlobalManager::ParticlesGlobalManagerPrivate : public osg::NodeCallback
+{
+public:
+    ParticlesGlobalManagerPrivate() : _updater(new osgParticle::ParticleSystemUpdater),
+                                      _commonGeode(new osg::Geode)
+    {
+    }
+
+    void operator()(osg::Node* node, osg::NodeVisitor* nv) override
+    {
+        std::lock_guard<std::mutex> g(_lock);
+        _enabled = !_enabledNode || _enabledNode->getBoolValue();
+
+        if (!_enabled)
+            return;
+
+        const auto q = SGQuatd::fromLonLatDeg(_longitudeNode->getFloatValue(), _latitudeNode->getFloatValue());
+        osg::Matrix om(toOsg(q));
+        osg::Vec3 v(0, 0, 9.81);
+
+        _gravity = om.preMult(v);
+
+        // NOTE: THIS WIND COMPUTATION DOESN'T SEEM TO AFFECT PARTICLES
+        // const osg::Vec3& zUpWind = _wind;
+        // osg::Vec3 w(zUpWind.y(), zUpWind.x(), -zUpWind.z());
+        // _localWind = om.preMult(w);
+    }
+
+    // only call this with the lock held!
+    osg::Group* internalGetCommonRoot()
+    {
+        if (!_commonRoot.valid()) {
+            SG_LOG(SG_PARTICLES, SG_DEBUG, "Particle common root called.");
+            _commonRoot = new osg::Group;
+            _commonRoot->setName("common particle system root");
+            _commonGeode->setName("common particle system geode");
+            _commonRoot->addChild(_commonGeode);
+            _commonRoot->addChild(_updater);
+            _commonRoot->setNodeMask(~simgear::MODELLIGHT_BIT);
+        }
+        return _commonRoot.get();
+    }
+
+    std::mutex _lock;
+    bool _frozen = false;
+    osg::ref_ptr<osgParticle::ParticleSystemUpdater> _updater;
+    osg::ref_ptr<osg::Group> _commonRoot;
+    osg::ref_ptr<osg::Geode> _commonGeode;
+    osg::Vec3 _wind;
+    bool _globalCallbackRegistered = false;
+    bool _enabled = true;
+    osg::Vec3 _gravity;
+    //  osg::Vec3 _localWind;
+
+    SGConstPropertyNode_ptr _enabledNode;
+    SGConstPropertyNode_ptr _longitudeNode, _latitudeNode;
+};
+
+static std::mutex static_managerLock;
+static std::unique_ptr<ParticlesGlobalManager> static_instance;
+
+ParticlesGlobalManager* ParticlesGlobalManager::instance()
+{
+    std::lock_guard<std::mutex> g(static_managerLock);
+    if (!static_instance) {
+        static_instance.reset(new ParticlesGlobalManager);
+    }
+
+    return static_instance.get();
 }
 
-
-//static members
-osg::Vec3 GlobalParticleCallback::gravity;
-osg::Vec3 GlobalParticleCallback::wind;
-bool GlobalParticleCallback::enabled = true;
-SGConstPropertyNode_ptr GlobalParticleCallback::enabledNode = 0;
-
-osg::ref_ptr<osg::Group> Particles::commonRoot;
-osg::ref_ptr<osgParticle::ParticleSystemUpdater> Particles::psu = new osgParticle::ParticleSystemUpdater;
-osg::ref_ptr<osg::Geode> Particles::commonGeode = new osg::Geode;
-osg::Vec3 Particles::_wind;
-bool Particles::_frozen = false;
-
-Particles::Particles() : 
-    useGravity(false),
-    useWind(false)
+void ParticlesGlobalManager::clear()
 {
+    std::lock_guard<std::mutex> g(static_managerLock);
+    static_instance.reset();
+}
+
+ParticlesGlobalManager::ParticlesGlobalManager() : d(new ParticlesGlobalManagerPrivate)
+{
+}
+
+ParticlesGlobalManager::~ParticlesGlobalManager()
+{
+    if (d->_globalCallbackRegistered) {
+        // is this actually necessary? possibly not
+        d->_updater->setUpdateCallback(nullptr);
+    }
+}
+
+bool ParticlesGlobalManager::isEnabled() const
+{
+    std::lock_guard<std::mutex> g(d->_lock);
+    return d->_enabled;
+}
+
+bool ParticlesGlobalManager::isFrozen() const
+{
+    std::lock_guard<std::mutex> g(d->_lock);
+    return d->_frozen;
+}
+
+osg::Vec3 ParticlesGlobalManager::getWindVector() const
+{
+    std::lock_guard<std::mutex> g(d->_lock);
+    return d->_wind;
 }
 
 template <typename Object>
 class PointerGuard{
 public:
-    PointerGuard() : _ptr(0) {}
     Object* get() { return _ptr; }
     Object* operator () ()
     {
@@ -97,24 +169,9 @@ public:
         return _ptr;
     }
 private:
-    Object* _ptr;
+    Object* _ptr = nullptr;
 };
 
-osg::Group* Particles::getCommonRoot()
-{
-    if(!commonRoot.valid())
-    {
-        SG_LOG(SG_PARTICLES, SG_DEBUG, "Particle common root called.");
-        commonRoot = new osg::Group;
-        commonRoot.get()->setName("common particle system root");
-        commonGeode.get()->setName("common particle system geode");
-        commonRoot.get()->addChild(commonGeode.get());
-        commonRoot.get()->addChild(psu.get());
-        commonRoot->setNodeMask( ~simgear::MODELLIGHT_BIT );
-    }
-    return commonRoot.get();
-}
-
 void transformParticles(osgParticle::ParticleSystem* particleSys,
                         const osg::Matrix& mat)
 {
@@ -129,10 +186,182 @@ void transformParticles(osgParticle::ParticleSystem* particleSys,
     }
 }
 
-osg::Group * Particles::appendParticles(const SGPropertyNode* configNode,
-                                          SGPropertyNode* modelRoot,
-                                          const osgDB::Options*
-                                          options)
+void Particles::operator()(osg::Node* node, osg::NodeVisitor* nv)
+{
+    auto globalManager = ParticlesGlobalManager::instance();
+
+    //SG_LOG(SG_PARTICLES, SG_ALERT, "callback!\n");
+    particleSys->setFrozen(globalManager->isFrozen());
+
+    using namespace osg;
+    if (shooterValue)
+        shooter->setInitialSpeedRange(shooterValue->getValue(),
+                                      (shooterValue->getValue() + shooterExtraRange));
+    if (counterValue)
+        counter->setRateRange(counterValue->getValue(),
+                              counterValue->getValue() + counterExtraRange);
+    else if (counterCond)
+        counter->setRateRange(counterStaticValue,
+                              counterStaticValue + counterStaticExtraRange);
+    if (!globalManager->isEnabled() || (counterCond && !counterCond->test()))
+        counter->setRateRange(0, 0);
+    bool colorchange = false;
+    for (int i = 0; i < 8; ++i) {
+        if (colorComponents[i]) {
+            staticColorComponents[i] = colorComponents[i]->getValue();
+            colorchange = true;
+        }
+    }
+    if (colorchange)
+        particleSys->getDefaultParticleTemplate().setColorRange(osgParticle::rangev4(Vec4(staticColorComponents[0], staticColorComponents[1], staticColorComponents[2], staticColorComponents[3]), Vec4(staticColorComponents[4], staticColorComponents[5], staticColorComponents[6], staticColorComponents[7])));
+    if (startSizeValue)
+        startSize = startSizeValue->getValue();
+    if (endSizeValue)
+        endSize = endSizeValue->getValue();
+    if (startSizeValue || endSizeValue)
+        particleSys->getDefaultParticleTemplate().setSizeRange(osgParticle::rangef(startSize, endSize));
+    if (lifeValue)
+        particleSys->getDefaultParticleTemplate().setLifeTime(lifeValue->getValue());
+
+    if (particleFrame.valid()) {
+        MatrixList mlist = node->getWorldMatrices();
+        if (!mlist.empty()) {
+            const Matrix& particleMat = particleFrame->getMatrix();
+            Vec3d emitOrigin(mlist[0](3, 0), mlist[0](3, 1), mlist[0](3, 2));
+            Vec3d displace = emitOrigin - Vec3d(particleMat(3, 0), particleMat(3, 1),
+                                                particleMat(3, 2));
+            if (displace * displace > 10000.0 * 10000.0) {
+                // Make new frame for particle system, coincident with
+                // the emitter frame, but oriented with local Z.
+                SGGeod geod = SGGeod::fromCart(toSG(emitOrigin));
+                Matrix newParticleMat = makeZUpFrame(geod);
+                Matrix changeParticleFrame = particleMat * Matrix::inverse(newParticleMat);
+                particleFrame->setMatrix(newParticleMat);
+                transformParticles(particleSys.get(), changeParticleFrame);
+            }
+        }
+    }
+    if (program.valid() && useWind)
+        program->setWind(globalManager->getWindVector());
+}
+
+void Particles::setupShooterSpeedData(const SGPropertyNode* configNode,
+                                      SGPropertyNode* modelRoot)
+{
+    shooterValue = read_value(configNode, modelRoot, "-m",
+                              -SGLimitsd::max(), SGLimitsd::max());
+    if (!shooterValue) {
+        SG_LOG(SG_GENERAL, SG_DEV_WARN, "Particles: shooter property error!\n");
+    }
+    shooterExtraRange = configNode->getFloatValue("extrarange", 0);
+}
+
+void Particles::setupCounterData(const SGPropertyNode* configNode,
+                                 SGPropertyNode* modelRoot)
+{
+    counterValue = read_value(configNode, modelRoot, "-m",
+                              -SGLimitsd::max(), SGLimitsd::max());
+    if (!counterValue) {
+        SG_LOG(SG_GENERAL, SG_DEV_WARN, "counter property error!\n");
+    }
+    counterExtraRange = configNode->getFloatValue("extrarange", 0);
+}
+
+void Particles::Particles::setupCounterCondition(const SGPropertyNode* configNode,
+                                                 SGPropertyNode* modelRoot)
+{
+    counterCond = sgReadCondition(modelRoot, configNode);
+}
+
+void Particles::setupCounterCondition(float aCounterStaticValue,
+                                      float aCounterStaticExtraRange)
+{
+    counterStaticValue = aCounterStaticValue;
+    counterStaticExtraRange = aCounterStaticExtraRange;
+}
+
+void Particles::setupStartSizeData(const SGPropertyNode* configNode,
+                                   SGPropertyNode* modelRoot)
+{
+    startSizeValue = read_value(configNode, modelRoot, "-m",
+                                -SGLimitsd::max(), SGLimitsd::max());
+    if (!startSizeValue) {
+        SG_LOG(SG_GENERAL, SG_DEV_WARN, "Particles: startSizeValue error!\n");
+    }
+}
+
+void Particles::setupEndSizeData(const SGPropertyNode* configNode,
+                                 SGPropertyNode* modelRoot)
+{
+    endSizeValue = read_value(configNode, modelRoot, "-m",
+                              -SGLimitsd::max(), SGLimitsd::max());
+    if (!endSizeValue) {
+        SG_LOG(SG_GENERAL, SG_DEV_WARN, "Particles: startSizeValue error!\n");
+    }
+}
+
+void Particles::setupLifeData(const SGPropertyNode* configNode,
+                              SGPropertyNode* modelRoot)
+{
+    lifeValue = read_value(configNode, modelRoot, "-m",
+                           -SGLimitsd::max(), SGLimitsd::max());
+    if (!lifeValue) {
+        SG_LOG(SG_GENERAL, SG_DEV_WARN, "Particles: lifeValue error!\n");
+    }
+}
+
+void Particles::setupColorComponent(const SGPropertyNode* configNode,
+                                    SGPropertyNode* modelRoot, int color,
+                                    int component)
+{
+    SGSharedPtr<SGExpressiond> colorValue = read_value(configNode, modelRoot, "-m",
+                                                       -SGLimitsd::max(),
+                                                       SGLimitsd::max());
+    if (!colorValue) {
+        SG_LOG(SG_GENERAL, SG_DEV_WARN, "Particles: color property error!\n");
+    }
+    colorComponents[(color * 4) + component] = colorValue;
+    //number of color components = 4
+}
+
+void Particles::setupStaticColorComponent(float r1, float g1, float b1, float a1,
+                                          float r2, float g2, float b2, float a2)
+{
+    staticColorComponents[0] = r1;
+    staticColorComponents[1] = g1;
+    staticColorComponents[2] = b1;
+    staticColorComponents[3] = a1;
+    staticColorComponents[4] = r2;
+    staticColorComponents[5] = g2;
+    staticColorComponents[6] = b2;
+    staticColorComponents[7] = a2;
+}
+
+void ParticlesGlobalManager::setWindVector(const osg::Vec3& wind)
+{
+    std::lock_guard<std::mutex> g(d->_lock);
+    d->_wind = wind;
+}
+
+void ParticlesGlobalManager::setWindFrom(const double from_deg, const double speed_kt)
+{
+    double map_rad = -from_deg * SG_DEGREES_TO_RADIANS;
+    double speed_mps = speed_kt * SG_KT_TO_MPS;
+
+    std::lock_guard<std::mutex> g(d->_lock);
+    d->_wind[0] = cos(map_rad) * speed_mps;
+    d->_wind[1] = sin(map_rad) * speed_mps;
+    d->_wind[2] = 0.0;
+}
+
+
+osg::Group* ParticlesGlobalManager::getCommonRoot()
+{
+    std::lock_guard<std::mutex> g(d->_lock);
+    return d->internalGetCommonRoot();
+}
+
+osg::ref_ptr<osg::Group> ParticlesGlobalManager::appendParticles(const SGPropertyNode* configNode, SGPropertyNode* modelRoot, const osgDB::Options* options)
 {
     SG_LOG(SG_PARTICLES, SG_DEBUG,
            "Setting up a particle system." << std::boolalpha
@@ -152,7 +381,7 @@ osg::Group * Particles::appendParticles(const SGPropertyNode* configNode,
         << "\n    Wind: " << configNode->getChild("program")->getBoolValue("wind", true)
         << std::noboolalpha);
 
-    osgParticle::ParticleSystem *particleSys;
+    osg::ref_ptr<osgParticle::ParticleSystem> particleSys;
 
     //create a generic particle system
     std::string type = configNode->getStringValue("type", "normal");
@@ -160,11 +389,10 @@ osg::Group * Particles::appendParticles(const SGPropertyNode* configNode,
         particleSys = new osgParticle::ParticleSystem;
     else
         particleSys = new osgParticle::ConnectedParticleSystem;
+
     //may not be used depending on the configuration
     PointerGuard<Particles> callback;
 
-    getPSU()->addParticleSystem(particleSys); 
-    getPSU()->setUpdateCallback(new GlobalParticleCallback(modelRoot));
     //contains counter, placer and shooter by default
     osgParticle::ModularEmitter* emitter = new osgParticle::ModularEmitter;
 
@@ -172,7 +400,7 @@ osg::Group * Particles::appendParticles(const SGPropertyNode* configNode,
 
     // Set up the alignment node ("stolen" from animation.cxx)
     // XXX Order of rotations is probably not correct.
-    osg::MatrixTransform *align = new osg::MatrixTransform;
+    osg::ref_ptr<osg::MatrixTransform> align = new osg::MatrixTransform;
     osg::Matrix res_matrix;
     res_matrix.makeRotate(
         configNode->getFloatValue("offsets/pitch-deg", 0.0)*SG_DEGREES_TO_RADIANS,
@@ -187,12 +415,8 @@ osg::Group * Particles::appendParticles(const SGPropertyNode* configNode,
                        configNode->getFloatValue("offsets/y-m", 0.0),
                        configNode->getFloatValue("offsets/z-m", 0.0));
     align->setMatrix(res_matrix * tmat);
-
     align->setName("particle align");
 
-    //if (dynamic_cast<CustomModularEmitter*>(emitter)==0) SG_LOG(SG_PARTICLES, SG_ALERT, "observer error\n");
-    //align->addObserver(dynamic_cast<CustomModularEmitter*>(emitter));
-
     align->addChild(emitter);
 
     //this name can be used in the XML animation as if it was a submodel
@@ -209,7 +433,6 @@ osg::Group * Particles::appendParticles(const SGPropertyNode* configNode,
         osg::Geode* g = new osg::Geode;
         g->addDrawable(particleSys);
         callback()->particleFrame->addChild(g);
-        getCommonRoot()->addChild(callback()->particleFrame.get());
     }
     std::string textureFile;
     if (configNode->hasValue("texture")) {
@@ -505,66 +728,38 @@ osg::Group * Particles::appendParticles(const SGPropertyNode* configNode,
         emitter->setUpdateCallback(callback.get());
     }
 
+    // touch shared data now (and not before)
+
+    {
+        std::lock_guard<std::mutex> g(d->_lock);
+        d->_updater->addParticleSystem(particleSys);
+
+        if (attach != "local") {
+            d->internalGetCommonRoot()->addChild(callback()->particleFrame);
+        }
+
+        if (!d->_globalCallbackRegistered) {
+            SG_LOG(SG_PARTICLES, SG_INFO, "Registering global particles callback");
+            d->_globalCallbackRegistered = true;
+            d->_longitudeNode = modelRoot->getNode("/position/longitude-deg", true);
+            d->_latitudeNode = modelRoot->getNode("/position/latitude-deg", true);
+            d->_updater->setUpdateCallback(d.get());
+        }
+    }
+
     return align;
 }
 
-void Particles::operator()(osg::Node* node, osg::NodeVisitor* nv)
+void ParticlesGlobalManager::setSwitchNode(const SGPropertyNode* n)
 {
-    //SG_LOG(SG_PARTICLES, SG_ALERT, "callback!\n");
-    this->particleSys->setFrozen(_frozen);
-
-    using namespace osg;
-    if (shooterValue)
-        shooter->setInitialSpeedRange(shooterValue->getValue(),
-                                      (shooterValue->getValue()
-                                       + shooterExtraRange));
-    if (counterValue)
-        counter->setRateRange(counterValue->getValue(),
-                              counterValue->getValue() + counterExtraRange);
-    else if (counterCond)
-        counter->setRateRange(counterStaticValue,
-                              counterStaticValue + counterStaticExtraRange);
-    if (!GlobalParticleCallback::getEnabled() || (counterCond && !counterCond->test()))
-        counter->setRateRange(0, 0);
-    bool colorchange=false;
-    for (int i = 0; i < 8; ++i) {
-        if (colorComponents[i]) {
-            staticColorComponents[i] = colorComponents[i]->getValue();
-            colorchange=true;
-        }
-    }
-    if (colorchange)
-        particleSys->getDefaultParticleTemplate().setColorRange(osgParticle::rangev4( Vec4(staticColorComponents[0], staticColorComponents[1], staticColorComponents[2], staticColorComponents[3]), Vec4(staticColorComponents[4], staticColorComponents[5], staticColorComponents[6], staticColorComponents[7])));
-    if (startSizeValue)
-        startSize = startSizeValue->getValue();
-    if (endSizeValue)
-        endSize = endSizeValue->getValue();
-    if (startSizeValue || endSizeValue)
-        particleSys->getDefaultParticleTemplate().setSizeRange(osgParticle::rangef(startSize, endSize));
-    if (lifeValue)
-        particleSys->getDefaultParticleTemplate().setLifeTime(lifeValue->getValue());
-
-    if (particleFrame.valid()) {
-        MatrixList mlist = node->getWorldMatrices();
-        if (!mlist.empty()) {
-            const Matrix& particleMat = particleFrame->getMatrix();
-            Vec3d emitOrigin(mlist[0](3, 0), mlist[0](3, 1), mlist[0](3, 2));
-            Vec3d displace
-                = emitOrigin - Vec3d(particleMat(3, 0), particleMat(3, 1),
-                                     particleMat(3, 2));
-            if (displace * displace > 10000.0 * 10000.0) {
-                // Make new frame for particle system, coincident with
-                // the emitter frame, but oriented with local Z.
-                SGGeod geod = SGGeod::fromCart(toSG(emitOrigin));
-                Matrix newParticleMat = makeZUpFrame(geod);
-                Matrix changeParticleFrame
-                    = particleMat * Matrix::inverse(newParticleMat);
-                particleFrame->setMatrix(newParticleMat);
-                transformParticles(particleSys.get(), changeParticleFrame);
-            }
-        }
-    }
-    if (program.valid() && useWind)
-        program->setWind(_wind);
+    std::lock_guard<std::mutex> g(d->_lock);
+    d->_enabledNode = n;
 }
+
+void ParticlesGlobalManager::setFrozen(bool b)
+{
+    std::lock_guard<std::mutex> g(d->_lock);
+    d->_frozen = b;
+}
+
 } // namespace simgear

simgear/scene/model/particles.hxx

@@ -1,5 +1,5 @@
 // particles.hxx - classes to manage particles
-// Copyright (C) 2008 Tiago Gusm<73>o
+// Copyright (C) 2008 Tiago Gusm<73>o
 //
 // This program is free software; you can redistribute it and/or
 // modify it under the terms of the GNU General Public License as
@@ -53,133 +53,37 @@ class ParticleSystemUpdater;
 #include <simgear/math/SGMatrix.hxx>
 
 
-// Has anyone done anything *really* stupid, like making min and max macros?
-#ifdef min
-#undef min
-#endif
-#ifdef max
-#undef max
-#endif
-
-#include "animation.hxx"
-
 namespace simgear
 {
 
-class GlobalParticleCallback : public osg::NodeCallback 
-{
-public:
-    GlobalParticleCallback(const SGPropertyNode* modelRoot) 
-    {
-        this->modelRoot=modelRoot;
-    }
-
-    virtual void operator()(osg::Node* node, osg::NodeVisitor* nv);
-
-    static const osg::Vec3 &getGravityVector()
-    {
-        return gravity;
-    }
-
-    static const osg::Vec3 &getWindVector()
-    {
-        return wind;
-    }
-
-    static void setSwitch(const SGPropertyNode* n)
-    {
-        enabledNode = n;
-    }
-
-    static bool getEnabled()
-    {
-        return enabled;
-    }
-
-private:
-    static osg::Vec3 gravity;
-    static osg::Vec3 wind;
-    SGConstPropertyNode_ptr modelRoot;
-    static SGConstPropertyNode_ptr enabledNode;
-    static bool enabled;
-};
-
-
+class ParticlesGlobalManager;
 
 class Particles : public osg::NodeCallback 
 {
 public:
-    Particles();
+    Particles() = default;
 
-    static osg::Group * appendParticles(const SGPropertyNode* configNode, SGPropertyNode* modelRoot, const osgDB::Options* options);
-
-    virtual void operator()(osg::Node* node, osg::NodeVisitor* nv);
+    void operator()(osg::Node* node, osg::NodeVisitor* nv) override;
 
     void setupShooterSpeedData(const SGPropertyNode* configNode,
-                               SGPropertyNode* modelRoot)
-    {
-        shooterValue = read_value(configNode, modelRoot, "-m",
-                                  -SGLimitsd::max(), SGLimitsd::max());
-        if(!shooterValue){
-            SG_LOG(SG_GENERAL, SG_ALERT, "shooter property error!\n");
-        }
-        shooterExtraRange = configNode->getFloatValue("extrarange",0);
-    }
+                               SGPropertyNode* modelRoot);
 
     void setupCounterData(const SGPropertyNode* configNode,
-                          SGPropertyNode* modelRoot)
-    {
-        counterValue = read_value(configNode, modelRoot, "-m",
-                                  -SGLimitsd::max(), SGLimitsd::max());
-        if(!counterValue){
-            SG_LOG(SG_GENERAL, SG_ALERT, "counter property error!\n");
-        }
-        counterExtraRange = configNode->getFloatValue("extrarange",0);
-    }
+                          SGPropertyNode* modelRoot);
 
     void setupCounterCondition(const SGPropertyNode* configNode,
-                               SGPropertyNode* modelRoot)
-    {
-        counterCond = sgReadCondition(modelRoot, configNode);
-    }
-
+                               SGPropertyNode* modelRoot);
     void setupCounterCondition(float counterStaticValue,
-                               float counterStaticExtraRange)
-    {
-        this->counterStaticValue = counterStaticValue;
-        this->counterStaticExtraRange = counterStaticExtraRange;
-    }
+                               float counterStaticExtraRange);
 
     void setupStartSizeData(const SGPropertyNode* configNode,
-                            SGPropertyNode* modelRoot)
-    {
-        startSizeValue = read_value(configNode, modelRoot, "-m",
-                                    -SGLimitsd::max(), SGLimitsd::max());
-        if(!startSizeValue) 
-        {
-            SG_LOG(SG_GENERAL, SG_ALERT, "startSizeValue error!\n");
-        }
-    }
+                            SGPropertyNode* modelRoot);
 
     void setupEndSizeData(const SGPropertyNode* configNode,
-                          SGPropertyNode* modelRoot)
-    {
-        endSizeValue = read_value(configNode, modelRoot, "-m",
-                                  -SGLimitsd::max(), SGLimitsd::max());
-        if(!endSizeValue){
-            SG_LOG(SG_GENERAL, SG_ALERT, "startSizeValue error!\n");
-        }
-    }
+                          SGPropertyNode* modelRoot);
 
     void setupLifeData(const SGPropertyNode* configNode,
-                       SGPropertyNode* modelRoot)
-    {
-        lifeValue = read_value(configNode, modelRoot, "-m",
-                               -SGLimitsd::max(), SGLimitsd::max());
-        if(!lifeValue){
-            SG_LOG(SG_GENERAL, SG_ALERT, "lifeValue error!\n");
-        }
-    }
+                       SGPropertyNode* modelRoot);
 
     void setupStaticSizeData(float startSize, float endSize)
     {
@@ -212,60 +116,14 @@ public:
     //component: r=0, g=1, ... ; color: 0=start color, 1=end color
     void setupColorComponent(const SGPropertyNode* configNode,
                              SGPropertyNode* modelRoot, int color,
-                             int component)
-    {
-        SGExpressiond *colorValue = read_value(configNode, modelRoot, "-m",
-                                               -SGLimitsd::max(),
-                                               SGLimitsd::max());
-        if(!colorValue){
-            SG_LOG(SG_GENERAL, SG_ALERT, "color property error!\n");
-        }
-        colorComponents[(color*4)+component] = colorValue;
-        //number of color components = 4
-    }
+                             int component);
 
-    void setupStaticColorComponent(float r1,float g1, float b1, float a1,
-                                   float r2, float g2, float b2, float a2)
-    {
-        staticColorComponents[0] = r1;
-        staticColorComponents[1] = g1;
-        staticColorComponents[2] = b1;
-        staticColorComponents[3] = a1;
-        staticColorComponents[4] = r2;
-        staticColorComponents[5] = g2;
-        staticColorComponents[6] = b2;
-        staticColorComponents[7] = a2;
-    }
+    void setupStaticColorComponent(float r1, float g1, float b1, float a1,
+                                   float r2, float g2, float b2, float a2);
 
-    static osg::Group * getCommonRoot();
-
-    static osg::Geode * getCommonGeode()
-    {
-        return commonGeode.get();
-    }
-
-    static osgParticle::ParticleSystemUpdater * getPSU()
-    {
-        return psu.get();
-    }
-
-    static void setFrozen(bool e) { _frozen = e; }
-
-    /**
-     *  Set and get the wind vector for particles in the
-     * atmosphere. This vector is in the Z-up Y-north frame, and the
-     * magnitude is the velocity in meters per second.
-     */
-    static void setWindVector(const osg::Vec3& wind) { _wind = wind; }
-    static void setWindFrom(const double from_deg, const double speed_kt) {
-	double map_rad = -from_deg * SG_DEGREES_TO_RADIANS;
-	double speed_mps = speed_kt * SG_KT_TO_MPS;
-	_wind[0] = cos(map_rad) * speed_mps;
-	_wind[1] = sin(map_rad) * speed_mps;
-	_wind[2] = 0.0;
-    }
-    static const osg::Vec3& getWindVector() { return _wind; }
 protected:
+    friend class ParticlesGlobalManager;
+
     float shooterExtraRange;
     float counterExtraRange;
     SGSharedPtr<SGExpressiond> shooterValue;
@@ -285,39 +143,54 @@ protected:
     osg::ref_ptr<osgParticle::ParticleSystem> particleSys;
     osg::ref_ptr<osgParticle::FluidProgram> program;
     osg::ref_ptr<osg::MatrixTransform> particleFrame;
-    
-    bool useGravity;
-    bool useWind;
-    static bool _frozen;
-    static osg::ref_ptr<osgParticle::ParticleSystemUpdater> psu;
-    static osg::ref_ptr<osg::Group> commonRoot;
-    static osg::ref_ptr<osg::Geode> commonGeode;
-    static osg::Vec3 _wind;
+
+    bool useGravity = false;
+    bool useWind = false;
 };
-}
-#endif
 
-
-/*
-class CustomModularEmitter : public osgParticle::ModularEmitter, public osg::Observer
+class ParticlesGlobalManager
 {
 public:
+    ~ParticlesGlobalManager();
 
-virtual void objectDeleted (void *)
-{
-SG_LOG(SG_GENERAL, SG_ALERT, "object deleted!\n");
+    static ParticlesGlobalManager* instance();
+    static void clear();
 
-SGParticles::getCommonRoot()->removeChild(this->getParticleSystem()->getParent(0));
-SGParticles::getPSU()->removeParticleSystem(this->getParticleSystem());
-}
+    bool isEnabled() const;
 
+    osg::ref_ptr<osg::Group> appendParticles(const SGPropertyNode* configNode, SGPropertyNode* modelRoot, const osgDB::Options* options);
 
-//   ~CustomModularEmitter()
-//   {
-//     SG_LOG(SG_GENERAL, SG_ALERT, "object deleted!\n");
-//     
-//     SGParticles::getCommonRoot()->removeChild(this->getParticleSystem()->getParent(0));
-//     SGParticles::getPSU()->removeParticleSystem(this->getParticleSystem());
-//   }
+    osg::Group* getCommonRoot();
+
+    osg::Geode* getCommonGeode();
+
+    osgParticle::ParticleSystemUpdater* getPSU();
+
+    void setFrozen(bool e);
+    bool isFrozen() const;
+
+    void setSwitchNode(const SGPropertyNode* n);
+
+    /**
+     *  Set and get the wind vector for particles in the
+     * atmosphere. This vector is in the Z-up Y-north frame, and the
+     * magnitude is the velocity in meters per second.
+     */
+    void setWindVector(const osg::Vec3& wind);
+    void setWindFrom(const double from_deg, const double speed_kt);
+
+    osg::Vec3 getWindVector() const;
+
+private:
+    ParticlesGlobalManager();
+
+    class ParticlesGlobalManagerPrivate;
+
+    // because Private inherits NodeCallback, we need to own it
+    // via an osg::ref_ptr
+    osg::ref_ptr<ParticlesGlobalManagerPrivate> d;
 };
-*/
+
+} // namespace simgear
+
+#endif

simgear/scene/tgdb/ReaderWriterSTG.cxx

@@ -221,29 +221,41 @@ struct ReaderWriterSTG::_ModelBin {
             if (signBuilder.getSignsGroup())
                 group->addChild(signBuilder.getSignsGroup());
 
-            if (_buildingList.size() > 0) {
+            if (!_buildingList.empty()) {
                 SGMaterialLibPtr matlib = _options->getMaterialLib();
                 bool useVBOs = (_options->getPluginStringData("SimGear::USE_VBOS") == "ON");
 
                 if (!matlib) {
                     SG_LOG( SG_TERRAIN, SG_ALERT, "Unable to get materials definition for buildings");
                 } else {
-                    for (std::list<_BuildingList>::iterator i = _buildingList.begin(); i != _buildingList.end(); ++i) {
+                    for (const auto& b : _buildingList) {
                         // Build buildings for each list of buildings
-                        SGGeod geodPos = SGGeod::fromDegM(i->_lon, i->_lat, 0.0);
-                        SGMaterial* mat = matlib->find(i->_material_name, geodPos);
-                        SGPath path = SGPath(i->_filename);
+                        SGGeod geodPos = SGGeod::fromDegM(b._lon, b._lat, 0.0);
+                        SGSharedPtr<SGMaterial> mat = matlib->find(b._material_name, geodPos);
+
+                        // trying to avoid crash on null material, see:
+                        // https://sentry.io/organizations/flightgear/issues/1867075869
+                        if (!mat) {
+                            SG_LOG(SG_TERRAIN, SG_ALERT, "Building specifies unknown material: " << b._material_name);
+                            continue;
+                        }
+
+                        const auto path = SGPath(b._filename);
                         SGBuildingBin* buildingBin = new SGBuildingBin(path, mat, useVBOs);
 
-                        SGBuildingBinList buildingBinList;
-                        buildingBinList.push_back(buildingBin);
+                        SGBuildingBinList bbList;
+                        bbList.push_back(buildingBin);
 
                         osg::MatrixTransform* matrixTransform;
-                        matrixTransform = new osg::MatrixTransform(makeZUpFrame(SGGeod::fromDegM(i->_lon, i->_lat, i->_elev)));
+                        matrixTransform = new osg::MatrixTransform(makeZUpFrame(SGGeod::fromDegM(b._lon, b._lat, b._elev)));
                         matrixTransform->setName("rotateBuildings");
                         matrixTransform->setDataVariance(osg::Object::STATIC);
-                        matrixTransform->addChild(createRandomBuildings(buildingBinList, osg::Matrix::identity(), _options));
+                        matrixTransform->addChild(createRandomBuildings(bbList, osg::Matrix::identity(), _options));
                         group->addChild(matrixTransform);
+
+                        std::for_each(bbList.begin(), bbList.end(), [](SGBuildingBin* bb) {
+                            delete bb;
+                        });
                     }
                 }
             }

simgear/scene/tgdb/SGBuildingBin.cxx

@@ -462,9 +462,17 @@ typedef QuadTreeBuilder<LOD*, SGBuildingBin::BuildingInstance, MakeBuildingLeaf,
       // F is the number of floors (integer)
       // WT is the texture index to use (integer) for walls. Buildings with the same WT value will have the same wall texture assigned.  There are 6 small, 6 medium and 4 large textures.
       // RT is the texture index to use (integer) for roofs. Buildings with the same RT value will have the same roof texture assigned.  There are 6 small, 6 medium and 4 large textures.
-      float x, y, z, r, w, d, h, p;
-      int b, s, o, f, wt, rt;
-      in >> x >> y >> z >> r >> b >> w >> d >> h >> p >> s >> o >> f >> wt >> rt;
+      float x = 0.0f, y = 0.0f, z = 0.0f, r = 0.0f, w = 0.0f, d = 0.0f, h = 0.0f, p = 0.0f;
+      int b = 0, s = 0, o = 0, f = 0, wt = 0, rt = 0;
+      in >> x >> y >> z >> r >> b;
+
+      if (in.bad() || in.fail()) {
+          SG_LOG(SG_TERRAIN, SG_WARN, "Error parsing build entry in: " << absoluteFileName << " line: \"" << line << "\"");
+          continue;
+      }
+
+      // these might fail, so check them after we look at failbit
+      in >> w >> d >> h >> p >> s >> o >> f >> wt >> rt;
 
       //SG_LOG(SG_TERRAIN, SG_ALERT, "Building entry " << x << " " << y << " " << z << " " << b );
       SGVec3f loc = SGVec3f(x,y,z);
@@ -489,14 +497,20 @@ typedef QuadTreeBuilder<LOD*, SGBuildingBin::BuildingInstance, MakeBuildingLeaf,
   };
 
   // Set up the building set based on the material definitions
-  SGBuildingBin::SGBuildingBin(const SGMaterial *mat, bool useVBOs) {
-    material_name = new std::string(mat->get_names()[0]);
-    SG_LOG(SG_TERRAIN, SG_DEBUG, "Building material " << material_name);
-    material = mat;
-    texture = new std::string(mat->get_building_texture());
-    lightMap = new std::string(mat->get_building_lightmap());
-    buildingRange = mat->get_building_range();
-    SG_LOG(SG_TERRAIN, SG_DEBUG, "Building texture " << texture);
+  SGBuildingBin::SGBuildingBin(const SGMaterial* mat, bool useVBOs) : _material(const_cast<SGMaterial*>(mat))
+  {
+      const auto& materialNames = mat->get_names();
+      if (materialNames.empty()) {
+          SG_LOG(SG_TERRAIN, SG_DEV_ALERT, "SGBuildingBin: material has zero names defined");
+      } else {
+          _materialName = materialNames.front();
+          SG_LOG(SG_TERRAIN, SG_DEBUG, "Building material " << _materialName);
+      }
+
+      _textureName = mat->get_building_texture();
+      _lightMapName = mat->get_building_lightmap();
+      buildingRange = mat->get_building_range();
+      SG_LOG(SG_TERRAIN, SG_DEBUG, "Building texture " << _textureName);
   }
 
   SGBuildingBin::~SGBuildingBin() {
@@ -644,15 +658,15 @@ typedef QuadTreeBuilder<LOD*, SGBuildingBin::BuildingInstance, MakeBuildingLeaf,
     if (buildingtype == SGBuildingBin::SMALL) {
       // Small building
       // Maximum number of floors is 3, and maximum width/depth is 192m.
-      width = material->get_building_small_min_width() + mt_rand(&seed) * mt_rand(&seed) * (material->get_building_small_max_width() - material->get_building_small_min_width());
-      depth = material->get_building_small_min_depth() + mt_rand(&seed) * mt_rand(&seed) * (material->get_building_small_max_depth() - material->get_building_small_min_depth());
-      floors = SGMisc<double>::round(material->get_building_small_min_floors() + mt_rand(&seed) * (material->get_building_small_max_floors() - material->get_building_small_min_floors()));
+      width = _material->get_building_small_min_width() + mt_rand(&seed) * mt_rand(&seed) * (_material->get_building_small_max_width() - _material->get_building_small_min_width());
+      depth = _material->get_building_small_min_depth() + mt_rand(&seed) * mt_rand(&seed) * (_material->get_building_small_max_depth() - _material->get_building_small_min_depth());
+      floors = SGMisc<double>::round(_material->get_building_small_min_floors() + mt_rand(&seed) * (_material->get_building_small_max_floors() - _material->get_building_small_min_floors()));
       height = floors * (2.8 + mt_rand(&seed));
 
       // Small buildings are never deeper than they are wide.
       if (depth > width) { depth = width; }
 
-      pitch_height = (mt_rand(&seed) < material->get_building_small_pitch()) ? 3.0 : 0.0;
+      pitch_height = (mt_rand(&seed) < _material->get_building_small_pitch()) ? 3.0 : 0.0;
 
       if (pitch_height == 0.0) {
         roof_shape = 0;
@@ -663,18 +677,18 @@ typedef QuadTreeBuilder<LOD*, SGBuildingBin::BuildingInstance, MakeBuildingLeaf,
       }
     } else if (buildingtype == SGBuildingBin::MEDIUM) {
       // MEDIUM BUILDING
-      width = material->get_building_medium_min_width() + mt_rand(&seed) * mt_rand(&seed) * (material->get_building_medium_max_width() - material->get_building_medium_min_width());
-      depth = material->get_building_medium_min_depth() + mt_rand(&seed) * mt_rand(&seed) * (material->get_building_medium_max_depth() - material->get_building_medium_min_depth());
-      floors = SGMisc<double>::round(material->get_building_medium_min_floors() + mt_rand(&seed) * (material->get_building_medium_max_floors() - material->get_building_medium_min_floors()));
+      width = _material->get_building_medium_min_width() + mt_rand(&seed) * mt_rand(&seed) * (_material->get_building_medium_max_width() - _material->get_building_medium_min_width());
+      depth = _material->get_building_medium_min_depth() + mt_rand(&seed) * mt_rand(&seed) * (_material->get_building_medium_max_depth() - _material->get_building_medium_min_depth());
+      floors = SGMisc<double>::round(_material->get_building_medium_min_floors() + mt_rand(&seed) * (_material->get_building_medium_max_floors() - _material->get_building_medium_min_floors()));
       height = floors * (2.8 + mt_rand(&seed));
 
-      while ((height > width) && (floors > material->get_building_medium_min_floors())) {
-        // Ensure that medium buildings aren't taller than they are wide
-        floors--;
-        height = floors * (2.8 + mt_rand(&seed));
+      while ((height > width) && (floors > _material->get_building_medium_min_floors())) {
+          // Ensure that medium buildings aren't taller than they are wide
+          floors--;
+          height = floors * (2.8 + mt_rand(&seed));
       }
 
-      pitch_height = (mt_rand(&seed) < material->get_building_medium_pitch()) ? 3.0 : 0.0;
+      pitch_height = (mt_rand(&seed) < _material->get_building_medium_pitch()) ? 3.0 : 0.0;
 
       if (pitch_height == 0.0) {
         roof_shape = 0;
@@ -685,11 +699,11 @@ typedef QuadTreeBuilder<LOD*, SGBuildingBin::BuildingInstance, MakeBuildingLeaf,
       }
     } else {
       // LARGE BUILDING
-      width = material->get_building_large_min_width() + mt_rand(&seed) * (material->get_building_large_max_width() - material->get_building_large_min_width());
-      depth = material->get_building_large_min_depth() + mt_rand(&seed) * (material->get_building_large_max_depth() - material->get_building_large_min_depth());
-      floors = SGMisc<double>::round(material->get_building_large_min_floors() + mt_rand(&seed) * (material->get_building_large_max_floors() - material->get_building_large_min_floors()));
+      width = _material->get_building_large_min_width() + mt_rand(&seed) * (_material->get_building_large_max_width() - _material->get_building_large_min_width());
+      depth = _material->get_building_large_min_depth() + mt_rand(&seed) * (_material->get_building_large_max_depth() - _material->get_building_large_min_depth());
+      floors = SGMisc<double>::round(_material->get_building_large_min_floors() + mt_rand(&seed) * (_material->get_building_large_max_floors() - _material->get_building_large_min_floors()));
       height = floors * (2.8 + mt_rand(&seed));
-      pitch_height = (mt_rand(&seed) < material->get_building_large_pitch()) ? 3.0 : 0.0;
+      pitch_height = (mt_rand(&seed) < _material->get_building_large_pitch()) ? 3.0 : 0.0;
 
       if (pitch_height == 0.0) {
         roof_shape = 0;
@@ -718,36 +732,35 @@ typedef QuadTreeBuilder<LOD*, SGBuildingBin::BuildingInstance, MakeBuildingLeaf,
   }
 
   SGBuildingBin::BuildingType SGBuildingBin::getBuildingType(float roll) {
+      if (roll < _material->get_building_small_fraction()) {
+          return SGBuildingBin::SMALL;
+      }
 
-    if (roll < material->get_building_small_fraction()) {
-      return SGBuildingBin::SMALL;
-    }
-
-    if (roll < (material->get_building_small_fraction() + material->get_building_medium_fraction())) {
-      return SGBuildingBin::MEDIUM;
-    }
+      if (roll < (_material->get_building_small_fraction() + _material->get_building_medium_fraction())) {
+          return SGBuildingBin::MEDIUM;
+      }
 
     return SGBuildingBin::LARGE;
   }
 
   float SGBuildingBin::getBuildingMaxRadius(BuildingType type) {
-    if (type == SGBuildingBin::SMALL) return material->get_building_small_max_width();
-    if (type == SGBuildingBin::MEDIUM) return material->get_building_medium_max_width();
-    if (type == SGBuildingBin::LARGE) return material->get_building_large_max_width();
-    return 0;
+      if (type == SGBuildingBin::SMALL) return _material->get_building_small_max_width();
+      if (type == SGBuildingBin::MEDIUM) return _material->get_building_medium_max_width();
+      if (type == SGBuildingBin::LARGE) return _material->get_building_large_max_width();
+      return 0;
   }
 
   float SGBuildingBin::getBuildingMaxDepth(BuildingType type) {
-    if (type == SGBuildingBin::SMALL) return material->get_building_small_max_depth();
-    if (type == SGBuildingBin::MEDIUM) return material->get_building_medium_max_depth();
-    if (type == SGBuildingBin::LARGE) return material->get_building_large_max_depth();
-    return 0;
+      if (type == SGBuildingBin::SMALL) return _material->get_building_small_max_depth();
+      if (type == SGBuildingBin::MEDIUM) return _material->get_building_medium_max_depth();
+      if (type == SGBuildingBin::LARGE) return _material->get_building_large_max_depth();
+      return 0;
   }
 
   ref_ptr<Group> SGBuildingBin::createBuildingsGroup(Matrix transInv, const SGReaderWriterOptions* options)
   {
     ref_ptr<Effect> effect;
-    EffectMap::iterator iter = buildingEffectMap.find(*texture);
+    auto iter = buildingEffectMap.find(_textureName);
 
     if ((iter == buildingEffectMap.end())||
         (!iter->second.lock(effect)))
@@ -756,16 +769,14 @@ typedef QuadTreeBuilder<LOD*, SGBuildingBin::BuildingInstance, MakeBuildingLeaf,
       makeChild(effectProp, "inherits-from")->setStringValue("Effects/building");
       SGPropertyNode* params = makeChild(effectProp, "parameters");
       // Main texture - n=0
-      params->getChild("texture", 0, true)->getChild("image", 0, true)
-          ->setStringValue(*texture);
+      params->getChild("texture", 0, true)->getChild("image", 0, true)->setStringValue(_textureName);
 
       // Light map - n=3
-      params->getChild("texture", 3, true)->getChild("image", 0, true)
-          ->setStringValue(*lightMap);
+      params->getChild("texture", 3, true)->getChild("image", 0, true)->setStringValue(_lightMapName);
 
       effect = makeEffect(effectProp, true, options);
       if (iter == buildingEffectMap.end())
-          buildingEffectMap.insert(EffectMap::value_type(*texture, effect));
+          buildingEffectMap.insert(EffectMap::value_type(_textureName, effect));
       else
           iter->second = effect; // update existing, but empty observer
     }

simgear/scene/tgdb/SGBuildingBin.hxx

@@ -145,20 +145,19 @@ public:
     };
 
 private:
+    const SGSharedPtr<SGMaterial> _material;
 
-  const SGMaterial *material;
+    std::string _materialName;
+    std::string _textureName;
+    std::string _lightMapName;
 
-  std::string* material_name;
-  std::string* texture;
-  std::string* lightMap;
-
-  // Visibility range for buildings
-  float buildingRange;
+    // Visibility range for buildings
+    float buildingRange;
 
 
-  // Information for an instance of a building - position and orientation
-  typedef std::vector<BuildingInstance> BuildingInstanceList;
-  BuildingInstanceList buildingLocations;
+    // Information for an instance of a building - position and orientation
+    typedef std::vector<BuildingInstance> BuildingInstanceList;
+    BuildingInstanceList buildingLocations;
 
 public:
 
@@ -187,7 +186,7 @@ public:
 
   bool checkMinDist (SGVec3f p, float radius);
 
-  std::string* getMaterialName() { return material_name; }
+  const std::string& getMaterialName() const { return _materialName; }
 
   BuildingType getBuildingType(float roll);
   float getBuildingMaxRadius(BuildingType);

simgear/scene/tgdb/SGTexturedTriangleBin.hxx

@@ -428,12 +428,12 @@ public:
       return 0;
 
     // FIXME: do not include all values here ...
-    osg::Vec3Array* vertices = new osg::Vec3Array;
-    osg::Vec3Array* normals = new osg::Vec3Array;
-    osg::Vec2Array* priTexCoords = new osg::Vec2Array;
-    osg::Vec2Array* secTexCoords = new osg::Vec2Array;
+    osg::ref_ptr<osg::Vec3Array> vertices = new osg::Vec3Array;
+    osg::ref_ptr<osg::Vec3Array> normals = new osg::Vec3Array;
+    osg::ref_ptr<osg::Vec2Array> priTexCoords = new osg::Vec2Array;
+    osg::ref_ptr<osg::Vec2Array> secTexCoords = new osg::Vec2Array;
 
-    osg::Vec4Array* colors = new osg::Vec4Array;
+    osg::ref_ptr<osg::Vec4Array> colors = new osg::Vec4Array;
     colors->push_back(osg::Vec4(1, 1, 1, 1));
 
     osg::Geometry* geometry = new osg::Geometry;

simgear/scene/tgdb/TreeBin.cxx

@@ -151,14 +151,26 @@ Geometry* makeSharedTreeGeometry(int numQuads)
     return result;
 }
 
-ref_ptr<Geometry> sharedTreeGeometry;
+static std::mutex static_sharedGeometryMutex;
+static ref_ptr<Geometry> sharedTreeGeometry;
+
+void clearSharedTreeGeometry()
+{
+    std::lock_guard<std::mutex> g(static_sharedGeometryMutex);
+    sharedTreeGeometry = {};
+}
 
 Geometry* createTreeGeometry(float width, float height, int varieties)
 {
-    if (!sharedTreeGeometry)
-        sharedTreeGeometry = makeSharedTreeGeometry(1600);
-    Geometry* quadGeom = simgear::clone(sharedTreeGeometry.get(),
-                                        CopyOp::SHALLOW_COPY);
+    Geometry* quadGeom = nullptr;
+    {
+        std::lock_guard<std::mutex> g(static_sharedGeometryMutex);
+        if (!sharedTreeGeometry)
+            sharedTreeGeometry = makeSharedTreeGeometry(1600);
+        quadGeom = simgear::clone(sharedTreeGeometry.get(),
+                                  CopyOp::SHALLOW_COPY);
+    }
+
     Vec3Array* params = new Vec3Array;
     params->push_back(Vec3(width, height, (float)varieties));
     quadGeom->setNormalArray(params);

simgear/scene/tgdb/TreeBin.hxx

@@ -35,11 +35,14 @@ namespace simgear
 {
 class TreeBin {
 public:
-  struct Tree {
-    SGVec3f position;
-    SGVec3f tnormal;
-    Tree(const SGVec3f& p, const SGVec3f& t) : position(p),tnormal(t)
-    { }
+    ~TreeBin() = default;
+
+    struct Tree {
+        SGVec3f position;
+        SGVec3f tnormal;
+        Tree(const SGVec3f& p, const SGVec3f& t) : position(p), tnormal(t)
+        {
+        }
   };
 
     typedef std::vector<Tree> TreeList;
@@ -55,19 +58,25 @@ public:
     { _trees.push_back(t); }
 
     void insert(const SGVec3f& p, const SGVec3f& tnorm)
-    {insert(Tree(p,tnorm));}
+    {
+        _trees.emplace_back(p, tnorm);
+    }
 
     unsigned getNumTrees() const
     { return _trees.size(); }
+
     const Tree& getTree(unsigned i) const
-    { return _trees[i]; }
+    {
+        assert(i < _trees.size());
+        return _trees.at(i);
+    }
+
     TreeList _trees;
     
-    ~TreeBin() {
-      _trees.clear();
-    }    
 };
 
+void clearSharedTreeGeometry();
+
 typedef std::list<TreeBin*> SGTreeBinList;
 
 osg::Group* createForest(SGTreeBinList& forestList, const osg::Matrix& transform,

simgear/scene/tgdb/pt_lights.cxx

@@ -280,9 +280,18 @@ SGLightFactory::getLights(const SGDirectionalLightBin& lights)
   //stateSet->setRenderBinDetails(POINT_LIGHTS_BIN, "DepthSortedBin");
   //stateSet->setMode(GL_LIGHTING, osg::StateAttribute::OFF);
 
+
+  static SGSceneFeatures* sceneFeatures = SGSceneFeatures::instance();
+  bool useTriangles = sceneFeatures->getEnableTriangleDirectionalLights();
+
   osg::DrawArrays* drawArrays;
-  drawArrays = new osg::DrawArrays(osg::PrimitiveSet::TRIANGLES,
-                                   0, vertices->size());
+  if (useTriangles)
+    drawArrays = new osg::DrawArrays(osg::PrimitiveSet::TRIANGLES,
+                                     0, vertices->size());
+  else
+   drawArrays = new osg::DrawArrays(osg::PrimitiveSet::POINTS,
+                                     0, vertices->size());
+
   geometry->addPrimitiveSet(drawArrays);
   return geometry;
 }

simgear/scene/tsync/terrasync.cxx

@@ -46,6 +46,7 @@
 #include <fstream>
 #include <string>
 #include <map>
+#include <algorithm>
 
 #include <simgear/version.h>
 
@@ -155,20 +156,16 @@ public:
 class SyncSlot
 {
 public:
-    SyncSlot() :
-        isNewDirectory(false),
-        busy(false),
-        pendingKBytes(0)
-    {}
+    SyncSlot() = default;
 
     SyncItem currentItem;
-    bool isNewDirectory;
-    std::queue<SyncItem> queue;
+    bool isNewDirectory = false;
+    std::deque<SyncItem> queue;
     std::unique_ptr<HTTPRepository> repository;
     SGTimeStamp stamp;
-    bool busy; ///< is the slot working or idle
-    unsigned int pendingKBytes;
-    unsigned int nextWarnTimeout;
+    bool busy = false; ///< is the slot working or idle
+    unsigned int pendingKBytes = 0;
+    unsigned int nextWarnTimeout = 0;
 };
 
 static const int SYNC_SLOT_TILES = 0; ///< Terrain and Objects sync
@@ -205,7 +202,6 @@ struct TerrasyncThreadState
         _updated_tile_count(0),
         _success_count(0),
         _consecutive_errors(0),
-        _allowed_errors(6),
         _cache_hits(0),
         _transfer_rate(0),
         _total_kb_downloaded(0),
@@ -219,7 +215,6 @@ struct TerrasyncThreadState
     int  _updated_tile_count;
     int  _success_count;
     int  _consecutive_errors;
-    int  _allowed_errors;
     int  _cache_hits;
     int _transfer_rate;
     // kbytes, not bytes, because bytes might overflow 2^31
@@ -306,13 +301,6 @@ public:
 
     void setInstalledDir(const SGPath& p)  { _installRoot = p; }
 
-   void   setAllowedErrorCount(int errors)
-    {
-        std::lock_guard<std::mutex> g(_stateLock);
-        _state._allowed_errors = errors;
-    }
-
-   void   setCachePath(const SGPath& p)     {_persistentCachePath = p;}
    void   setCacheHits(unsigned int hits)
     {
         std::lock_guard<std::mutex> g(_stateLock);
@@ -328,7 +316,12 @@ public:
         }
         return st;
     }
-private:
+
+    bool isDirActive(const std::string& path) const;
+
+    void setCachePath(const SGPath &p) { _persistentCachePath = p; }
+
+  private:
     void incrementCacheHits()
     {
         std::lock_guard<std::mutex> g(_stateLock);
@@ -341,15 +334,22 @@ private:
     void runInternal();
     void updateSyncSlot(SyncSlot& slot);
 
+    void beginSyncAirports(SyncSlot& slot);
+    void beginSyncTile(SyncSlot& slot);
+    void beginNormalSync(SyncSlot& slot);
+
+    void drainWaitingTiles();
+
     // commond helpers between both internal and external models
 
     SyncItem::Status isPathCached(const SyncItem& next) const;
-    void initCompletedTilesPersistentCache();
-    void writeCompletedTilesPersistentCache() const;
     void updated(SyncItem item, bool isNewDirectory);
     void fail(SyncItem failedItem);
     void notFound(SyncItem notFoundItem);
 
+    void initCompletedTilesPersistentCache();
+    void writeCompletedTilesPersistentCache() const;
+
     HTTP::Client _http;
     SyncSlot _syncSlots[NUM_SYNC_SLOTS];
 
@@ -370,7 +370,7 @@ private:
     string _dnsdn;
 
     TerrasyncThreadState _state;
-    std::mutex _stateLock;
+    mutable std::mutex _stateLock;
 };
 
 SGTerraSync::WorkerThread::WorkerThread() :
@@ -398,6 +398,18 @@ void SGTerraSync::WorkerThread::stop()
     SyncItem w(string(), SyncItem::Stop);
     request(w);
     join();
+
+    // clear the sync slots, in case we restart
+    for (unsigned int slot = 0; slot < NUM_SYNC_SLOTS; ++slot) {
+        _syncSlots[slot] = {};
+    }
+
+    // clear these so if re-init-ing, we check again
+    _completedTiles.clear();
+    _notFoundItems.clear();
+
+    _http.reset();
+    _http.setUserAgent("terrascenery-" SG_STRINGIZE(SIMGEAR_VERSION));
 }
 
 bool SGTerraSync::WorkerThread::start()
@@ -417,12 +429,18 @@ bool SGTerraSync::WorkerThread::start()
     SGPath path(_local_dir);
     if (!path.exists())
     {
-        SG_LOG(SG_TERRASYNC,SG_ALERT,
-               "Cannot start scenery download. Directory '" << _local_dir <<
-               "' does not exist. Set correct directory path or create directory folder.");
-        _state._fail_count++;
-        _state._stalled = true;
-        return false;
+        const SGPath parentDir = path.dirPath();
+        if (parentDir.exists()) {
+            // attempt to create terraSync dir ourselves
+            bool ok = path.create_dir(0755);
+            if (!ok) {
+                SG_LOG(SG_TERRASYNC, SG_ALERT,
+                       "Cannot start scenery download. Directory '" << _local_dir << "' does not exist. Set correct directory path or create directory folder.");
+                _state._fail_count++;
+                _state._stalled = true;
+                return false;
+            }
+        }
     }
 
     path.append("version");
@@ -439,10 +457,8 @@ bool SGTerraSync::WorkerThread::start()
     _stop = false;
     _state = TerrasyncThreadState(); // clean state
 
-    // not really an alert - but we want to (always) see this message, so user is
-    // aware we're downloading scenery (and using bandwidth).
-    SG_LOG(SG_TERRASYNC,SG_ALERT,
-           "Starting automatic scenery download/synchronization to '"<< _local_dir << "'.");
+    SG_LOG(SG_TERRASYNC, SG_MANDATORY_INFO,
+           "Starting automatic scenery download/synchronization to '" << _local_dir << "'.");
 
     SGThread::start();
     return true;
@@ -524,7 +540,6 @@ void SGTerraSync::WorkerThread::run()
     }
 
     initCompletedTilesPersistentCache();
-
     runInternal();
 
     {
@@ -536,12 +551,13 @@ void SGTerraSync::WorkerThread::run()
 void SGTerraSync::WorkerThread::updateSyncSlot(SyncSlot &slot)
 {
     if (slot.repository.get()) {
+        slot.repository->process();
         if (slot.repository->isDoingSync()) {
 #if 1
             if (slot.stamp.elapsedMSec() > (int)slot.nextWarnTimeout) {
                 SG_LOG(SG_TERRASYNC, SG_INFO, "sync taking a long time:" << slot.currentItem._dir << " taken " << slot.stamp.elapsedMSec());
                 SG_LOG(SG_TERRASYNC, SG_INFO, "HTTP request count:" << _http.hasActiveRequests());
-                slot.nextWarnTimeout += 10000;
+                slot.nextWarnTimeout += 30 * 1000;
             }
 #endif
             // convert bytes to kbytes here
@@ -551,6 +567,7 @@ void SGTerraSync::WorkerThread::updateSyncSlot(SyncSlot &slot)
 
         // check result
         HTTPRepository::ResultCode res = slot.repository->failure();
+
         if (res == HTTPRepository::REPO_ERROR_NOT_FOUND) {
             notFound(slot.currentItem);
         } else if (res != HTTPRepository::REPO_NO_ERROR) {
@@ -565,33 +582,25 @@ void SGTerraSync::WorkerThread::updateSyncSlot(SyncSlot &slot)
         slot.busy = false;
         slot.repository.reset();
         slot.pendingKBytes = 0;
+        slot.currentItem = {};
     }
 
     // init and start sync of the next repository
     if (!slot.queue.empty()) {
         slot.currentItem = slot.queue.front();
-        slot.queue.pop();
+        slot.queue.pop_front();
 
         SGPath path(_local_dir);
         path.append(slot.currentItem._dir);
         slot.isNewDirectory = !path.exists();
-        if (slot.isNewDirectory) {
-            int rc = path.create_dir( 0755 );
-            if (rc) {
-                SG_LOG(SG_TERRASYNC,SG_ALERT,
-                       "Cannot create directory '" << path << "', return code = " << rc );
-                fail(slot.currentItem);
-                return;
-            }
-        } // of creating directory step
+        const auto type = slot.currentItem._type;
 
-        slot.repository.reset(new HTTPRepository(path, &_http));
-        slot.repository->setBaseUrl(_httpServer + "/" + slot.currentItem._dir);
-
-        if (_installRoot.exists()) {
-            SGPath p = _installRoot;
-            p.append(slot.currentItem._dir);
-            slot.repository->setInstalledCopyPath(p);
+        if (type == SyncItem::AirportData) {
+            beginSyncAirports(slot);
+        } else if (type == SyncItem::Tile) {
+            beginSyncTile(slot);
+        } else {
+            beginNormalSync(slot);
         }
 
         try {
@@ -605,7 +614,7 @@ void SGTerraSync::WorkerThread::updateSyncSlot(SyncSlot &slot)
             return;
         }
 
-        slot.nextWarnTimeout = 20000;
+        slot.nextWarnTimeout = 30 * 1000;
         slot.stamp.stamp();
         slot.busy = true;
         slot.pendingKBytes = slot.repository->bytesToDownload();
@@ -614,23 +623,115 @@ void SGTerraSync::WorkerThread::updateSyncSlot(SyncSlot &slot)
     }
 }
 
+void SGTerraSync::WorkerThread::beginSyncAirports(SyncSlot& slot)
+{
+    if (!slot.isNewDirectory) {
+        beginNormalSync(slot);
+        return;
+    }
+
+    SG_LOG(SG_TERRASYNC, SG_INFO, "doing Airports download via tarball");
+
+    // we want to sync the 'root' TerraSync dir, but not all of it, just
+    // the Airports_archive.tar.gz file so we use our TerraSync local root
+    // as the path (since the archive will add Airports/)
+    slot.repository.reset(new HTTPRepository(_local_dir, &_http));
+    slot.repository->setBaseUrl(_httpServer);
+
+    // filter callback to *only* sync the Airport_archive tarball,
+    // and ensure no other contents are touched
+    auto f = [](const HTTPRepository::SyncItem& item) {
+        if (!item.directory.empty())
+            return false;
+        return (item.filename.find("Airports_archive.") == 0);
+    };
+
+    slot.repository->setFilter(f);
+}
+
+void SGTerraSync::WorkerThread::beginSyncTile(SyncSlot& slot)
+{
+    // avoid 404 requests by doing a sync which excludes all paths
+    // except our tile path. In the case of a missing 1x1 tile, we will
+    // stop becuase all directories are filtered out, which is what we want
+
+    auto comps = strutils::split(slot.currentItem._dir, "/");
+    if (comps.size() != 3) {
+        SG_LOG(SG_TERRASYNC, SG_ALERT, "Bad tile path:" << slot.currentItem._dir);
+        beginNormalSync(slot);
+        return;
+    }
+
+    const auto tileCategory = comps.front();
+    const auto tenByTenDir = comps.at(1);
+    const auto oneByOneDir = comps.at(2);
+
+    const auto path = SGPath::fromUtf8(_local_dir) / tileCategory;
+    slot.repository.reset(new HTTPRepository(path, &_http));
+    slot.repository->setBaseUrl(_httpServer + "/" + tileCategory);
+
+    if (_installRoot.exists()) {
+      SGPath p = _installRoot / tileCategory;
+      slot.repository->setInstalledCopyPath(p);
+    }
+
+    const auto dirPrefix = tenByTenDir + "/" + oneByOneDir;
+
+    // filter callback to *only* sync the 1x1 dir we want, if it exists
+    // if doesn't, we'll simply stop, which is what we want
+    auto f = [tenByTenDir, oneByOneDir, dirPrefix](const HTTPRepository::SyncItem& item) {
+        // only allow the specific 10x10 and 1x1 dirs we want
+        if (item.directory.empty()) {
+            return item.filename == tenByTenDir;
+        } else if (item.directory == tenByTenDir) {
+            return item.filename == oneByOneDir;
+        }
+
+        // allow arbitrary children below dirPrefix, including sub-dirs
+        if (item.directory.find(dirPrefix) == 0) {
+            return true;
+        }
+
+        SG_LOG(SG_TERRASYNC, SG_ALERT, "Tile sync: saw weird path:" << item.directory << " file " << item.filename);
+        return false;
+    };
+
+    slot.repository->setFilter(f);
+}
+
+void SGTerraSync::WorkerThread::beginNormalSync(SyncSlot& slot)
+{
+    SGPath path(_local_dir);
+    path.append(slot.currentItem._dir);
+    slot.repository.reset(new HTTPRepository(path, &_http));
+    slot.repository->setBaseUrl(_httpServer + "/" + slot.currentItem._dir);
+
+    if (_installRoot.exists()) {
+      SGPath p = _installRoot;
+      p.append(slot.currentItem._dir);
+      slot.repository->setInstalledCopyPath(p);
+    }
+}
+
 void SGTerraSync::WorkerThread::runInternal()
 {
-    unsigned dnsRetryCount = 0;
-
     while (!_stop) {
         // try to find a terrasync server
         if( !hasServer() ) {
-          if( ++dnsRetryCount > 5 ) {
-            SG_LOG(SG_TERRASYNC, SG_WARN, "Can't find a terrasync server. TS disabled.");
-            break;
-          }
-          if( hasServer( findServer() ) ) {
-            SG_LOG(SG_TERRASYNC, SG_INFO, "terrasync scenery provider of the day is '" << _httpServer << "'");
-          }
-          continue;
+            const auto haveServer = findServer();
+            if (haveServer) {
+                hasServer(true);
+
+                std::lock_guard<std::mutex> g(_stateLock);
+                _state._consecutive_errors = 0;
+
+                SG_LOG(SG_TERRASYNC, SG_INFO, "terrasync scenery provider of the day is '" << _httpServer << "'");
+            } else {
+                std::lock_guard<std::mutex> g(_stateLock);
+                _state._consecutive_errors++;
+            }
+            continue;
         }
-        dnsRetryCount = 0;
 
         try {
             _http.update(10);
@@ -648,21 +749,7 @@ void SGTerraSync::WorkerThread::runInternal()
         if (_stop)
             break;
 
-        // drain the waiting tiles queue into the sync slot queues.
-        while (!waitingTiles.empty()) {
-            SyncItem next = waitingTiles.pop_front();
-            SyncItem::Status cacheStatus = isPathCached(next);
-            if (cacheStatus != SyncItem::Invalid) {
-                incrementCacheHits();
-                SG_LOG(SG_TERRASYNC, SG_DEBUG, "\nTerraSync Cache hit for: '" << next._dir << "'");
-                next._status = cacheStatus;
-                _freshTiles.push_back(next);
-                continue;
-            }
-
-            unsigned int slot = syncSlotForType(next._type);
-            _syncSlots[slot].queue.push(next);
-        }
+        drainWaitingTiles();
 
         bool anySlotBusy = false;
         unsigned int newPendingCount = 0;
@@ -691,7 +778,7 @@ void SGTerraSync::WorkerThread::runInternal()
 
 SyncItem::Status SGTerraSync::WorkerThread::isPathCached(const SyncItem& next) const
 {
-    TileAgeCache::const_iterator ii = _completedTiles.find( next._dir );
+    auto ii = _completedTiles.find(next._dir);
     if (ii == _completedTiles.end()) {
         ii = _notFoundItems.find( next._dir );
         // Invalid means 'not cached', otherwise we want to return to
@@ -719,6 +806,7 @@ void SGTerraSync::WorkerThread::fail(SyncItem failedItem)
     _state._fail_count++;
     failedItem._status = SyncItem::Failed;
     _freshTiles.push_back(failedItem);
+    // not we also end up here for partial syncs
     SG_LOG(SG_TERRASYNC,SG_INFO,
            "Failed to sync'" << failedItem._dir << "'");
     _completedTiles[ failedItem._dir ] = now + UpdateInterval::FailedAttempt;
@@ -760,68 +848,117 @@ void SGTerraSync::WorkerThread::updated(SyncItem item, bool isNewDirectory)
     writeCompletedTilesPersistentCache();
 }
 
-void SGTerraSync::WorkerThread::initCompletedTilesPersistentCache()
+void SGTerraSync::WorkerThread::drainWaitingTiles()
 {
-    if (!_persistentCachePath.exists()) {
-        return;
-    }
-
-    SGPropertyNode_ptr cacheRoot(new SGPropertyNode);
-    time_t now = time(0);
-
-    try {
-        readProperties(_persistentCachePath, cacheRoot);
-    } catch (sg_exception& e) {
-        SG_LOG(SG_TERRASYNC, SG_INFO, "corrupted persistent cache, discarding " << e.getFormattedMessage());
-        return;
-    }
-
-    for (int i=0; i<cacheRoot->nChildren(); ++i) {
-        SGPropertyNode* entry = cacheRoot->getChild(i);
-        bool isNotFound = (strcmp(entry->getName(), "not-found") == 0);
-        string tileName = entry->getStringValue("path");
-        time_t stamp = entry->getIntValue("stamp");
-        if (stamp < now) {
+    // drain the waiting tiles queue into the sync slot queues.
+    while (!waitingTiles.empty()) {
+        SyncItem next = waitingTiles.pop_front();
+        SyncItem::Status cacheStatus = isPathCached(next);
+        if (cacheStatus != SyncItem::Invalid) {
+            incrementCacheHits();
+            SG_LOG(SG_TERRASYNC, SG_BULK, "\nTerraSync Cache hit for: '" << next._dir << "'");
+            next._status = cacheStatus;
+            _freshTiles.push_back(next);
             continue;
         }
 
-        if (isNotFound) {
-            _completedTiles[tileName] = stamp;
-        } else {
-            _notFoundItems[tileName] = stamp;
-        }
+        const auto slot = syncSlotForType(next._type);
+        _syncSlots[slot].queue.push_back(next);
     }
 }
 
-void SGTerraSync::WorkerThread::writeCompletedTilesPersistentCache() const
+bool SGTerraSync::WorkerThread::isDirActive(const std::string& path) const
 {
-    // cache is disabled
-    if (_persistentCachePath.isNull()) {
-        return;
+    // check waiting tiles first. we have to copy it to check safely,
+    // but since it's normally empty, this is not a big deal.
+    const auto copyOfWaiting = waitingTiles.copy();
+    auto it = std::find_if(copyOfWaiting.begin(), copyOfWaiting.end(), [&path](const SyncItem& i) {
+        return i._dir == path;
+    });
+
+    if (it != copyOfWaiting.end()) {
+        return true;
     }
 
-    sg_ofstream f(_persistentCachePath, std::ios::trunc);
-    if (!f.is_open()) {
-        return;
+    // check each sync slot in turn
+    std::lock_guard<std::mutex> g(_stateLock);
+    for (unsigned int slot = 0; slot < NUM_SYNC_SLOTS; ++slot) {
+        const auto& syncSlot = _syncSlots[slot];
+        if (syncSlot.currentItem._dir == path)
+            return true;
+
+        auto it = std::find_if(syncSlot.queue.begin(), syncSlot.queue.end(), [&path](const SyncItem& i) {
+            return i._dir == path;
+        });
+
+        if (it != syncSlot.queue.end()) {
+            return true;
+        }
+    } // of sync slots iteration
+
+    return false;
+}
+
+void SGTerraSync::WorkerThread::initCompletedTilesPersistentCache() {
+  if (!_persistentCachePath.exists()) {
+    return;
+  }
+
+  SGPropertyNode_ptr cacheRoot(new SGPropertyNode);
+  time_t now = time(0);
+
+  try {
+    readProperties(_persistentCachePath, cacheRoot);
+  } catch (sg_exception &e) {
+    SG_LOG(SG_TERRASYNC, SG_INFO, "corrupted persistent cache, discarding");
+    return;
+  }
+
+  for (int i = 0; i < cacheRoot->nChildren(); ++i) {
+    SGPropertyNode *entry = cacheRoot->getChild(i);
+    bool isNotFound = (strcmp(entry->getName(), "not-found") == 0);
+    string tileName = entry->getStringValue("path");
+    time_t stamp = entry->getIntValue("stamp");
+    if (stamp < now) {
+      continue;
     }
 
-    SGPropertyNode_ptr cacheRoot(new SGPropertyNode);
-    TileAgeCache::const_iterator it = _completedTiles.begin();
-    for (; it != _completedTiles.end(); ++it) {
-        SGPropertyNode* entry = cacheRoot->addChild("entry");
-        entry->setStringValue("path", it->first);
-        entry->setIntValue("stamp", it->second);
+    if (isNotFound) {
+      _notFoundItems[tileName] = stamp;
+    } else {
+      _completedTiles[tileName] = stamp;
     }
+  }
+}
 
-    it = _notFoundItems.begin();
-    for (; it != _notFoundItems.end(); ++it) {
-        SGPropertyNode* entry = cacheRoot->addChild("not-found");
-        entry->setStringValue("path", it->first);
-        entry->setIntValue("stamp", it->second);
-    }
+void SGTerraSync::WorkerThread::writeCompletedTilesPersistentCache() const {
+  // cache is disabled
+  if (_persistentCachePath.isNull()) {
+    return;
+  }
 
-    writeProperties(f, cacheRoot, true /* write_all */);
-    f.close();
+  sg_ofstream f(_persistentCachePath, std::ios::trunc);
+  if (!f.is_open()) {
+    return;
+  }
+
+  SGPropertyNode_ptr cacheRoot(new SGPropertyNode);
+  TileAgeCache::const_iterator it = _completedTiles.begin();
+  for (; it != _completedTiles.end(); ++it) {
+    SGPropertyNode *entry = cacheRoot->addChild("entry");
+    entry->setStringValue("path", it->first);
+    entry->setIntValue("stamp", it->second);
+  }
+
+  it = _notFoundItems.begin();
+  for (; it != _notFoundItems.end(); ++it) {
+    SGPropertyNode *entry = cacheRoot->addChild("not-found");
+    entry->setStringValue("path", it->first);
+    entry->setIntValue("stamp", it->second);
+  }
+
+  writeProperties(f, cacheRoot, true /* write_all */);
+  f.close();
 }
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -850,6 +987,12 @@ SGTerraSync::~SGTerraSync()
 
 void SGTerraSync::setRoot(SGPropertyNode_ptr root)
 {
+    if (!root) {
+        _terraRoot.clear();
+        _renderingRoot.clear();
+        return;
+    }
+
     _terraRoot = root->getNode("/sim/terrasync",true);
     _renderingRoot = root->getNode("/sim/rendering", true);
 }
@@ -897,11 +1040,17 @@ void SGTerraSync::reinit()
 #else
         _workerThread->setDNSDN( _terraRoot->getStringValue("dnsdn","terrasync.flightgear.org") );
 #endif
-        _workerThread->setLocalDir(_terraRoot->getStringValue("scenery-dir",""));
+
+        SGPath sceneryRoot{_terraRoot->getStringValue("scenery-dir", "")};
+        _workerThread->setLocalDir(sceneryRoot.utf8Str());
 
         SGPath installPath(_terraRoot->getStringValue("installation-dir"));
         _workerThread->setInstalledDir(installPath);
-        _workerThread->setAllowedErrorCount(_terraRoot->getIntValue("max-errors",5));
+
+        if (_terraRoot->getBoolValue("enable-persistent-cache", true)) {
+          _workerThread->setCachePath(sceneryRoot / "RecheckCache");
+        }
+
         _workerThread->setCacheHits(_terraRoot->getIntValue("cache-hit", 0));
 
         if (_workerThread->start())
@@ -944,12 +1093,7 @@ void SGTerraSync::bind()
     _downloadedKBtesNode = _terraRoot->getNode("downloaded-kbytes", true);
     _enabledNode = _terraRoot->getNode("enabled", true);
     _availableNode = _terraRoot->getNode("available", true);
-    //_busyNode->setAttribute(SGPropertyNode::WRITE, false);
-    //_activeNode->setAttribute(SGPropertyNode::WRITE, false);
-    //_updateCountNode->setAttribute(SGPropertyNode::WRITE, false);
-    //_errorCountNode->setAttribute(SGPropertyNode::WRITE, false);
-    //_tileCountNode->setAttribute(SGPropertyNode::WRITE, false);
-
+    _maxErrorsNode = _terraRoot->getNode("max-errors", true);
 }
 
 void SGTerraSync::unbind()
@@ -961,7 +1105,20 @@ void SGTerraSync::unbind()
 
     _terraRoot.clear();
     _stalledNode.clear();
+    _activeNode.clear();
     _cacheHits.clear();
+    _renderingRoot.clear();
+    _busyNode.clear();
+    _updateCountNode.clear();
+    _errorCountNode.clear();
+    _tileCountNode.clear();
+    _cacheHitsNode.clear();
+    _transferRateBytesSecNode.clear();
+    _pendingKbytesNode.clear();
+    _downloadedKBtesNode.clear();
+    _enabledNode.clear();
+    _availableNode.clear();
+    _maxErrorsNode.clear();
 }
 
 void SGTerraSync::update(double)
@@ -969,16 +1126,21 @@ void SGTerraSync::update(double)
     auto enabled = _enabledNode->getBoolValue();
     auto worker_running = _workerThread->isRunning();
 
+    // hold enabled false until retry time passes
+    if (enabled && (_retryTime > SGTimeStamp::now())) {
+        enabled = false;
+    }
+
     // see if the enabled status has changed; and if so take the appropriate action.
     if (enabled && !worker_running)
     {
         reinit();
-        SG_LOG(SG_TERRASYNC, SG_ALERT, "Terrasync started");
+        SG_LOG(SG_TERRASYNC, SG_MANDATORY_INFO, "Terrasync started");
     }
     else if (!enabled && worker_running)
     {
         reinit();
-        SG_LOG(SG_TERRASYNC, SG_ALERT, "Terrasync stopped");
+        SG_LOG(SG_TERRASYNC, SG_MANDATORY_INFO, "Terrasync stopped");
     }
     TerrasyncThreadState copiedState(_workerThread->threadsafeCopyState());
 
@@ -994,35 +1156,31 @@ void SGTerraSync::update(double)
     _stalledNode->setBoolValue(_workerThread->isStalled());
     _activeNode->setBoolValue(worker_running);
 
+    int allowedErrors = _maxErrorsNode->getIntValue();
+    if (worker_running && (copiedState._consecutive_errors >= allowedErrors)) {
+        _workerThread->stop();
+
+        _retryBackOffSeconds = std::min(_retryBackOffSeconds + 60, 60u * 15);
+        const int seconds = static_cast<int>(sg_random() * _retryBackOffSeconds);
+        _retryTime = SGTimeStamp::now() + SGTimeStamp::fromSec(seconds);
+        SG_LOG(SG_TERRASYNC, SG_ALERT, "Terrasync paused due to " << copiedState._consecutive_errors << " consecutive errors during sync; will retry in " << seconds << " seconds.");
+    }
+
     while (_workerThread->hasNewTiles())
     {
-        SyncItem next = _workerThread->getNewTile();
-
-        if ((next._type == SyncItem::Tile) || (next._type == SyncItem::AIData)) {
-            _activeTileDirs.erase(next._dir);
-        }
-    } // of freshly synced items
+        // ensure they are popped
+        _workerThread->getNewTile();
+    }
 }
 
 bool SGTerraSync::isIdle() {return _workerThread->isIdle();}
 
 void SGTerraSync::syncAirportsModels()
 {
-    static const char* bounds = "MZAJKL"; // airport sync order: K-L, A-J, M-Z
-    // note "request" method uses LIFO order, i.e. processes most recent request first
-    for( unsigned i = 0; i < strlen(bounds)/2; i++ )
-    {
-        for ( char synced_other = bounds[2*i]; synced_other <= bounds[2*i+1]; synced_other++ )
-        {
-            ostringstream dir;
-            dir << "Airports/" << synced_other;
-            SyncItem w(dir.str(), SyncItem::AirportData);
-            _workerThread->request( w );
-        }
-    }
-
-    SyncItem w("Models", SyncItem::SharedModels);
-    _workerThread->request( w );
+  SyncItem w("Airports", SyncItem::AirportData);
+  SyncItem a("Models", SyncItem::SharedModels);
+  _workerThread->request(w);
+  _workerThread->request(a);
 }
 
 string_list SGTerraSync::getSceneryPathSuffixes() const
@@ -1046,17 +1204,16 @@ string_list SGTerraSync::getSceneryPathSuffixes() const
 
 void SGTerraSync::syncAreaByPath(const std::string& aPath)
 {
-    string_list scenerySuffixes = getSceneryPathSuffixes();
-    string_list::const_iterator it = scenerySuffixes.begin();
+    if (!_workerThread->isRunning()) {
+        return;
+    }
 
-    for (; it != scenerySuffixes.end(); ++it)
-    {
-        std::string dir = *it + "/" + aPath;
-        if (_activeTileDirs.find(dir) != _activeTileDirs.end()) {
+    for (const auto& suffix : getSceneryPathSuffixes()) {
+        const auto dir = suffix + "/" + aPath;
+        if (_workerThread->isDirActive(dir)) {
             continue;
         }
 
-        _activeTileDirs.insert(dir);
         SyncItem w(dir, SyncItem::Tile);
         _workerThread->request( w );
     }
@@ -1075,13 +1232,9 @@ bool SGTerraSync::isTileDirPending(const std::string& sceneryDir) const
         return false;
     }
 
-    string_list scenerySuffixes = getSceneryPathSuffixes();
-    string_list::const_iterator it = scenerySuffixes.begin();
-
-    for (; it != scenerySuffixes.end(); ++it)
-    {
-        string s = *it + "/" + sceneryDir;
-        if (_activeTileDirs.find(s) != _activeTileDirs.end()) {
+    for (const auto& suffix : getSceneryPathSuffixes()) {
+        const auto s = suffix + "/" + sceneryDir;
+        if (_workerThread->isDirActive(s)) {
             return true;
         }
     }
@@ -1091,14 +1244,16 @@ bool SGTerraSync::isTileDirPending(const std::string& sceneryDir) const
 
 void SGTerraSync::scheduleDataDir(const std::string& dataDir)
 {
-    if (_activeTileDirs.find(dataDir) != _activeTileDirs.end()) {
+    if (!_workerThread->isRunning()) {
+        return;
+    }
+
+    if (_workerThread->isDirActive(dataDir)) {
         return;
     }
 
-    _activeTileDirs.insert(dataDir);
     SyncItem w(dataDir, SyncItem::AIData);
     _workerThread->request( w );
-
 }
 
 bool SGTerraSync::isDataDirPending(const std::string& dataDir) const
@@ -1107,7 +1262,7 @@ bool SGTerraSync::isDataDirPending(const std::string& dataDir) const
         return false;
     }
 
-    return (_activeTileDirs.find(dataDir) != _activeTileDirs.end());
+    return _workerThread->isDirActive(dataDir);
 }
 
 void SGTerraSync::reposition()

simgear/scene/tsync/terrasync.hxx

@@ -107,6 +107,7 @@ private:
     SGPropertyNode_ptr _transferRateBytesSecNode;
     SGPropertyNode_ptr _pendingKbytesNode;
     SGPropertyNode_ptr _downloadedKBtesNode;
+    SGPropertyNode_ptr _maxErrorsNode;
 
     // we manually bind+init TerraSync during early startup
     // to get better overlap of slow operations (Shared Models sync
@@ -117,8 +118,10 @@ private:
     simgear::TiedPropertyList _tiedProperties;
     BufferedLogCallback* _log;
 
-    typedef std::set<std::string> string_set;
-    string_set _activeTileDirs;
+    /// if we disabled TerraSync due to errors, this is the time at which we will restart it
+    /// automatically.
+    SGTimeStamp _retryTime;
+    unsigned int _retryBackOffSeconds = 0;
 };
 
 }

simgear/sound/readwav.cxx

@@ -380,7 +380,8 @@ namespace simgear
 ALvoid* loadWAVFromFile(const SGPath& path, unsigned int& format, ALsizei& size, ALfloat& freqf, unsigned int& block_align)
 {
   if (!path.exists()) {
-    throw sg_io_exception("loadWAVFromFile: file not found", path);
+    SG_LOG(SG_IO, SG_DEV_ALERT, "loadWAVFromFile: file not found:" << path);
+    return nullptr;
   }
 
   Buffer b;
@@ -395,13 +396,15 @@ ALvoid* loadWAVFromFile(const SGPath& path, unsigned int& format, ALsizei& size,
   fd = gzopen(ps.c_str(), "rb");
 #endif
   if (!fd) {
-    throw sg_io_exception("loadWAVFromFile: unable to open file", path);
+    SG_LOG(SG_IO, SG_DEV_ALERT, "loadWAVFromFile: unable to open file:" << path);
+    return nullptr;
   }
 
   try {
       loadWavFile(fd, &b);
   } catch (sg_exception& e) {
-      throw sg_io_exception(e.getFormattedMessage() + "\nfor: " + path.str());
+      SG_LOG(SG_IO, SG_DEV_ALERT, "loadWAVFromFile:" << e.getFormattedMessage() << "\nfor: " << path);
+      return nullptr;
   }
 
   ALvoid* data = b.data;

simgear/sound/soundmgr_openal.cxx

@@ -594,6 +594,7 @@ unsigned int SGSoundMgr::request_buffer(SGSoundSample *sample)
         case SG_SAMPLE_STEREO8:
             SG_LOG(SG_SOUND, SG_POPUP, "Stereo sound detected:\n" << sample->get_sample_name() << "\nUse two separate mono files instead if required.");
             format = AL_FORMAT_STEREO8;
+            break;
         default:
             SG_LOG(SG_SOUND, SG_ALERT, "unsupported audio format");
             return buffer;
@@ -812,13 +813,14 @@ bool SGSoundMgr::load( const std::string &samplepath,
 
     auto data = simgear::loadWAVFromFile(samplepath, format, size, freqf, blocksz);
     freq = (ALsizei)freqf;
-    if (data == nullptr) {
-        throw sg_io_exception("Failed to load wav file", sg_location(samplepath));
+    if (!data) {
+        return false;
     }
 
     if (format == AL_FORMAT_STEREO8 || format == AL_FORMAT_STEREO16) {
          free(data);
-        throw sg_io_exception("Warning: STEREO files are not supported for 3D audio effects: " + samplepath);
+        SG_LOG(SG_IO, SG_DEV_ALERT, "Warning: STEREO files are not supported for 3D audio effects: " << samplepath);
+        return false;
     }
 
     *dbuf = (void *)data;

simgear/sound/xmlsound.cxx

@@ -84,7 +84,7 @@ SGXmlSound::~SGXmlSound()
     _pitch.clear();
 }
 
-void
+bool
 SGXmlSound::init( SGPropertyNode *root,
                   SGPropertyNode *node,
                   SGSampleGroup *sgrp,
@@ -315,8 +315,8 @@ SGXmlSound::init( SGPropertyNode *root,
    string soundFileStr = node->getStringValue("path", "");
    _sample = new SGSoundSample(soundFileStr.c_str(), path);
    if (!_sample->file_path().exists()) {
-      throw sg_io_exception("XML sound: couldn't find file: '" + soundFileStr + "'");
-      return;
+      SG_LOG(SG_SOUND, SG_WARN, "XML sound: couldn't find file: '" + soundFileStr + "'");
+      return false;
    }
 
    _sample->set_relative_position( offset_pos );
@@ -328,6 +328,8 @@ SGXmlSound::init( SGPropertyNode *root,
    _sample->set_volume( v );
    _sample->set_pitch( p );
    _sgrp->add( _sample, _name );
+
+   return true;
 }
 
 void
@@ -414,9 +416,11 @@ SGXmlSound::update (double dt)
       double v = 1.0;
 
       if (_volume[i].expr) {
-         v = _volume[i].expr->getValue(nullptr);
-         expr = true;
-         continue;
+          double expression_value = _volume[i].expr->getValue(nullptr);
+          if (expression_value >= 0)
+              volume *= expression_value;
+		  expr = true;
+		  continue;
       }
 
       if (_volume[i].prop) {
@@ -437,19 +441,23 @@ SGXmlSound::update (double dt)
 
       if (_volume[i].max && (v > _volume[i].max))
          v = _volume[i].max;
-
       else if (v < _volume[i].min)
          v = _volume[i].min;
 
-      if (_volume[i].subtract)				// Hack!
-         volume = _volume[i].offset - v;
 
+      if (_volume[i].subtract){				// Hack!
+          v = v + _volume[i].offset;
+          if (v >= 0)
+            volume = volume * v;
+      }
       else {
-         volume_offset += _volume[i].offset;
-         volume *= v;
+          if (v >= 0) {
+              volume_offset += _volume[i].offset;
+              volume = volume * v;
+          }
       }
    }
-
+   
    //
    // Update the pitch
    //
@@ -462,9 +470,9 @@ SGXmlSound::update (double dt)
       double p = 1.0;
 
       if (_pitch[i].expr) {
-         p = _pitch[i].expr->getValue(nullptr);
-         expr = true;
-         continue;
+          pitch *= _pitch[i].expr->getValue(nullptr);
+          expr = true;
+          continue;
       }
 
       if (_pitch[i].prop) {

simgear/sound/xmlsound.hxx

@@ -107,7 +107,7 @@ public:
    * @param avionics    A pointer to the pre-initialized avionics sample group.
    * @param path        The path where the audio files remain.
    */
-  virtual void init( SGPropertyNode *root,
+  virtual bool init( SGPropertyNode *root,
                      SGPropertyNode *child,
                      SGSampleGroup *sgrp,
                      SGSampleGroup *avionics,

simgear/structure/SGAction.cxx

@@ -0,0 +1,86 @@
+
+
+#if 0
+
+set bindings for action seperate from defintion ?
+    - in XML, especially aircraft XML
+
+define actions from command / Nasal
+add behaviours from Nasal 
+
+define keymapping
+    - manager of keybindings defined against actions?
+    - for a toggle or enum, define behaviour
+        - each key repeat cycles
+        - alternate key to go the other way (G/shift-G)
+
+release bindings for momentary actions:
+    button up / key-up
+
+send activate, release
+
+    send deactivate, release for 'alternate' action
+
+#endif
+
+
+void SGAction::setValueExpression()
+{
+    // watch all the properties
+}
+
+void SGAction::setValueCondition()
+{
+    // 
+}
+
+void SGAction::updateProperties()
+{
+    //
+    _node->setBoolValue("enabled", isEnabled());
+    switch (_type) {
+    case Momentary:
+    case Toggle:
+        _node->setBoolValue("value", getValue());
+        break;
+
+    case Enumerated: 
+        if (!_valueEnumeration.empty()) {
+            // map to the string value
+            _node->setStringValue("value", _valueEnumeration.at(getValue()));
+        } else {
+            // set as an integer
+            _node->setIntValue("value", getValue());
+        }
+    }
+
+    // set description
+}
+
+bool SGAction::isEnabled()
+{
+    if (_enableCondition) {
+
+    } else {
+        return _enabled;
+    }
+
+    updateProperties();
+}
+
+int SGAction::getValue()
+{
+    if (type == Enumerated) {
+        if (_valueExpression) {
+            // invoke it
+        }
+    } else {
+        if (_valueCondition) {
+            return _valueCondition.test();
+        }
+    }
+
+    return _value;
+}
+
+// commands enable-action, disable-action

simgear/structure/SGBinding.cxx

@@ -18,24 +18,16 @@
 #include <simgear/structure/exception.hxx>
 
 SGBinding::SGBinding()
-  : _command(0),
-    _arg(new SGPropertyNode),
-    _setting(0)
+    : _arg(new SGPropertyNode)
 {
 }
 
 SGBinding::SGBinding(const std::string& commandName)
-    : _command(0),
-    _arg(0),
-    _setting(0)
 {
     _command_name = commandName;
 }
 
 SGBinding::SGBinding(const SGPropertyNode* node, SGPropertyNode* root)
-  : _command(0),
-    _arg(0),
-    _setting(0)
 {
   read(node, root);
 }
@@ -43,8 +35,8 @@ SGBinding::SGBinding(const SGPropertyNode* node, SGPropertyNode* root)
 void
 SGBinding::clear()
 {
-    _command = NULL;
     _arg.clear();
+    _root.clear();
     _setting.clear();
 }
 
@@ -57,14 +49,12 @@ SGBinding::read(const SGPropertyNode* node, SGPropertyNode* root)
 
   _command_name = node->getStringValue("command", "");
   if (_command_name.empty()) {
-    SG_LOG(SG_INPUT, SG_WARN, "No command supplied for binding.");
-    _command = 0;
+      SG_LOG(SG_INPUT, SG_DEV_ALERT, "No command supplied for binding.");
   }
 
   _arg = const_cast<SGPropertyNode*>(node);
   _root = const_cast<SGPropertyNode*>(root);
-  _setting = 0;
-
+  _setting.clear();
 }
 
 void
@@ -78,30 +68,29 @@ SGBinding::fire() const
 void
 SGBinding::innerFire () const
 {
-  if (_command == 0)
-    _command = SGCommandMgr::instance()->getCommand(_command_name);
-  if (_command == 0) {
-    SG_LOG(SG_INPUT, SG_WARN, "No command attached to binding:" << _command_name);
-  } else {
-      try {
-          if (!(*_command)(_arg, _root)) {
-                SG_LOG(SG_INPUT, SG_ALERT, "Failed to execute command "
-                       << _command_name);
-          }
-      } catch (sg_exception& e) {
+    auto cmd = SGCommandMgr::instance()->getCommand(_command_name);
+    if (!cmd) {
+        SG_LOG(SG_INPUT, SG_WARN, "No command found for binding:" << _command_name);
+        return;
+    }
+
+    try {
+        if (!(*cmd)(_arg, _root)) {
+            SG_LOG(SG_INPUT, SG_ALERT, "Failed to execute command " << _command_name);
+        }
+    } catch (sg_exception& e) {
         SG_LOG(SG_GENERAL, SG_ALERT, "command '" << _command_name << "' failed with exception\n"
-          << "\tmessage:" << e.getMessage() << " (from " << e.getOrigin() << ")");
-      }
-  }
+                                                 << "\tmessage:" << e.getMessage() << " (from " << e.getOrigin() << ")");
+    }
 }
 
 void
 SGBinding::fire (SGPropertyNode* params) const
 {
   if (test()) {
-    if (params != NULL) {
-      copyProperties(params, _arg);
-    }
+      if (params != nullptr) {
+          copyProperties(params, _arg);
+      }
 
     innerFire();
   }
@@ -122,8 +111,9 @@ SGBinding::fire (double setting) const
   if (test()) {
                                 // A value is automatically added to
                                 // the args
-    if (_setting == 0)          // save the setting node for efficiency
-      _setting = _arg->getChild("setting", 0, true);
+                                if (!_setting) { // save the setting node for efficiency
+                                    _setting = _arg->getChild("setting", 0, true);
+                                }
     _setting->setDoubleValue(setting);
     innerFire();
   }

simgear/structure/SGBinding.hxx

@@ -77,16 +77,6 @@ public:
    */
   const std::string &getCommandName () const { return _command_name; }
 
-
-  /**
-   * Get the command itself.
-   *
-   * @return The command associated with this binding, or 0 if none
-   * is present.
-   */
-  SGCommandMgr::Command* getCommand () const { return _command; }
-
-
   /**
    * Get the argument that will be passed to the command.
    *
@@ -140,7 +130,6 @@ private:
   SGBinding (const SGBinding &binding);
 
   std::string _command_name;
-  mutable SGCommandMgr::Command* _command;
   mutable SGPropertyNode_ptr _arg;
   mutable SGPropertyNode_ptr _setting;
   mutable SGPropertyNode_ptr _root;

simgear/structure/exception.cxx

@@ -13,6 +13,8 @@
 
 #include <simgear/misc/sg_path.hxx>
 
+static ThrowCallback static_callback;
+
 ////////////////////////////////////////////////////////////////////////
 // Implementation of sg_location class.
 ////////////////////////////////////////////////////////////////////////
@@ -49,19 +51,7 @@ sg_location::sg_location (const char* path, int line, int column)
   setPath(path);
 }
 
-sg_location::~sg_location ()
-{
-}
-
-const char*
-sg_location::getPath () const
-{
-  return _path;
-}
-
-void
-sg_location::setPath (const char* path)
-{
+void sg_location::setPath(const char *path) {
   if (path) {
     strncpy(_path, path, max_path);
     _path[max_path -1] = '\0';
@@ -70,17 +60,9 @@ sg_location::setPath (const char* path)
   }
 }
 
-int
-sg_location::getLine () const
-{
-  return _line;
-}
+const char *sg_location::getPath() const { return _path; }
 
-void
-sg_location::setLine (int line)
-{
-  _line = line;
-}
+int sg_location::getLine() const { return _line; }
 
 int
 sg_location::getColumn () const
@@ -88,11 +70,6 @@ sg_location::getColumn () const
   return _column;
 }
 
-void
-sg_location::setColumn (int column)
-{
-  _column = column;
-}
 
 int
 sg_location::getByte () const
@@ -100,12 +77,6 @@ sg_location::getByte () const
   return _byte;
 }
 
-void
-sg_location::setByte (int byte)
-{
-  _byte = byte;
-}
-
 std::string
 sg_location::asString () const
 {
@@ -126,8 +97,8 @@ sg_location::asString () const
   return out.str();
 }
 
+bool sg_location::isValid() const { return strlen(_path) > 0; }
 
-
 ////////////////////////////////////////////////////////////////////////
 // Implementation of sg_throwable class.
 ////////////////////////////////////////////////////////////////////////
@@ -138,10 +109,14 @@ sg_throwable::sg_throwable ()
   _origin[0] = '\0';
 }
 
-sg_throwable::sg_throwable (const char* message, const char* origin)
-{
+sg_throwable::sg_throwable(const char *message, const char *origin,
+                           const sg_location &loc) {
   setMessage(message);
   setOrigin(origin);
+
+  if (static_callback) {
+    static_callback(_message, _origin, loc);
+  }
 }
 
 sg_throwable::~sg_throwable ()
@@ -228,16 +203,13 @@ sg_exception::sg_exception ()
 {
 }
 
-sg_exception::sg_exception (const char* message, const char* origin)
-  : sg_throwable(message, origin)
-{
-}
+sg_exception::sg_exception(const char *message, const char *origin,
+                           const sg_location &loc)
+    : sg_throwable(message, origin, loc) {}
 
-sg_exception::sg_exception( const std::string& message,
-                            const std::string& origin )
-  : sg_throwable(message.c_str(), origin.c_str())
-{
-}
+sg_exception::sg_exception(const std::string &message,
+                           const std::string &origin, const sg_location &loc)
+    : sg_throwable(message.c_str(), origin.c_str(), loc) {}
 
 sg_exception::~sg_exception ()
 {
@@ -257,13 +229,10 @@ sg_io_exception::sg_io_exception (const char* message, const char* origin)
 {
 }
 
-sg_io_exception::sg_io_exception (const char* message,
-				  const sg_location &location,
-				  const char* origin)
-  : sg_exception(message, origin),
-    _location(location)
-{
-}
+sg_io_exception::sg_io_exception(const char *message,
+                                 const sg_location &location,
+                                 const char *origin)
+    : sg_exception(message, origin, location), _location(location) {}
 
 sg_io_exception::sg_io_exception( const std::string& message,
                                   const std::string& origin )
@@ -271,13 +240,10 @@ sg_io_exception::sg_io_exception( const std::string& message,
 {
 }
 
-sg_io_exception::sg_io_exception( const std::string& message,
-                                  const sg_location &location,
-                                  const std::string& origin )
-  : sg_exception(message, origin),
-    _location(location)
-{
-}
+sg_io_exception::sg_io_exception(const std::string &message,
+                                 const sg_location &location,
+                                 const std::string &origin)
+    : sg_exception(message, origin, location), _location(location) {}
 
 sg_io_exception::~sg_io_exception ()
 {
@@ -382,4 +348,9 @@ sg_range_exception::sg_range_exception(const std::string& message,
 sg_range_exception::~sg_range_exception ()
 {
 }
+
+////////////////////////////////////////////////////////////////////////
+
+void setThrowCallback(ThrowCallback cb) { static_callback = cb; }
+
 // end of exception.cxx

simgear/structure/exception.hxx

@@ -11,6 +11,7 @@
 #define __SIMGEAR_MISC_EXCEPTION_HXX 1
 
 #include <exception>
+#include <functional>
 #include <simgear/compiler.h>
 #include <string>
 
@@ -31,17 +32,20 @@ public:
   sg_location(const std::string& path, int line = -1, int column = -1);
   sg_location(const SGPath& path, int line = -1, int column = -1);
   explicit sg_location(const char* path, int line = -1, int column = -1);
-  virtual ~sg_location();
-  virtual const char* getPath() const;
-  virtual void setPath (const char* path);
-  virtual int getLine () const;
-  virtual void setLine (int line);
-  virtual int getColumn () const;
-  virtual void setColumn (int column);
-  virtual int getByte () const;
-  virtual void setByte (int byte);
-  virtual std::string asString () const;
+
+  ~sg_location() = default;
+
+  const char *getPath() const;
+  int getLine() const;
+  int getColumn() const;
+  int getByte() const;
+
+  std::string asString() const;
+  bool isValid() const;
+
 private:
+  void setPath(const char *p);
+
   char _path[max_path];
   int _line;
   int _column;
@@ -57,7 +61,9 @@ class sg_throwable : public std::exception
 public:
   enum {MAX_TEXT_LEN = 1024};
   sg_throwable ();
-  sg_throwable (const char* message, const char* origin = 0);
+  sg_throwable(const char *message, const char *origin = 0,
+               const sg_location &loc = {});
+
   virtual ~sg_throwable ();
   virtual const char* getMessage () const;
   virtual const std::string getFormattedMessage () const;
@@ -86,7 +92,7 @@ class sg_error : public sg_throwable
 public:
   sg_error ();
   sg_error (const char* message, const char* origin = 0);
-  sg_error (const std::string& message, const std::string& origin = "");  
+  sg_error(const std::string &message, const std::string &origin = {});
   virtual ~sg_error ();
 };
 
@@ -109,8 +115,10 @@ class sg_exception : public sg_throwable
 {
 public:
   sg_exception ();
-  sg_exception (const char* message, const char* origin = 0);
-  sg_exception (const std::string& message, const std::string& = "");
+  sg_exception(const char *message, const char *origin = 0,
+               const sg_location &loc = {});
+  sg_exception(const std::string &message, const std::string & = {},
+               const sg_location &loc = {});
   virtual ~sg_exception ();
 };
 
@@ -193,6 +201,17 @@ public:
   virtual ~sg_range_exception ();
 };
 
+using ThrowCallback = std::function<void(
+    const char *message, const char *origin, const sg_location &loc)>;
+
+/**
+ * @brief Specify a callback to be invoked when an exception is created.
+ *
+ * This is used to capture a stack-trace for our crash/error reporting system,
+ * if a callback is defined
+ */
+void setThrowCallback(ThrowCallback cb);
+
 #endif
 
 // end of exception.hxx

simgear/threads/SGQueue.hxx

@@ -272,30 +272,35 @@ template<class T>
 class SGBlockingDeque
 {
 public:
+    using value_type = T;
+    using container_type = std::deque<T>;
+
     /**
      * Create a new SGBlockingDequeue.
      */
-    SGBlockingDeque() {}
+    SGBlockingDeque() = default;
 
     /**
      * Destroy this dequeue.
      */
-    virtual ~SGBlockingDeque() {}
+    ~SGBlockingDeque() = default;
 
     /**
      *
      */
-    virtual void clear() {
-    std::lock_guard<std::mutex> g(mutex);
-    this->queue.clear();
+    void clear()
+    {
+        std::lock_guard<std::mutex> g(mutex);
+        this->queue.clear();
     }
 
     /**
      *
      */
-    virtual bool empty() {
-    std::lock_guard<std::mutex> g(mutex);
-    return this->queue.empty();
+    bool empty() const
+    {
+        std::lock_guard<std::mutex> g(mutex);
+        return this->queue.empty();
     }
 
     /**
@@ -303,10 +308,11 @@ public:
      *
      * @param item The object to add.
      */
-    virtual void push_front( const T& item ) {
-    std::lock_guard<std::mutex> g(mutex);
-    this->queue.push_front( item );
-    not_empty.signal();
+    void push_front(const T& item)
+    {
+        std::lock_guard<std::mutex> g(mutex);
+        this->queue.push_front(item);
+        not_empty.signal();
     }
 
     /**
@@ -314,10 +320,11 @@ public:
      *
      * @param item The object to add.
      */
-    virtual void push_back( const T& item ) {
-    std::lock_guard<std::mutex> g(mutex);
-    this->queue.push_back( item );
-    not_empty.signal();
+    void push_back(const T& item)
+    {
+        std::lock_guard<std::mutex> g(mutex);
+        this->queue.push_back(item);
+        not_empty.signal();
     }
 
     /**
@@ -326,14 +333,15 @@ public:
      *
      * @return The next available object.
      */
-    virtual T front() {
-    std::lock_guard<std::mutex> g(mutex);
+    T front() const
+    {
+        std::lock_guard<std::mutex> g(mutex);
 
-    assert(this->queue.empty() != true);
-    //if (queue.empty()) throw ??
+        assert(this->queue.empty() != true);
+        //if (queue.empty()) throw ??
 
-    T item = this->queue.front();
-    return item;
+        T item = this->queue.front();
+        return item;
     }
 
     /**
@@ -342,18 +350,19 @@ public:
      *
      * @return The next available object.
      */
-    virtual T pop_front() {
-    std::lock_guard<std::mutex> g(mutex);
+    T pop_front()
+    {
+        std::lock_guard<std::mutex> g(mutex);
 
-    while (this->queue.empty())
-        not_empty.wait(mutex);
+        while (this->queue.empty())
+            not_empty.wait(mutex);
 
-    assert(this->queue.empty() != true);
-    //if (queue.empty()) throw ??
+        assert(this->queue.empty() != true);
+        //if (queue.empty()) throw ??
 
-    T item = this->queue.front();
-    this->queue.pop_front();
-    return item;
+        T item = this->queue.front();
+        this->queue.pop_front();
+        return item;
     }
 
     /**
@@ -362,18 +371,19 @@ public:
      *
      * @return The next available object.
      */
-    virtual T pop_back() {
-    std::lock_guard<std::mutex> g(mutex);
+    T pop_back()
+    {
+        std::lock_guard<std::mutex> g(mutex);
 
-    while (this->queue.empty())
-        not_empty.wait(mutex);
+        while (this->queue.empty())
+            not_empty.wait(mutex);
 
-    assert(this->queue.empty() != true);
-    //if (queue.empty()) throw ??
+        assert(this->queue.empty() != true);
+        //if (queue.empty()) throw ??
 
-    T item = this->queue.back();
-    this->queue.pop_back();
-    return item;
+        T item = this->queue.back();
+        this->queue.pop_back();
+        return item;
     }
 
     /**
@@ -381,8 +391,9 @@ public:
      *
      * @return Size of queue.
      */
-    virtual size_t size() {
-    std::lock_guard<std::mutex> g(mutex);
+    size_t size() const
+    {
+        std::lock_guard<std::mutex> g(mutex);
         return this->queue.size();
     }
 
@@ -391,12 +402,19 @@ public:
     	while (this->queue.empty())
     	    not_empty.wait(mutex);
     }
+
+    container_type copy() const
+    {
+        std::lock_guard<std::mutex> g(mutex);
+        return queue;
+    }
+
 private:
 
     /**
      * Mutex to serialise access.
      */
-    std::mutex mutex;
+    mutable std::mutex mutex;
 
     /**
      * Condition to signal when queue not empty.
@@ -409,7 +427,7 @@ private:
     SGBlockingDeque& operator=( const SGBlockingDeque& );
 
 protected:
-    std::deque<T> queue;
+    container_type queue;
 };
 
 #endif // SGQUEUE_HXX_INCLUDED

version

@@ -1 +0,0 @@
-2020.2.1