Skip to content

Commit 8b662b6

Browse files
committed
ompi_mpi_init: fix race condition
There was a race condition in 35438ae: if multiple threads invoked ompi_mpi_init() simultaneously (which could happen from both MPI and OSHMEM), the code did not catch this condition -- Bad Things would happen. Now use an atomic cmp/set to ensure that only one thread is able to advance ompi_mpi_init from NOT_INITIALIZED to INIT_STARTED. Additionally, change the prototype of ompi_mpi_init() so that oshmem_init() can safely invoke ompi_mpi_init() multiple times (as long as MPI_FINALIZE has not started) without displaying an error. If multiple threads invoke oshmem_init() simultaneously, one of them will actually do the initialization, and the rest will loop waiting for it to complete. Signed-off-by: Jeff Squyres <[email protected]> (cherry picked from commit 67ba8da)
1 parent 73c2768 commit 8b662b6

File tree

5 files changed

+42
-30
lines changed

5 files changed

+42
-30
lines changed

ompi/mpi/c/init.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
* University of Stuttgart. All rights reserved.
1010
* Copyright (c) 2004-2005 The Regents of the University of California.
1111
* All rights reserved.
12-
* Copyright (c) 2007-2015 Cisco Systems, Inc. All rights reserved.
12+
* Copyright (c) 2007-2018 Cisco Systems, Inc. All rights reserved
1313
* Copyright (c) 2007-2008 Sun Microsystems, Inc. All rights reserved.
1414
* Copyright (c) 2015 Research Organization for Information Science
1515
* and Technology (RIST). All rights reserved.
@@ -63,9 +63,9 @@ int MPI_Init(int *argc, char ***argv)
6363
don't lose anything) */
6464

6565
if (NULL != argc && NULL != argv) {
66-
err = ompi_mpi_init(*argc, *argv, required, &provided);
66+
err = ompi_mpi_init(*argc, *argv, required, &provided, false);
6767
} else {
68-
err = ompi_mpi_init(0, NULL, required, &provided);
68+
err = ompi_mpi_init(0, NULL, required, &provided, false);
6969
}
7070

7171
/* Since we don't have a communicator to invoke an errorhandler on

ompi/mpi/c/init_thread.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
* Copyright (c) 2010 Oak Ridge National Labs. All rights reserved.
1414
* Copyright (c) 2015 Research Organization for Information Science
1515
* and Technology (RIST). All rights reserved.
16-
* Copyright (c) 2015 Cisco Systems, Inc. All rights reserved.
16+
* Copyright (c) 2015-2018 Cisco Systems, Inc. All rights reserved
1717
* Copyright (c) 2016 Los Alamos National Security, LLC. All rights
1818
* reserved.
1919
* $COPYRIGHT$
@@ -63,9 +63,9 @@ int MPI_Init_thread(int *argc, char ***argv, int required,
6363
don't lose anything) */
6464

6565
if (NULL != argc && NULL != argv) {
66-
err = ompi_mpi_init(*argc, *argv, required, provided);
66+
err = ompi_mpi_init(*argc, *argv, required, provided, false);
6767
} else {
68-
err = ompi_mpi_init(0, NULL, required, provided);
68+
err = ompi_mpi_init(0, NULL, required, provided, false);
6969
}
7070

7171
/* Since we don't have a communicator to invoke an errorhandler on

ompi/runtime/mpiruntime.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,8 @@ void ompi_mpi_thread_level(int requested, int *provided);
175175
* @param argv argv, typically from main() (IN)
176176
* @param requested Thread support that is requested (IN)
177177
* @param provided Thread support that is provided (OUT)
178+
* @param reinit_ok Return successfully (with no error) if someone has
179+
* already called ompi_mpi_init().
178180
*
179181
* @returns MPI_SUCCESS if successful
180182
* @returns Error code if unsuccessful
@@ -186,7 +188,8 @@ void ompi_mpi_thread_level(int requested, int *provided);
186188
*
187189
* It is permissable to pass in (0, NULL) for (argc, argv).
188190
*/
189-
int ompi_mpi_init(int argc, char **argv, int requested, int *provided);
191+
int ompi_mpi_init(int argc, char **argv, int requested, int *provided,
192+
bool reinit_ok);
190193

