A long, long time ago, a bug was inadvertently introduced into the threaded

metar fetcher.  Effectively this caused the metar thread and the main
thread to both attempt to fetch weather data.  This could lead to long pauses
when the main thread decided to fetch the weather, and introduced a race
condition that could cause a segfault/crash.

Investigating this issue, I discovered that even longer ago, someone confused
#defines and #ifdef symbols with C/C++ variables.  If I #define XYZ 0 it is
defined so #ifdef XYZ is true, not false like a variable.  Our thread
detection made this mistake and there were follow up patches to work around
it.

So I fixed the configure script (ahhh, reading the autoconf manual is highly
recommended excercise for people editing the configure.ac file.)  I also
discovered that we were hardwiring with_threads=yes with no way via configure
options to disable threads from the build so I fixed that.

Then I patched up the #ifdef's scattered through the code to match the
configure script changes, oh and by the way, I stumbled upon a past typo
that led to the race condition in the metar fetching thread and fixed that.

diff --git a/src/Environment/environment_ctrl.cxx b/src/Environment/environment_ctrl.cxx
index a947c48a901a963a625da79787ed98fe42e79245..87b79544aa9245e98c86652b810fd5e7565a68cb 100644 (file)
--- a/src/Environment/environment_ctrl.cxx
+++ b/src/Environment/environment_ctrl.cxx
@@ -330,7 +330,7 @@ FGMetarEnvironmentCtrl::FGMetarEnvironmentCtrl ()
       _dt( 0.0 ),
       _error_dt( 0.0 )
 {
-#if defined(ENABLE_THREADS) && ENABLE_THREADS
+#if defined(ENABLE_THREADS)
     thread = new MetarThread(this);
     thread->start( 1 );
 #endif // ENABLE_THREADS
@@ -338,7 +338,7 @@ FGMetarEnvironmentCtrl::FGMetarEnvironmentCtrl ()
 
 FGMetarEnvironmentCtrl::~FGMetarEnvironmentCtrl ()
 {
-#if defined(ENABLE_THREADS) && ENABLE_THREADS
+#if defined(ENABLE_THREADS)
    thread->cancel();
    thread->join();
 #endif // ENABLE_THREADS
@@ -491,7 +491,7 @@ FGMetarEnvironmentCtrl::update(double delta_time_sec)
         }
     }
 
