Skip to content

Dist_graph_create*: Finalize the created communicators - #970

Open
eschnett wants to merge 4 commits into
masterfrom
claude/correctness-issue-7-distgraph-free
Open

Dist_graph_create*: Finalize the created communicators#970
eschnett wants to merge 4 commits into
masterfrom
claude/correctness-issue-7-distgraph-free

Conversation

@eschnett

@eschnett eschnett commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@eschnett

Copy link
Copy Markdown
Contributor Author

The tests here fail due to unrelated issues. Merging #982 should help.

Comment thread src/topology.jl
length(sources), sources, source_weights,
length(destinations), destinations, destination_weights,
Info(infokws...), reorder, graph_comm)
graph_comm != COMM_NULL && finalizer(free, graph_comm)

@vchuravy vchuravy Sep 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When does graph_comm == COMM_NULL happen? If that is a case we need to worry about I would rather have the check in free since the docs now tells the user to call free.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The standard doesn't mention that it could be COMM_NULL, I think it cannot. This code just follows the usual pattern, and that pattern isn't necessary here.

Should be we be defensive? This way we keep the check here and also add it inside the finalizer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants