GNOME Bugzilla – Bug 701446
Fix callback annotation in gupnp_service_proxy_add_notify
Last modified: 2014-05-24 16:31:10 UTC
Second call to the callback was causing the segfault. Changing annotation from async to notified fixed the problem. (tested with python) From https://live.gnome.org/GObjectIntrospection/Annotations * async - Only valid for the duration of the first callback invokation. Can only be called once. * notified - valid until the GDestroyNotify argument is called. Can be called multiple times before the GDestroyNotify is called. Patch attached.
Created attachment 245840 [details] [review] Fix callback annotation in gupnp_service_proxy_add_notify
Review of attachment 245840 [details] [review]: Sorry that it took so long to review this, somehow went under the radar ::: libgupnp/gupnp-service-proxy.c @@ -1545,3 +1545,3 @@ * @variable: The variable to add notification for * @type: The type of the variable - * @callback: (scope async): The callback to call when @variable changes + * @callback: (scope notified): The callback to call when @variable changes From the description in the linked wiki-page, wouldn't "call" be more correct? Since we don't have a GDestroyNotify here
Thanks, it looks like "notified" is a closest but not fully correct. With "call", callback would be valid only for a duration of the gupnp_service_proxy_add_notify function call. The correct solution would be to create new gupnp_service_proxy_add_notify_gi function with GDestroyNotify, used by introspection. Should I go ahead and propose the patch?
Created attachment 263204 [details] [review] Add _add_notify_full variant with a GDestroyNotify This is to help introspection with the call-back.
Review of attachment 263204 [details] [review]: Thanks for the patch, I've tested it with gupnp 0.20.4 - it runs fine. On service proxy finalize callback notify should also be called. In our case gupnp_service_proxy_finalize() invokes notify_data_free(), notify_data_free() should call callback notify before freeing it. ::: libgupnp/gupnp-service-proxy.c @@ +1592,3 @@ + callback, + NULL, + user_data); Parameters order should be ([..] callback, user_data, NULL)
Good catch, thanks
*** Bug 730410 has been marked as a duplicate of this bug. ***
Created attachment 277118 [details] [review] Add _add_notify_full variant with a GDestroyNotify This is to help introspection with the call-back.
Attachment 277118 [details] pushed as 9ffedf4 - Add _add_notify_full variant with a GDestroyNotify