get_errorhandler: Finalize the error handlers

src/errhandler.jl

@@ -23,8 +23,14 @@ add_load_time_hook!(LoadTimeHookSetVal(ERRORS_RETURN,    API.MPI_ERRORS_RETURN
 
 Errhandler() = Errhandler(ERRHANDLER_NULL.val)
 
+# The predefined error handlers must never be freed, and we don't want to attach a
+# finalizer to them either: `get_errorhandler` is allowed to hand one back, in
+# which case the caller does not need to free.
+is_predefined(errh::Errhandler) =
+    errh == ERRHANDLER_NULL || errh == ERRORS_ARE_FATAL || errh == ERRORS_RETURN
+
 function free(errh::Errhandler)
-    if errh != ERRHANDLER_NULL && errh != ERRORS_ARE_FATAL && errh != ERRORS_RETURN && !Finalized()
+    if !is_predefined(errh) && !Finalized()
         # int MPI_Errhandler_free(MPI_Errhandler *errhandler)
         API.MPI_Errhandler_free(errh)
     end
@@ -59,16 +65,25 @@ Get the current [`Errhandler`](@ref) for the relevant MPI object.
 function get_errorhandler(comm::Comm)
     errh = Errhandler()
     API.MPI_Comm_get_errhandler(comm, errh)
+    # The MPI standard specifies that these behave as if a new error handler object were
+    # created, so the returned handler needs to be finalized
+    is_predefined(errh) || finalizer(free, errh)
     return errh
 end
 function get_errorhandler(win::Win)
     errh = Errhandler()
     API.MPI_Win_get_errhandler(win, errh)
+    # The MPI standard specifies that these behave as if a new error handler object were
+    # created, so the returned handler needs to be finalized
+    is_predefined(errh) || finalizer(free, errh)
     return errh
 end
 function get_errorhandler(file::File.FileHandle)
     errh = Errhandler()
     API.MPI_File_get_errhandler(file, errh)
+    # The MPI standard specifies that these behave as if a new error handler object were
+    # created, so the returned handler needs to be finalized
+    is_predefined(errh) || finalizer(free, errh)
     return errh
 end
 

test/test_errorhandler.jl

@@ -24,10 +24,41 @@ end
 error_handler_c = @cfunction(error_handler, Cvoid, (Ptr{MPI.MPI_Comm}, Ptr{Cint}))
 
 comm = MPI.Comm(DUMMY_COMM)
-errh = MPI.get_errorhandler(comm)
+errh = MPI.Errhandler()
 
 MPI.API.MPI_Comm_create_errhandler(error_handler_c, errh)
 MPI.set_errorhandler!(comm, errh)
 @test_logs (:warn, "custom error handler") MPI.API.MPI_Comm_call_errhandler(comm, DUMMY_ERR)
 
+# `get_errorhandler` owns what it returns (MPI-5.0 §9.3: it behaves as if a new
+# error handler object were created), so an owned handler must be freed at
+# finalization.  A predefined handler is owned by nobody and must be left alone.
+# COMM_SELF was set to the predefined ERRORS_ARE_FATAL above.  (COMM_WORLD is
+# no longer usable for this: DUMMY_COMM is MPI_COMM_WORLD and just got a custom
+# handler.)
+let errh = MPI.get_errorhandler(MPI.COMM_SELF)
+    @test errh == MPI.ERRORS_ARE_FATAL
+    @test MPI.is_predefined(errh)
+    finalize(errh)
+    @test errh == MPI.ERRORS_ARE_FATAL            # no finalizer, nothing freed
+end
+
+let custom = MPI.Errhandler()
+    MPI.API.MPI_Comm_create_errhandler(error_handler_c, custom)
+    @test !MPI.is_predefined(custom)
+
+    dup = MPI.Comm_dup(MPI.COMM_WORLD)
+    MPI.set_errorhandler!(dup, custom)
+    got = MPI.get_errorhandler(dup)
+    @test !MPI.is_predefined(got)
+
+    # MPI_Errhandler_free sets the handle to MPI_ERRHANDLER_NULL, so this
+    # observes that the finalizer really did free it.
+    finalize(got)
+    @test got == MPI.ERRHANDLER_NULL
+
+    MPI.free(dup)
+    MPI.free(custom)
+end
+
 MPI.Finalize()