JSBSim-Team / jsbsim

An open source flight dynamics & control software library
GNU Lesser General Public License v2.1
1.22k stars 420 forks source link

Make the Python binding of FGPropertyNode compatible with SGSharedPtr ref counting #1000

Closed bcoconni closed 4 months ago

bcoconni commented 4 months ago

This PR modifies the Python binding of FGPropertyNode so that the reference counting of the SGPropertyNode instance that it points to is updated.

This is to avoid that the C++ instance of FGPropertyNode/SGPropertyNode that the Python object relies on is being deleted by another event. Such a scenario would result in a dangling pointer and eventually to a segfault of the Python interpreter.

In other words, this PR is meant to avoid the scenario below from occurring.

fdm = jsbsim.FGFDMExec()
pm = fdm.get_property_manager()
node = pm.get_node('some/node')

# [...] A lot of code

del fdm

# [...] Some code until the Python garbage collector actually deletes fdm

node.get_double_value()  # BOOOMM !!!
codecov[bot] commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (952ef8d) 24.87% compared to head (d30e99f) 24.87%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1000 +/- ## ======================================= Coverage 24.87% 24.87% ======================================= Files 168 168 Lines 18908 18908 ======================================= Hits 4704 4704 Misses 14204 14204 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

bcoconni commented 4 months ago

For those wondering what the difference is between FGPropertyNode and SGPropertyNode:

agodemar commented 4 months ago

Thanks @bcoconni, well done

seanmcleod commented 4 months ago

@bcoconni @agodemar quite a milestone, the 1000th pull request for the JSBSim repo at Github! :wink:

bcoconni commented 4 months ago

It's actually the 298th-ish PR: the number 1000 is actually a counter that accumulates over all the items in the repo (PR, issues and discussions).

That's quite a feat nonetheless. Congrats to the community 🥳