-#if defined(ENABLE_THREADS) && ENABLE_THREADS
+#if !defined(ENABLE_THREADS)
     // No loader thread running so manually fetch the data
     string id = "";
     while ( !request_queue.empty() ) {
@@ -582,7 +582,7 @@ FGMetarEnvironmentCtrl::fetch_data( const string &icao )
     } catch (const sg_io_exception& e) {
         SG_LOG( SG_GENERAL, SG_WARN, "Error fetching live weather data: "
                 << e.getFormattedMessage().c_str() );
-#if defined(ENABLE_THREADS) && ENABLE_THREADS
+#if defined(ENABLE_THREADS)
         if (_error_count++ >= 3) {
            SG_LOG( SG_GENERAL, SG_WARN, "Stop fetching data permanently.");
            thread->cancel();
@@ -701,7 +701,7 @@ FGMetarEnvironmentCtrl::update_metar_properties( const FGMetar *m )
 }
 
 
-#if defined(ENABLE_THREADS) && ENABLE_THREADS
+#if defined(ENABLE_THREADS)
 /**
  *
  */

diff --git a/src/Environment/environment_ctrl.hxx b/src/Environment/environment_ctrl.hxx
index be009962e8e5163e2125b1b31d3a011c1bbb27ae..c38e033fcb317f9678ca3a3bec8bdb5887e68625 100644 (file)
--- a/src/Environment/environment_ctrl.hxx
+++ b/src/Environment/environment_ctrl.hxx
@@ -27,7 +27,7 @@
 #include <simgear/structure/subsystem_mgr.hxx>
 #include <simgear/environment/metar.hxx>
 
-#if defined(ENABLE_THREADS) && ENABLE_THREADS
+#if defined(ENABLE_THREADS)
 # include <simgear/threads/SGThread.hxx>
 # include <simgear/threads/SGQueue.hxx>
 #endif
@@ -185,7 +185,7 @@ private:
 
 private:
 
-#if defined(ENABLE_THREADS) && ENABLE_THREADS
+#if defined(ENABLE_THREADS)
     /**
      * FIFO queue which holds a pointer to the fetched metar data.
      */
@@ -207,7 +207,7 @@ private:
     queue < FGMetarResult > result_queue;
 #endif
 
-#if defined(ENABLE_THREADS) && ENABLE_THREADS
+#if defined(ENABLE_THREADS)
     /**
      * This class represents the thread of execution responsible for
      * fetching the metar data.

diff --git a/src/Scenery/FGTileLoader.cxx b/src/Scenery/FGTileLoader.cxx
index 03cb493d56e89ac88c18e0509d27a0e0fea5a69b..ca61e8294943acfd1343ddf956654364d751f214 100644 (file)
--- a/src/Scenery/FGTileLoader.cxx
+++ b/src/Scenery/FGTileLoader.cxx
@@ -41,7 +41,7 @@ extern ssgBranch *ground;
  */
 FGTileLoader::FGTileLoader()
 {
-#if defined(ENABLE_THREADS) && ENABLE_THREADS
+#if defined(ENABLE_THREADS)
     // Create and start the loader threads.
     for (int i = 0; i < MAX_THREADS; ++i)
     {
@@ -56,7 +56,7 @@ FGTileLoader::FGTileLoader()
  */
 FGTileLoader::~FGTileLoader()
 {
-#if defined(ENABLE_THREADS) && ENABLE_THREADS
+#if defined(ENABLE_THREADS)
     // Wake up its time to die.
     // queue_cond.broadcast();
 
@@ -121,7 +121,7 @@ void
 FGTileLoader::update()
 {
 
-#if defined(ENABLE_THREADS) && ENABLE_THREADS
+#if defined(ENABLE_THREADS)
     // send a signal to the pager thread that it is allowed to load
     // another tile
     mutex.lock();
@@ -155,7 +155,7 @@ FGTileLoader::update()
 }
 
 
-#if defined(ENABLE_THREADS) && ENABLE_THREADS
+#if defined(ENABLE_THREADS)
 /**
  * 
  */

diff --git a/src/Scenery/FGTileLoader.hxx b/src/Scenery/FGTileLoader.hxx
index 3d912fa753ce73715a4bafd50534c3dba442371c..ef32ec4a07f492868ef6b589c5010c12b157b086 100644 (file)
--- a/src/Scenery/FGTileLoader.hxx
+++ b/src/Scenery/FGTileLoader.hxx
@@ -27,7 +27,7 @@
 #include <simgear/bucket/newbucket.hxx>
 #include <simgear/misc/sg_path.hxx>
 
-#if defined(ENABLE_THREADS) && ENABLE_THREADS
+#if defined(ENABLE_THREADS)
 #  include <simgear/threads/SGThread.hxx>
 #  include <simgear/threads/SGQueue.hxx>
 #else
@@ -97,7 +97,7 @@ private:
 
 private:
 
-#if defined(ENABLE_THREADS) && ENABLE_THREADS
+#if defined(ENABLE_THREADS)
     /**
      * FIFO queue of tiles to load from data files.
      */
@@ -113,7 +113,7 @@ private:
      */
     string_list tile_path;
 
-#if defined(ENABLE_THREADS) && ENABLE_THREADS
+#if defined(ENABLE_THREADS)
     /**
      * Maximum number of threads to create for loading tiles.
      */

diff --git a/src/Scenery/tilemgr.cxx b/src/Scenery/tilemgr.cxx
index af7195b44cb62aa23fff879a769a6541ac3ac9ef..353be612d132d43f5bba91ed3f27df334b386015 100644 (file)
--- a/src/Scenery/tilemgr.cxx
+++ b/src/Scenery/tilemgr.cxx
@@ -47,7 +47,7 @@
 
 #define TEST_LAST_HIT_CACHE
 
-#if defined(ENABLE_THREADS) && ENABLE_THREADS
+#if defined(ENABLE_THREADS)
 SGLockedQueue<FGTileEntry *> FGTileMgr::attach_queue;
 SGLockedQueue<FGDeferredModel *> FGTileMgr::model_queue;
 #else
@@ -92,7 +92,7 @@ int FGTileMgr::init() {
     }
 
     while ( ! model_queue.empty() ) {
-#if defined(ENABLE_THREADS) && ENABLE_THREADS
+#if defined(ENABLE_THREADS)
         FGDeferredModel* dm = model_queue.pop();
 #else
         FGDeferredModel* dm = model_queue.front();
@@ -299,7 +299,7 @@ void FGTileMgr::update_queues()
 
             // cout << "loading next model ..." << endl;
             // load the next tile in the queue
-#if defined(ENABLE_THREADS) && ENABLE_THREADS
+#if defined(ENABLE_THREADS)
             FGDeferredModel* dm = model_queue.pop();
 #else
             FGDeferredModel* dm = model_queue.front();
@@ -337,7 +337,7 @@ void FGTileMgr::update_queues()
     loader.update();
 
     if ( !attach_queue.empty() ) {
-#if defined(ENABLE_THREADS) && ENABLE_THREADS
+#if defined(ENABLE_THREADS)
         FGTileEntry* e = attach_queue.pop();
 #else
         FGTileEntry* e = attach_queue.front();
@@ -364,7 +364,7 @@ void FGTileMgr::update_queues()
                 // get real serious and agressively free up some tiles so
                 // we don't explode our memory usage.
 
-                SG_LOG( SG_TERRAIN, SG_WARN,
+                SG_LOG( SG_TERRAIN, SG_ALERT,
                         "Warning: catching up on tile delete queue" );
             }
 

diff --git a/src/Scenery/tilemgr.hxx b/src/Scenery/tilemgr.hxx
index 18af47d9422dc39d5a56cf5ababab07ce14bc830..9d41d788c72bb7915d7acfad03dffc8734a35e4a 100644 (file)
--- a/src/Scenery/tilemgr.hxx
+++ b/src/Scenery/tilemgr.hxx
@@ -36,7 +36,7 @@
 #include <queue>
 
 #include <simgear/bucket/newbucket.hxx>
-#if defined(ENABLE_THREADS) && ENABLE_THREADS
+#if defined(ENABLE_THREADS)
 #  include <simgear/threads/SGQueue.hxx>
 #endif // ENABLE_THREADS
 
@@ -118,7 +118,7 @@ private:
      * model_queue is the set of models that need to be loaded by the
      * primary render thread.
      */
-#if defined(ENABLE_THREADS) && ENABLE_THREADS
+#if defined(ENABLE_THREADS)
     static SGLockedQueue<FGTileEntry *> attach_queue;
     static SGLockedQueue<FGDeferredModel *> model_queue;
 #else