Skip to content

Conversation

rhc54
Copy link
Member

@rhc54 rhc54 commented Jun 13, 2025

Server upcall functions were required to return prior to executing the callback function. This was a limitation of the implementation and not a requirement of the Standard - the statements should never have been included here.

FWIW: the limitation has subsequently been removed from the reference implementation.

In the pmix_server_iof_fn_t, clarify that an error status can be returned either directly by the function or in the callback function. It does not require that the host execute the callback to return an error, as was implied by the current language.

@rhc54 rhc54 added this to the PMIx v6.0 Standard milestone Jun 13, 2025
@rhc54 rhc54 self-assigned this Jun 13, 2025
@rhc54 rhc54 added the Errata label Jun 13, 2025
@rhc54
Copy link
Member Author

rhc54 commented Jun 13, 2025

Please use emoji reactions ON THIS COMMENT to indicate your position on this proposal.

  • You do not need to vote on every proposal
  • If you have no opinion, don't vote - that is also useful data
  • If you've already commented on this issue, please still vote so
    we know your current thoughts
  • Not all proposals solve exactly the same problem, so we may end
    up accepting proposals that appear to have some overlap
    This is not a binding majority-rule vote, but it will be a very
    significant input into the corresponding ASC decision.

Here are the meanings for the emojis:

  • Hooray or Rocket: I support this so strongly that I
    want to be an advocate for it
  • Heart: I think this is an ideal solution
  • Thumbs up: I'd be happy with this solution
  • Confused: I'd rather we not do this, but I can tolerate it
  • Thumbs down: I'd be actively unhappy, and may even consider
    other technologies instead
    If you want to explain in more detail, feel free to add another
    comment, but please also vote on this comment.

Server upcall functions were required to return prior to executing
the callback function. This was a limitation of the implementation
and not a requirement of the Standard - the statements should
never have been included here.

FWIW: the limitation has subsequently been removed from the
reference implementation.

In the pmix_server_iof_fn_t, clarify that an error status can be
returned either directly by the function or in the callback function.
It does not require that the host execute the callback to return
an error, as was implied by the current language.

Signed-off-by: Ralph Castain <[email protected]>
@abouteiller
Copy link
Contributor

Note for when merging, we will need to add a changelog errata entry

@neicker
Copy link

neicker commented Jul 17, 2025

While this seem to cover all such mentions in Server-Specific Interfaces section, I found a few more also in other parts:

  • Chap_API_Event.tex:118: "The callback function must not be executed prior to returning from the \ac{API}, and ..."
  • Chap_API_Query.tex:191: "Note that the library must not invoke the callback function prior to returning from the \ac{API}."

Furthermore a similar sentence is in one of the commands:

  • pmix.sty:900: "\newcommand{\returnsimplenb}{.... Note that the library must not invoke the callback function prior to returning from the \ac{API}. ... .}"

This one is used at several places all over the document including in the Server-Specific Interfaces section.

Unclear to me if we want to handle them in this PR or if this would be a new issue.

@neicker
Copy link

neicker commented Jul 17, 2025

In the Revision History section for version 2.1 it is stated:

  • Clarify return values for non-blocking APIs and emphasize that callback functions must not be invoked prior to return from the API

Thus, it seems there was a clear intention at that time. Do we have to explain why we have a different view on this now?

@rhc54
Copy link
Member Author

rhc54 commented Jul 17, 2025

In the Revision History section for version 2.1 it is stated:

* Clarify return values for non-blocking APIs and emphasize that callback functions must not be invoked prior to return from the API

Thus, it seems there was a clear intention at that time. Do we have to explain why we have a different view on this now?

We'll note that we changed it in the revision history. Probably have a little blurb, but nothing too verbose. Just didn't want to include it here as the revision history for v6 is created in a different PR - no need to maximize conflict.

@rhc54
Copy link
Member Author

rhc54 commented Jul 17, 2025

While this seem to cover all such mentions in Server-Specific Interfaces section, I found a few more also in other parts:

* Chap_API_Event.tex:118: "The callback function must not be executed prior to returning from the \ac{API}, and ..."

* Chap_API_Query.tex:191: "Note that the library must not invoke the callback function prior to returning from the \ac{API}."

Furthermore a similar sentence is in one of the commands:

* pmix.sty:900: "\newcommand{\returnsimplenb}{.... Note that the library must not invoke the callback function prior to returning from the \ac{API}. ... .}"

This one is used at several places all over the document including in the Server-Specific Interfaces section.

Unclear to me if we want to handle them in this PR or if this would be a new issue.

I can fix them here - thanks for spotting them!

@rhc54
Copy link
Member Author

rhc54 commented Jul 17, 2025

@neicker I fixed the places you mentioned. See if they now look okay.

@neicker
Copy link

neicker commented Jul 19, 2025

@neicker I fixed the places you mentioned. See if they now look okay.

Looks good to me.

@naughtont3
Copy link
Contributor

2025 ASC Q3 Voting Result: Passed (9 Yes, 0 No, 0 Abstain)

@naughtont3 naughtont3 added the First Vote Passed ASC first vote passed label Jul 21, 2025
@naughtont3 naughtont3 requested review from dsolt and naughtont3 and removed request for dsolt July 21, 2025 15:41
@rhc54 rhc54 merged commit 87d6470 into pmix:master Jul 21, 2025
1 check passed
@rhc54 rhc54 deleted the topic/errata branch July 21, 2025 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Errata First Vote Passed ASC first vote passed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants