Skip to content

Commit 48feba8

Browse files
rhc54rhc54
committed
Provide blocking ability to the register/deregister fns
If the caller doesn't provide a cbfunc for PMIx_server_register_nspace and friends, then don't just return and let the processing of the request be done without pause as they could get into race conditions. Signed-off-by: Ralph Castain <[email protected]> (cherry picked from commit 04d83be)
1 parent 48a885c commit 48feba8

File tree

2 files changed

+81
-19
lines changed

2 files changed

+81
-19
lines changed

‎src/server/pmix_server.c‎

Lines changed: 73 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -526,6 +526,13 @@ PMIX_EXPORT pmix_status_t PMIx_server_finalize(void)
526526
return PMIX_SUCCESS;
527527
}
528528

529+
static void opcbfunc(pmix_status_t status, void *cbdata)
530+
{
531+
pmix_lock_t *lock = (pmix_lock_t*)cbdata;
532+
lock->status = status;
533+
PMIX_WAKEUP_THREAD(lock);
534+
}
535+
529536
static void _register_nspace(int sd, short args, void *cbdata)
530537
{
531538
pmix_setup_caddy_t *cd = (pmix_setup_caddy_t*)cbdata;
@@ -585,9 +592,7 @@ static void _register_nspace(int sd, short args, void *cbdata)
585592
cd->info, cd->ninfo);
586593

587594
release:
588-
if (NULL != cd->opcbfunc) {
589-
cd->opcbfunc(rc, cd->cbdata);
590-
}
595+
cd->opcbfunc(rc, cd->cbdata);
591596
PMIX_RELEASE(cd);
592597
}
593598

@@ -597,6 +602,8 @@ PMIX_EXPORT pmix_status_t PMIx_server_register_nspace(const pmix_nspace_t nspace
597602
pmix_op_cbfunc_t cbfunc, void *cbdata)
598603
{
599604
pmix_setup_caddy_t *cd;
605+
pmix_status_t rc;
606+
pmix_lock_t mylock;
600607

601608
PMIX_ACQUIRE_THREAD(&pmix_global_lock);
602609
if (pmix_globals.init_cntr <= 0) {
@@ -616,6 +623,22 @@ PMIX_EXPORT pmix_status_t PMIx_server_register_nspace(const pmix_nspace_t nspace
616623
cd->info = info;
617624
}
618625

626+
/* if the provided callback is NULL, then substitute
627+
* our own internal cbfunc and block here */
628+
if (NULL == cbfunc) {
629+
PMIX_CONSTRUCT_LOCK(&mylock);
630+
cd->opcbfunc = opcbfunc;
631+
cd->cbdata = &mylock;
632+
PMIX_THREADSHIFT(cd, _register_nspace);
633+
PMIX_WAIT_THREAD(&mylock);
634+
rc = mylock.status;
635+
PMIX_DESTRUCT_LOCK(&mylock);
636+
if (PMIX_SUCCESS == rc) {
637+
rc = PMIX_OPERATION_SUCCEEDED;
638+
}
639+
return rc;
640+
}
641+
619642
/* we have to push this into our event library to avoid
620643
* potential threading issues */
621644
PMIX_THREADSHIFT(cd, _register_nspace);
@@ -753,9 +776,7 @@ static void _deregister_nspace(int sd, short args, void *cbdata)
753776
}
754777

755778
/* release the caller */
756-
if (NULL != cd->opcbfunc) {
757-
cd->opcbfunc(rc, cd->cbdata);
758-
}
779+
cd->opcbfunc(rc, cd->cbdata);
759780
PMIX_RELEASE(cd);
760781
}
761782

