Skip to content

Fix installing code-signed binaries on macOS#941

Merged
aiuto merged 1 commit intobazelbuild:mainfrom
luispadron:luis/fix-concurrent-installs
Mar 4, 2025
Merged

Fix installing code-signed binaries on macOS#941
aiuto merged 1 commit intobazelbuild:mainfrom
luispadron:luis/fix-concurrent-installs

Conversation

@luispadron
Copy link
Contributor

@luispadron luispadron commented Feb 25, 2025 •
edited
Loading

When running a pkg_install on a code-signed binary on macOS, that is already executing from a previous install... I was getting Killed: 9 when attempting to run the newly installed executable.

This can be reproduced semi-consistently with something like:

# Run an install & execute the installed binary, background immediately, then do it again.
bazel run //:install --destdir bin && ./bin/foo &; \
bazel run //:install --destdir bin && ./bin/foo

This is apparently a long-standing "quirk" on macOS when executing code-signed binaries.

This code is incorrect because it modifies the command-line tool’s executable file in place. macOS caches information about the code’s signature in the kernel. It doesn’t flush that cache when you modify the file’s contents. Modifying the file in place yields a mismatch between the file’s contents and the in-kernel cache, which can cause a hard-to-reproduce code-signing crash the next time you run the tool.

To update a file that contains signed code without risking this crash, write the updated code to a temporary file and replace the existing file with that temporary one

Using a temporary file eliminates the risk of a code-signing crash because the in-kernel cache is associated with the old file, which remains unmodified. The new file gets its own in-kernel cache, built from the contents of that new file.

Source: https://developer.apple.com/documentation/security/updating-mac-software

Maybe we could do this for just macOS but this should be supported on all platforms.

@luispadron luispadron force-pushed the luis/fix-concurrent-installs branch from 23b7014 to 28646eb Compare February 25, 2025 04:13
@luispadron luispadron marked this pull request as ready for review February 25, 2025 04:54
@luispadron luispadron changed the title Fix installing an installed running process Fix installing an installed running process on macOS Feb 25, 2025
@luispadron luispadron force-pushed the luis/fix-concurrent-installs branch 2 times, most recently from df66b56 to a865451 Compare February 25, 2025 05:00
@luispadron luispadron changed the title Fix installing an installed running process on macOS Fix installing code-signed binaries on macOS Feb 25, 2025
@luispadron luispadron force-pushed the luis/fix-concurrent-installs branch from a865451 to 94112a9 Compare February 25, 2025 05:15
@aiuto
Copy link
Collaborator

aiuto commented Feb 27, 2025

An install tests seems to be failing .https://buildkite.com/bazel/rules-pkg/builds/3583#01953b88-df12-4579-afb0-a560668ecd50

There are also a bunch of other broken tests which I have a fix for in #943
Let's get that in and see if this PR is actually clean.

@aiuto
Copy link
Collaborator

aiuto commented Mar 2, 2025

Can you rebase against head and push again. I did some things to fix the baseline continuous build, so I want to see the tests stay clean.

@luispadron luispadron force-pushed the luis/fix-concurrent-installs branch from 4bc0c7d to 0818c65 Compare March 4, 2025 04:13
@luispadron luispadron force-pushed the luis/fix-concurrent-installs branch from 0818c65 to 9f1d430 Compare March 4, 2025 04:32
@luispadron
Copy link
Contributor Author

luispadron commented Mar 4, 2025

@aiuto thanks! There was a legit error on linux due to calling replace on different file-systems. I fixed this with dir in the tempfile call.

@aiuto aiuto merged commit 31cab20 into bazelbuild:main Mar 4, 2025
3 checks passed
@luispadron
Copy link
Contributor Author

luispadron commented Mar 11, 2025

@aiuto any plans to create a release soon?

@aiuto
Copy link
Collaborator

aiuto commented Mar 18, 2025

1.1 came out last week.

rdesgroppes added a commit to rdesgroppes/rules_pkg that referenced this pull request Nov 5, 2025
@rdesgroppes
Fix Windows file locking issue in pkg_install
d740cae 
`NamedTemporaryFile` turns out to lock the file on Windows, preventing
`os.replace` from moving it:
```
$ bazel run //[redacted]:install -- --destdir=d:/est
[...]
INFO: Running command line: [redacted]/install.exe <args omitted>
INFO: Installing to [redacted]
Traceback (most recent call last):
  File "[redacted]\install_install_script.py", line 92, in _do_file_copy
    os.replace(tmp_file.name, dest)
PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: '[redacted]\\tmp4wd6gg1t' -> 'd:/est/filename.ext'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "[redacted]\install_install_script.py", line 307, in <module>
    sys.exit(main(sys.argv))
             ^^^^^^^^^^^^^^
  File "[redacted]\install_install_script.py", line 303, in main
    installer.do_the_thing()
  File "[redacted]\install_install_script.py", line 206, in do_the_thing
    self._install_file(entry)
  File "[redacted]\install_install_script.py", line 126, in _install_file
    self._do_file_copy(entry.src, entry.dest)
  File "[redacted]\install_install_script.py", line 94, in _do_file_copy
    pathlib.Path(tmp_file.name).unlink(missing_ok=True)
  File "[redacted]\rules_python++python+python_3_12_x86_64-pc-windows-msvc\Lib\pathlib.py", line 1342, in unlink
    os.unlink(self)
PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: '[redacted]\\tmp4wd6gg1t'

Intuition: _Any problem in computer science can be solved with another
level of indirection._ (https://bwlampson.site/Slides/TuringLecture.htm)

The change therefore proposes to use `TemporaryDirectory` instead of
`NamedTemporaryFile` as an indirection to avoid a file lock from being
held, which allows the file to be freely written and moved on all
platforms while maintaining the same atomic replace behavior for macOS
code-signed binaries introduced in commit
31cab20 (bazelbuild#941).
@rdesgroppes rdesgroppes mentioned this pull request Nov 5, 2025
Merged
rdesgroppes added a commit to rdesgroppes/rules_pkg that referenced this pull request Nov 5, 2025
@rdesgroppes
Fix Windows file locking issue in pkg_install
8dc3a32 
`NamedTemporaryFile` turns out to lock the file on Windows, preventing
`os.replace` from moving it:
```
$ bazel run //[redacted]:install -- --destdir=d:\est
[...]
INFO: Running command line: [redacted]/install.exe <args omitted>
INFO: Installing to [redacted]
Traceback (most recent call last):
  File "[redacted]\install_install_script.py", line 92, in _do_file_copy
    os.replace(tmp_file.name, dest)
PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: '[redacted]\\tmp4wd6gg1t' -> 'd:\est/filename.ext'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "[redacted]\install_install_script.py", line 307, in <module>
    sys.exit(main(sys.argv))
             ^^^^^^^^^^^^^^
  File "[redacted]\install_install_script.py", line 303, in main
    installer.do_the_thing()
  File "[redacted]\install_install_script.py", line 206, in do_the_thing
    self._install_file(entry)
  File "[redacted]\install_install_script.py", line 126, in _install_file
    self._do_file_copy(entry.src, entry.dest)
  File "[redacted]\install_install_script.py", line 94, in _do_file_copy
    pathlib.Path(tmp_file.name).unlink(missing_ok=True)
  File "[redacted]\rules_python++python+python_3_12_x86_64-pc-windows-msvc\Lib\pathlib.py", line 1342, in unlink
    os.unlink(self)
PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: '[redacted]\\tmp4wd6gg1t'

