Fix technical undefined behavior

[raphlinus]

I believe that the current drop implementation has technical undefined behavior, because holds a reference to the IUnknown object while that object is in the process of being deallocated. This patch retrieves the function pointer, drops the reference, then does the release.

[raphlinus]

This PR is partly for discussion purposes. I'd really appreciate resolution of the following questions:

• Is this, in fact, an instance of undefined behavior? I've asked around and haven't gotten a solid answer. Either way, what is the rationale? In any case, I believe it's at most "technically UB" because the code clearly doesn't do anything between the time of the release call and the time the reference goes out of scope.

• Do we care about fixing such things? My gut feeling is "yes", as mem::uninitialized was also technically UB. But it's equally plausible we have higher standards for Rust core than Windows bindings, because under the hood the latter are seething with unsafe anyway.

• Does my fix actually get rid of the UB? I believe so, but reasoning about such things is hard, so again would like input from an expert.

Also, this is not entirely an isolated issue. It comes from a deeper exploration of what it means for a COM binding to be safe, and I'm trying to formulate rules that cover, among other things, the interaction between allowing a COM object to be cloned and the ability to define &mut self methods on a wrapper newtype for that COM object. Understanding the rationale about why the existing wio code is or is not unsafe will help me greatly, as both have to do deeply with the scope of references (mutable or no) to underlying IFoo objects during method calls.

@rylev who has expressed interest in COM wrapping on Twitter, and @Connicpu who is trying to balance safety and ergonomics in Direct2D and related crates.

[MaulingMonkey]

Is this, in fact, an instance of undefined behavior?

I believe so. It violates one of the two core rules defined in the rustonomicon: "A reference cannot outlive its referent". In more general cases, it's a dangling reference which can obviously be abused to dereference freed memory and other such nastiness. While it might degrade into only "technically" undefined behavior here since we don't use it afterwards... I prefer not to trust LLVM to do what I want with undefined behavior, "technically" or not.

Do we care about fixing such things?

While I can't speak for others, I do.

Does my fix actually get rid of the UB?

If Unknown takes a &self, maybe not, but otherwise I believe so. Famous last words... but I'm under the impression that rust pointers are supposed to behave roughly like C pointers, in that they can legally dangle without causing instant undefined behavior (they still of course generate undefined behavior upon dereferencing if dangling.)