Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Dec 11, 2023

Conversation

bcoconni
Copy link
Member

@bcoconni bcoconni commented Dec 9, 2023

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 !!!

This is to make sure that the C++ instance is not deleted while a Python object is still referencing it.
Copy link

codecov bot commented Dec 9, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

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

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bcoconni
Copy link
Member Author

bcoconni commented Dec 9, 2023

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

@agodemar
Copy link
Contributor

Thanks @bcoconni, well done

@agodemar agodemar merged commit dde0285 into JSBSim-Team:master Dec 11, 2023
@seanmcleod
Copy link
Member

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

@bcoconni
Copy link
Member Author

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 🥳

@bcoconni bcoconni deleted the FGPropertyNode_SGSharedPtr branch December 14, 2023 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants