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.

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

fix: reduce unnecessary filesytem copies when spawning plugins #628

Merged
merged 1 commit into from
Nov 14, 2024

Conversation

patrickjcasey
Copy link
Contributor

Here is a video of this branch having two macOS hc check commands work at the same time!

Screen.Recording.2024-11-14.at.12.34.58.PM.mov

To trigger this bug on macOS, the following steps were the easiest way I could find to reliably trigger this

  1. Have a plugin running
  2. While it is still running, copy over the running binary location on disk
  3. Attempt to run this new binary (it will fail)

To avoid this issue, hc now checks the sha256 of the source and destination before copying the files and skips the copy if the hashes are identical

@patrickjcasey patrickjcasey linked an issue Nov 14, 2024 that may be closed by this pull request
@patrickjcasey patrickjcasey force-pushed the patrickjcasey/macos-plugins-fail-to-spawn branch from 13a65f6 to 0054e60 Compare November 14, 2024 17:41
@patrickjcasey patrickjcasey changed the title fix: reduce unnecessary filesytem copies fix: reduce unnecessary filesytem copies when spawning plugins Nov 14, 2024
@j-lanson
Copy link
Collaborator

Resolves #600 .

Copy link
Collaborator

@j-lanson j-lanson left a comment

Choose a reason for hiding this comment

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

Looks good, just one small nit

hipcheck/src/plugin/retrieval.rs Outdated Show resolved Hide resolved
@patrickjcasey patrickjcasey force-pushed the patrickjcasey/macos-plugins-fail-to-spawn branch from 0054e60 to f2b0900 Compare November 14, 2024 18:21
@patrickjcasey
Copy link
Contributor Author

Screen.Recording.2024-11-14.at.1.18.27.PM.mov

hc check running simultaneously on the latest code in this branch

On macOS, there are issues with overwriting a binary with a copy
command, if it is already running, due to how the kernel caches binary
signatures. To mitigate this, copying to a tempfile, then moving it
resolves the issue
@patrickjcasey patrickjcasey force-pushed the patrickjcasey/macos-plugins-fail-to-spawn branch from f2b0900 to 7e00a49 Compare November 14, 2024 18:23
Copy link
Collaborator

@j-lanson j-lanson left a comment

Choose a reason for hiding this comment

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

Looks good, thanks a bunch Patrick!

@patrickjcasey patrickjcasey merged commit 0bc8963 into main Nov 14, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

hc fails to start plugins on macOS
2 participants