Open MPI logo

Open MPI Development Mailing List Archives

  |   Home   |   Support   |   FAQ   |   all Development mailing list

From: Jeff Squyres (jsquyres_at_[hidden])
Date: 2007-08-25 16:32:39


On Aug 25, 2007, at 1:20 PM, Rainer Keller wrote:

>>> /* Protection for recursive invocation */
>>> if (have_been_invoked) {
>>> return OMPI_SUCCESS;
>>> }
>>> have_been_invoked = true;
>>
>> This, IMHO, is a wrong thing to do. The intent of ompi_mpi_abort()
>> was that it never returned. But now it is? That seems wrong to me.

If you read the rest of my e-mail, I was saying that I think it's
wrong, too. :-) You did not answer my other question...

> Recursive or not, if we add __opal_attribute_noreturn__, the
> compiler (and
> Coverity, as David said in a private mail) then knows, that the
> function
> won't exit.

The real question is whether the function can be (or is) called
recursively. If it can't / isn't, then we can replace the first
"return OMPI_SUCCESS" with an endless sleep and we should be ok. It
is *not* ok to replace it with "exit(errcode)" for the
THREAD_MULTIPLE scenarios where a different thread is busy processing
the actual abort. And do we need better protection for
have_been_invoked, such as a lock or an atomic operation?

If this function can't be / is never invoked recursively, then we
should make it noisy (in a developer / debug build) if it ever *is*
invoked recursively and probably put it into either an endless sleep
or invoke abort() because that will clearly be a bug.

I unfortunately do not remember whether I put that recursive
protection in to fix a real problem or whether I was trying to be
(incorrectly) proactive...

Comments?

-- 
Jeff Squyres
Cisco Systems