ompi/errhandler: make set/get actually thread safe

The current versions of these functions have a fatal flaw. If a
errhandler set and free call is made by another thread while the
thread calling get is between the cmpset and retain then we will
retain an invalid object. Fixing this by just using locking. This is
not a critical path so this should be ok.

Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>

ompi/mpi/c/comm_get_errhandler.c

@@ -13,7 +13,7 @@
  * Copyright (c) 2007-2009 Cisco Systems, Inc.  All rights reserved.
  * Copyright (c) 2015      Research Organization for Information Science
  *                         and Technology (RIST). All rights reserved.
- * Copyright (c) 2016      Los Alamos National Security, LLC. All rights
+ * Copyright (c) 2016-2017 Los Alamos National Security, LLC. All rights
  *                         reserved.
  * $COPYRIGHT$
  *
@@ -43,8 +43,6 @@ static const char FUNC_NAME[] = "MPI_Comm_get_errhandler";
 
 int MPI_Comm_get_errhandler(MPI_Comm comm, MPI_Errhandler *errhandler)
 {
-    MPI_Errhandler tmp;
-
     /* Error checking */
     MEMCHECKER(
         memchecker_comm(comm);
@@ -65,16 +63,12 @@ int MPI_Comm_get_errhandler(MPI_Comm comm, MPI_Errhandler *errhandler)
     }
   }
 
-  /* On 64 bits environments we have to make sure the reading of the
+  opal_mutex_lock (&comm->c_lock);
-     error_handler became atomic. */
-  do {
-      tmp = comm->error_handler;
-  } while (!OPAL_ATOMIC_BOOL_CMPSET_PTR(&(comm->error_handler), tmp, tmp));
-
   /* Retain the errhandler, corresponding to object refcount decrease
      in errhandler_free.c. */
-  *errhandler = tmp;
+  OBJ_RETAIN(comm->error_handler);
-  OBJ_RETAIN(tmp);
+  *errhandler = comm->error_handler;
+  opal_mutex_unlock (&comm->c_lock);
 
   /* All done */
 

ompi/mpi/c/comm_set_errhandler.c

@@ -12,7 +12,7 @@
  *                         All rights reserved.
  * Copyright (c) 2015      Research Organization for Information Science
  *                         and Technology (RIST). All rights reserved.
- * Copyright (c) 2016      Los Alamos National Security, LLC. All rights
+ * Copyright (c) 2016-2017 Los Alamos National Security, LLC. All rights
  *                         reserved.
  * $COPYRIGHT$
  *
@@ -69,11 +69,12 @@ int MPI_Comm_set_errhandler(MPI_Comm comm, MPI_Errhandler errhandler)
     /* Prepare the new error handler */
     OBJ_RETAIN(errhandler);
 
-    /* Ditch the old errhandler, and decrement its refcount.  On 64
+    opal_mutex_lock (&comm->c_lock);
-       bits environments we have to make sure the reading of the
+    /* Ditch the old errhandler, and decrement its refcount. */
-       error_handler became atomic. */
+    tmp = comm->error_handler;
-    tmp = OPAL_ATOMIC_SWAP_PTR(&comm->error_handler, errhandler);
+    comm->error_handler = errhandler;
     OBJ_RELEASE(tmp);
+    opal_mutex_unlock (&comm->c_lock);
 
     /* All done */
     return MPI_SUCCESS;

ompi/mpi/c/file_get_errhandler.c

@@ -13,7 +13,7 @@
  * Copyright (c) 2008      Cisco Systems, Inc.  All rights reserved.
  * Copyright (c) 2015      Research Organization for Information Science
  *                         and Technology (RIST). All rights reserved.
- * Copyright (c) 2016      Los Alamos National Security, LLC. All rights
+ * Copyright (c) 2016-2017 Los Alamos National Security, LLC. All rights
  *                         reserved.
  * $COPYRIGHT$
  *
@@ -42,8 +42,6 @@ static const char FUNC_NAME[] = "MPI_File_get_errhandler";
 
 int MPI_File_get_errhandler( MPI_File file, MPI_Errhandler *errhandler)
 {
-    MPI_Errhandler tmp;
-
     OPAL_CR_NOOP_PROGRESS();
 
   /* Error checking */
@@ -64,16 +62,12 @@ int MPI_File_get_errhandler( MPI_File file, MPI_Errhandler *errhandler)
     }
   }
 
