Merge pull request open-mpi#13423 from naughtont3/tjn-fixdatarace-lastused

naughtont3 · web-flow · commit dcb4a0144fef · 2025-10-21T11:03:05.000-04:00
avoid data race on set LASTUSED attribute
diff --git a/ompi/errhandler/errcode.c b/ompi/errhandler/errcode.c
@@ -280,7 +280,8 @@ int ompi_mpi_errcode_finalize (void)
          * we have to free.
          */
         errc = (ompi_mpi_errcode_t *)opal_pointer_array_get_item(&ompi_mpi_errcodes, i);
-        OBJ_RELEASE (errc);
+        if (NULL != errc)
+            OBJ_RELEASE (errc);
     }
 
     OBJ_DESTRUCT(&ompi_success);
@@ -440,13 +441,24 @@ int ompi_mpi_errcode_remove(int errnum)
             if (OPAL_SUCCESS == ret) {
                 if (errnum == ompi_mpi_errcode_lastused) {
                     ompi_mpi_errcode_lastused--;
+                    ret = ompi_mpi_errcode_lastused;
                 }
             }
         }
     }
 
     opal_mutex_unlock(&errcode_lock);
 
+    /* Release the object on success */
+    if (ret >= 0) {
+        OBJ_RELEASE(errcodep);
+    }
+
+    /*
+     * Return lastused value captured under lock so caller has
+     * consistent value to set MPI_LASTUSEDCODE attribute.
+     * On error, we return less than zero (e.g., OMPI_ERROR).
+     */
     return ret;
 }
 
@@ -472,6 +484,7 @@ int ompi_mpi_errclass_remove(int errclass)
                 if (OPAL_SUCCESS == ret) {
                     if (errclass == ompi_mpi_errcode_lastused) {
                         ompi_mpi_errcode_lastused--;
+                        ret = ompi_mpi_errcode_lastused;
                     }
                 }
             }
@@ -480,6 +493,16 @@ int ompi_mpi_errclass_remove(int errclass)
 
     opal_mutex_unlock(&errcode_lock);
 
+    /* Release the object on success */
+    if (ret >= 0) {
+        OBJ_RELEASE(errcodep);
+    }
+
+    /*
+     * Return lastused value captured under lock so caller has
+     * consistent value to set MPI_LASTUSEDCODE attribute.
+     * On error, we return less than zero (e.g., OMPI_ERROR).
+     */
     return ret;
 }
 
diff --git a/ompi/mpi/c/add_error_class.c.in b/ompi/mpi/c/add_error_class.c.in
@@ -33,7 +33,7 @@
 
 PROTOTYPE ERROR_CLASS add_error_class(INT_OUT errorclass)
 {
-    int err_class;
+    int eclass;
     int rc;
 
     if ( MPI_PARAM_CHECK ) {
@@ -45,10 +45,10 @@ PROTOTYPE ERROR_CLASS add_error_class(INT_OUT errorclass)
         }
     }
 
-    err_class = ompi_mpi_errclass_add();
-    if ( 0 > err_class ) {
+    eclass = ompi_mpi_errclass_add();
+    if ( 0 > eclass ) {
         return OMPI_ERRHANDLER_NOHANDLE_INVOKE(MPI_ERR_INTERN,
-                                      FUNC_NAME);
+                                               FUNC_NAME);
     }
 
 
@@ -61,13 +61,14 @@ PROTOTYPE ERROR_CLASS add_error_class(INT_OUT errorclass)
                               MPI_COMM_WORLD,
                               &MPI_COMM_WORLD->c_keyhash,
                               MPI_LASTUSEDCODE,
-                              ompi_mpi_errcode_lastused,
+                              eclass,   /* lastused */
                               true);
     if ( MPI_SUCCESS != rc ) {
-	return OMPI_ERRHANDLER_NOHANDLE_INVOKE(rc, FUNC_NAME);
+        return OMPI_ERRHANDLER_NOHANDLE_INVOKE(rc, FUNC_NAME);
     }
 
-    *errorclass = err_class;
+    *errorclass = eclass;
+
     return MPI_SUCCESS;
 }
 
diff --git a/ompi/mpi/c/add_error_code.c.in b/ompi/mpi/c/add_error_code.c.in
@@ -33,30 +33,30 @@
 
 PROTOTYPE ERROR_CLASS add_error_code(INT errorclass, INT_OUT errorcode)
 {
-    int code;
+    int ecode;
     int rc;
 
     if ( MPI_PARAM_CHECK ) {
         OMPI_ERR_INIT_FINALIZE(FUNC_NAME);
 
         if ( ompi_mpi_errcode_is_invalid(errorclass) )
             return OMPI_ERRHANDLER_NOHANDLE_INVOKE(MPI_ERR_ARG,
-                                          FUNC_NAME);
+                                                   FUNC_NAME);
 
-	if ( !ompi_mpi_errnum_is_class ( errorclass) )
-	    return OMPI_ERRHANDLER_NOHANDLE_INVOKE(MPI_ERR_ARG,
-					  FUNC_NAME);
+        if ( !ompi_mpi_errnum_is_class ( errorclass) )
+            return OMPI_ERRHANDLER_NOHANDLE_INVOKE(MPI_ERR_ARG,
+                                                   FUNC_NAME);
 
         if (NULL == errorcode) {
-            return OMPI_ERRHANDLER_NOHANDLE_INVOKE(
-                                          MPI_ERR_ARG, FUNC_NAME);
+            return OMPI_ERRHANDLER_NOHANDLE_INVOKE(MPI_ERR_ARG,
+                                                   FUNC_NAME);
         }
     }
 
