rust/hg-core/src/dirstate_tree/owning.rs
author Raphaël Gomès <rgomes@octobus.net>
Tue, 05 Apr 2022 10:55:28 +0200
branchstable
changeset 49000 dd6b67d5c256
parent 48999 cfd270d83169
child 49930 e98fd81bb151
child 50243 6cce0afc1454
permissions -rw-r--r--
rust: fix unsound `OwningDirstateMap` As per the previous patch, `OwningDirstateMap` is unsound. Self-referential structs are difficult to implement correctly in Rust since the compiler is free to move structs around as much as it wants to. They are also very rarely needed in practice, so the state-of-the-art on how they should be done within the Rust rules is still a bit new. The crate `ouroboros` is an attempt at providing a safe way (in the Rust sense) of declaring self-referential structs. It is getting a lot attention and was improved very quickly when soundness issues were found in the past: rather than relying on our own (limited) review circle, we might as well use the de-facto common crate to fix this problem. This will give us a much better chance of finding issues should any new ones be discovered as well as the benefit of fewer `unsafe` APIs of our own. I was starting to think about how I would present a safe API to the old struct but soon realized that the callback-based approach was already done in `ouroboros`, along with a lot more care towards refusing incorrect structs. In short: we don't return a mutable reference to the `DirstateMap` anymore, we expect users of its API to pass a `FnOnce` that takes the map as an argument. This allows our `OwningDirstateMap` to control the input and output lifetimes of the code that modifies it to prevent such issues. Changing to `ouroboros` meant changing every API with it, but it is relatively low churn in the end. It correctly identified the example buggy modification of `copy_map_insert` outlined in the previous patch as violating the borrow rules. Differential Revision: https://phab.mercurial-scm.org/D12429
Ignore whitespace changes - Everywhere: Within whitespace: At end of lines:
49000
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
     1
use crate::{DirstateError, DirstateParents};
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
     2
47954
4afd6cc447b9 rust: Make OwningDirstateMap generic and move it into hg-core
Simon Sapin <simon.sapin@octobus.net>
parents: 47682
diff changeset
     3
use super::dirstate_map::DirstateMap;
4afd6cc447b9 rust: Make OwningDirstateMap generic and move it into hg-core
Simon Sapin <simon.sapin@octobus.net>
parents: 47682
diff changeset
     4
use std::ops::Deref;
47123
d8ac62374943 dirstate-tree: Make `DirstateMap` borrow from a bytes buffer
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
     5
49000
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
     6
use ouroboros::self_referencing;
48999
cfd270d83169 rust: explain why the current `OwningDirstateMap` is unsound
Raphaël Gomès <rgomes@octobus.net>
parents: 48567
diff changeset
     7
47123
d8ac62374943 dirstate-tree: Make `DirstateMap` borrow from a bytes buffer
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
     8
/// Keep a `DirstateMap<'on_disk>` next to the `on_disk` buffer that it
47954
4afd6cc447b9 rust: Make OwningDirstateMap generic and move it into hg-core
Simon Sapin <simon.sapin@octobus.net>
parents: 47682
diff changeset
     9
/// borrows.
49000
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    10
#[self_referencing]
47954
4afd6cc447b9 rust: Make OwningDirstateMap generic and move it into hg-core
Simon Sapin <simon.sapin@octobus.net>
parents: 47682
diff changeset
    11
pub struct OwningDirstateMap {
4afd6cc447b9 rust: Make OwningDirstateMap generic and move it into hg-core
Simon Sapin <simon.sapin@octobus.net>
parents: 47682
diff changeset
    12
    on_disk: Box<dyn Deref<Target = [u8]> + Send>,
49000
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    13
    #[borrows(on_disk)]
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    14
    #[covariant]
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    15
    map: DirstateMap<'this>,
47123
d8ac62374943 dirstate-tree: Make `DirstateMap` borrow from a bytes buffer
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
    16
}
d8ac62374943 dirstate-tree: Make `DirstateMap` borrow from a bytes buffer
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
    17
d8ac62374943 dirstate-tree: Make `DirstateMap` borrow from a bytes buffer
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
    18
