Bug#746475: flightgear: crash with Nasal bindings

[Florent Rougon]

Package: flightgear
Version: 3.0.0-1
Severity: normal
Tags: fixed-upstream


Origin: upstream, https://gitorious.org/fg/flightgear/commit/b3c7cb7c151858ef79f9371a29be49915e5d3803
Bug: https://code.google.com/p/flightgear-bugs/issues/detail?id=1397
Author: Nicholas Scheel
Date: Tue Apr 15 14:13:46 2014 +0100
Description: Fix crash with Nasal bindings.
 naBindFunction doesn't save the function code to the global
 hash, so pass an explicit context to various 'call' overloads so
 the function can't be GC-ed in between parsing and calling.

--- a/src/Scripting/NasalSys.cxx
+++ b/src/Scripting/NasalSys.cxx
@@ -235,6 +235,11 @@
   return callMethod(code, naNil(), argc, args, locals);
 }
 
+naRef FGNasalSys::callWithContext(naContext ctx, naRef code, int argc, naRef* args, naRef locals)
+{
+  return callMethodWithContext(ctx, code, naNil(), argc, args, locals);
+}
+
 // Does a naCall() in a new context.  Wrapped here to make lock
 // tracking easier.  Extension functions are called with the lock, but
 // we have to release it before making a new naCall().  So rather than
@@ -247,6 +252,11 @@
   return naCallMethod(code, self, argc, args, locals);
 }
 
+naRef FGNasalSys::callMethodWithContext(naContext ctx, naRef code, naRef self, int argc, naRef* args, naRef locals)
+{
+  return naCallMethodCtx(ctx, code, self, argc, args, locals);
+}
+
 FGNasalSys::~FGNasalSys()
 {
     nasalSys = 0;
@@ -254,11 +264,15 @@
 
 bool FGNasalSys::parseAndRun(const char* sourceCode)
 {
-    naRef code = parse("FGNasalSys::parseAndRun()", sourceCode,
+    naContext ctx = naNewContext();
+    naRef code = parse(ctx, "FGNasalSys::parseAndRun()", sourceCode,
                        strlen(sourceCode));
-    if(naIsNil(code))
+    if(naIsNil(code)) {
+        naFreeContext(ctx);
         return false;
-    call(code, 0, 0, naNil());
+    }
+    callWithContext(ctx, code, 0, 0, naNil());
+    naFreeContext(ctx);
     return true;
 }
 
@@ -1067,11 +1081,13 @@
                               const SGPropertyNode* cmdarg,
                               int argc, naRef* args)
 {
-    naRef code = parse(fileName, src, len);
-    if(naIsNil(code))
+    naContext ctx = naNewContext();
+    naRef code = parse(ctx, fileName, src, len);
+    if(naIsNil(code)) {
+        naFreeContext(ctx);
         return false;
+    }
 
-    naContext ctx = naNewContext();
     
     // See if we already have a module hash to use.  This allows the
     // user to, for example, add functions to the built-in math
@@ -1084,7 +1100,7 @@
 
     _cmdArg = (SGPropertyNode*)cmdarg;
 
-    call(code, argc, args, locals);
+    callWithContext(ctx, code, argc, args, locals);
     hashset(_globals, moduleName, locals);
     
     naFreeContext(ctx);
@@ -1100,10 +1116,9 @@
     naFreeContext(ctx);
 }
 
-naRef FGNasalSys::parse(const char* filename, const char* buf, int len)
+naRef FGNasalSys::parse(naContext ctx, const char* filename, const char* buf, int len)
 {
     int errLine = -1;
-    naContext ctx = naNewContext();
     naRef srcfile = naNewString(ctx);
     naStr_fromdata(srcfile, (char*)filename, strlen(filename));
     naRef code = naParseCode(ctx, srcfile, 1, (char*)buf, len, &errLine);
@@ -1111,14 +1126,11 @@
         SG_LOG(SG_NASAL, SG_ALERT,
                "Nasal parse error: " << naGetError(ctx) <<
                " in "<< filename <<", line " << errLine);
-        naFreeContext(ctx);
         return naNil();
     }
 
     // Bind to the global namespace before returning
-    naRef bound = naBindFunction(ctx, code, _globals);
-    naFreeContext(ctx);
-    return bound;
+    return naBindFunction(ctx, code, _globals);
 }
 
 bool FGNasalSys::handleCommand( const char* moduleName,
@@ -1126,22 +1138,24 @@
                                 const char* src,
                                 const SGPropertyNode* arg )
 {
-    naRef code = parse(fileName, src, strlen(src));
-    if(naIsNil(code)) return false;
+    naContext ctx = naNewContext();
+    naRef code = parse(ctx, fileName, src, strlen(src));
+    if(naIsNil(code)) {
+        naFreeContext(ctx);
+        return false;
+    }
 
     // Commands can be run "in" a module.  Make sure that module
     // exists, and set it up as the local variables hash for the
     // command.
     naRef locals = naNil();
     if(moduleName[0]) {
-        naContext ctx = naNewContext();
         naRef modname = naNewString(ctx);
         naStr_fromdata(modname, (char*)moduleName, strlen(moduleName));
         if(!naHash_get(_globals, modname, &locals)) {
             locals = naNewHash(ctx);
             naHash_set(_globals, modname, locals);
         }
-        naFreeContext(ctx);
     }
 
     // Cache this command's argument for inspection via cmdarg().  For
@@ -1149,7 +1163,8 @@
     // code doesn't need it.
     _cmdArg = (SGPropertyNode*)arg;
 
-    call(code, 0, 0, locals);
+    callWithContext(ctx, code, 0, 0, locals);
+    naFreeContext(ctx);
     return true;
 }
 
--- a/src/Scripting/NasalSys.hxx
+++ b/src/Scripting/NasalSys.hxx
@@ -102,8 +102,10 @@
     void globalsSet(const char* key, naRef val);
 
     naRef call(naRef code, int argc, naRef* args, naRef locals);
+    naRef callWithContext(naContext ctx, naRef code, int argc, naRef* args, naRef locals);
   
     naRef callMethod(naRef code, naRef self, int argc, naRef* args, naRef locals);
+    naRef callMethodWithContext(naContext ctx, naRef code, naRef self, int argc, naRef* args, naRef locals);
   
     naRef propNodeGhost(SGPropertyNode* handle);
   
@@ -169,7 +171,7 @@
     void loadScriptDirectory(simgear::Dir nasalDir);
     void addModule(std::string moduleName, simgear::PathList scripts);
     static void logError(naContext);
-    naRef parse(const char* filename, const char* buf, int len);
+    naRef parse(naContext ctx, const char* filename, const char* buf, int len);
     naRef genPropsModule();
 
     naContext _context;

-- 
Florent