-    code = ompi_mpi_errcode_add ( errorclass);
-    if ( 0 > code ) {
+    ecode = ompi_mpi_errcode_add ( errorclass);
+    if ( 0 > ecode ) {
         return OMPI_ERRHANDLER_NOHANDLE_INVOKE(MPI_ERR_INTERN,
-                                      FUNC_NAME);
+                                               FUNC_NAME);
     }
 
     /*
@@ -68,12 +68,13 @@ PROTOTYPE ERROR_CLASS add_error_code(INT errorclass, INT_OUT errorcode)
                               MPI_COMM_WORLD,
                               &MPI_COMM_WORLD->c_keyhash,
                               MPI_LASTUSEDCODE,
-                              ompi_mpi_errcode_lastused,
+                              ecode,    /* lastused */
                               true);
     if ( MPI_SUCCESS != rc ) {
-	return OMPI_ERRHANDLER_NOHANDLE_INVOKE(rc, FUNC_NAME);
+        return OMPI_ERRHANDLER_NOHANDLE_INVOKE(rc, FUNC_NAME);
     }
 
-    *errorcode = code;
+    *errorcode = ecode;
+
     return MPI_SUCCESS;
 }
diff --git a/ompi/mpi/c/remove_error_class.c.in b/ompi/mpi/c/remove_error_class.c.in
@@ -34,6 +34,7 @@
 
 PROTOTYPE ERROR_CLASS remove_error_class(INT errorclass)
 {
+    int lastused;
     int rc;
 
     if ( MPI_PARAM_CHECK ) {
@@ -46,10 +47,10 @@ PROTOTYPE ERROR_CLASS remove_error_class(INT errorclass)
 
     }
 
-    rc = ompi_mpi_errclass_remove( errorclass );
-    if ( MPI_SUCCESS != rc ) {
+    lastused = ompi_mpi_errclass_remove( errorclass );
+    if ( 0 > lastused ) {
         return OMPI_ERRHANDLER_NOHANDLE_INVOKE(MPI_ERR_INTERN,
-                                      FUNC_NAME);
+                                               FUNC_NAME);
     }
 
 
@@ -62,7 +63,7 @@ PROTOTYPE ERROR_CLASS remove_error_class(INT errorclass)
                               MPI_COMM_WORLD,
                               &MPI_COMM_WORLD->c_keyhash,
                               MPI_LASTUSEDCODE,
-                              ompi_mpi_errcode_lastused,
+                              lastused,
                               true);
     if ( MPI_SUCCESS != rc ) {
         return OMPI_ERRHANDLER_NOHANDLE_INVOKE(rc, FUNC_NAME);
diff --git a/ompi/mpi/c/remove_error_code.c.in b/ompi/mpi/c/remove_error_code.c.in
@@ -34,7 +34,7 @@
 
 PROTOTYPE ERROR_CLASS remove_error_code(INT errorcode)
 {
-    int code;
+    int lastused;
     int rc;
 
     if ( MPI_PARAM_CHECK ) {
@@ -47,10 +47,10 @@ PROTOTYPE ERROR_CLASS remove_error_code(INT errorcode)
 
     }
 
-    code = ompi_mpi_errcode_remove ( errorcode );
-    if ( 0 > code ) {
+    lastused = ompi_mpi_errcode_remove ( errorcode );
+    if ( 0 > lastused ) {
         return OMPI_ERRHANDLER_NOHANDLE_INVOKE(MPI_ERR_INTERN,
-                                      FUNC_NAME);
+                                               FUNC_NAME);
     }
 
     /*
@@ -62,7 +62,7 @@ PROTOTYPE ERROR_CLASS remove_error_code(INT errorcode)
                               MPI_COMM_WORLD,
                               &MPI_COMM_WORLD->c_keyhash,
                               MPI_LASTUSEDCODE,
-                              ompi_mpi_errcode_lastused,
+                              lastused,
                               true);
     if ( MPI_SUCCESS != rc ) {
         return OMPI_ERRHANDLER_NOHANDLE_INVOKE(rc, FUNC_NAME);

Original file line number	Diff line number	Diff line change
`@@ -280,7 +280,8 @@ int ompi_mpi_errcode_finalize (void)`
`280`	`280`	`* we have to free.`
`281`	`281`	`*/`
`282`	`282`	`errc = (ompi_mpi_errcode_t *)opal_pointer_array_get_item(&ompi_mpi_errcodes, i);`
`283`		`- OBJ_RELEASE (errc);`
	`283`	`+ if (NULL != errc)`
	`284`	`+ OBJ_RELEASE (errc);`
`284`	`285`	`}`
`285`	`286`
`286`	`287`	`OBJ_DESTRUCT(&ompi_success);`
`@@ -440,13 +441,24 @@ int ompi_mpi_errcode_remove(int errnum)`
`440`	`441`	`if (OPAL_SUCCESS == ret) {`
`441`	`442`	`if (errnum == ompi_mpi_errcode_lastused) {`
`442`	`443`	`ompi_mpi_errcode_lastused--;`
	`444`	`+ ret = ompi_mpi_errcode_lastused;`
`443`	`445`	`}`
`444`	`446`	`}`
`445`	`447`	`}`
`446`	`448`	`}`
`447`	`449`
`448`	`450`	`opal_mutex_unlock(&errcode_lock);`
`449`	`451`
	`452`	`+ /* Release the object on success */`
	`453`	`+ if (ret >= 0) {`
	`454`	`+ OBJ_RELEASE(errcodep);`
	`455`	`+ }`
	`456`	`+`
	`457`	`+ /*`
	`458`	`+ * Return lastused value captured under lock so caller has`
	`459`	`+ * consistent value to set MPI_LASTUSEDCODE attribute.`
	`460`	`+ * On error, we return less than zero (e.g., OMPI_ERROR).`
	`461`	`+ */`
`450`	`462`	`return ret;`
`451`	`463`	`}`
`452`	`464`
`@@ -472,6 +484,7 @@ int ompi_mpi_errclass_remove(int errclass)`
`472`	`484`	`if (OPAL_SUCCESS == ret) {`
`473`	`485`	`if (errclass == ompi_mpi_errcode_lastused) {`
`474`	`486`	`ompi_mpi_errcode_lastused--;`
	`487`	`+ ret = ompi_mpi_errcode_lastused;`
`475`	`488`	`}`
`476`	`489`	`}`
`477`	`490`	`}`
`@@ -480,6 +493,16 @@ int ompi_mpi_errclass_remove(int errclass)`
`480`	`493`
`481`	`494`	`opal_mutex_unlock(&errcode_lock);`
`482`	`495`
	`496`	`+ /* Release the object on success */`
	`497`	`+ if (ret >= 0) {`
	`498`	`+ OBJ_RELEASE(errcodep);`
	`499`	`+ }`
	`500`	`+`
	`501`	`+ /*`
	`502`	`+ * Return lastused value captured under lock so caller has`
	`503`	`+ * consistent value to set MPI_LASTUSEDCODE attribute.`
	`504`	`+ * On error, we return less than zero (e.g., OMPI_ERROR).`
	`505`	`+ */`
`483`	`506`	`return ret;`
`484`	`507`	`}`
`485`	`508`
Original file line number	Diff line number	Diff line change
`@@ -33,7 +33,7 @@`
`33`	`33`
`34`	`34`	`PROTOTYPE ERROR_CLASS add_error_class(INT_OUT errorclass)`
`35`	`35`	`{`
`36`		`- int err_class;`
	`36`	`+ int eclass;`
`37`	`37`	`int rc;`
`38`	`38`
`39`	`39`	`if ( MPI_PARAM_CHECK ) {`
`@@ -45,10 +45,10 @@ PROTOTYPE ERROR_CLASS add_error_class(INT_OUT errorclass)`
`45`	`45`	`}`
`46`	`46`	`}`
`47`	`47`
`48`		`- err_class = ompi_mpi_errclass_add();`
`49`		`- if ( 0 > err_class ) {`
	`48`	`+ eclass = ompi_mpi_errclass_add();`
	`49`	`+ if ( 0 > eclass ) {`
`50`	`50`	`return OMPI_ERRHANDLER_NOHANDLE_INVOKE(MPI_ERR_INTERN,`
`51`		`- FUNC_NAME);`
	`51`	`+ FUNC_NAME);`
`52`	`52`	`}`
`53`	`53`
`54`	`54`
`@@ -61,13 +61,14 @@ PROTOTYPE ERROR_CLASS add_error_class(INT_OUT errorclass)`
`61`	`61`	`MPI_COMM_WORLD,`
`62`	`62`	`&MPI_COMM_WORLD->c_keyhash,`
`63`	`63`	`MPI_LASTUSEDCODE,`
`64`		`- ompi_mpi_errcode_lastused,`
	`64`	`+ eclass, /* lastused */`
`65`	`65`	`true);`
`66`	`66`	`if ( MPI_SUCCESS != rc ) {`
`67`		`- return OMPI_ERRHANDLER_NOHANDLE_INVOKE(rc, FUNC_NAME);`
	`67`	`+ return OMPI_ERRHANDLER_NOHANDLE_INVOKE(rc, FUNC_NAME);`
`68`	`68`	`}`
`69`	`69`
`70`		`- *errorclass = err_class;`
	`70`	`+ *errorclass = eclass;`
	`71`	`+`
`71`	`72`	`return MPI_SUCCESS;`
`72`	`73`	`}`
`73`	`74`