Reset: ensure Nasal shutdown is clean

Some improvements / bullet-proofing while trying to track down the
slow-on-reset issue.

src/Scripting/NasalSys.cxx

@@ -90,7 +90,6 @@ void FGNasalModuleListener::valueChanged(SGPropertyNode*)
 
 //////////////////////////////////////////////////////////////////////////
 
-
 class TimerObj : public SGReferenced
 {
 public:
@@ -103,15 +102,17 @@ public:
     char nm[128];
     snprintf(nm, 128, "nasal-timer-%p", this);
     _name = nm;
-    _gcRoot =  sys->gcSave(f);
+    _gcRoot =  naGCSave(f);
-    _gcSelf = sys->gcSave(self);
+    _gcSelf = naGCSave(self);
+    sys->addPersistentTimer(this);
   }
 
   virtual ~TimerObj()
   {
     stop();
-    _sys->gcRelease(_gcRoot);
+    naGCRelease(_gcRoot);
-    _sys->gcRelease(_gcSelf);
+    naGCRelease(_gcSelf);
+      _sys->removePersistentTimer(this);
   }
 
   bool isRunning() const { return _isRunning; }
@@ -168,7 +169,7 @@ public:
       // event manager).
       _isRunning = false;
 
-    naRef *args = NULL;
+    naRef *args = nullptr;
     _sys->callMethod(_func, _self, 0, args, naNil() /* locals */);
   }
 
@@ -966,22 +967,24 @@ void FGNasalSys::shutdown()
 
     shutdownNasalPositioned();
 
-    map<int, FGNasalListener *>::iterator it, end = _listener.end();
+    for (auto l : _listener)
-    for(it = _listener.begin(); it != end; ++it)
+        delete l.second;
-        delete it->second;
     _listener.clear();
 
-    NasalCommandDict::iterator j = _commands.begin();
+    for (auto c : _commands) {
-    for (; j != _commands.end(); ++j) {
+        globals->get_commands()->removeCommand(c.first);
-        globals->get_commands()->removeCommand(j->first);
     }
     _commands.clear();
 
-    std::vector<FGNasalModuleListener*>::iterator k = _moduleListeners.begin();
+    for(auto ml : _moduleListeners)
-    for(; k!= _moduleListeners.end(); ++k)
+        delete ml;
-        delete *k;
     _moduleListeners.clear();
 
+    for (auto t : _nasalTimers) {
+        delete t;
+    }
+    _nasalTimers.clear();
+    
     naClearSaved();
 
     _string = naNil(); // will be freed by _context
@@ -994,6 +997,15 @@ void FGNasalSys::shutdown()
     _globals = naNil();
 
     naGC();
+    
+    // Destroy all queued ghosts : important to ensure persistent timers are
+    // destroyed now.
+    nasal::ghostProcessDestroyList();
+    
+    if (!_persistentTimers.empty()) {
+        SG_LOG(SG_NASAL, SG_DEV_WARN, "Extant persistent timer count:" << _persistentTimers.size());
+    }
+    
     _inited = false;
 }
 
@@ -1374,11 +1386,8 @@ void FGNasalSys::setTimer(naContext c, int argc, naRef* args)
     name.append(std::to_string(naGetLine(c, 0)));
 
     // Generate and register a C++ timer handler
-    NasalTimer* t = new NasalTimer;
+    NasalTimer* t = new NasalTimer(handler, this);
-    t->handler = handler;
+    _nasalTimers.push_back(t);
-    t->gcKey = gcSave(handler);
-    t->nasal = this;
-
     globals->get_event_mgr()->addEvent(name,
                                        t, &NasalTimer::timerExpired,
                                        delta.num, simtime);
@@ -1387,7 +1396,10 @@ void FGNasalSys::setTimer(naContext c, int argc, naRef* args)
 void FGNasalSys::handleTimer(NasalTimer* t)
 {
     call(t->handler, 0, 0, naNil());
-    gcRelease(t->gcKey);
+    auto it =  std::find(_nasalTimers.begin(), _nasalTimers.end(), t);
+    assert(it != _nasalTimers.end());
+    _nasalTimers.erase(it);
+    delete t;
 }
 
 int FGNasalSys::gcSave(naRef r)
