search

Kimundi / owning-ref-rs

A library for creating references that carry their owner with them.
MIT License
362 stars 50 forks source link

Send/Sync impls are unsafe #26

Closed Storyyeller closed 7 years ago

Storyyeller commented 7 years ago

I believe the following impls are unsafe

unsafe impl<O: Send, T: ?Sized> Send for OwningRef<O, T> {}
unsafe impl<O: Sync, T: ?Sized> Sync for OwningRef<O, T> {}
unsafe impl<O: Send, T: ?Sized> Send for OwningRefMut<O, T> {}
unsafe impl<O: Sync, T: ?Sized> Sync for OwningRefMut<O, T> {}

You need to check not only O, but also &T/&mut T. For example, if T is a Cell, this would improperly implement Sync. This can happen either if O contains a mutex that you map() into, or you map() into an unrelated 'static reference.

Kimundi commented 7 years ago

I think you are right - I'll add the necessary constraints and see what happens.

Kimundi commented 7 years ago

I pushed a new version, feel free to close this if the new impls look right to you.

Storyyeller commented 7 years ago

Looks good to me. Thanks for the speedy fix!

Storyyeller commented 7 years ago

P.S. You may want to consider yanking the previous versions if you haven't done so already to reduce the risk that people accidentally depend on them.