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

@@ -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)

simgear-version

@@ -1 +1 @@
-2020.3.0
+2020.3.3

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))

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/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;
+        if (!r->isCancelled()) {
-        r->qname = result->dnssrv_qname;
+            r->cname = result->dnssrv_cname;
-        r->ttl = result->dnssrv_ttl;
+            r->qname = result->dnssrv_qname;
-        for (int i = 0; i < result->dnssrv_nrr; i++) {
+            r->ttl = result->dnssrv_ttl;
-            SRVRequest::SRV_ptr srv(new SRVRequest::SRV);
+            for (int i = 0; i < result->dnssrv_nrr; i++) {
-            r->entries.push_back(srv);
+                SRVRequest::SRV_ptr srv(new SRVRequest::SRV);
-            srv->priority = result->dnssrv_srv[i].priority;
+                r->entries.push_back(srv);
-            srv->weight = result->dnssrv_srv[i].weight;
+                srv->priority = result->dnssrv_srv[i].priority;
-            srv->port = result->dnssrv_srv[i].port;
+                srv->weight = result->dnssrv_srv[i].weight;
-            srv->target = result->dnssrv_srv[i].name;
+                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;
+        if (!r->isCancelled()) {
-        r->qname = result->dnstxt_qname;
+            r->cname = result->dnstxt_cname;
-        r->ttl = result->dnstxt_ttl;
+            r->qname = result->dnstxt_qname;
-        for (int i = 0; i < result->dnstxt_nrr; i++) {
+            r->ttl = result->dnstxt_ttl;
-          //TODO: interprete the .len field of dnstxt_txt?
+            for (int i = 0; i < result->dnstxt_nrr; i++) {
-          string txt = string((char*)result->dnstxt_txt[i].txt);
+                //TODO: interprete the .len field of dnstxt_txt?
-          r->entries.push_back( txt );
+                auto rawTxt = reinterpret_cast<char*>(result->dnstxt_txt[i].txt);
-          string_list tokens = simgear::strutils::split( txt, "=", 1 );
+                if (!rawTxt) {
-          if( tokens.size() == 2 ) {
+                    continue;
-            r->attributes[tokens[0]] = tokens[1];
+                }
-          }
+
+                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;
+        if (!r->isCancelled()) {
-        r->qname = result->dnsnaptr_qname;
+            r->cname = result->dnsnaptr_cname;
-        r->ttl = result->dnsnaptr_ttl;
+            r->qname = result->dnsnaptr_qname;
-        for (int i = 0; i < result->dnsnaptr_nrr; i++) {
+            r->ttl = result->dnsnaptr_ttl;
-            if( !r->qservice.empty() && r->qservice != result->dnsnaptr_naptr[i].service )
+            for (int i = 0; i < result->dnsnaptr_nrr; i++) {
-                continue;
+                if (!r->qservice.empty() && r->qservice != result->dnsnaptr_naptr[i].service)
+                    continue;
 
-            //TODO: case ignore and result flags may have more than one flag
+                //TODO: case ignore and result flags may have more than one flag
-            if( !r->qflags.empty() && r->qflags != result->dnsnaptr_naptr[i].flags )
+                if (!r->qflags.empty() && r->qflags != result->dnsnaptr_naptr[i].flags)
-                continue;
+                    continue;
 
-            NAPTRRequest::NAPTR_ptr naptr(new NAPTRRequest::NAPTR);
+                NAPTRRequest::NAPTR_ptr naptr(new NAPTRRequest::NAPTR);
-            r->entries.push_back(naptr);
+                r->entries.push_back(naptr);
-            naptr->order = result->dnsnaptr_naptr[i].order;
+                naptr->order = result->dnsnaptr_naptr[i].order;
-            naptr->preference = result->dnsnaptr_naptr[i].preference;
+                naptr->preference = result->dnsnaptr_naptr[i].preference;
-            naptr->flags = result->dnsnaptr_naptr[i].flags;
+                naptr->flags = result->dnsnaptr_naptr[i].flags;
-            naptr->service = result->dnsnaptr_naptr[i].service;
+                naptr->service = result->dnsnaptr_naptr[i].service;
-            naptr->regexp = result->dnsnaptr_naptr[i].regexp;
+                naptr->regexp = result->dnsnaptr_naptr[i].regexp;
-            naptr->replacement = result->dnsnaptr_naptr[i].replacement;
+                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/HTTPRepository.cxx

@@ -81,6 +81,45 @@ std::string innerResultCodeAsString(HTTPRepository::ResultCode code) {
   return "Unknown response code";
 }
 
+struct HashCacheEntry {
+    std::string filePath;
+    time_t modTime;
+    size_t lengthBytes;
+    std::string hashHex;
+};
+
+using HashCache = std::unordered_map<std::string, HashCacheEntry>;
+
+std::string computeHashForPath(const SGPath& p)
+{
+    if (!p.exists())
+        return {};
+
+    sha1nfo info;
+    sha1_init(&info);
+
+    const int bufSize = 1024 * 1024;
+    char* buf = static_cast<char*>(malloc(bufSize));
+    if (!buf) {
+        sg_io_exception("Couldn't allocate SHA1 computation buffer");
+    }
+
+    size_t readLen;
+    SGBinaryFile f(p);
+    if (!f.open(SG_IO_IN)) {
+        free(buf);
+        throw sg_io_exception("Couldn't open file for compute hash", p);
+    }
+    while ((readLen = f.read(buf, bufSize)) > 0) {
+        sha1_write(&info, buf, readLen);
+    }
+
+    f.close();
+    free(buf);
+    std::string hashBytes((char*)sha1_result(&info), HASH_LENGTH);
+    return strutils::encodeHex(hashBytes);
+}
+
 } // namespace
 
 class HTTPDirectory
@@ -111,6 +150,9 @@ class HTTPDirectory
     typedef std::vector<ChildInfo> ChildInfoList;
     ChildInfoList children;
 
+    mutable HashCache hashes;
+    mutable bool hashCacheDirty = false;
+
 public:
     HTTPDirectory(HTTPRepoPrivate* repo, const std::string& path) :
         _repository(repo),
@@ -124,6 +166,8 @@ public:
               // already exists on disk
               parseDirIndex(children);
               std::sort(children.begin(), children.end());
+
+              parseHashCache();
           } catch (sg_exception& ) {
               // parsing cache failed
               children.clear();
@@ -149,7 +193,7 @@ public:
     {
         SGPath fpath(absolutePath());
         fpath.append(".dirindex");
-        _repository->updatedFileContents(fpath, hash);
+        updatedFileContents(fpath, hash);
 
         children.clear();
         parseDirIndex(children);
@@ -177,7 +221,7 @@ public:
 		char* buf = nullptr;
 		size_t bufSize = 0;
 
-		for (const auto& child : children) {
+                for (auto &child : children) {
                   if (child.type != HTTPRepository::FileType)
                     continue;
 
@@ -189,30 +233,36 @@ public:
                   cp.append(child.name);
                   if (!cp.exists()) {
                     continue;
-			}
+                  }
 
-			SGBinaryFile src(cp);
+                  SGBinaryFile src(cp);
-			SGBinaryFile dst(child.path);
+                  SGBinaryFile dst(child.path);
-			src.open(SG_IO_IN);
+                  src.open(SG_IO_IN);
-			dst.open(SG_IO_OUT);
+                  dst.open(SG_IO_OUT);
 
-			if (bufSize < cp.sizeInBytes()) {
+                  if (bufSize < cp.sizeInBytes()) {
-				bufSize = cp.sizeInBytes();
+                    bufSize = cp.sizeInBytes();
-				free(buf);
+                    free(buf);
-				buf = (char*) malloc(bufSize);
+                    buf = (char *)malloc(bufSize);
-				if (!buf) {
+                    if (!buf) {
-					continue;
+                      continue;
-				}
+                    }
-			}
+                  }
 
-			src.read(buf, cp.sizeInBytes());
+                  src.read(buf, cp.sizeInBytes());
-			dst.write(buf, cp.sizeInBytes());
+                  dst.write(buf, cp.sizeInBytes());
-			src.close();
+                  src.close();
-			dst.close();
+                  dst.close();
 
-		}
+                  // reset caching
+                  child.path.set_cached(false);
+                  child.path.set_cached(true);
 
-		free(buf);
+                  std::string hash = computeHashForPath(child.path);
+                  updatedFileContents(child.path, hash);
+                }
+
+                free(buf);
     }
 
     void updateChildrenBasedOnHash()
@@ -274,7 +324,7 @@ public:
           if (c.type == HTTPRepository::DirectoryType) {
             // If it's a directory,perform a recursive check.
             HTTPDirectory *childDir = childDirectory(c.name);
-            childDir->updateChildrenBasedOnHash();
+            _repository->scheduleUpdateOfChildren(childDir);
           }
         }
       } // of repository-defined (well, .dirIndex) children iteration
@@ -372,6 +422,66 @@ public:
         return _relativePath;
     }
 
+    class ArchiveExtractTask {
+    public:
+      ArchiveExtractTask(SGPath p, const std::string &relPath)
+          : relativePath(relPath), file(p), extractor(p.dir()) {
+        if (!file.open(SG_IO_IN)) {
+          SG_LOG(SG_TERRASYNC, SG_ALERT,
+                 "Unable to open " << p << " to extract");
+          return;
+        }
+
+        buffer = (uint8_t *)malloc(bufferSize);
+      }
+
+      ArchiveExtractTask(const ArchiveExtractTask &) = delete;
+
+      HTTPRepoPrivate::ProcessResult run(HTTPRepoPrivate *repo) {
+        size_t rd = file.read((char *)buffer, bufferSize);
+        extractor.extractBytes(buffer, rd);
+
+        if (file.eof()) {
+          extractor.flush();
+          file.close();
+
+          if (!extractor.isAtEndOfArchive()) {
+            SG_LOG(SG_TERRASYNC, SG_ALERT, "Corrupt tarball " << relativePath);
+            repo->failedToUpdateChild(relativePath,
+                                      HTTPRepository::REPO_ERROR_IO);
+            return HTTPRepoPrivate::ProcessFailed;
+          }
+
+          if (extractor.hasError()) {
+            SG_LOG(SG_TERRASYNC, SG_ALERT, "Error extracting " << relativePath);
+            repo->failedToUpdateChild(relativePath,
+                                      HTTPRepository::REPO_ERROR_IO);
+            return HTTPRepoPrivate::ProcessFailed;
+          }
+
+          return HTTPRepoPrivate::ProcessDone;
+        }
+
+        return HTTPRepoPrivate::ProcessContinue;
+      }
+
+      ~ArchiveExtractTask() { free(buffer); }
+
+    private:
+      // intentionally small so we extract incrementally on Windows
+      // where Defender throttles many small files, sorry
+      // if you make this bigger we will be more efficient but stall for
+      // longer when extracting the Airports_archive
+      const int bufferSize = 1024 * 64;
+
+      std::string relativePath;
+      uint8_t *buffer = nullptr;
+      SGBinaryFile file;
+      ArchiveExtractor extractor;
+    };
+
+    using ArchiveExtractTaskPtr = std::shared_ptr<ArchiveExtractTask>;
+
     void didUpdateFile(const std::string& file, const std::string& hash, size_t sz)
     {
         // check hash matches what we expected
@@ -387,7 +497,7 @@ public:
                     _relativePath + "/" + file,
                     HTTPRepository::REPO_ERROR_CHECKSUM);
             } else {
-                _repository->updatedFileContents(it->path, hash);
+                updatedFileContents(it->path, hash);
                 _repository->updatedChildSuccessfully(_relativePath + "/" +
                                                       file);
 
@@ -410,33 +520,22 @@ public:
                   }
 
                   if (pathAvailable) {
-                    // If this is a tarball, then extract it.
+                    // we use a Task helper to extract tarballs incrementally.
-                    SGBinaryFile f(p);
+                    // without this, archive extraction blocks here, which
-                    if (! f.open(SG_IO_IN)) SG_LOG(SG_TERRASYNC, SG_ALERT, "Unable to open " << p << " to extract");
+                    // prevents other repositories downloading / updating.
+                    // Unfortunately due Windows AV (Defender, etc) we cna block
+                    // here for many minutes.
 
-                    SG_LOG(SG_TERRASYNC, SG_INFO, "Extracting " << absolutePath() << "/" << file << " to " << p.dir());
+                    // use a lambda to own this shared_ptr; this means when the
-                    SGPath extractDir = p.dir();
+                    // lambda is destroyed, the ArchiveExtraTask will get
-                    ArchiveExtractor ex(extractDir);
+                    // cleaned up.
-
+                    ArchiveExtractTaskPtr t =
-                    uint8_t *buf = (uint8_t *)alloca(2048);
+                        std::make_shared<ArchiveExtractTask>(p, _relativePath);
-                    while (!f.eof()) {
+                    auto cb = [t](HTTPRepoPrivate *repo) {
-                      size_t bufSize = f.read((char *)buf, 2048);
+                      return t->run(repo);
-                      ex.extractBytes(buf, bufSize);
+                    };
-                    }
-
-                    ex.flush();
-                    if (! ex.isAtEndOfArchive()) {
-                      SG_LOG(SG_TERRASYNC, SG_ALERT, "Corrupt tarball " << p);
-                      _repository->failedToUpdateChild(
-                          _relativePath, HTTPRepository::REPO_ERROR_IO);
-                    }
-
-                    if (ex.hasError()) {
-                      SG_LOG(SG_TERRASYNC, SG_ALERT, "Error extracting " << p);
-                      _repository->failedToUpdateChild(
-                          _relativePath, HTTPRepository::REPO_ERROR_IO);
-                    }
 
+                    _repository->addTask(cb);
                   } else {
                     SG_LOG(SG_TERRASYNC, SG_ALERT, "Unable to remove old file/directory " << removePath);
                   } // of pathAvailable
@@ -452,6 +551,50 @@ public:
         fpath.append(file);
         _repository->failedToUpdateChild(fpath, status);
     }
+
+    std::string hashForPath(const SGPath& p) const
+    {
+        const auto ps = p.utf8Str();
+        auto it = hashes.find(ps);
+        if (it != hashes.end()) {
+            const auto& entry = it->second;
+            // ensure data on disk hasn't changed.
+            // we could also use the file type here if we were paranoid
+            if ((p.sizeInBytes() == entry.lengthBytes) && (p.modTime() == entry.modTime)) {
+                return entry.hashHex;
+            }
+
+            // entry in the cache, but it's stale so remove and fall through
+            hashes.erase(it);
+        }
+
+        std::string hash = computeHashForPath(p);
+        updatedFileContents(p, hash);
+        return hash;
+    }
+
+    bool isHashCacheDirty() const
+    {
+        return hashCacheDirty;
+    }
+
+    void writeHashCache() const
+    {
+        if (!hashCacheDirty)
+            return;
+
+        hashCacheDirty = false;
+
+        SGPath cachePath = absolutePath() / ".dirhash";
+        sg_ofstream stream(cachePath, std::ios::out | std::ios::trunc | std::ios::binary);
+        for (const auto& e : hashes) {
+            const auto& entry = e.second;
+            stream << entry.filePath << "*" << entry.modTime << "*"
+                   << entry.lengthBytes << "*" << entry.hashHex << "\n";
+        }
+        stream.close();
+    }
+
 private:
 
     struct ChildWithName
@@ -575,7 +718,7 @@ private:
             ok = _repository->deleteDirectory(fpath, path);
         } else {
             // remove the hash cache entry
-            _repository->updatedFileContents(path, std::string());
+            updatedFileContents(path, std::string());
             ok = path.remove();
         }
 
@@ -593,9 +736,80 @@ private:
       if (child.type == HTTPRepository::TarballType)
         p.concat(
             ".tgz"); // For tarballs the hash is against the tarball file itself
-      return _repository->hashForPath(p);
+      return hashForPath(p);
     }
 
