From dde69e1be2bf71a671cef77d1517d7fdb1ff892b Mon Sep 17 00:00:00 2001 From: Ralph Castain Date: Sat, 18 Jun 2016 12:28:46 -0700 Subject: [PATCH] Cleanup CIDs 1362763, 1362762, 1362760, 1362759, 1362758, 1362757, 1362756, 1362755, 1362754. Unsure how to resolve 1362761. Fixes #1792 --- .../pmix/src/event/pmix_event_notification.c | 2 +- .../pmix/src/event/pmix_event_registration.c | 122 ++++++++++++++---- .../pmix/pmix2x/pmix/src/server/pmix_server.c | 27 +--- .../pmix2x/pmix/src/server/pmix_server_ops.c | 29 +---- .../pmix2x/pmix/src/server/pmix_server_ops.h | 6 +- opal/mca/pmix/pmix2x/pmix2x.c | 6 +- opal/mca/pmix/pmix2x/pmix2x_client.c | 3 + 7 files changed, 120 insertions(+), 75 deletions(-) diff --git a/opal/mca/pmix/pmix2x/pmix/src/event/pmix_event_notification.c b/opal/mca/pmix/pmix2x/pmix/src/event/pmix_event_notification.c index 9180221158..3869465841 100644 --- a/opal/mca/pmix/pmix2x/pmix/src/event/pmix_event_notification.c +++ b/opal/mca/pmix/pmix2x/pmix/src/event/pmix_event_notification.c @@ -527,7 +527,7 @@ static pmix_status_t notify_client_of_event(pmix_status_t status, pmix_output_verbose(2, pmix_globals.debug_output, "pmix_server_notify_event status =%d, source = %s:%d, ninfo =%lu", - status, source->nspace, source->rank, ninfo); + status, cd->source.nspace, cd->source.rank, ninfo); /* we have to push this into our event library to avoid * potential threading issues */ diff --git a/opal/mca/pmix/pmix2x/pmix/src/event/pmix_event_registration.c b/opal/mca/pmix/pmix2x/pmix/src/event/pmix_event_registration.c index 51eeb62c9e..c402bb1d80 100644 --- a/opal/mca/pmix/pmix2x/pmix/src/event/pmix_event_registration.c +++ b/opal/mca/pmix/pmix2x/pmix/src/event/pmix_event_registration.c @@ -181,7 +181,7 @@ static pmix_status_t _add_hdlr(pmix_list_t *list, pmix_list_item_t *item, if (NULL == cd->codes) { registered = false; PMIX_LIST_FOREACH(active, &pmix_globals.events.actives, pmix_active_code_t) { - if (INT_MIN == active->code) { + if (PMIX_MAX_ERR_CONSTANT == active->code) { /* we have registered a default */ registered = true; break; @@ -189,7 +189,7 @@ static pmix_status_t _add_hdlr(pmix_list_t *list, pmix_list_item_t *item, } if (!registered) { active = PMIX_NEW(pmix_active_code_t); - active->code = INT_MIN; + active->code = PMIX_MAX_ERR_CONSTANT; pmix_list_append(&pmix_globals.events.actives, &active->super); /* ensure we register it */ need_register = true; @@ -221,12 +221,14 @@ static pmix_status_t _add_hdlr(pmix_list_t *list, pmix_list_item_t *item, PMIX_RETAIN(cd); cd2->cd = cd; cd2->ninfo = pmix_list_get_size(xfer); - PMIX_INFO_CREATE(cd2->info, cd2->ninfo); - n=0; - PMIX_LIST_FOREACH(ixfer, xfer, pmix_info_caddy_t) { - (void)strncpy(cd2->info[n].key, ixfer->info[n].key, PMIX_MAX_KEYLEN); - pmix_value_load(&cd2->info[n].value, &ixfer->info[n].value.data, ixfer->info[n].value.type); - ++n; + if (0 < cd2->ninfo) { + PMIX_INFO_CREATE(cd2->info, cd2->ninfo); + n=0; + PMIX_LIST_FOREACH(ixfer, xfer, pmix_info_caddy_t) { + (void)strncpy(cd2->info[n].key, ixfer->info[n].key, PMIX_MAX_KEYLEN); + pmix_value_load(&cd2->info[n].value, &ixfer->info[n].value.data, ixfer->info[n].value.type); + ++n; + } } /* if we are a client, and we haven't already registered a handler of this @@ -427,18 +429,46 @@ PMIX_EXPORT void PMIx_Register_event_handler(pmix_status_t codes[], size_t ncode static void dereg_event_hdlr(int sd, short args, void *cbdata) { pmix_shift_caddy_t *cd = (pmix_shift_caddy_t*)cbdata; - pmix_buffer_t *msg; - pmix_single_event_t *sing; - pmix_multi_event_t *multi; + pmix_buffer_t *msg = NULL; + pmix_single_event_t *sing, *s2; + pmix_multi_event_t *multi, *m2; pmix_default_event_t *def; pmix_cmd_t cmd = PMIX_DEREGEVENTS_CMD; pmix_status_t rc = PMIX_SUCCESS; + pmix_status_t wildcard = PMIX_MAX_ERR_CONSTANT; + size_t n; + bool found, foundcode; + + /* if I am not the server, then I need to notify the server + * to remove my registration */ + if (!pmix_globals.server) { + msg = PMIX_NEW(pmix_buffer_t); + if (PMIX_SUCCESS != (rc = pmix_bfrop.pack(msg, &cmd, 1, PMIX_CMD))) { + PMIX_RELEASE(msg); + goto cleanup; + } + } /* the registration can be in any of three places, so check them all */ PMIX_LIST_FOREACH(def, &pmix_globals.events.default_events, pmix_default_event_t) { if (def->index == cd->ref) { /* found it */ pmix_list_remove_item(&pmix_globals.events.default_events, &def->super); + if (NULL != msg) { + /* if there are no more default handlers registered, tell + * the server to dereg the default handler */ + if (0 == pmix_list_get_size(&pmix_globals.events.default_events)) { + n = 1; + if (PMIX_SUCCESS != (rc = pmix_bfrop.pack(msg, &n, 1, PMIX_SIZE))) { + PMIX_RELEASE(msg); + goto cleanup; + } + if (PMIX_SUCCESS != (rc = pmix_bfrop.pack(msg, &wildcard, 1, PMIX_STATUS))) { + PMIX_RELEASE(msg); + goto cleanup; + } + } + } PMIX_RELEASE(def); goto report; } @@ -447,6 +477,30 @@ static void dereg_event_hdlr(int sd, short args, void *cbdata) if (sing->index == cd->ref) { /* found it */ pmix_list_remove_item(&pmix_globals.events.single_events, &sing->super); + if (NULL != msg) { + /* if there are no more handlers registered for this code, tell + * the server to dereg the handler for this code */ + found = false; + PMIX_LIST_FOREACH(s2, &pmix_globals.events.single_events, pmix_single_event_t) { + if (s2->code = sing->code) { + found = true; + break; + } + } + if (!found) { + n = 1; + if (PMIX_SUCCESS != (rc = pmix_bfrop.pack(msg, &n, 1, PMIX_SIZE))) { + PMIX_RELEASE(msg); + PMIX_RELEASE(sing); + goto cleanup; + } + if (PMIX_SUCCESS != (rc = pmix_bfrop.pack(msg, &sing->code, 1, PMIX_STATUS))) { + PMIX_RELEASE(msg); + PMIX_RELEASE(sing); + goto cleanup; + } + } + } PMIX_RELEASE(sing); goto report; } @@ -455,6 +509,40 @@ static void dereg_event_hdlr(int sd, short args, void *cbdata) if (multi->index == cd->ref) { /* found it */ pmix_list_remove_item(&pmix_globals.events.multi_events, &multi->super); + if (NULL != msg) { + /* if there are no more handlers registered for this code, tell + * the server to dereg the handler for this code */ + found = false; + PMIX_LIST_FOREACH(s2, &pmix_globals.events.single_events, pmix_single_event_t) { + if (m2->ncodes != multi->ncodes) { + continue; + } + foundcode = true; + for (n=0; n < multi->ncodes; n++) { + if (m2->codes[n] != multi->codes[n]) { + foundcode = false; + break; + } + } + if (foundcode) { + found = true; + break; + } + } + if (!found) { + n = multi->ncodes; + if (PMIX_SUCCESS != (rc = pmix_bfrop.pack(msg, &n, 1, PMIX_SIZE))) { + PMIX_RELEASE(msg); + PMIX_RELEASE(multi); + goto cleanup; + } + if (PMIX_SUCCESS != (rc = pmix_bfrop.pack(msg, &multi->codes, n, PMIX_STATUS))) { + PMIX_RELEASE(msg); + PMIX_RELEASE(multi); + goto cleanup; + } + } + } PMIX_RELEASE(multi); goto report; } @@ -467,17 +555,7 @@ static void dereg_event_hdlr(int sd, short args, void *cbdata) return; report: - if (!pmix_globals.server) { - /* notify the server */ - msg = PMIX_NEW(pmix_buffer_t); - if (PMIX_SUCCESS != (rc = pmix_bfrop.pack(msg, &cmd, 1, PMIX_CMD))) { - PMIX_RELEASE(msg); - goto cleanup; - } - if (PMIX_SUCCESS != (rc = pmix_bfrop.pack(msg, &cd->ref, 1, PMIX_SIZE))) { - PMIX_ERROR_LOG(rc); - goto cleanup; - } + if (NULL != msg) { /* push the message into our event base to send to the server */ PMIX_ACTIVATE_SEND_RECV(&pmix_client_globals.myserver, msg, NULL, NULL); } diff --git a/opal/mca/pmix/pmix2x/pmix/src/server/pmix_server.c b/opal/mca/pmix/pmix2x/pmix/src/server/pmix_server.c index 0a062ad04d..2dec6939d3 100644 --- a/opal/mca/pmix/pmix2x/pmix/src/server/pmix_server.c +++ b/opal/mca/pmix/pmix2x/pmix/src/server/pmix_server.c @@ -1919,7 +1919,7 @@ static void cnct_cbfunc(pmix_status_t status, void *cbdata) PMIX_THREADSHIFT(scd, _cnct); } -static void regevents_cbfunc (pmix_status_t status, void *cbdata) +static void regevents_cbfunc(pmix_status_t status, void *cbdata) { pmix_status_t rc; pmix_server_caddy_t *cd = (pmix_server_caddy_t*) cbdata; @@ -1937,23 +1937,6 @@ static void regevents_cbfunc (pmix_status_t status, void *cbdata) PMIX_RELEASE(cd); } -static void deregevents_cbfunc (pmix_status_t status, void *cbdata) -{ - pmix_status_t rc; - pmix_server_caddy_t *cd = (pmix_server_caddy_t*) cbdata; - pmix_buffer_t *reply = PMIX_NEW(pmix_buffer_t); - - pmix_output_verbose(2, pmix_globals.debug_output, - "server:deregevents_cbfunc called status = %d", status); - - if (PMIX_SUCCESS != (rc = pmix_bfrop.pack(reply, &status, 1, PMIX_STATUS))) { - PMIX_ERROR_LOG(rc); - } - // send reply - PMIX_SERVER_QUEUE_REPLY(cd->peer, cd->hdr.tag, reply); - PMIX_RELEASE(cd); -} - static void notifyerror_cbfunc (pmix_status_t status, void *cbdata) { pmix_status_t rc; @@ -2128,12 +2111,10 @@ static pmix_status_t server_switchyard(pmix_peer_t *peer, uint32_t tag, return rc; } if (PMIX_DEREGEVENTS_CMD == cmd) { - PMIX_PEER_CADDY(cd, peer, tag); - if (PMIX_SUCCESS != (rc = pmix_server_deregister_events(peer, buf, deregevents_cbfunc, cd))) { - PMIX_RELEASE(cd); - } - return rc; + pmix_server_deregister_events(peer, buf); + return PMIX_SUCCESS; } + if (PMIX_NOTIFY_CMD == cmd) { PMIX_PEER_CADDY(cd, peer, tag); rc = pmix_server_event_recvd_from_client(peer, buf, notifyerror_cbfunc, cd); diff --git a/opal/mca/pmix/pmix2x/pmix/src/server/pmix_server_ops.c b/opal/mca/pmix/pmix2x/pmix/src/server/pmix_server_ops.c index 43a4fd3388..6218ff010b 100644 --- a/opal/mca/pmix/pmix2x/pmix/src/server/pmix_server_ops.c +++ b/opal/mca/pmix/pmix2x/pmix/src/server/pmix_server_ops.c @@ -986,7 +986,7 @@ pmix_status_t pmix_server_register_events(pmix_peer_t *peer, pmix_status_t rc; pmix_status_t *codes = NULL; pmix_info_t *info = NULL; - size_t ninfo, ncodes, n, k; + size_t ninfo=0, ncodes, n, k; pmix_regevents_info_t *reginfo; pmix_peer_events_info_t *prev; pmix_notify_caddy_t *cd; @@ -1089,7 +1089,6 @@ pmix_status_t pmix_server_register_events(pmix_peer_t *peer, prev->peer = peer; prev->enviro_events = enviro_events; pmix_list_append(®info->peers, &prev->super); - found = true; } } else { /* if we get here, then we didn't find an existing registration for this code */ @@ -1171,10 +1170,8 @@ cleanup: return PMIX_SUCCESS; } -pmix_status_t pmix_server_deregister_events(pmix_peer_t *peer, - pmix_buffer_t *buf, - pmix_op_cbfunc_t cbfunc, - void *cbdata) +void pmix_server_deregister_events(pmix_peer_t *peer, + pmix_buffer_t *buf) { int32_t cnt; pmix_status_t rc, *codes = NULL, *cdptr, maxcode = PMIX_MAX_ERR_CONSTANT; @@ -1191,7 +1188,7 @@ pmix_status_t pmix_server_deregister_events(pmix_peer_t *peer, cnt=1; if (PMIX_SUCCESS != (rc = pmix_bfrop.unpack(buf, &ncodes, &cnt, PMIX_SIZE))) { PMIX_ERROR_LOG(rc); - return rc; + return; } /* unpack the array of codes */ if (0 < ncodes) { @@ -1203,22 +1200,6 @@ pmix_status_t pmix_server_deregister_events(pmix_peer_t *peer, } } - /* unpack the number of info objects */ - cnt=1; - if (PMIX_SUCCESS != (rc = pmix_bfrop.unpack(buf, &ninfo, &cnt, PMIX_SIZE))) { - PMIX_ERROR_LOG(rc); - return rc; - } - /* unpack the array of info objects */ - if (0 < ninfo) { - PMIX_INFO_CREATE(info, ninfo); - cnt=ninfo; - if (PMIX_SUCCESS != (rc = pmix_bfrop.unpack(buf, info, &cnt, PMIX_INFO))) { - PMIX_ERROR_LOG(rc); - goto cleanup; - } - } - /* find the event registration info so we can delete them */ if (NULL == codes) { cdptr = &maxcode; @@ -1258,7 +1239,7 @@ cleanup: if (NULL != info) { PMIX_INFO_FREE(info, ninfo); } - return rc; + return; } diff --git a/opal/mca/pmix/pmix2x/pmix/src/server/pmix_server_ops.h b/opal/mca/pmix/pmix2x/pmix/src/server/pmix_server_ops.h index 336c0beaea..5ddb02eb53 100644 --- a/opal/mca/pmix/pmix2x/pmix/src/server/pmix_server_ops.h +++ b/opal/mca/pmix/pmix2x/pmix/src/server/pmix_server_ops.h @@ -265,10 +265,8 @@ pmix_status_t pmix_server_register_events(pmix_peer_t *peer, pmix_op_cbfunc_t cbfunc, void *cbdata); -pmix_status_t pmix_server_deregister_events(pmix_peer_t *peer, - pmix_buffer_t *buf, - pmix_op_cbfunc_t cbfunc, - void *cbdata); +void pmix_server_deregister_events(pmix_peer_t *peer, + pmix_buffer_t *buf); pmix_status_t pmix_server_event_recvd_from_client(pmix_peer_t *peer, pmix_buffer_t *buf, diff --git a/opal/mca/pmix/pmix2x/pmix2x.c b/opal/mca/pmix/pmix2x/pmix2x.c index 70904a3f72..1afdec6354 100644 --- a/opal/mca/pmix/pmix2x/pmix2x.c +++ b/opal/mca/pmix/pmix2x/pmix2x.c @@ -396,7 +396,11 @@ void pmix2x_event_hdlr(size_t evhdlr_registration_id, } iptr = OBJ_NEW(opal_value_t); iptr->key = strdup(info[n].key); - pmix2x_value_unload(iptr, &info[n].value); + if (OPAL_SUCCESS != (rc = pmix2x_value_unload(iptr, &info[n].value))) { + OPAL_ERROR_LOG(rc); + OBJ_RELEASE(iptr); + continue; + } opal_list_append(cd->info, &iptr->super); } } diff --git a/opal/mca/pmix/pmix2x/pmix2x_client.c b/opal/mca/pmix/pmix2x/pmix2x_client.c index e2827db11b..c54b35896a 100644 --- a/opal/mca/pmix/pmix2x/pmix2x_client.c +++ b/opal/mca/pmix/pmix2x/pmix2x_client.c @@ -150,6 +150,7 @@ int pmix2x_abort(int flag, const char *msg, } } if (NULL == job) { + PMIX_PROC_FREE(parray, cnt); return OPAL_ERR_NOT_FOUND; } (void)strncpy(parray[n].nspace, job->nspace, PMIX_MAX_NSLEN); @@ -319,6 +320,7 @@ int pmix2x_fencenb(opal_list_t *procs, int collect_data, } } if (NULL == job) { + PMIX_PROC_FREE(parray, cnt); return OPAL_ERR_NOT_FOUND; } (void)strncpy(parray[n].nspace, job->nspace, PMIX_MAX_NSLEN); @@ -1043,6 +1045,7 @@ int pmix2x_connect(opal_list_t *procs) } if (NULL == job) { OPAL_ERROR_LOG(OPAL_ERR_NOT_FOUND); + PMIX_PROC_FREE(parray, cnt); return OPAL_ERR_NOT_FOUND; } (void)strncpy(parray[n].nspace, job->nspace, PMIX_MAX_NSLEN);