Intuition: _Any problem in computer science can be solved with another
level of indirection._ (https://bwlampson.site/Slides/TuringLecture.htm)

The change therefore proposes to use `TemporaryDirectory` instead of
`NamedTemporaryFile` as an indirection to avoid a file lock from being
held, which allows the file to be freely written and moved on all
platforms while maintaining the same atomic replace behavior for macOS
code-signed binaries introduced in commit
31cab20 (bazelbuild#941).
rdesgroppes added a commit to rdesgroppes/rules_pkg that referenced this pull request Nov 5, 2025
@rdesgroppes
Fix Windows file locking issue in pkg_install
c968563 
`NamedTemporaryFile` turns out to lock the file on Windows, preventing
`os.replace` from moving it:
```
windows> bazel run --enable_runfiles //[redacted]:install -- --destdir=d:\est
[...]
INFO: Running command line: [redacted]/install.exe <args omitted>
INFO: Installing to [redacted]
Traceback (most recent call last):
  File "[redacted]\install_install_script.py", line 92, in _do_file_copy
    os.replace(tmp_file.name, dest)
PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: '[redacted]\\tmp4wd6gg1t' -> 'd:\est/filename.ext'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "[redacted]\install_install_script.py", line 307, in <module>
    sys.exit(main(sys.argv))
             ^^^^^^^^^^^^^^
  File "[redacted]\install_install_script.py", line 303, in main
    installer.do_the_thing()
  File "[redacted]\install_install_script.py", line 206, in do_the_thing
    self._install_file(entry)
  File "[redacted]\install_install_script.py", line 126, in _install_file
    self._do_file_copy(entry.src, entry.dest)
  File "[redacted]\install_install_script.py", line 94, in _do_file_copy
    pathlib.Path(tmp_file.name).unlink(missing_ok=True)
  File "[redacted]\rules_python++python+python_3_12_x86_64-pc-windows-msvc\Lib\pathlib.py", line 1342, in unlink
    os.unlink(self)
PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: '[redacted]\\tmp4wd6gg1t'

Intuition: _Any problem in computer science can be solved with another
level of indirection._ (https://bwlampson.site/Slides/TuringLecture.htm)

The change therefore proposes to use `TemporaryDirectory` instead of
`NamedTemporaryFile` as an indirection to avoid a file lock from being
held, which allows the file to be freely written and moved on all
platforms while maintaining the same atomic replace behavior for macOS
code-signed binaries introduced in commit
31cab20 (bazelbuild#941).
aiuto pushed a commit that referenced this pull request Nov 6, 2025
@rdesgroppes
Fix Windows file locking issue in pkg_install (#983)
6cdaba6 
`NamedTemporaryFile` turns out to lock the file on Windows, preventing
`os.replace` from moving it:
```
windows> bazel run --enable_runfiles //[redacted]:install -- --destdir=d:\est
[...]
INFO: Running command line: [redacted]/install.exe <args omitted>
INFO: Installing to [redacted]
Traceback (most recent call last):
  File "[redacted]\install_install_script.py", line 92, in _do_file_copy
    os.replace(tmp_file.name, dest)
PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: '[redacted]\\tmp4wd6gg1t' -> 'd:\est/filename.ext'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "[redacted]\install_install_script.py", line 307, in <module>
    sys.exit(main(sys.argv))
             ^^^^^^^^^^^^^^
  File "[redacted]\install_install_script.py", line 303, in main
    installer.do_the_thing()
  File "[redacted]\install_install_script.py", line 206, in do_the_thing
    self._install_file(entry)
  File "[redacted]\install_install_script.py", line 126, in _install_file
    self._do_file_copy(entry.src, entry.dest)
  File "[redacted]\install_install_script.py", line 94, in _do_file_copy
    pathlib.Path(tmp_file.name).unlink(missing_ok=True)
  File "[redacted]\rules_python++python+python_3_12_x86_64-pc-windows-msvc\Lib\pathlib.py", line 1342, in unlink
    os.unlink(self)
PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: '[redacted]\\tmp4wd6gg1t'

Intuition: _Any problem in computer science can be solved with another
level of indirection._ (https://bwlampson.site/Slides/TuringLecture.htm)

The change therefore proposes to use `TemporaryDirectory` instead of
`NamedTemporaryFile` as an indirection to avoid a file lock from being
held, which allows the file to be freely written and moved on all
platforms while maintaining the same atomic replace behavior for macOS
code-signed binaries introduced in commit
31cab20 (#941).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cgrindel cgrindel cgrindel approved these changes

@aiuto aiuto aiuto approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@luispadron @aiuto @cgrindel