Gio::DBus::InterfaceVTable memory leak in register_object() of stub

[martin-ejdestig]

At least it looks like it to me:

guint {{ class_name_with_namespace }}::register_object(...)
{
    ...
    Gio::DBus::InterfaceVTable *interface_vtable =
        new Gio::DBus::InterfaceVTable(
            sigc::mem_fun(this, &{{ interface.cpp_class_name }}::on_method_call),
            sigc::mem_fun(this, &{{ interface.cpp_class_name }}::on_interface_get_property),
            sigc::mem_fun(this, &{{ interface.cpp_class_name }}::on_interface_set_property));
    guint id = 0;
    try {
        id = connection->register_object(object_path,
            introspection_data->lookup_interface("{{ interface.name }}"),
            *interface_vtable);    // NOTE: register_object() takes const InterfaceVTable &
        m_connection = connection;
        m_objectPath = object_path;
    } catch(const Glib::Error &ex) {
        g_warning("Registration of object failed");
    }
    // NOTE: interface_vtable leaked...
    return id;
}

The documentation says:

"The only correct use of this class is to declare a global instance of it (or an instance local to the main function) and pass pointers to the instance to the methods that require such a parameter. The instance can be used for multiple registrations and the memory it uses will be freed at the end of execution. Any other use (like creating an instance local to a function and using that) may cause memory leaks or errors (if the instance is destroyed too early)."

Bad for a long running service if objects are created and removed dynamically and each creation leaks.

Do not see a simple fix though since we cannot have a single instance and use sigc::mem_fun for the slots.

[martin-ejdestig]

If solution is to create a singleton of some sort, introspection_data can probably be stored in it as well. Just noticed that it is recreated for each object (writing down here now to remember :). E.g.

class FooStub::InterfaceInfo
{
    static instance()
    {
        static InterfaceInfo instance;
        return instance;
    }

    ...

    Gio::DBus::InterfaceVTable interface_vtable;
    Glib::RefPtr<Gio::DBus::NodeInfo> introspection_data;
}

Something like that. (Free double checked locking guaranteed since C++11. :)

[martin-ejdestig]

introspection_data can probably be stored in it as well

Though, should verify by reading up on glib/glibmm docs and code in detail for how this is all supposed to work first, of course. :)

[mardy]

Yes, one possibility is to use simple functions (and not class methods) and pass this as a parameter. Here's an example: https://github.com/GNOME/glibmm/blob/master/examples/dbus/server_without_bus.cc

On the other hand, using a singleton would roughly take the same amount of memory (with the negative point that it wouldn't be share across different processes using the same library), so it wouldn't be such a bad idea as well.

Do you see some issues in doing it the way it's done in the example I linked?

[martin-ejdestig]

Hum, I do not see how this would be passed as a parameter to on_method_call for different instances.

Except for not seeing how this is passed, it is probably a bad idea to have it as a global (static initialization order fiasco)... but easily fixed if the this problem is solved. :)

[mardy]

I think you are right... in the C version, there's a user_data parameter which can be passed when registering the object and retrieved from the GDBusMethodInvocation object, but it's missing from the C++ API.

I filed a bug about it, let's see if we get some useful suggestion: https://gitlab.gnome.org/GNOME/glibmm/issues/42

[martin-ejdestig]

An alternative could perhaps be to have an std::unordered_map with object paths as keys and FooStub * as values in something like the FooStub::InterfaceInfo singleton I suggested above. And then proxy calls through static methods in it that looks up the stub instance from the object path received as argument in the slots.

But let's hope there is a better suggestion from Glibmm.