Refs trac:502, #503. This commit as a result of review of r12122.

* Update comments in some MPI_FILE_* functions to reflect that the
   MPI specs have different page numbers in the ps and pdf (woof!).
 * Update comments to say "Retain" where we meant retain (not "return)
 * Add a check in MPI_ERRHANDLER_FREE to raise an MPI exception if the
   user attempts to free an intrinsic errhandler *and* the refcount is
   1 (meaning that it would actually free the intrinsic).  This
   protects erroneous programs from segv'ing.
 * Remove lengthy comment from comm_get_errhandler.c which is no
   longer valid (because of the MPI-2 errata that says that users *do*
   have to call MPI_ERRHANDLER_FREE).

This commit was SVN r12128.

The following SVN revision numbers were found above:
  r12122 --> open-mpi/ompi@407b3cb788

The following Trac tickets were found above:
  Ticket 502 --> https://svn.open-mpi.org/trac/ompi/ticket/502

ompi/mpi/c/comm_get_errhandler.c

@@ -48,29 +48,8 @@ int MPI_Comm_get_errhandler(MPI_Comm comm, MPI_Errhandler *errhandler)
     }
   }
 
-  /* Return the errhandler.  A quandry.  Should we increase the
+  /* Retain the errhandler, corresponding to object refcount
-     refcount here?
+     decrease in errhandler_free.c. */
-
-     - Consider that if we *get* an errhandler, we don't have to free
-       it.  It's just a handle that was returned to the user.  If they
-       never free it (and we increased the refcount), then it'll never
-       be freed.
-
-     - However, if we *don't* increase it and the user *does* free it,
-       then this could cause the refcount to go to 0 prematurely, and
-       a communicator could be left with a stale error handler.
-
-     Add to the mix that MPI-1:196:8-11 says that MPI_ERRHANDLER_FREE
-     will only free the error handler when all the communicators using
-     it have been freed.
-
-     All in all, it seems like we should increase the refcount to be
-     safe here.  We're still conformant -- error handlers won't be
-     freed until all the communicators (or other objects using them)
-     are freed *and* any outstanding handles returned by this function
-     (or its peers) are also freed.
-  */
-
   OBJ_RETAIN(comm->error_handler);
   *errhandler = comm->error_handler;
 

ompi/mpi/c/errhandler_free.c

@@ -37,13 +37,33 @@ int MPI_Errhandler_free(MPI_Errhandler *errhandler)
 
   if (MPI_PARAM_CHECK) {
     OMPI_ERR_INIT_FINALIZE(FUNC_NAME);
-    if (NULL == errhandler) {
+    /* Raise an MPI exception if we got NULL or if we got an intrinsic
+       *and* the reference count is 1 (meaning that this FREE would
+       actually free the underlying intrinsic object).  This is ugly
+       but necessary -- see below. */
+    if (NULL == errhandler ||
+        (ompi_errhandler_is_intrinsic(*errhandler) && 
+         1 == (*errhandler)->super.obj_reference_count)) {
       return OMPI_ERRHANDLER_INVOKE(MPI_COMM_WORLD, MPI_ERR_ARG,
                                    "MPI_Errhandler_free");
     }
   }
 
-  /* We have a valid errhandler, release it */
+  /* Return the errhandler.  According to MPI-2 errata, any errhandler
+     obtained by MPI_*_GET_ERRHANDLER or MPI_ERRHANDLER_GET must also
+     be freed by MPI_ERRHANDLER_FREE (including intrinsic error
+     handlers).  For example, this is valid:
+
+     int main() {
+         MPI_Errhandler errhdl;
+         MPI_Init(NULL, NULL);
+         MPI_Comm_get_errhandler(MPI_COMM_WORLD, &errhdl);
+         MPI_Errhandler_free(&errhdl);
+         MPI_Finalize();
+         return 0;
+     }
+     
+     So decrease the refcount here. */
 
   OBJ_RELEASE(*errhandler);
   *errhandler = MPI_ERRHANDLER_NULL;

ompi/mpi/c/file_delete.c

@@ -50,11 +50,12 @@ int MPI_File_delete(char *filename, MPI_Info info)
         OMPI_ERRHANDLER_CHECK(rc, MPI_FILE_NULL, rc, FUNC_NAME);
     }
 