+    void parseHashCache()
+    {
+        hashes.clear();
+        SGPath cachePath = absolutePath() / ".dirhash";
+        if (!cachePath.exists()) {
+            return;
+        }
+
+        sg_ifstream stream(cachePath, std::ios::in);
+
+        while (!stream.eof()) {
+            std::string line;
+            std::getline(stream, line);
+            line = simgear::strutils::strip(line);
+            if (line.empty() || line[0] == '#')
+                continue;
+
+            string_list tokens = simgear::strutils::split(line, "*");
+            if (tokens.size() < 4) {
+                SG_LOG(SG_TERRASYNC, SG_WARN, "invalid entry in '" << cachePath << "': '" << line << "' (ignoring line)");
+                continue;
+            }
+            const std::string nameData = simgear::strutils::strip(tokens[0]);
+            const std::string timeData = simgear::strutils::strip(tokens[1]);
+            const std::string sizeData = simgear::strutils::strip(tokens[2]);
+            const std::string hashData = simgear::strutils::strip(tokens[3]);
+
+            if (nameData.empty() || timeData.empty() || sizeData.empty() || hashData.empty()) {
+                SG_LOG(SG_TERRASYNC, SG_WARN, "invalid entry in '" << cachePath << "': '" << line << "' (ignoring line)");
+                continue;
+            }
+
+            HashCacheEntry entry;
+            entry.filePath = nameData;
+            entry.hashHex = hashData;
+            entry.modTime = strtol(timeData.c_str(), NULL, 10);
+            entry.lengthBytes = strtol(sizeData.c_str(), NULL, 10);
+            hashes.insert(std::make_pair(entry.filePath, entry));
+        }
+    }
+
+    void updatedFileContents(const SGPath& p, const std::string& newHash) const
+    {
+        // remove the existing entry
+        const auto ps = p.utf8Str();
+        auto it = hashes.find(ps);
+        if (it != hashes.end()) {
+            hashes.erase(it);
+            hashCacheDirty = true;
+        }
+
+        if (newHash.empty()) {
+            return; // we're done
+        }
+
+        // use a cloned SGPath and reset its caching to force one stat() call
+        SGPath p2(p);
+        p2.set_cached(false);
+        p2.set_cached(true);
+
+        HashCacheEntry entry;
+        entry.filePath = ps;
+        entry.hashHex = newHash;
+        entry.modTime = p2.modTime();
+        entry.lengthBytes = p2.sizeInBytes();
+        hashes.insert(std::make_pair(ps, entry));
+
+        hashCacheDirty = true;
+    }
+
+
     HTTPRepoPrivate* _repository;
     std::string _relativePath; // in URL and file-system space
 };
