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

Destroy and teardown should somehow mark layers of the stack as torn down on success #2368

Open
jbaublitz opened this issue Jan 6, 2021 · 3 comments
Assignees

Comments

@jbaublitz
Copy link
Member

@mulkieran While working on #2355, I noticed that a lot of our destroy methods take &mut self. Would it make more sense here to use self and let Rust's ownership checking guarantee that the entity can't be used after it's destroyed? I need to at least change this for the crypt layer as I've tried to make the code simpler by guaranteeing that if you have a CryptHandle, the device is active. However, I'm wondering if this would add a benefit across our code base.

@jbaublitz jbaublitz self-assigned this Jan 6, 2021
@jbaublitz jbaublitz assigned bmr-cymru and unassigned jbaublitz Aug 2, 2022
@jbaublitz
Copy link
Member Author

I think this is now obsolete.

@jbaublitz jbaublitz assigned jbaublitz and unassigned bmr-cymru Mar 17, 2023
@jbaublitz jbaublitz reopened this Mar 17, 2023
@jbaublitz
Copy link
Member Author

jbaublitz commented Mar 17, 2023

Reopening this because of #3274 and #3285

@jbaublitz
Copy link
Member Author

I think taking self is actually a bad idea just because of the fallible nature of the teardown methods. I'd almost rather still take &mut self but create a new data type to mark a layer as deactivated so that it can't be accessed afterwards. I think this is a larger and longer term redesign so I'm going to put this to the side for now.

@jbaublitz jbaublitz changed the title Should our destroy methods take self instead of &mut self? Destroy and teardown should somehow mark layers of the stack as torn down on success Mar 23, 2023
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

No branches or pull requests

2 participants