Mercurial > public > mercurial-scm > hg-stable
changeset 53017:3ac28aa1430e
rust-annotate: do not impl Sync for Lines
I failed to notice before that bdiff_diff mutates its inputs by storing
bookkeeping information in the `n` and `e` fields. This means &Lines can't be
shared across threads, so Lines cannot be Sync. Fortunately nothing was using
this. The parallel file reading and line splitting only relies on Send.
The reason I didn't notice this to begin with was that Lines contains a
*mut ffi::bdiffline, which is accessible from &Lines, so I never wrote or
thought about &mut Lines. This pattern (using a mutable pointer stored in an
immutable struct) would be expressed with RefCell if this was pure safe Rust.
| author | Mitchell Kember <mkember@janestreet.com> |
|---|---|
| date | Mon, 24 Feb 2025 16:56:09 -0500 |
| parents | e9d942fe7b95 |
| children | 65f6f1fe43ef |
| files | rust/hg-core/src/bdiff.rs |
| diffstat | 1 files changed, 5 insertions(+), 5 deletions(-) [+] |
line wrap: on
line diff
--- a/rust/hg-core/src/bdiff.rs Tue Feb 25 09:27:09 2025 -0500 +++ b/rust/hg-core/src/bdiff.rs Mon Feb 24 16:56:09 2025 -0500 @@ -6,7 +6,7 @@ /// A file split into lines, ready for diffing. pub struct Lines<'a> { /// The array of lines, allocated by bdiff.c. - /// Must never be mutated apart from freeing it in `Drop`. + /// Must never be mutated by Rust code apart from freeing it in `Drop`. array: *mut ffi::bdiff_line, /// Length of the array. len: u32, @@ -66,14 +66,14 @@ } } -// Safety: It is safe for multiple threads to share `&Lines` because -// `self.array` is never mutated except in `Drop`. -unsafe impl Sync for Lines<'_> {} - // Safety: It is safe to send `Lines` to a different thread because // `self.array` is never copied so only one thread will free it. unsafe impl Send for Lines<'_> {} +// It is *not* safe to share `&Lines` between threads because `ffi::bdiff_diff` +// mutates lines by storing bookkeeping information in `n` and `e`. +static_assertions_next::assert_impl!(Lines<'_>: !Sync); + #[derive(Clone)] pub struct LinesIter<'a, 'b> { lines: &'a Lines<'b>,