@@ -606,7 +820,6 @@ HTTPRepository::HTTPRepository(const SGPath& base, HTTP::Client *cl) :
     _d->http = cl;
     _d->basePath = base;
     _d->rootDir.reset(new HTTPDirectory(_d.get(), ""));
-    _d->parseHashCache();
 }
 
 HTTPRepository::~HTTPRepository()
@@ -654,6 +867,38 @@ bool HTTPRepository::isDoingSync() const
     return _d->isUpdating;
 }
 
+void HTTPRepository::process()
+{
+    int processedCount = 0;
+    const int maxToProcess = 16;
+
+    while (processedCount < maxToProcess) {
+      if (_d->pendingTasks.empty()) {
+        break;
+      }
+
+      auto task = _d->pendingTasks.front();
+      auto result = task(_d.get());
+      if (result == HTTPRepoPrivate::ProcessContinue) {
+        // assume we're not complete
+        return;
+      }
+
+      _d->pendingTasks.pop_front();
+      ++processedCount;
+    }
+
+    _d->checkForComplete();
+}
+
+void HTTPRepoPrivate::checkForComplete()
+{
+  if (pendingTasks.empty() && activeRequests.empty() &&
+      queuedRequests.empty()) {
+    isUpdating = false;
+  }
+}
+
 size_t HTTPRepository::bytesToDownload() const
 {
     size_t result = 0;
@@ -868,7 +1113,7 @@ HTTPRepository::failure() const
               return;
             }
 
