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.

simgear-version

@@ -1 +1 @@
-2020.3.1
+2020.3.3

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,22 +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++) {
-          auto rawTxt = reinterpret_cast<char*>(result->dnstxt_txt[i].txt);
+                //TODO: interprete the .len field of dnstxt_txt?
-          if (!rawTxt) {
+                auto rawTxt = reinterpret_cast<char*>(result->dnstxt_txt[i].txt);
-              continue;
+                if (!rawTxt) {
-          }
+                    continue;
+                }
 
-          const string txt{rawTxt};
+                const string txt{rawTxt};
-          r->entries.push_back(txt);
+                r->entries.push_back(txt);
-          string_list tokens = simgear::strutils::split( txt, "=", 1 );
+                string_list tokens = simgear::strutils::split(txt, "=", 1);
-          if( tokens.size() == 2 ) {
+                if (tokens.size() == 2) {
-            r->attributes[tokens[0]] = tokens[1];
+                    r->attributes[tokens[0]] = tokens[1];
-          }
+                }
+            }
         }
         free(result);
     }
@@ -176,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;
 }
 
 
@@ -195,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();
@@ -223,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;
 }
 
 
@@ -242,6 +266,7 @@ Client::Client() :
 
 void Client::makeRequest(const Request_ptr& r)
 {
+    d->_activeRequests.push_back(r);
     r->submit(this);
 }
 
@@ -252,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

@@ -221,42 +221,48 @@ public:
 		char* buf = nullptr;
 		size_t bufSize = 0;
 
-		for (const auto& child : children) {
+                for (auto &child : children) {
-            if (child.type != HTTPRepository::FileType)
+                  if (child.type != HTTPRepository::FileType)
-                continue;
+                    continue;
 
-            if (child.path.exists())
+                  if (child.path.exists())
-                continue;
+                    continue;
 
-            SGPath cp = _repository->installedCopyPath;
+                  SGPath cp = _repository->installedCopyPath;
-            cp.append(relativePath());
+                  cp.append(relativePath());
-            cp.append(child.name);
+                  cp.append(child.name);
-            if (!cp.exists()) {
+                  if (!cp.exists()) {
-                continue;
+                    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()

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/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/Root.cxx

@@ -722,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())) 
@@ -761,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();

simgear/scene/tsync/terrasync.cxx

@@ -603,12 +603,6 @@ void SGTerraSync::WorkerThread::updateSyncSlot(SyncSlot &slot)
             beginNormalSync(slot);
         }
 
-        if (_installRoot.exists()) {
-            SGPath p = _installRoot;
-            p.append(slot.currentItem._dir);
-            slot.repository->setInstalledCopyPath(p);
-        }
-
         try {
             slot.repository->update();
         } catch (sg_exception& e) {
@@ -676,6 +670,11 @@ void SGTerraSync::WorkerThread::beginSyncTile(SyncSlot& slot)
     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
@@ -706,6 +705,12 @@ void SGTerraSync::WorkerThread::beginNormalSync(SyncSlot& slot)
     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()