Github user jeking3 commented on a diff in the pull request: https://github.com/apache/thrift/pull/1200#discussion_r102356814 --- Diff: lib/c_glib/src/thrift/c_glib/protocol/thrift_protocol.c --- @@ -61,15 +61,15 @@ thrift_protocol_set_property (GObject *object, guint property_id, switch (property_id) { case PROP_THRIFT_PROTOCOL_TRANSPORT: - protocol->transport = g_value_get_object (value); --- End diff -- "should" live as long. Why should we impose a requirement that the consuming application, using an object oriented-ish language like glib, be required to ensure the underlying transport outlives the protocol that is consuming it? The copy should not lead to memory issues because the dispose method in this class unreferences and clears the pointer. This means the consuming application does a g_object_new, that's one reference. The protocol consumes it, that's a second reference. If the consuming application were to unref the transport, it would crash. But by using reference counts on objects the way they were intended to be used, a consuming application could quite literally unref everything but the server before serve() and then unref the server when it returns, and there would be no leaks or problems. I don't think it's a good practice to force the consuming application to guarantee the lifetime of pointers that reference each-other when the object library itself provides reference counting and garbage collection. That's why I changed it here and in other classes like ThriftServer.
--- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---