Some callers of nbd_aio_pread_structured_callback or nbd_aio_block_status_callback share the same opaque pointer to both calls, with the intent to only free the pointer on the completion callback. Although our documentation does not explicitly specify the order in which callbacks are made, allowing the callback(FREE) to occur prior to the read(FREE) puts a burden on the client to not dereference the user_data in the read() function if the VALID bit is not set. If the client follows our suggested practice of an immediate return 0 if the VAILD bit is not set, then this is the case, but we can't guarantee all clients will use that pattern. And, it's fairly easy to guarantee that ALL of our read/extent callbacks are made prior to any of our callback() use, by utilizing the FINISH state of structured replies to catch the few cases where we are unable to send VALID|FREE. Note that nbd_internal_retire_and_free_command still has to check whether we have freed the callbacks, as nbd_close() can strand commands before the FINISH state. --- generator/states-reply-structured.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/generator/states-reply-structured.c b/generator/states-reply-structured.c index 20b0e9e..ff5b727 100644 --- a/generator/states-reply-structured.c +++ b/generator/states-reply-structured.c @@ -540,11 +540,20 @@ valid_flags (struct nbd_handle *h) return 0; REPLY.STRUCTURED_REPLY.FINISH: + struct command *cmd = h->reply_cmd; uint16_t flags; flags = be16toh (h->sbuf.sr.structured_reply.flags); - if (flags & NBD_REPLY_FLAG_DONE) + if (flags & NBD_REPLY_FLAG_DONE) { + if (cmd->type == NBD_CMD_BLOCK_STATUS && 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); + cmd->cb.fn.read = NULL; SET_NEXT_STATE (%^FINISH_COMMAND); + } else { h->reply_cmd = NULL; SET_NEXT_STATE (%.READY); -- 2.20.1