-  /* On 64 bits environments we have to make sure the reading of the
+  opal_mutex_lock (&file->f_mutex);
-     error_handler became atomic. */
-  do {
-      tmp = file->error_handler;
-  } while (!OPAL_ATOMIC_BOOL_CMPSET_PTR(&(file->error_handler), tmp, tmp));
-
   /* Retain the errhandler, corresponding to object refcount
      decrease in errhandler_free.c. */
-  *errhandler = tmp;
+  *errhandler = file->error_handler;
-  OBJ_RETAIN(tmp);
+  OBJ_RETAIN(file->error_handler);
+  opal_mutex_unlock (&file->f_mutex);
 
   /* All done */
 

ompi/mpi/c/file_set_errhandler.c

@@ -13,7 +13,7 @@
  * Copyright (c) 2008      Cisco Systems, Inc.  All rights reserved.
  * Copyright (c) 2015      Research Organization for Information Science
  *                         and Technology (RIST). All rights reserved.
- * Copyright (c) 2016      Los Alamos National Security, LLC. All rights
+ * Copyright (c) 2016-2017 Los Alamos National Security, LLC. All rights
  *                         reserved.
  * $COPYRIGHT$
  *
@@ -68,11 +68,12 @@ int MPI_File_set_errhandler( MPI_File file, MPI_Errhandler errhandler)
     /* Prepare the new error handler */
     OBJ_RETAIN(errhandler);
 
-    /* Ditch the old errhandler, and decrement its refcount.  On 64
+    opal_mutex_lock (&file->f_mutex);
-       bits environments we have to make sure the reading of the
+    /* Ditch the old errhandler, and decrement its refcount. */
-       error_handler became atomic. */
+    tmp = file->error_handler;
-    tmp = OPAL_ATOMIC_SWAP_PTR (&file->error_handler, errhandler);
+    file->error_handler = errhandler;
     OBJ_RELEASE(tmp);
+    opal_mutex_unlock (&file->f_mutex);
 
     /* All done */
     return MPI_SUCCESS;

ompi/mpi/c/win_get_errhandler.c

@@ -42,8 +42,6 @@ static const char FUNC_NAME[] = "MPI_Win_get_errhandler";
 
 int MPI_Win_get_errhandler(MPI_Win win, MPI_Errhandler *errhandler)
 {
-    MPI_Errhandler tmp;
-
     OPAL_CR_NOOP_PROGRESS();
 
     if (MPI_PARAM_CHECK) {
@@ -57,16 +55,12 @@ int MPI_Win_get_errhandler(MPI_Win win, MPI_Errhandler *errhandler)
         }
     }
 
-    /* On 64 bits environments we have to make sure the reading of the
+    opal_mutex_lock (&win->w_lock);
-       error_handler became atomic. */
-    do {
-        tmp = win->error_handler;
-    } while (!OPAL_ATOMIC_BOOL_CMPSET_PTR(&(win->error_handler), tmp, tmp));
-
     /* Retain the errhandler, corresponding to object refcount
        decrease in errhandler_free.c. */
     OBJ_RETAIN(win->error_handler);
     *errhandler = win->error_handler;
+    opal_mutex_unlock (&win->w_lock);
 
     /* All done */
     return MPI_SUCCESS;

ompi/mpi/c/win_set_errhandler.c

@@ -13,7 +13,7 @@
  * Copyright (c) 2008-2009 Cisco Systems, Inc.  All rights reserved.
  * Copyright (c) 2015      Research Organization for Information Science
  *                         and Technology (RIST). All rights reserved.
- * Copyright (c) 2016      Los Alamos National Security, LLC. All rights
+ * Copyright (c) 2016-2017 Los Alamos National Security, LLC. All rights
  *                         reserved.
  * $COPYRIGHT$
  *
@@ -63,11 +63,12 @@ int MPI_Win_set_errhandler(MPI_Win win, MPI_Errhandler errhandler)
     /* Prepare the new error handler */
     OBJ_RETAIN(errhandler);
 
-    /* Ditch the old errhandler, and decrement its refcount.  On 64
+    opal_mutex_lock (&win->w_lock);
-       bits environments we have to make sure the reading of the
+    /* Ditch the old errhandler, and decrement its refcount. */
-       error_handler became atomic. */
+    tmp = win->error_handler;
-    tmp = OPAL_ATOMIC_SWAP_PTR(&win->error_handler, errhandler);
+    win->error_handler = errhandler;
     OBJ_RELEASE(tmp);
+    opal_mutex_unlock (&win->w_lock);
 
     /* All done */
     return MPI_SUCCESS;