I have had the unfortunate honor of finding an extremely subtle soundness issue with
Erasable::unerase's requirements for implementors (link is to prior version). The short of it is that 1.0.0 documentation suggests creating a temporary shared reference (
&_) from the input pointer, and doing so can potentially cause undefined behavior for some cases the
Erasable trait is supposed to support.
The new documentation (link to documentation for git head) explicitly calls out the issue, and with version 1.1.0 published, implementors of the trait will be required to uphold the new, slightly more restrictive requirements for implementation. The issue is that this is a semver-compatible release.
I've taken some measures to make this less breaking of a fix, and detail them below. I'd like community input on if this seems reasonable, and if perhaps there's a better way to do this than what I've implemented currently. (Additionally, the sooner I can publish the fixed version and yank the problematic one, the better. v1.0.0 is the only version currently published.)
The migration path is made possible via a new default-provided associated const on the trait,
ACK_1_1_0. It's whole point is that it is defaulted to
false and implementors must override it to
true. (To aid in this, the
ERASABLE_ENFORCE_1_1_0_SEMANTICS environment variable will remove the default implementation.) Additionally, users of the
Erasable trait are encouraged to check the value of
ACK_1_1_0 if they actually would be made unsound by
unerase manifesting a temporary shared reference.
On top of this, the edge case that would be unsound is a very unlikely edge case: there has to be an erasable pointer type with shared mutability that is shared between threads, such that a write happens that potentially races with the reference in
unerase. Oh, and the trait and method are already
unsafe and exist to work with raw pointers, so an implementor should be paying attention to these details. It's not a soundness hole in safe code (that my libraries have written), it's a potential soundness hole given a use of my
unsafe APIs in a way previously documented as OK but that I can't think of a real way to end up running into without contrived code specifically to exploit it.
But still, I pride myself in my code being sound, even in obscure edge cases that require
unsafe (but are technically not disallowed by documentation), so the issue needs to be addressed. And if I can do so without a semver-major version bump, that'd be ideal. (And yes, the README does loudly call this out now.))