impl OwningDirstateMap {
47954
4afd6cc447b9 rust: Make OwningDirstateMap generic and move it into hg-core
Simon Sapin <simon.sapin@octobus.net>
parents: 47682
diff changeset
    19
    pub fn new_empty<OnDisk>(on_disk: OnDisk) -> Self
4afd6cc447b9 rust: Make OwningDirstateMap generic and move it into hg-core
Simon Sapin <simon.sapin@octobus.net>
parents: 47682
diff changeset
    20
    where
49000
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    21
        OnDisk: Deref<Target = [u8]> + Send + 'static,
47954
4afd6cc447b9 rust: Make OwningDirstateMap generic and move it into hg-core
Simon Sapin <simon.sapin@octobus.net>
parents: 47682
diff changeset
    22
    {
4afd6cc447b9 rust: Make OwningDirstateMap generic and move it into hg-core
Simon Sapin <simon.sapin@octobus.net>
parents: 47682
diff changeset
    23
        let on_disk = Box::new(on_disk);
47123
d8ac62374943 dirstate-tree: Make `DirstateMap` borrow from a bytes buffer
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
    24
49000
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    25
        OwningDirstateMapBuilder {
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    26
            on_disk,
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    27
            map_builder: |bytes| DirstateMap::empty(&bytes),
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    28
        }
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    29
        .build()
47123
d8ac62374943 dirstate-tree: Make `DirstateMap` borrow from a bytes buffer
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
    30
    }
d8ac62374943 dirstate-tree: Make `DirstateMap` borrow from a bytes buffer
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
    31
49000
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    32
    pub fn new_v1<OnDisk>(
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    33
        on_disk: OnDisk,
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    34
    ) -> Result<(Self, DirstateParents), DirstateError>
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    35
    where
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    36
        OnDisk: Deref<Target = [u8]> + Send + 'static,
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    37
    {
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    38
        let on_disk = Box::new(on_disk);
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    39
        let mut parents = DirstateParents::NULL;
47954
4afd6cc447b9 rust: Make OwningDirstateMap generic and move it into hg-core
Simon Sapin <simon.sapin@octobus.net>
parents: 47682
diff changeset
    40
49000
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    41
        Ok((
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    42
            OwningDirstateMapTryBuilder {
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    43
                on_disk,
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    44
                map_builder: |bytes| {
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    45
                    DirstateMap::new_v1(&bytes).map(|(dmap, p)| {
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    46
                        parents = p.unwrap_or(DirstateParents::NULL);
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    47
                        dmap
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    48
                    })
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    49
                },
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    50
            }
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    51
            .try_build()?,
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    52
            parents,
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    53
        ))
47123
d8ac62374943 dirstate-tree: Make `DirstateMap` borrow from a bytes buffer
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
    54
    }
d8ac62374943 dirstate-tree: Make `DirstateMap` borrow from a bytes buffer
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
    55
49000
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    56
    pub fn new_v2<OnDisk>(
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    57
        on_disk: OnDisk,
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    58
        data_size: usize,
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    59
        metadata: &[u8],
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    60
    ) -> Result<Self, DirstateError>
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    61
    where
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    62
        OnDisk: Deref<Target = [u8]> + Send + 'static,
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    63
    {
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    64
        let on_disk = Box::new(on_disk);
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    65
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    66
        OwningDirstateMapTryBuilder {
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    67
            on_disk,
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    68
            map_builder: |bytes| {
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    69
                DirstateMap::new_v2(&bytes, data_size, metadata)
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    70
            },
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    71
        }
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    72
        .try_build()
47123
d8ac62374943 dirstate-tree: Make `DirstateMap` borrow from a bytes buffer
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
    73
    }
47954
4afd6cc447b9 rust: Make OwningDirstateMap generic and move it into hg-core
Simon Sapin <simon.sapin@octobus.net>
parents: 47682
diff changeset
    74
49000
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    75
    pub fn with_dmap_mut<R>(
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    76
        &mut self,
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    77
        f: impl FnOnce(&mut DirstateMap) -> R,
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    78
    ) -> R {
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    79
        self.with_map_mut(f)
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    80
    }
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    81
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    82
    pub fn get_map(&self) -> &DirstateMap {
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    83
        self.borrow_map()
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    84
    }
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    85
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    86
    pub fn on_disk(&self) -> &[u8] {
dd6b67d5c256 rust: fix unsound `OwningDirstateMap`
Raphaël Gomès <rgomes@octobus.net>
parents: 48999
diff changeset
    87
        self.borrow_on_disk()
47954
4afd6cc447b9 rust: Make OwningDirstateMap generic and move it into hg-core
Simon Sapin <simon.sapin@octobus.net>
parents: 47682
diff changeset
    88
    }
47123
d8ac62374943 dirstate-tree: Make `DirstateMap` borrow from a bytes buffer
Simon Sapin <simon.sapin@octobus.net>
parents:
diff changeset
    89
}