search for: completion_cb_freed

...eneral Public @@ -42,6 +42,8 @@ static unsigned debug_fn_called; static unsigned debug_fn_freed; static unsigned read_cb_called; static unsigned read_cb_freed; +static unsigned block_status_cb_called; +static unsigned block_status_cb_freed; static unsigned completion_cb_called; static unsigned completion_cb_freed; @@ -74,6 +76,21 @@ read_cb_free (void *opaque) read_cb_freed++; } +static int +block_status_cb (void *opaque, const char *meta, uint64_t offset, + uint32_t *entries, size_t nr_entries, int *error) +{ + assert (!block_status_cb_freed); + block_status_cb_called++; + return 0...

...alid; -static unsigned read_cb_free; -static unsigned completion_cb_valid; -static unsigned completion_cb_free; +static unsigned debug_fn_called; +static unsigned debug_fn_freed; +static unsigned read_cb_called; +static unsigned read_cb_freed; +static unsigned completion_cb_called; +static unsigned completion_cb_freed; static int -debug_fn (unsigned valid_flag, void *opaque, +debug_fn (void *opaque, const char *context, const char *msg) { - if (valid_flag & LIBNBD_CALLBACK_VALID) - debug_fn_valid++; - if (valid_flag & LIBNBD_CALLBACK_FREE) - debug_fn_free++; + debug_fn_called++;...

...alid; -static unsigned read_cb_free; -static unsigned completion_cb_valid; -static unsigned completion_cb_free; +static unsigned debug_fn_called; +static unsigned debug_fn_freed; +static unsigned read_cb_called; +static unsigned read_cb_freed; +static unsigned completion_cb_called; +static unsigned completion_cb_freed; static int -debug_fn (unsigned valid_flag, void *opaque, - const char *context, const char *msg) +debug_fn (void *opaque, const char *context, const char *msg) { - if (valid_flag & LIBNBD_CALLBACK_VALID) - debug_fn_valid++; - if (valid_flag & LIBNBD_CALLBACK_FREE) - de...

As promised in my earlier thread on libnbd completion callback question. Eric Blake (2): generator: Refactor handling of closures in unlocked functions generator: Free closures on failure docs/libnbd.pod | 2 +- generator/C.ml | 48 +++++++++++------ generator/C.mli | 1 + lib/debug.c | 7 +-- lib/opt.c | 31 ++++++-----

This is my proposal for fixing the documentation to match practice (namely, that completion.callback is not invoked in the cases where the aio call itself reports errors); we could instead try to go the other direction and tweak the generator to guarantee that both completion.callback and completion.free are reached no matter what, but that felt more invasive to me. Eric Blake (2): api: Tighten

...or (ESRCH, "no subprocess exists"); diff --git a/tests/closure-lifetimes.c b/tests/closure-lifetimes.c index 5a7cd9c..70788b6 100644 --- a/tests/closure-lifetimes.c +++ b/tests/closure-lifetimes.c @@ -143,7 +143,7 @@ main (int argc, char *argv[]) assert (read_cb_freed == 1); assert (completion_cb_freed == 1); - nbd_kill_command (nbd, 0); + nbd_kill_subprocess (nbd, 0); nbd_close (nbd); /* Test command callbacks are freed if the handle is closed without @@ -162,7 +162,7 @@ main (int argc, char *argv[]) .free = completion...

As proposed here: https://www.redhat.com/archives/libguestfs/2019-August/msg00130.html I didn't actually read Eric's replies to that yet because I've been concentrating on writing these patches all day. Anyway here they are and I'll look at what Eric said about the proposal next. Rich.

Patches 1 & 2 are rather complex, but the end result is that we pass closures + user_data + free function in single struct parameters as I described previously in this email: https://www.redhat.com/archives/libguestfs/2019-August/msg00210.html Patch 3 adds a convenient FREE_CALLBACK macro which seems a worthwhile simplification if you buy into 1 & 2. Patch 4 adds another macro which is

On 7/25/19 8:07 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

I think I've addressed everything that was raised in review. Some of the highlights: - Callbacks should be freed reliably along all exit paths. - There's a simple test of closure lifetimes. - I've tried to use VALID|FREE in all the places where I'm confident that it's safe and correct to do. There may be more places. Note this is an optimization and shouldn't

...; -static unsigned completion_cb_valid; > -static unsigned completion_cb_free; > +static unsigned debug_fn_called; > +static unsigned debug_fn_freed; > +static unsigned read_cb_called; > +static unsigned read_cb_freed; > +static unsigned completion_cb_called; > +static unsigned completion_cb_freed; > > static int > -debug_fn (unsigned valid_flag, void *opaque, > - const char *context, const char *msg) > +debug_fn (void *opaque, const char *context, const char *msg) > { > - if (valid_flag & LIBNBD_CALLBACK_VALID) > - debug_fn_valid++; > - if (v...

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

Reverts commit 387cbe67c3db27e8a61117fedb6e7fad76e409ef. --- generator/generator | 18 +++++++++++++++++- lib/handle.c | 28 +++++++++++++++++++++++++++- tests/closure-lifetimes.c | 4 +++- 3 files changed, 47 insertions(+), 3 deletions(-) diff --git a/generator/generator b/generator/generator index 2cd83f1..25e4aa5 100755 --- a/generator/generator +++ b/generator/generator

As discussed in this thread, the parameter is an invitation to write code with race conditions: https://www.redhat.com/archives/libguestfs/2019-July/thread.html#00309 --- docs/libnbd.pod | 6 +- examples/glib-main-loop.c | 10 ++-- examples/strict-structured-reads.c | 2 +- generator/generator | 57

The original nbd_aio_* (non-callback) functions are removed and replaced with the renamed callback variants. This is a simple mechanical change to the API: (1) Any existing call to nbd_aio_*_callback can simply be renamed to nbd_aio_* (2) Any existing call to nbd_aio_* must have two extra NULL parameters added before the final flags parameter. In non-C languages, only change (1) is

This applies on top of the OClosure v2 series posted a few minutes ago. Rich.

The definition of functions that take a callback is changed so that the callback and user_data are combined into a single structure, eg: int64_t nbd_aio_pread (struct nbd_handle *h, void *buf, size_t count, uint64_t offset, - int (*completion_callback) (/*..*/), void *user_data, + nbd_completion_callback completion_callback, uint32_t flags); Several