Skip to content

Can't use &mut self in tree nodes #55

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

Open
ckaran opened this issue Feb 8, 2021 · 6 comments
Open

Can't use &mut self in tree nodes #55

ckaran opened this issue Feb 8, 2021 · 6 comments
Assignees
Labels
bug Something isn't working

Comments

@ckaran
Copy link
Collaborator

ckaran commented Feb 8, 2021

See the comments at #54 (comment)_ but the gist is that as @RustyYato pointed out you can't use &mut self as I've been using it. Fix this up.

@ckaran ckaran self-assigned this Feb 8, 2021
@ckaran ckaran added the bug Something isn't working label Feb 8, 2021
@RustyYato
Copy link
Owner

I opened a related issue on rust-lang/unsafe-code-guidelines#272, because it looks like it won't be possible to provide a safe API without allocating, which is unfortunate.

@ckaran
Copy link
Collaborator Author

ckaran commented Feb 8, 2021

Yup, just saw it pop up on my feed. I wish I could help in the decisions on this, but I don't (yet) know rust well enough to make that happen. I'll keep reading as new posts pop up in my feed though!

@RustyYato
Copy link
Owner

The conclusion to the discussion was that nodes that hold intrusive collection nodes should be guarded to prevent &mut _ access. One way is to do this is with a stack pin (Pin<&T>), something similar to pin_utils::pin_mut or a wrapper around Pin<Rc<T>>

@ckaran
Copy link
Collaborator Author

ckaran commented Feb 11, 2021

OK, I think that Pin<Rc<T>> might be the easiest to follow. I think I'm going to have to make both thread-safe and thread-unsafe versions, so there will also need to be a Pin<Arc<Mutex<T>>>. Right?

I'm going to admit that I'm learning as I go for some of this, the aliasing rules get weird for me...

@RustyYato
Copy link
Owner

RustyYato commented Feb 11, 2021

I would postpone the thread-safe version. Rust aliasing rules mixed with thread safety is a nightmare. I think a Mutex would be easiest, but it basically makes a &mut _, so it might be unusable. I think ReentrantMutex (from parking_lot) will provide the guarantees that we want. But users of Link and DoubleLink can use those without issue.

For the tutorial, I would stick to Pin<Rc<Node>> if you want to show how to create a safe API, or just show the unsafe API.

@ckaran
Copy link
Collaborator Author

ckaran commented Feb 11, 2021

OK, I'll stick with Pin<Rc<Node>>. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants