Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Assign PIDs in process creation order #2908

Merged
merged 3 commits into from
May 4, 2023
Merged

Assign PIDs in process creation order #2908

merged 3 commits into from
May 4, 2023

Conversation

sporksmith
Copy link
Contributor

@sporksmith sporksmith commented May 4, 2023 •
edited

  • Drop guarantee about PID assignment order. Afaik Linux doesn't provide any similar guarantee.
  • Assign PIDs in process creation order instead of based on the ordering in the config file. This is closer to what Linux does in practice. The previous behavior would have been especially strange once we implemented fork.
  • Fix a bug where shadow crashes if the process is already gone at its shutdown_time.

@github-actions github-actions bot added Component: Build Build/install tools and dependencies Component: Documentation In-repository documentation, under docs/ Component: Main Composing the core Shadow executable Component: Testing Unit and integration tests and frameworks labels May 4, 2023
@sporksmith
Drop guarantee about PID assignment
827c0eb 
We previously added this guarantee so that graceful shutdown could be
done via the `kill` command. This can now be done more easily using
`shutdown_signal` and `shutdown_time`.
@sporksmith
Host::add_application: assign process IDs in creation order
9c07b35 
This more closely reflects the behavior on a Linux system.
@sporksmith sporksmith added this to the Code health and maintenance milestone May 4, 2023
@sporksmith sporksmith marked this pull request as ready for review May 4, 2023 19:29
stevenengler
stevenengler approved these changes May 4, 2023
View reviewed changes
src/main/host/host.rs Outdated Show resolved Hide resolved
@sporksmith sporksmith enabled auto-merge May 4, 2023 19:37
@sporksmith sporksmith merged commit 7ec372d into shadow:main May 4, 2023
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stevenengler stevenengler stevenengler approved these changes

Assignees

@sporksmith sporksmith

Labels
Component: Build Build/install tools and dependencies Component: Documentation In-repository documentation, under docs/ Component: Main Composing the core Shadow executable Component: Testing Unit and integration tests and frameworks
Projects
None yet
Milestone
Code health and maintenance
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@sporksmith @stevenengler