sysrepo / sysrepo-cpp

C++ bindings for the sysrepo library
BSD 3-Clause "New" or "Revised" License
7 stars 6 forks source link

Clarification: Using wrapUnmanagedSession in sr_plugin_init_cb called by sysrepo plugin deamon #22

Closed rbu9fe closed 8 months ago

rbu9fe commented 8 months ago

I'm running sysrepo plugins which are being initialized by the sysrepo plugin daemon calling sr_plugin_init_cb(sr_session_ctx_t *running_session, void **private_data). Here I found an issue where wrapUnmanagedSession has been exported as a public API so that it can be used to create a sysrepo object from the incoming running_session. However, this effectively takes over the ownership of running_session meaning that the underlying session will be destroyed in the object's destructor so that any other plugin, that's running in parallel, may crash afterwards since the sysrepo plugin daemon calls sr_plugin_init_cb of all plugins with the very same session. Is my understanding correct?

jktjkt commented 8 months ago

Nope, the session is considered "unmanaged", which means that the C++ wrappers won't manage the lifetime of that object. You're supposed to somehow guarantee that the session doesn't get destroyed from outside of the C++ bindings as long as there are no C++ objects which reference that session. Also, make sure that someone closes the session once you've ceased using the C++ class instance.

However, this is "tricky". For example, any libyang::DataNode instances that are derived from this unmanaged session must not outlive the actual C-level sysrepo session. If you use the native C++ approach, this will be guaranteed, but if you (for example) want to store a libyang::DataNode after some out-of-band C code has closed the unmanaged session, this will crash.

To bring this into perspective with the C plugin API, the problem here (as I understand it) is that "something" which is outside of the C++ bindings will decide that "now it's the time to close the session". The C++ code cannot do anything with that, and there's no mechanism in place to invalidate all the data structures which reference data obtained through the session (and that's by design).

TL;DR: if you make sure that you never access anything after sr_plugin_cleanup_cb is called, you'll be fine. Just remember that the C++ bindings hide a lot of there relations, so this "never access stuff" is a transitive requirement which also applies to libyang data.

rbu9fe commented 8 months ago

Got it, thanks a lot for the clarification!