-    /* Note that MPI-2:9.7 (p261) says that errors in MPI_FILE_OPEN
+    /* Note that MPI-2:9.7 (p265 in the ps; 261 in the pdf) says that
-       (before the file handle is created) should invoke the default
+       errors in MPI_FILE_OPEN (before the file handle is created)
-       error handler on MPI_FILE_NULL.  Hence, if we get a file handle
+       should invoke the default error handler on MPI_FILE_NULL.
-       out of ompi_file_open(), invoke the error handler on that.  If
+       Hence, if we get a file handle out of ompi_file_open(), invoke
-       not, invoke the error handler on MPI_FILE_NULL. */
+       the error handler on that.  If not, invoke the error handler on
+       MPI_FILE_NULL. */
 
     /* The io framework is only initialized lazily.  If it hasn't
        already been initialized, do so now (note that MPI_FILE_OPEN

ompi/mpi/c/file_get_errhandler.c

@@ -39,8 +39,9 @@ int MPI_File_get_errhandler( MPI_File file, MPI_Errhandler *errhandler)
   if (MPI_PARAM_CHECK) {
     OMPI_ERR_INIT_FINALIZE(FUNC_NAME);
 
-    /* Note that MPI-2:9.7 (p261) explicitly says that you are
+    /* Note that MPI-2:9.7 (p265 in the ps; 261 in the pdf) explicitly
-       allowed to set the error handler on MPI_FILE_NULL */
+       says that you are allowed to set the error handler on
+       MPI_FILE_NULL */
 
     if (NULL == file) {
       return OMPI_ERRHANDLER_INVOKE(MPI_COMM_WORLD, MPI_ERR_FILE,
@@ -51,9 +52,8 @@ int MPI_File_get_errhandler( MPI_File file, MPI_Errhandler *errhandler)
     }
   }
 
-  /* Return the errhandler.  See lengthy comment in
+  /* Retain the errhandler, corresponding to object refcount
-     comm_get_errhandler.c about why we increment the refcount. */
+     decrease in errhandler_free.c. */
-
   OBJ_RETAIN(file->error_handler);
   *errhandler = file->error_handler;
 

ompi/mpi/c/file_open.c

@@ -56,11 +56,12 @@ int MPI_File_open(MPI_Comm comm, char *filename, int amode,
 	
     }
 
-    /* Note that MPI-2:9.7 (p261) says that errors in MPI_FILE_OPEN
+    /* Note that MPI-2:9.7 (p265 in the ps; p261 in the pdf) says that
-       (before the file handle is created) should invoke the default
+       errors in MPI_FILE_OPEN (before the file handle is created)
-       error handler on MPI_FILE_NULL.  Hence, if we get a file handle
+       should invoke the default error handler on MPI_FILE_NULL.
-       out of ompi_file_open(), invoke the error handler on that.  If
+       Hence, if we get a file handle out of ompi_file_open(), invoke
-       not, invoke the error handler on MPI_FILE_NULL. */
+       the error handler on that.  If not, invoke the error handler on
+       MPI_FILE_NULL. */
 
     /* The io framework is only initialized lazily.  If it hasn't
        already been initialized, do so now (note that MPI_FILE_OPEN

ompi/mpi/c/file_set_errhandler.c

@@ -39,8 +39,9 @@ int MPI_File_set_errhandler( MPI_File file, MPI_Errhandler errhandler)
     if (MPI_PARAM_CHECK) {
         OMPI_ERR_INIT_FINALIZE(FUNC_NAME);
 
-        /* Note that MPI-2:9.7 (p261) explicitly says that you are
+        /* Note that MPI-2:9.7 (p265 in the ps; p261 in the pdf)
-           allowed to set the error handler on MPI_FILE_NULL */
+           explicitly says that you are allowed to set the error
+           handler on MPI_FILE_NULL */
 
         if (NULL == file) {
             return OMPI_ERRHANDLER_INVOKE(MPI_FILE_NULL, MPI_ERR_FILE,

ompi/mpi/c/win_get_errhandler.c

@@ -45,8 +45,8 @@ int MPI_Win_get_errhandler(MPI_Win win, MPI_Errhandler *errhandler)
         }
     }
 
-    /* Return the errhandler.  See lengthy comment in
+    /* Retain the errhandler, corresponding to object refcount
-       comm_get_errhandler.c about why we increment the refcount. */
+       decrease in errhandler_free.c. */
     OBJ_RETAIN(win->error_handler);
     *errhandler = win->error_handler;