@@ -764,6 +785,7 @@ PMIX_EXPORT void PMIx_server_deregister_nspace(const pmix_nspace_t nspace,
764785
void *cbdata)
765786
{
766787
pmix_setup_caddy_t *cd;
788+
pmix_lock_t mylock;
767789

768790
pmix_output_verbose(2, pmix_server_globals.base_output,
769791
"pmix:server deregister nspace %s",
@@ -784,6 +806,18 @@ PMIX_EXPORT void PMIx_server_deregister_nspace(const pmix_nspace_t nspace,
784806
cd->opcbfunc = cbfunc;
785807
cd->cbdata = cbdata;
786808

809+
/* if the provided callback is NULL, then substitute
810+
* our own internal cbfunc and block here */
811+
if (NULL == cbfunc) {
812+
PMIX_CONSTRUCT_LOCK(&mylock);
813+
cd->opcbfunc = opcbfunc;
814+
cd->cbdata = &mylock;
815+
PMIX_THREADSHIFT(cd, _deregister_nspace);
816+
PMIX_WAIT_THREAD(&mylock);
817+
PMIX_DESTRUCT_LOCK(&mylock);
818+
return;
819+
}
820+
787821
/* we have to push this into our event library to avoid
788822
* potential threading issues */
789823
PMIX_THREADSHIFT(cd, _deregister_nspace);
@@ -1060,9 +1094,7 @@ static void _register_client(int sd, short args, void *cbdata)
10601094

10611095
cleanup:
10621096
/* let the caller know we are done */
1063-
if (NULL != cd->opcbfunc) {
1064-
cd->opcbfunc(rc, cd->cbdata);
1065-
}
1097+
cd->opcbfunc(rc, cd->cbdata);
10661098
PMIX_RELEASE(cd);
10671099
}
10681100

@@ -1071,6 +1103,8 @@ PMIX_EXPORT pmix_status_t PMIx_server_register_client(const pmix_proc_t *proc,
10711103
pmix_op_cbfunc_t cbfunc, void *cbdata)
10721104
{
10731105
pmix_setup_caddy_t *cd;
1106+
pmix_status_t rc;
1107+
pmix_lock_t mylock;
10741108

10751109
PMIX_ACQUIRE_THREAD(&pmix_global_lock);
10761110
if (pmix_globals.init_cntr <= 0) {
@@ -1095,6 +1129,22 @@ PMIX_EXPORT pmix_status_t PMIx_server_register_client(const pmix_proc_t *proc,
10951129
cd->opcbfunc = cbfunc;
10961130
cd->cbdata = cbdata;
10971131

1132+
/* if the provided callback is NULL, then substitute
1133+
* our own internal cbfunc and block here */
1134+
if (NULL == cbfunc) {
1135+
PMIX_CONSTRUCT_LOCK(&mylock);
1136+
cd->opcbfunc = opcbfunc;
1137+
cd->cbdata = &mylock;
1138+
PMIX_THREADSHIFT(cd, _register_client);
1139+
PMIX_WAIT_THREAD(&mylock);
1140+
rc = mylock.status;
1141+
PMIX_DESTRUCT_LOCK(&mylock);
1142+
if (PMIX_SUCCESS == rc) {
1143+
rc = PMIX_OPERATION_SUCCEEDED;
1144+
}
1145+
return rc;
1146+
}
1147+
10981148
/* we have to push this into our event library to avoid
10991149
* potential threading issues */
11001150
PMIX_THREADSHIFT(cd, _register_client);
@@ -1175,16 +1225,15 @@ static void _deregister_client(int sd, short args, void *cbdata)
11751225
}
11761226

11771227
cleanup:
1178-
if (NULL != cd->opcbfunc) {
1179-
cd->opcbfunc(PMIX_SUCCESS, cd->cbdata);
1180-
}
1228+
cd->opcbfunc(PMIX_SUCCESS, cd->cbdata);
11811229
PMIX_RELEASE(cd);
11821230
}
11831231

11841232
PMIX_EXPORT void PMIx_server_deregister_client(const pmix_proc_t *proc,
11851233
pmix_op_cbfunc_t cbfunc, void *cbdata)
11861234
{
11871235
pmix_setup_caddy_t *cd;
1236+
pmix_lock_t mylock;
11881237

11891238
PMIX_ACQUIRE_THREAD(&pmix_global_lock);
11901239
if (pmix_globals.init_cntr <= 0) {
@@ -1212,6 +1261,18 @@ PMIX_EXPORT void PMIx_server_deregister_client(const pmix_proc_t *proc,
12121261
cd->opcbfunc = cbfunc;
12131262
cd->cbdata = cbdata;
12141263

1264+
/* if the provided callback is NULL, then substitute
1265+
* our own internal cbfunc and block here */
1266+
if (NULL == cbfunc) {
1267+
PMIX_CONSTRUCT_LOCK(&mylock);
1268+
cd->opcbfunc = opcbfunc;
1269+
cd->cbdata = &mylock;
1270+
PMIX_THREADSHIFT(cd, _deregister_client);
1271+
PMIX_WAIT_THREAD(&mylock);
1272+
PMIX_DESTRUCT_LOCK(&mylock);
1273+
return;
1274+
}
1275+
12151276
/* we have to push this into our event library to avoid
12161277
* potential threading issues */
12171278
PMIX_THREADSHIFT(cd, _deregister_client);

‎test/test_server.c‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ static void set_namespace(int local_size, int univ_size,
150150
PMIx_generate_regex(NODE_NAME, &regex);
151151
pmix_strncpy(info[4].key, PMIX_NODE_MAP, PMIX_MAX_KEYLEN);
152152
info[4].value.type = PMIX_STRING;
153-
info[4].value.data.string = regex;
153+
info[4].value.data.string = strdup(regex);
154154

155155
/* generate the global proc map */
156156
fill_seq_ranks_array(univ_size, 0, &ranks);
@@ -161,7 +161,7 @@ static void set_namespace(int local_size, int univ_size,
161161
free(ranks);
162162
pmix_strncpy(info[5].key, PMIX_PROC_MAP, PMIX_MAX_KEYLEN);
163163
info[5].value.type = PMIX_STRING;
164-
info[5].value.data.string = ppn;
164+
info[5].value.data.string = strdup(ppn);
165165

166166
pmix_strncpy(info[6].key, PMIX_JOB_SIZE, PMIX_MAX_KEYLEN);
167167
info[6].value.type = PMIX_UINT32;
@@ -917,17 +917,18 @@ int server_launch_clients(int local_size, int univ_size, int base_rank,
917917
/* fork/exec the test */
918918
for (n = 0; n < local_size; n++) {
919919
proc.rank = base_rank + rank_counter;
920-
if (PMIX_SUCCESS != (rc = PMIx_server_setup_fork(&proc, client_env))) {//n
921-
TEST_ERROR(("Server fork setup failed with error %d", rc));
920+
rc = PMIx_server_register_client(&proc, myuid, mygid, NULL, NULL, NULL);
921+
if (PMIX_SUCCESS != rc && PMIX_OPERATION_SUCCEEDED != rc) {
922+
TEST_ERROR(("Server register client failed with error %d", rc));
922923
PMIx_server_finalize();
923924
cli_kill_all();
924-
return rc;
925+
return 0;
925926
}
926-
if (PMIX_SUCCESS != (rc = PMIx_server_register_client(&proc, myuid, mygid, NULL, NULL, NULL))) {//n
927+
if (PMIX_SUCCESS != (rc = PMIx_server_setup_fork(&proc, client_env))) {//n
927928
TEST_ERROR(("Server fork setup failed with error %d", rc));
928929
PMIx_server_finalize();
929930
cli_kill_all();
930-
return 0;
931+
return rc;
931932
}
932933

933934
cli_info[cli_counter].pid = fork();

0 commit comments

Comments
 (0)