-            std::string curHash = _directory->repository()->hashForPath(path());
+            std::string curHash = _directory->hashForPath(path());
             if (hash != curHash) {
               simgear::Dir d(_directory->absolutePath());
               if (!d.exists()) {
@@ -967,6 +1212,7 @@ HTTPRepository::failure() const
             http->cancelRequest(*rq, "Repository object deleted");
         }
 
+        flushHashCaches();
         directories.clear(); // wil delete them all
     }
 
@@ -986,154 +1232,6 @@ HTTPRepository::failure() const
         return r;
     }
 
-
-    class HashEntryWithPath
-    {
-    public:
-        HashEntryWithPath(const SGPath& p) : path(p.utf8Str()) {}
-        bool operator()(const HTTPRepoPrivate::HashCacheEntry& entry) const
-        { return entry.filePath == path; }
-    private:
-        std::string path;
-    };
-
-    std::string HTTPRepoPrivate::hashForPath(const SGPath& p)
-    {
-        HashCache::iterator it = std::find_if(hashes.begin(), hashes.end(), HashEntryWithPath(p));
-        if (it != hashes.end()) {
-            // ensure data on disk hasn't changed.
-            // we could also use the file type here if we were paranoid
-            if ((p.sizeInBytes() == it->lengthBytes) && (p.modTime() == it->modTime)) {
-                return it->hashHex;
-            }
-
-            // entry in the cache, but it's stale so remove and fall through
-            hashes.erase(it);
-        }
-
-        std::string hash = computeHashForPath(p);
-        updatedFileContents(p, hash);
-        return hash;
-    }
-
-    std::string HTTPRepoPrivate::computeHashForPath(const SGPath& p)
-    {
-        if (!p.exists())
-            return {};
-
-        sha1nfo info;
-        sha1_init(&info);
-
-        const int bufSize = 1024 * 1024;
-        char* buf = static_cast<char*>(malloc(bufSize));
-        if (!buf) {
-            sg_io_exception("Couldn't allocate SHA1 computation buffer");
-        }
-
-        size_t readLen;
-        SGBinaryFile f(p);
-        if (!f.open(SG_IO_IN)) {
-            free(buf);
-            throw sg_io_exception("Couldn't open file for compute hash", p);
-        }
-        while ((readLen = f.read(buf, bufSize)) > 0) {
-            sha1_write(&info, buf, readLen);
-        }
-
-        f.close();
-        free(buf);
-        std::string hashBytes((char*) sha1_result(&info), HASH_LENGTH);
-        return strutils::encodeHex(hashBytes);
-    }
-
-    void HTTPRepoPrivate::updatedFileContents(const SGPath& p, const std::string& newHash)
-    {
-        // remove the existing entry
-        auto it = std::find_if(hashes.begin(), hashes.end(), HashEntryWithPath(p));
-        if (it != hashes.end()) {
-            hashes.erase(it);
-            ++hashCacheDirty;
-        }
-
-        if (newHash.empty()) {
-            return; // we're done
-        }
-
-        // use a cloned SGPath and reset its caching to force one stat() call
-        SGPath p2(p);
-        p2.set_cached(false);
-        p2.set_cached(true);
-
-        HashCacheEntry entry;
-        entry.filePath = p.utf8Str();
-        entry.hashHex = newHash;
-        entry.modTime = p2.modTime();
-        entry.lengthBytes = p2.sizeInBytes();
-        hashes.push_back(entry);
-
-        ++hashCacheDirty ;
-    }
-
-    void HTTPRepoPrivate::writeHashCache()
-    {
-        if (hashCacheDirty == 0) {
-            return;
-        }
-
-        SGPath cachePath = basePath;
-        cachePath.append(".hashes");
-        sg_ofstream stream(cachePath, std::ios::out | std::ios::trunc | std::ios::binary);
-        HashCache::const_iterator it;
-        for (it = hashes.begin(); it != hashes.end(); ++it) {
-            stream << it->filePath << "*" << it->modTime << "*"
-            << it->lengthBytes << "*" << it->hashHex << "\n";
-        }
-        stream.close();
-        hashCacheDirty = 0;
-    }
-
-    void HTTPRepoPrivate::parseHashCache()
-    {
-        hashes.clear();
-        SGPath cachePath = basePath;
-        cachePath.append(".hashes");
-        if (!cachePath.exists()) {
-            return;
-        }
-
-        sg_ifstream stream(cachePath, std::ios::in);
-
-        while (!stream.eof()) {
-            std::string line;
-            std::getline(stream,line);
-            line = simgear::strutils::strip(line);
-            if( line.empty() || line[0] == '#' )
-                continue;
-
-			string_list tokens = simgear::strutils::split(line, "*");
-            if( tokens.size() < 4 ) {
-                SG_LOG(SG_TERRASYNC, SG_WARN, "invalid entry in '" << cachePath << "': '" << line << "' (ignoring line)");
-                continue;
-            }
-            const std::string nameData = simgear::strutils::strip(tokens[0]);
-            const std::string timeData = simgear::strutils::strip(tokens[1]);
-            const std::string sizeData = simgear::strutils::strip(tokens[2]);
-            const std::string hashData = simgear::strutils::strip(tokens[3]);
-
-            if (nameData.empty() || timeData.empty() || sizeData.empty() || hashData.empty() ) {
-                SG_LOG(SG_TERRASYNC, SG_WARN, "invalid entry in '" << cachePath << "': '" << line << "' (ignoring line)");
-                continue;
-            }
-
-            HashCacheEntry entry;
-            entry.filePath = nameData;
-            entry.hashHex = hashData;
-            entry.modTime = strtol(timeData.c_str(), NULL, 10);
-            entry.lengthBytes = strtol(sizeData.c_str(), NULL, 10);
-            hashes.push_back(entry);
-        }
-    }
-
     class DirectoryWithPath
     {
     public:
@@ -1163,16 +1261,12 @@ HTTPRepository::failure() const
         if (it != directories.end()) {
           assert((*it)->absolutePath() == absPath);
           directories.erase(it);
-                } else {
+        } else {
-			// we encounter this code path when deleting an orphaned directory
+            // we encounter this code path when deleting an orphaned directory
-		}
+        }
 
