Remove extraneous changes beyond avoiding the false positive warnings

[kannanjgithub]

super.shutdown() is never expected to throw, so some of the changes were not required.
Also removing the redundant unit test.

Rework of PR #12705.

[becomeStar]

Thanks for the clarification.

I may be missing something, but I wanted to better understand the intent of the dummy this.getClass() check in the final ordering.

Since phantom.clearSafely() sets shutdown=true via getAndSet(true), it seems that once this call has executed, the orphan-warning false positive should already be avoided even if the wrapper becomes unreachable afterward.

This might be a minor point, but would placing the dummy this.getClass() check before phantom.clearSafely() potentially reduce the chance of GC happening before the shutdown is marked, and thus lower the risk of false positives?

Just trying to make sure I understand the reasoning correctly.

[ejona86]

Since phantom.clearSafely() sets shutdown=true via getAndSet(true), it seems that once this call has executed, the orphan-warning false positive should already be avoided even if the wrapper becomes unreachable afterward.

The concern was that when inlining or eager field reads happen, fields from ManagedChannelOrphanWrapper could be on the stack such that the object itself is no longer needed. That would cause the wrapper to be unreachable before the getAndSet(true).

That may seem strange, but as Reference.reachabilityFence() says: "The garbage collector may reclaim an object even if the fields of that object are still in use". So the JVM does allow such oddities. But the case it mentions also seems like it'd impact virtually all finalizers, including the few cases Effective Java says finalizers may actually be appropriate: forgetting to call close() and releasing native resources. And it seems those are indeed the cases mentioned in the discussion referenced indirectly from JEP 193.