Re: [Flightgear-devel] Segmentation fault acquiring SGPropertyLock

[Julian S. <ju...@op...>]

On Mon, 17 Jul 2023 21:25:22 +0100
Julian Smith <ju...@op...> wrote:

> On Mon, 17 Jul 2023 21:54:24 +0200
> Florent Rougon <f.r...@fr...> wrote:

> > #9  0x000055555642e306 in SGPropertyNode::fireValueChanged() (this=0x0)
> >     at .../simgear/simgear/props/props.cxx:3805  
> 
> `this=0x0` is the bug i think.
> 
> And it comes from:
> 
> > #10 0x00005555560c9082 in FGProperties::setFreeze(bool)
> >     (this=0x55555a19c0f0, f=true)
> >     at .../flightgear/src/Main/fg_props.cxx:234  
> 
> which is doing:
> 
> void FGProperties::setFreeze(bool f)
> {
>   if (_simFreeze == f)
>       return;
> 
>   _simFreeze = f;
> 
>   // Pause the particle system
>   auto p = simgear::ParticlesGlobalManager::instance();
>   p->setFrozen(f);
> 
>   _simFreezeNode->fireValueChanged();
> }
> 
> So the global `simFreezeNode` is null. I guess this is because the
> command-line option `--enable-freeze` is trying to freeze things too
> early, before the FGProperties subsystem has been initialised?

This diff seems to fix things for me:

diff --git a/src/Main/fg_props.cxx b/src/Main/fg_props.cxx
index 84e662322..b8701da56 100644
--- a/src/Main/fg_props.cxx
+++ b/src/Main/fg_props.cxx
@@ -418,7 +418,8 @@ FGProperties::bind ()
   // Simulation
   _tiedProperties.Tie("/sim/logging/priority", getLoggingPriority, setLoggingPriority);
   _tiedProperties.Tie("/sim/logging/classes", getLoggingClasses, setLoggingClasses);
-  _simFreezeNode = _tiedProperties.Tie("/sim/freeze/master", this, &FGProperties::getFreeze, &FGProperties::setFreeze);
+  _simFreezeNode = fgGetNode("/sim/freeze/master", true);
+  _tiedProperties.Tie(_simFreezeNode, this, &FGProperties::getFreeze, &FGProperties::setFreeze);
   _simFreezeNode->setAttribute(SGPropertyNode::LISTENER_SAFE, true);
 
   _tiedProperties.Tie<double>("/sim/time/elapsed-sec", getElapsedTime_sec);


The problem was that tiedProperties.Tie() was calling
FGProperties::setFreeze(true) before it returned, but we were only setting
_simFreezeNode *after* tiedProperties.Tie() had returned.

tiedProperties.Tie() simply returns the node that it was given as its
first arg, so the diff simply sets _simFreezeNode to the value it would
have taken later, before calling tiedProperties.Tie().

Please note that it's entirely possible that i've missed something
subtle here. I haven't considered what's going on in any great detail,
and i've never used tied properties myself.


- Jules

-- 
http://op59.net