RFR: 8185746: Remove Mutex destructor assertion

[Kim Barrett]

> On Aug 2, 2017, at 9:25 PM, coleen.phillimore at oracle.com wrote:
> 
> 
> 
> On 8/2/17 8:09 PM, David Holmes wrote:
>> On 3/08/2017 9:55 AM, Kim Barrett wrote:
>>>> On Aug 2, 2017, at 7:49 PM, coleen.phillimore at oracle.com wrote:
>>>> 
>>>> 
>>>> Looks good.
>>> 
>>> Thanks.
>>> 
>>>> On 8/2/17 7:36 PM, Kim Barrett wrote:
>>>>>> On Aug 2, 2017, at 7:31 PM, Kim Barrett <kim.barrett at oracle.com> wrote:
>>>>>> 
>>>>>>> On Aug 2, 2017, at 7:23 PM, David Holmes <david.holmes at oracle.com> wrote:
>>>>>>> 
>>>>>>> Hi Kim,
>>>>>>> 
>>>>>>> On 3/08/2017 8:41 AM, Kim Barrett wrote:
>>>>>>>> Please review this small change to improve the debugging experience
>>>>>>>> when a mutex is destroyed in a bad state.
>>>>>>>> I've removed the assert in ~Mutex, which was making the same checks as
>>>>>>>> in ~Monitor, but providing much less information.
>>>>>>> Okay. Please can you add a comment to the ~Mutex noting that eg:
>>>>>>> 
>>>>>>> // Defer any state checking to ~Monitor
>>>>>>> Mutex::~Mutex() { }
>>>>>> Well, I was waffling over just removing it completely.
>>>>> Note also that the structure here makes it pretty easy to accidentally
>>>>> invoke undefined behavior by destructor slicing. That's an entirely
>>>>> different problem, but it bugs me...
>>>> Can you describe what you mean by "destructor slicing"?  Do you mean calling ~Monitor on an instance of Mutex?
>>> 
>>> Yes, exactly that.  Typically by something like
>>> 
>>> Monitor* m = <mutex>;
>>> …
>>> delete m;
>> 
>> virtual destructor needed?
> 
> Yes, that was my question.  I guess with Monitor, we don't want to pay the word for the vtable?  But why not, it already has an embedded 64 character string.

Plus the cache line padding for most instances.  There’s quite a (known) mess in that hierarchy.
See comments for MONITOR_NAME_LEN and for the Mutex class.