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.

Sign up for GitHub

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

Jump to bottom

Use &mut and mutex lock to safely make Image implement Send #19

Merged
merged 5 commits into from
Dec 8, 2024
Merged

Use &mut and mutex lock to safely make Image implement Send #19

merged 5 commits into from
Dec 8, 2024

Conversation

ids1024
Copy link
Contributor

@ids1024 ids1024 commented Dec 3, 2024

Alternative to #18, as discussed.

Probably should be tested in Smithay to make sure the API changes work well.

@ids1024
Add DerefMut implementations
51f0619
@ids1024
Take &mut self in mutating methods
89ab367 
This makes it possible to depend on a borrowed image not being mutated
for safety.

This is generally more idiomatic in Rust anyway. If needed, the usual
methods can be used for interior mutability.
@ids1024 ids1024 mentioned this pull request Dec 3, 2024
Merged
@Drakulix
Copy link

Drakulix commented Dec 4, 2024

I hate how brittle this seems. (It is way too easy to forget calling the Mutex when new methods are ever introduced.) But it looks like it should work just fine.

@ids1024
Copy link
Contributor Author

ids1024 commented Dec 4, 2024

Yeah. But it's not too bad since only Drop and set_alpha_map/clear_alpha_map need to use the Mutex.

I wish there was some way to test something like this though.

@ids1024
Copy link
Contributor Author

ids1024 commented Dec 4, 2024

Added a Sync implementation as well as Send. I believe that is correct. And I renamed the feature to sync (I don't think there's a standard name for a feature like this, but that seems reasonable.)

@ids1024
Add sync feature to make Image implement Send/Sync
6703a65 
This uses a global mutex when ref counts are changed, which should
prevent data races, as long as `&mut` prevent other mutation.
@ids1024
Update Safety doc on Image::from_ptr to mention mutability
7d0f46d
@ids1024
Fix clippy lints
9d26552
@ids1024 ids1024 marked this pull request as ready for review December 4, 2024 21:00
cmeissl
cmeissl approved these changes Dec 8, 2024
View reviewed changes
Copy link
Owner

@cmeissl cmeissl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@cmeissl cmeissl merged commit 4dee2cf into cmeissl:main Dec 8, 2024
8 checks passed
@ids1024 ids1024 mentioned this pull request Dec 13, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cmeissl cmeissl cmeissl approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ids1024 @Drakulix @cmeissl