From 0378250846b77e3d5d14ac55f98d7e83bd621df9 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Fri, 14 Jun 2019 11:10:40 +0200 Subject: [PATCH] core: Make the refactored global wait work. * src/fdtable.h (FDTABLE_FLAG_NOT_DONE): New flag. * src/fdtable.c (_gpgme_fdtable_io_cb_count): Rename to ... (_gpgme_fdtable_get_count): this and add arg 'flags'. (_gpgme_fdtable_run_io_cbs): Add return arg 'r_owner'. * src/wait.c (gpgme_wait_ext): Improve tracing. Act upon error codes from running the callbacks. * tests/gpg/t-wait.c (main): Remove the sleep. -- Now t-wait works again. There are still problems with the user defined events but it is another step in our refactoring work. Signed-off-by: Werner Koch --- src/fdtable.c | 56 +++++++++++++++++++++++++------- src/fdtable.h | 16 ++++++---- src/wait.c | 79 ++++++++++++++++++++++++++++++++++------------ tests/gpg/t-wait.c | 2 +- 4 files changed, 114 insertions(+), 39 deletions(-) diff --git a/src/fdtable.c b/src/fdtable.c index 13d6ef94..ecec3477 100644 --- a/src/fdtable.c +++ b/src/fdtable.c @@ -417,7 +417,7 @@ _gpgme_fdtable_remove (int fd) handlervalue = fdtable[idx].close_notify.value; fdtable[idx].close_notify.value = NULL; - /* The handler might call into the fdtable again, so of we have a + /* The handler might call into the fdtable again, so if we have a * handler we can't immediately close it but instead record the fact * and remove the entry from the table only after the handler has * been run. */ @@ -445,10 +445,15 @@ _gpgme_fdtable_remove (int fd) } -/* Return the number of active I/O callbacks for OWNER or for all if - * OWNER is 0. */ +/* Return the number of FDs for OWNER (or for all if OWNER is 0) + * which match FLAGS. Recognized flag values are: + * 0 - Number FDs with IO callbacks + * FDTABLE_FLAG_ACTIVE - Number of FDs in the active state. + * FDTABLE_FLAG_DONE - Number of FDs in the done state. + * FDTABLE_FLAG_NOT_DONE - Number of FDs not in the done state. + */ unsigned int -_gpgme_fdtable_io_cb_count (uint64_t owner) +_gpgme_fdtable_get_count (uint64_t owner, unsigned int flags) { int idx; unsigned int count = 0; @@ -456,11 +461,23 @@ _gpgme_fdtable_io_cb_count (uint64_t owner) LOCK (fdtable_lock); for (idx=0; idx < fdtablesize; idx++) if (fdtable[idx].fd != -1 && (!owner || fdtable[idx].owner == owner)) - count++; + { + if (fdtable[idx].closing) + continue; + + if ((flags & FDTABLE_FLAG_DONE) && fdtable[idx].done) + count++; + else if ((flags & FDTABLE_FLAG_NOT_DONE) && !fdtable[idx].done) + count++; + else if ((flags & FDTABLE_FLAG_ACTIVE) && fdtable[idx].active) + count++; + else if (!flags && fdtable[idx].io_cb.cb) + count++; + } UNLOCK (fdtable_lock); - TRACE (DEBUG_SYSIO, __func__, NULL, "ctx=%lu count=%u", - (unsigned long)owner, count); + TRACE (DEBUG_SYSIO, __func__, NULL, "ctx=%lu flags=0x%u -> count=%u", + (unsigned long)owner, flags, count); return count; } @@ -468,9 +485,10 @@ _gpgme_fdtable_io_cb_count (uint64_t owner) /* Run all signaled IO callbacks of OWNER or all signaled callbacks if * OWNER is 0. Returns an error code on the first real error * encountered. If R_OP_ERR is not NULL an optional operational error - * can be stored tehre. For EOF the respective flags are set. */ + * can be stored there. For EOF the respective flags are set. */ gpg_error_t -_gpgme_fdtable_run_io_cbs (uint64_t owner, gpg_error_t *r_op_err) +_gpgme_fdtable_run_io_cbs (uint64_t owner, gpg_error_t *r_op_err, + uint64_t *r_owner) { gpg_error_t err; int idx; @@ -485,6 +503,8 @@ _gpgme_fdtable_run_io_cbs (uint64_t owner, gpg_error_t *r_op_err) if (r_op_err) *r_op_err = 0; + if (r_owner) + *r_owner = 0; for (;;) { @@ -543,6 +563,8 @@ _gpgme_fdtable_run_io_cbs (uint64_t owner, gpg_error_t *r_op_err) if (err) { _gpgme_cancel_with_err (serial, err, 0); + if (r_owner) + *r_owner = serial; return TRACE_ERR (err); } else if (iocb_data.op_err) @@ -558,6 +580,8 @@ _gpgme_fdtable_run_io_cbs (uint64_t owner, gpg_error_t *r_op_err) * operation. */ if (r_op_err) *r_op_err = iocb_data.op_err; + if (r_owner) + *r_owner = serial; return TRACE_ERR (0); } else if (!cb_count && actx) @@ -606,6 +630,8 @@ _gpgme_fdtable_get_fds (io_select_t *r_fds, uint64_t owner, unsigned int flags) continue; if ((flags & FDTABLE_FLAG_DONE) && !fdtable[idx].done) continue; + if ((flags & FDTABLE_FLAG_NOT_DONE) && fdtable[idx].done) + continue; if ((flags & FDTABLE_FLAG_FOR_READ) && !fdtable[idx].for_read) continue; if ((flags & FDTABLE_FLAG_FOR_WRITE) && !fdtable[idx].for_write) @@ -651,6 +677,11 @@ _gpgme_fdtable_get_done (uint64_t owner, TRACE_BEG (DEBUG_SYSIO, __func__, NULL, "ctx=%lu", (unsigned long)owner); + if (r_status) + *r_status = 0; + if (r_op_err) + *r_op_err = 0; + LOCK (fdtable_lock); for (idx=0; idx < fdtablesize; idx++) @@ -660,8 +691,11 @@ _gpgme_fdtable_get_done (uint64_t owner, /* Found. If an owner has been given also clear the done * flags from all other fds of this owner. Note that they * have the same status info anyway. */ - *r_status = fdtable[idx].done_status; - *r_op_err = fdtable[idx].done_op_err; + TRACE_LOG ("found fd=%d", fdtable[idx].fd); + if (r_status) + *r_status = fdtable[idx].done_status; + if (r_op_err) + *r_op_err = fdtable[idx].done_op_err; fdtable[idx].done = 0; serial = fdtable[idx].owner; if (owner) diff --git a/src/fdtable.h b/src/fdtable.h index 6f621849..431c7969 100644 --- a/src/fdtable.h +++ b/src/fdtable.h @@ -26,11 +26,12 @@ /* Flags used by _gpgme_fdtable_get_fds. */ #define FDTABLE_FLAG_ACTIVE 1 /* Only those with the active flag set. */ #define FDTABLE_FLAG_DONE 2 /* Only those with the done flag set */ -#define FDTABLE_FLAG_FOR_READ 4 /* Only those with the signaled flag set. */ -#define FDTABLE_FLAG_FOR_WRITE 8 /* Only those with the for_read flag set. */ -#define FDTABLE_FLAG_SIGNALED 16 /* Only those with the signaled flag set. */ -#define FDTABLE_FLAG_NOT_SIGNALED 32 /* Ditto reversed. */ -#define FDTABLE_FLAG_CLEAR 128 /* Clear the signaled flag. */ +#define FDTABLE_FLAG_NOT_DONE 4 /* Only those with the done flag cleared. */ +#define FDTABLE_FLAG_FOR_READ 16 /* Only those with the signaled flag set. */ +#define FDTABLE_FLAG_FOR_WRITE 32 /* Only those with the for_read flag set. */ +#define FDTABLE_FLAG_SIGNALED 64 /* Only those with the signaled flag set. */ +#define FDTABLE_FLAG_NOT_SIGNALED 128 /* Ditto reversed. */ +#define FDTABLE_FLAG_CLEAR 256 /* Clear the signaled flag. */ /* The handler type associated with an FD. It is called with the FD @@ -65,10 +66,11 @@ void _gpgme_fdtable_set_signaled (io_select_t fds, unsigned int nfds); gpg_error_t _gpgme_fdtable_remove (int fd); /* Return the number of active I/O callbacks for OWNER. */ -unsigned int _gpgme_fdtable_io_cb_count (uint64_t owner); +unsigned int _gpgme_fdtable_get_count (uint64_t owner, unsigned int flags); /* Run all the signaled IO callbacks of OWNER. */ -gpg_error_t _gpgme_fdtable_run_io_cbs (uint64_t owner, gpg_error_t *r_op_err); +gpg_error_t _gpgme_fdtable_run_io_cbs (uint64_t owner, gpg_error_t *r_op_err, + uint64_t *r_owner); /* Return a list of FDs matching the OWNER and FLAGS. */ unsigned int _gpgme_fdtable_get_fds (io_select_t *r_fds, diff --git a/src/wait.c b/src/wait.c index b88b7813..091a93b9 100644 --- a/src/wait.c +++ b/src/wait.c @@ -58,10 +58,10 @@ user_io_cb_handler (void *data, int fd) serial = tag->serial; assert (serial); - err = _gpgme_fdtable_run_io_cbs (serial, &op_err); + err = _gpgme_fdtable_run_io_cbs (serial, &op_err, NULL); if (err || op_err) ; - else if (!_gpgme_fdtable_io_cb_count (serial)) + else if (!_gpgme_fdtable_get_count (serial, 0)) { /* No more active callbacks - emit a DONE. */ struct gpgme_io_event_done_data done_data = { 0, 0 }; @@ -318,10 +318,20 @@ gpgme_wait_ext (gpgme_ctx_t ctx, gpgme_error_t *status, int nr; uint64_t serial; + TRACE_BEG (DEBUG_SYSIO, __func__, NULL, "ctx=%lu hang=%d", + CTXSERIAL (ctx), hang); + do { /* Get all fds of CTX (or all if CTX is NULL) we want to wait * for and which are in the active state. */ + TRACE_LOG + ("active=%u done=%u !done=%d cbs=%u", + _gpgme_fdtable_get_count (ctx?ctx->serial:0,FDTABLE_FLAG_ACTIVE), + _gpgme_fdtable_get_count (ctx?ctx->serial:0,FDTABLE_FLAG_DONE), + _gpgme_fdtable_get_count (ctx?ctx->serial:0,FDTABLE_FLAG_NOT_DONE), + _gpgme_fdtable_get_count (ctx?ctx->serial:0,0)); + free (fds); nfds = _gpgme_fdtable_get_fds (&fds, ctx? ctx->serial : 0, ( FDTABLE_FLAG_ACTIVE @@ -335,11 +345,22 @@ gpgme_wait_ext (gpgme_ctx_t ctx, gpgme_error_t *status, *status = err; if (op_err) *op_err = 0; - free (fds); - return NULL; + ctx = NULL; + goto leave; } - /* Nothing to select. Run the select anyway, so that we use - * its timeout. */ + /* Nothing to select. */ + + if (!_gpgme_fdtable_get_count (ctx? ctx->serial : 0, + FDTABLE_FLAG_NOT_DONE)) + { + if (status) + *status = 0; + if (op_err) + *op_err = 0; + goto leave; + } + /* There are not yet active FDS. Use the select's standard + * timeout and then try again. */ } nr = _gpgme_io_select (fds, nfds, 0); @@ -349,30 +370,48 @@ gpgme_wait_ext (gpgme_ctx_t ctx, gpgme_error_t *status, *status = gpg_error_from_syserror (); if (op_err) *op_err = 0; - free (fds); - return NULL; + ctx = NULL; + goto leave; } _gpgme_fdtable_set_signaled (fds, nfds); - _gpgme_fdtable_run_io_cbs (ctx? ctx->serial : 0, NULL); - serial = _gpgme_fdtable_get_done (ctx? ctx->serial : 0, status, op_err); - if (serial) + err = _gpgme_fdtable_run_io_cbs (ctx? ctx->serial : 0, op_err, &serial); + if (err || (op_err && *op_err)) { - _gpgme_get_ctx (serial, &ctx); + if (status) + *status = err; + if (serial) + _gpgme_get_ctx (serial, &ctx); hang = 0; } - else if (!hang) + else { - ctx = NULL; - if (status) - *status = 0; - if (op_err) - *op_err = 0; + serial = _gpgme_fdtable_get_done (ctx? ctx->serial : 0, status, + op_err); + if (serial) + { + _gpgme_get_ctx (serial, &ctx); + hang = 0; + } + else if (!hang) + { + ctx = NULL; + if (status) + *status = 0; + if (op_err) + *op_err = 0; + } } } while (hang); + leave: free (fds); + if (status) + TRACE_LOG ("status=%d", *status); + if (op_err) + TRACE_LOG ("op_err=%d", *op_err); + TRACE_SUC ("result=%lu", ctx? ctx->serial : 0); return ctx; } @@ -434,12 +473,12 @@ _gpgme_sync_wait (gpgme_ctx_t ctx, volatile int *cond, gpg_error_t *r_op_err) } _gpgme_fdtable_set_signaled (fds, nfds); - err = _gpgme_fdtable_run_io_cbs (ctx->serial, r_op_err); + err = _gpgme_fdtable_run_io_cbs (ctx->serial, r_op_err, NULL); if (err || (r_op_err && *r_op_err)) goto leave; } - if (!_gpgme_fdtable_io_cb_count (ctx->serial)) + if (!_gpgme_fdtable_get_count (ctx->serial, 0)) { /* No more matching fds with IO callbacks. */ struct gpgme_io_event_done_data data = {0, 0}; diff --git a/tests/gpg/t-wait.c b/tests/gpg/t-wait.c index c19502cd..0428e8d8 100644 --- a/tests/gpg/t-wait.c +++ b/tests/gpg/t-wait.c @@ -59,7 +59,7 @@ main (void) fail_if_err (err); while (gpgme_wait (ctx, &err, 0) == NULL && err == 0) - sleep(1); + ; if (gpgme_err_code (err) != GPG_ERR_NO_DATA) {