-		Dir dir(absPath);
+        Dir dir(absPath);
 		bool result = dir.remove(true);
-
-		// update the hash cache too
-		updatedFileContents(absPath, std::string());
-
         return result;
     }
 
@@ -1208,17 +1302,10 @@ HTTPRepository::failure() const
         http->makeRequest(rr);
       }
 
-      // rate limit how often we write this, since otherwise
+      if (countDirtyHashCaches() > 32) {
-      // it dominates the time on Windows. 256 seems about right,
+          flushHashCaches();
-      // causes a write a few times a minute.
-      if (hashCacheDirty > 256) {
-        writeHashCache();
-      }
-
-      if (activeRequests.empty() && queuedRequests.empty()) {
-        isUpdating = false;
-        writeHashCache();
       }
+      checkForComplete();
     }
 
     void HTTPRepoPrivate::failedToGetRootIndex(HTTPRepository::ResultCode st)
@@ -1279,4 +1366,38 @@ HTTPRepository::failure() const
           failures.end());
     }
 
+    void HTTPRepoPrivate::scheduleUpdateOfChildren(HTTPDirectory* dir)
+    {
+      auto updateChildTask = [dir](const HTTPRepoPrivate *) {
+        dir->updateChildrenBasedOnHash();
+        return ProcessDone;
+      };
+
+      addTask(updateChildTask);
+    }
+
+    void HTTPRepoPrivate::addTask(RepoProcessTask task) {
+      pendingTasks.push_back(task);
+    }
+
+    int HTTPRepoPrivate::countDirtyHashCaches() const
+    {
+        int result = rootDir->isHashCacheDirty() ? 1 : 0;
+        for (const auto& dir : directories) {
+            if (dir->isHashCacheDirty()) {
+                ++result;
+            }
+        }
+
+        return result;
+    }
+
+    void HTTPRepoPrivate::flushHashCaches()
+    {
+        rootDir->writeHashCache();
+        for (const auto& dir : directories) {
+            dir->writeHashCache();
+        }
+    }
+
 } // of namespace simgear

