Skip to content

Commit

Permalink
Fix segfaults caused by modifying existing shared library (#295)
Browse files Browse the repository at this point in the history
* Fix segfaults caused by modifying existing shared library

When the built shared library is copied to the correct location, it
actually *modifies* any existing shared library, which causes any
running process using it to segfault.

To fix this, we first delete any existing shared library (which is safe
to do) and then copy the newly built shared library to the correct
location as a new file.

Related to #257

* Use os.rename

* Use os.replace

* Update setuptools_rust/build.py

Co-authored-by: Adam Reichold <adamreichold@users.noreply.github.com>

* Update changelog

Co-authored-by: Adam Reichold <adamreichold@users.noreply.github.com>
  • Loading branch information
erikjohnston and adamreichold committed Oct 4, 2022
1 parent 96684af commit e9aef63
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 1 deletion.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Changelog

## Unreleased
### Fixed
- Fix a bug where rebuilding the library would cause any running processes using it to segfault. [#295](https://github.com/PyO3/setuptools-rust/pull/295)

## 1.5.2 (2022-09-19)
### Fixed
- Fix regression in `dylib` build artifacts not being found since 1.5.0. [#290](https://github.com/PyO3/setuptools-rust/pull/290)
Expand Down
13 changes: 12 additions & 1 deletion setuptools_rust/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,18 @@ def install_extension(
os.makedirs(os.path.dirname(ext_path), exist_ok=True)

log.info("Copying rust artifact from %s to %s", dylib_path, ext_path)
shutil.copyfile(dylib_path, ext_path)

# We want to atomically replace any existing library file. We can't
# just copy the new library directly on top of the old one as that
# causes the existing library to be modified (rather the replaced).
# This means that any process that currently uses the shared library
# will see it modified and likely segfault.
#
# We first copy the file to the same directory, as `os.rename`
# doesn't work across file system boundaries.
temp_ext_path = ext_path + "~"
shutil.copyfile(dylib_path, temp_ext_path)
os.replace(temp_ext_path, ext_path)

if sys.platform != "win32" and not debug_build:
args = []
Expand Down

0 comments on commit e9aef63

Please sign in to comment.