@@ -1402,12 +1414,27 @@ void FGNasalSys::gcRelease(int key)
 
 
 //------------------------------------------------------------------------------
-void FGNasalSys::NasalTimer::timerExpired()
+
+NasalTimer::NasalTimer(naRef h, FGNasalSys* sys) :
+    handler(h), nasal(sys)
+{
+    assert(sys);
+    gcKey = naGCSave(handler);
+}
+
+NasalTimer::~NasalTimer()
+{
+    naGCRelease(gcKey);
+}
+
+void NasalTimer::timerExpired()
 {
     nasal->handleTimer(this);
-    delete this;
+    // note handleTimer calls delete on us, don't do anything
+    // which requires 'this' to be valid here
 }
 
+
 int FGNasalSys::_listenerId = 0;
 
 // setlistener(<property>, <func> [, <initial=0> [, <persistent=1>]])
@@ -1506,6 +1533,18 @@ void FGNasalSys::removeCommand(const std::string& name)
     _commands.erase(it);
 }
 
+void FGNasalSys::addPersistentTimer(TimerObj* pto)
+{
+    _persistentTimers.push_back(pto);
+}
+
+void FGNasalSys::removePersistentTimer(TimerObj* obj)
+{
+    auto it = std::find(_persistentTimers.begin(), _persistentTimers.end(), obj);
+    assert(it != _persistentTimers.end());
+    _persistentTimers.erase(it);
+}
+
 //////////////////////////////////////////////////////////////////////////
 // FGNasalListener class.
 

src/Scripting/NasalSys.hxx

@@ -25,6 +25,8 @@ class SGCondition;
 class FGNasalModelData;
 class NasalCommand;
 class FGNasalModuleListener;
+struct NasalTimer;  ///< timer created by settimer
+class TimerObj;     ///< persistent timer created by maketimer
 
 namespace simgear { class BufferedLogCallback; }
 
@@ -35,10 +37,9 @@ class FGNasalSys : public SGSubsystem
 public:
     FGNasalSys();
     virtual ~FGNasalSys();
-    virtual void init();
+    void init() override;
-    virtual void shutdown();
+    void shutdown() override;
-    
+    void update(double dt) override;
-    virtual void update(double dt);
 
     // Loads a nasal script from an external file and inserts it as a
     // global module of the specified name.
@@ -167,19 +168,6 @@ private:
     // callback).
     bool _delay_load;
 
-    //
-    // FGTimer subclass for handling Nasal timer callbacks.
-    // See the implementation of the settimer() extension function for
-    // more notes.
-    //
-    struct NasalTimer {
-        virtual void timerExpired();
-        virtual ~NasalTimer() {}
-        naRef handler;
-        int gcKey;
-        FGNasalSys* nasal;
-    };
-
     // Listener
     std::map<int, FGNasalListener *> _listener;
     std::vector<FGNasalListener *> _dead_listener;
@@ -210,8 +198,25 @@ private:
     NasalCommandDict _commands;
     
     naRef _wrappedNodeFunc;
-public:
+    
+    // track NasalTimer instances (created via settimer() call) -
+    // this allows us to clean these up on shutdown
+    std::vector<NasalTimer*> _nasalTimers;
+
+    // NasalTimer is a friend to invoke handleTimer and do the actual
+    // dispatch of the settimer-d callback
+    friend NasalTimer;
+    
     void handleTimer(NasalTimer* t);
+    
+    // track persistent timers. These are owned from the Nasal side, so we
+    // only track a non-owning reference here.
+    std::vector<TimerObj*> _persistentTimers;
+    
+    friend TimerObj;
+    
+    void addPersistentTimer(TimerObj* pto);
+    void removePersistentTimer(TimerObj* obj);
 };
 
 #if 0

src/Scripting/NasalSys_private.hxx

@@ -68,4 +68,20 @@ private:
     naRef _start_element, _end_element, _data, _pi;
 };
 
+//
+// See the implementation of the settimer() extension function for
+// more notes.
+//
+struct NasalTimer
+{
+    NasalTimer(naRef handler, FGNasalSys* sys);
+    
+    void timerExpired();
+    ~NasalTimer();
+    
+    naRef handler;
+    int gcKey = 0;
+    FGNasalSys* nasal = nullptr;
+};
+
 #endif // of __NASALSYS_PRIVATE_HXX