simgear/io/HTTPRepository.hxx

@@ -62,6 +62,11 @@ public:
 
   virtual bool isDoingSync() const;
 
+  /**
+        @brief call this periodically to progress non-network tasks
+     */
+  void process();
+
   virtual ResultCode failure() const;
 
   virtual size_t bytesToDownload() const;

simgear/io/HTTPRepository_private.hxx

@@ -19,8 +19,11 @@
 
 #pragma once
 
+#include <deque>
+#include <functional>
 #include <memory>
 #include <string>
+#include <unordered_map>
 
 #include <simgear/io/HTTPClient.hxx>
 #include <simgear/misc/sg_path.hxx>
@@ -48,20 +51,11 @@ protected:
   size_t _contentSize = 0;
 };
 
-typedef SGSharedPtr<HTTPRepoGetRequest> RepoRequestPtr;
+using RepoRequestPtr = SGSharedPtr<HTTPRepoGetRequest>;
 
 class HTTPRepoPrivate {
 public:
-  struct HashCacheEntry {
-    std::string filePath;
-    time_t modTime;
-    size_t lengthBytes;
-    std::string hashHex;
-  };
 
-  typedef std::vector<HashCacheEntry> HashCache;
-  HashCache hashes;
-  int hashCacheDirty = 0;
 
   HTTPRepository::FailureVec failures;
   int maxPermittedFailures = 16;
@@ -89,18 +83,14 @@ public:
   HTTP::Request_ptr updateDir(HTTPDirectory *dir, const std::string &hash,
                               size_t sz);
 
-  std::string hashForPath(const SGPath &p);
-  void updatedFileContents(const SGPath &p, const std::string &newHash);
-  void parseHashCache();
-  std::string computeHashForPath(const SGPath &p);
-  void writeHashCache();
-
   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;
 
@@ -113,10 +103,24 @@ public:
   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/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/test_repository.cxx

@@ -409,6 +409,7 @@ void waitForUpdateComplete(HTTP::Client* cl, HTTPRepository* repo)
         cl->update();
         testServer.poll();
 
+        repo->process();
         if (!repo->isDoingSync()) {
             return;
         }
@@ -423,6 +424,7 @@ void runForTime(HTTP::Client *cl, HTTPRepository *repo, int msec = 15) {
   while (start.elapsedMSec() < msec) {
     cl->update();
     testServer.poll();
+    repo->process();
     SGTimeStamp::sleepForMSec(1);
   }
 }

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/package/Catalog.cxx

@@ -643,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());
+      // don't auto-re-add Catalogs the user has explicilty rmeoved, that would
-        // new catalog being added
+      // suck
-        createFromUrl(root(), altUrl);
+      const auto removedByUser = root()->explicitlyRemovedCatalogs();
-
+      auto it = std::find(removedByUser.begin(), removedByUser.end(), altId);
-        // and we can go idle now
+      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);
@@ -661,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

