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: soundness issue in Somr #9

Merged
merged 1 commit into from Aug 26, 2023
Merged

Conversation

zxch3n
Copy link
Contributor

@zxch3n zxch3n commented Aug 25, 2023

Currently, the following code causes undefined behavior.

    #[test]
    fn test_inner_mut() {
        let five = Somr::new(5);
        fn add(a: &Somr<i32>, b: &Somr<i32>) {
            a.get_mut().map(|x| *x += *b.get().unwrap()).unwrap();
        }

        add(&five, &five);
        assert_eq!(five.get().copied().unwrap(), 10);
    }
CleanShot 2023-08-25 at 23 04 22@2x

The original fn get_mut(&self) -> Option<&mut T> is error-prone, as the compiler cannot prevent programmers from creating multiple mut ref to the same element. Making it unsafe could reduce the scope that needs to be audited.

@zxch3n zxch3n temporarily deployed to development August 25, 2023 15:26 — with GitHub Actions Inactive
@darkskygit darkskygit temporarily deployed to development August 25, 2023 16:08 — with GitHub Actions Inactive
darkskygit
darkskygit previously approved these changes Aug 25, 2023
@zxch3n zxch3n temporarily deployed to development August 26, 2023 05:25 — with GitHub Actions Inactive
@darkskygit darkskygit merged commit 6a409f0 into y-crdt:main Aug 26, 2023
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants