1
0
Fork 0

#553: MP-model-loading related segfault

MP aircraft are loaded by a separate OSG thread (introduced after FG2.4.0).
The OSG thread also calls the "modelLoaded" callback. However, we mustn't
allow the OSG thread to call FGNasalModelData::modelLoaded directly:
FGNasalModelData isn't thread-safe. There are obvious issues (_callCount++;),
tricky issues like calling the Nasal _parser_ or creating hashes and
modifying the global Nasal namespace. It doesn't use locks to protect
against another thread executing a Nasal context or running garbage
collection. It also executes Nasal code itself (the model's "load" hook),
which we also cannot allow in a separate thread...
This patch returns all Nasal parts of MP-aircraft loading (parsing,
module creation, execution) to the main thread, while keeping the
multi-threaded OSG part (loading of MP-aircraft model files itself).
The same issue exists with scenery models (see other commit).

To summarize with 2 words: It s*cks... ;-)
This commit is contained in:
ThorstenB 2012-01-08 13:28:49 +01:00
parent 277ba10b39
commit 246feef85f
2 changed files with 57 additions and 29 deletions

View file

@ -75,8 +75,7 @@ FGAIBase::FGAIBase(object_type ot, bool enableHot) :
_refID( _newAIModelID() ), _refID( _newAIModelID() ),
_otype(ot), _otype(ot),
_initialized(false), _initialized(false),
_aimodel(0), _modeldata(0),
_fxpath(""),
_fx(0) _fx(0)
{ {
@ -232,13 +231,19 @@ void FGAIBase::update(double dt) {
pitch*speed ); pitch*speed );
_fx->set_velocity( velocity ); _fx->set_velocity( velocity );
} }
else if ((_aimodel)&&(fgGetBool("/sim/sound/aimodels/enabled",false))) else if ((_modeldata)&&(_modeldata->needInitilization()))
{ {
string fxpath = _aimodel->get_sound_path(); // process deferred nasal initialization,
// which must be done in main thread
_modeldata->init();
// sound initialization
if (fgGetBool("/sim/sound/aimodels/enabled",false))
{
string fxpath = _modeldata->get_sound_path();
if (fxpath != "") if (fxpath != "")
{ {
_fxpath = fxpath; props->setStringValue("sim/sound/path", fxpath.c_str());
props->setStringValue("sim/sound/path", _fxpath.c_str());
// initialize the sound configuration // initialize the sound configuration
SGSoundMgr *smgr = globals->get_soundmgr(); SGSoundMgr *smgr = globals->get_soundmgr();
@ -249,6 +254,7 @@ void FGAIBase::update(double dt) {
_fx->init(); _fx->init();
} }
} }
}
} }
/** update LOD properties of the model */ /** update LOD properties of the model */
@ -324,8 +330,8 @@ bool FGAIBase::init(bool search_in_AI_path)
else else
_installed = true; _installed = true;
_aimodel = new FGAIModelData(props); _modeldata = new FGAIModelData(props);
osg::Node * mdl = SGModelLib::loadDeferredModel(f, props, _aimodel); osg::Node * mdl = SGModelLib::loadDeferredModel(f, props, _modeldata);
_model = new osg::LOD; _model = new osg::LOD;
_model->setName("AI-model range animation node"); _model->setName("AI-model range animation node");
@ -912,7 +918,8 @@ int FGAIBase::_newAIModelID() {
FGAIModelData::FGAIModelData(SGPropertyNode *root) FGAIModelData::FGAIModelData(SGPropertyNode *root)
: _nasal( new FGNasalModelData(root) ), : _nasal( new FGNasalModelData(root) ),
_path("") _ready(false),
_initialized(false)
{ {
} }
@ -923,9 +930,21 @@ FGAIModelData::~FGAIModelData()
void FGAIModelData::modelLoaded(const string& path, SGPropertyNode *prop, osg::Node *n) void FGAIModelData::modelLoaded(const string& path, SGPropertyNode *prop, osg::Node *n)
{ {
const char* fxpath = prop->getStringValue("sound/path"); // WARNING: All this is called in a separate OSG thread! Only use thread-safe stuff
if (fxpath) { // here that is fine to be run concurrently with stuff in the main loop!
_path = string(fxpath); if (_ready)
} return;
_nasal->modelLoaded(path, prop, n); _fxpath = _prop->getStringValue("sound/path");
_prop = prop;
_path = path;
_ready = true;
}
// do Nasal initialization (must be called in the main loop)
void FGAIModelData::init()
{
// call FGNasalSys to create context and run hooks (not-thread safe!)
_nasal->modelLoaded(_path, _prop, 0);
_prop = 0;
_initialized = true;
} }

View file

@ -21,7 +21,6 @@
#define _FG_AIBASE_HXX #define _FG_AIBASE_HXX
#include <string> #include <string>
#include <list>
#include <simgear/constants.h> #include <simgear/constants.h>
#include <simgear/math/SGMath.hxx> #include <simgear/math/SGMath.hxx>
@ -38,7 +37,6 @@
using std::string; using std::string;
using std::list;
class SGMaterial; class SGMaterial;
class FGAIManager; class FGAIManager;
@ -230,9 +228,8 @@ private:
bool _initialized; bool _initialized;
osg::ref_ptr<osg::LOD> _model; //The 3D model LOD object osg::ref_ptr<osg::LOD> _model; //The 3D model LOD object
osg::ref_ptr<FGAIModelData> _aimodel; osg::ref_ptr<FGAIModelData> _modeldata;
string _fxpath;
SGSharedPtr<FGFX> _fx; SGSharedPtr<FGFX> _fx;
public: public:
@ -453,12 +450,24 @@ class FGAIModelData : public simgear::SGModelData {
public: public:
FGAIModelData(SGPropertyNode *root = 0); FGAIModelData(SGPropertyNode *root = 0);
~FGAIModelData(); ~FGAIModelData();
/** osg callback, thread-safe */
void modelLoaded(const string& path, SGPropertyNode *prop, osg::Node *n); void modelLoaded(const string& path, SGPropertyNode *prop, osg::Node *n);
inline string& get_sound_path() { return _path; };
/** init hook to be called after model is loaded.
* Not thread-safe. Call from main thread only. */
void init(void);
bool needInitilization(void) { return _ready && !_initialized;}
bool isInitialized(void) { return _initialized;}
inline std::string& get_sound_path() { return _fxpath;}
private: private:
FGNasalModelData *_nasal; FGNasalModelData *_nasal;
string _path; SGPropertyNode_ptr _prop;
std::string _path, _fxpath;
bool _ready;
bool _initialized;
}; };
#endif // _FG_AIBASE_HXX #endif // _FG_AIBASE_HXX