@@ -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 )

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;
 
     /**

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,12 +722,6 @@ 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 (aCat->isUserEnabled() &&
         (aReason == Delegate::STATUS_REFRESHED) && 
         (catIt == d->catalogs.end())) 
@@ -723,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();
@@ -783,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();
 
@@ -854,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/scene/tsync/terrasync.cxx

@@ -334,6 +334,10 @@ public:
     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
@@ -547,6 +551,7 @@ 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) {
@@ -588,47 +593,14 @@ void SGTerraSync::WorkerThread::updateSyncSlot(SyncSlot &slot)
         SGPath path(_local_dir);
         path.append(slot.currentItem._dir);
         slot.isNewDirectory = !path.exists();
-        if (slot.isNewDirectory) {
+        const auto type = slot.currentItem._type;
-            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
-
-        // optimise initial Airport download
-        if (slot.isNewDirectory &&
-            (slot.currentItem._type == SyncItem::AirportData)) {
-          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);
 
+        if (type == SyncItem::AirportData) {
+            beginSyncAirports(slot);
+        } else if (type == SyncItem::Tile) {
+            beginSyncTile(slot);
         } else {
-          slot.repository.reset(new HTTPRepository(path, &_http));
+            beginNormalSync(slot);
-          slot.repository->setBaseUrl(_httpServer + "/" +
-                                      slot.currentItem._dir);
-        }
-
-        if (_installRoot.exists()) {
-            SGPath p = _installRoot;
-            p.append(slot.currentItem._dir);
-            slot.repository->setInstalledCopyPath(p);
         }
 
         try {
@@ -651,6 +623,96 @@ 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()
 {
     while (!_stop) {

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

@@ -813,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) {
+    if (!data) {
-        throw sg_io_exception("Failed to load wav file", sg_location(samplepath));
+        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/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