Skip to content

Add process groups to unix backends#802

Merged
talex5 merged 1 commit intoocaml-multicore:mainfrom
patricoferris:process-groups
Aug 21, 2025
Merged

Add process groups to unix backends#802
talex5 merged 1 commit intoocaml-multicore:mainfrom
patricoferris:process-groups

Conversation

@patricoferris
Copy link
Collaborator

This is, I hope, most of the plumbing for #595. It remains to decide on how best to expose this in Eio.Process if at all.

Also if anyone has a good, reproducible test for this, that would be good too!

Copy link
Collaborator

@talex5 talex5 left a comment

Choose a reason for hiding this comment

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

Looks reasonable to me.

val setpgid : pid:int -> pgid:int -> t
(** [setpgid ~pid ~pgid] sets the process group ID to [pgid] for the process [pid].

If [pid] is [0] then the process ID of the calling process shall be used. *)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would pid ever not be zero? If you want to set the group of an existing process then you can do that at any time; no need for a fork action.

I guess the other interesting thing to mention is that setting both to 0 puts the child in a new process group.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point, I have made the API only take the pgid and added comments explaining the effect of setpgid 0 in 1eb1197

Co-authored-by: Thomas Leonard <talex5@gmail.com>
@talex5
Copy link
Collaborator

talex5 commented Aug 20, 2025

I've rebased it to fix the CI problems. OK to merge once that passes.

@talex5 talex5 merged commit 71a33f4 into ocaml-multicore:main Aug 21, 2025
5 checks passed
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