Add wrapper type for safety #32
Conversation
| // The following trait bounds ensure that the above unsafe trait implementations | ||
| // won't allow undefined behavior to be introduced. | ||
| #[cfg(debug_assertions)] | ||
| trait AutoSendSync: Send + Sync {} |
There was a problem hiding this comment.
Sorry how does this work? My understanding of how marker traits are handled is that they just tell the auto-trait resolver, "stop digging, you've struck gold." There's no compiler side verification of Send-iness of Sync-iness, which is why they're unsafe.
There was a problem hiding this comment.
A type can't implement AutoSendSync unless it satisfies the Send + Sync trait requirements as well
There was a problem hiding this comment.
Yeah I understand that, but it doesn't prevent UB. I can still implement AutoSendSync on a type that I've screwed up Send and/or Sync implementations on. So I guess the comment confused me at best. I'm not sure I understand the value add here other than maybe to mark any other types we may hit this bug on before Rust 1.60.0?
To be clear, the issue the original PR is fixing is not UB, it's bypassing a bug in the auto-trait resolver
There was a problem hiding this comment.
You can't implement AutoSendSync unless a type already implements Send + Sync. So if you break the type, the AutoSendSync will cause a compiler error. Without it, your unsafe impl could allow new UB to be undetected
There was a problem hiding this comment.
Ok I get it after some offline brain bashing. Didn't realize that the trait resolver would check field types on explicitly declared marker traits.
I'm cool with this. Going to move it to sdk and mark it deprecated under the assumption we'll need it again
|
Closing in favor of solana-labs#23321 |
Problem
Summary of Changes
Fixes #