191194
/**
192195
* Finalize the Open MPI MPI environment

ompi/runtime/ompi_mpi_init.c

Lines changed: 25 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,8 @@ static void fence_release(int status, void *cbdata)
368368
*active = false;
369369
}
370370

371-
int ompi_mpi_init(int argc, char **argv, int requested, int *provided)
371+
int ompi_mpi_init(int argc, char **argv, int requested, int *provided,
372+
bool reinit_ok)
372373
{
373374
int ret;
374375
ompi_proc_t** procs;
@@ -387,29 +388,32 @@ int ompi_mpi_init(int argc, char **argv, int requested, int *provided)
387388

388389
ompi_hook_base_mpi_init_top(argc, argv, requested, provided);
389390

390-
/* Ensure that we were not already initialized or finalized.
391-
392-
This lock is held for the duration of ompi_mpi_init() and
393-
ompi_mpi_finalize(). Hence, if we get it, then no other thread
394-
is inside the critical section (and we don't have to check the
395-
*_started bool variables). */
396-
opal_atomic_rmb();
397-
int32_t state = ompi_mpi_state;
391+
/* Ensure that we were not already initialized or finalized. */
392+
int32_t expected = OMPI_MPI_STATE_NOT_INITIALIZED;
393+
int32_t desired = OMPI_MPI_STATE_INIT_STARTED;
394+
opal_atomic_wmb();
395+
if (!opal_atomic_cmpset_32(&ompi_mpi_state, &expected, desired)) {
396+
// If we failed to atomically transition ompi_mpi_state from
397+
// NOT_INITIALIZED to INIT_STARTED, then someone else already
398+
// did that, and we should return.
399+
if (expected >= OMPI_MPI_STATE_FINALIZE_STARTED) {
400+
opal_show_help("help-mpi-runtime.txt",
401+
"mpi_init: already finalized", true);
402+
return MPI_ERR_OTHER;
403+
} else if (expected >= OMPI_MPI_STATE_INIT_STARTED) {
404+
// In some cases (e.g., oshmem_shmem_init()), we may call
405+
// ompi_mpi_init() multiple times. In such cases, just
406+
// silently return successfully.
407+
if (reinit_ok) {
408+
return MPI_SUCCESS;
409+
}
398410

399-
if (state >= OMPI_MPI_STATE_FINALIZE_STARTED) {
400-
opal_show_help("help-mpi-runtime.txt",
401-
"mpi_init: already finalized", true);
402-
return MPI_ERR_OTHER;
403-
} else if (state >= OMPI_MPI_STATE_INIT_STARTED) {
404-
opal_show_help("help-mpi-runtime.txt",
405-
"mpi_init: invoked multiple times", true);
406-
return MPI_ERR_OTHER;
411+
opal_show_help("help-mpi-runtime.txt",
412+
"mpi_init: invoked multiple times", true);
413+
return MPI_ERR_OTHER;
414+
}
407415
}
408416

409-
/* Indicate that we have *started* MPI_INIT* */
410-
opal_atomic_wmb();
411-
opal_atomic_swap_32(&ompi_mpi_state, OMPI_MPI_STATE_INIT_STARTED);
412-
413417
/* Figure out the final MPI thread levels. If we were not
414418
compiled for support for MPI threads, then don't allow
415419
MPI_THREAD_MULTIPLE. Set this stuff up here early in the

oshmem/runtime/oshmem_shmem_init.c

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -144,8 +144,13 @@ int oshmem_shmem_init(int argc, char **argv, int requested, int *provided)
144144
int ret = OSHMEM_SUCCESS;
145145

146146
if (!oshmem_shmem_initialized) {
147-
if (ompi_mpi_state < OMPI_MPI_STATE_INIT_COMPLETED) {
148-
ret = ompi_mpi_init(argc, argv, requested, provided);
147+
ret = ompi_mpi_init(argc, argv, requested, provided, true);
148+
149+
// It's posible that another thread is initializing MPI and
150+
// has not completed yet. Keep checking until it is
151+
// completed.
152+
while (ompi_mpi_state < OMPI_MPI_STATE_INIT_COMPLETED) {
153+
usleep(1);
149154
}
150155

151156
if (OSHMEM_SUCCESS != ret) {

0 commit comments

Comments
 (0)