Re: [Libguestfs] [PATCH libnbd v2 2/5] lib: Implement closure lifetimes.
On 7/24/19 11:54 AM, Richard W.M. Jones wrote:
Previously closures had a crude flag which tells if they are
persistent or transient. Transient closures (flag = false) last for
the lifetime of the currently called libnbd function. Persistent
closures had an indefinite lifetime which could last for as long as
the handle. In language bindings handling persistent closures was
wasteful as we needed to register a "close callback" to free the
closure when the handle is closed. But if you had submitted thousands
of asynchronous commands you would end up registering thousands of
close callbacks.
+++ b/lib/aio.c
@@ -90,6 +90,17 @@ nbd_unlocked_aio_command_completed (struct nbd_handle *h,
else
h->cmds_done = cmd->next;
+ /* Free the callbacks. */
+ if (cmd->type != NBD_CMD_READ && cmd->cb.fn.extent)
+ cmd->cb.fn.extent (LIBNBD_CALLBACK_FREE, cmd->cb.fn_user_data,
+ NULL, 0, NULL, 0, NULL);
+ if (cmd->type == NBD_CMD_READ && cmd->cb.fn.read)
+ cmd->cb.fn.read (LIBNBD_CALLBACK_FREE, cmd->cb.fn_user_data,
+ NULL, 0, 0, 0, NULL);
+ if (cmd->cb.callback)
+ cmd->cb.callback (LIBNBD_CALLBACK_FREE, cmd->cb.user_data,
+ 0, NULL);
+
free (cmd);
nbd_aio_command_completed is skipped when a user calls nbd_close. While
we've documented that nbd_close loses the exit status of any unretired
command (different from the fact that completion callbacks run on
transition to DEAD but the handle is still around), it is still probably
worth tweaking nbd_close's use of free_cmd_list() to still call FREE on
any pending callbacks on commands stranded by abrupt close to allow the
user to still avoid memory leaks.
+++ b/lib/debug.c
@@ -40,9 +40,11 @@ nbd_unlocked_get_debug (struct nbd_handle *h)
int
nbd_unlocked_set_debug_callback (struct nbd_handle *h,
- int (*debug_fn) (void *, const char *, const char *),
- void *data)
+ debug_fn debug_fn, void *data)
{
+ if (h->debug_fn)
+ /* ignore return value */
+ h->debug_fn (LIBNBD_CALLBACK_FREE, h->debug_data, NULL, NULL);
Is it worth outputting a debug message just before freeing things? Or
put differently, should this be something like
h->debug_fn (LIBNBD_CALLBACK_VALID | LIBNBD_CALLBACK_FREE,
h->debug_data, context, "changing debug callback");
Also, should nbd_close() output a debug message about the handle
disappearing (and if so, obviously prior to calling callback(FREE)).
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 488 bytes)