RFR: 8213481: [REDO] Fix incorrect copy constructors in hotspot

David Holmes david.holmes at oracle.com
Tue Nov 27 21:34:23 UTC 2018


On 28/11/2018 4:12 am, Kim Barrett wrote:
>> On Nov 26, 2018, at 10:21 PM, David Holmes <david.holmes at oracle.com> wrote:
>>
>> On 27/11/2018 1:15 pm, Kim Barrett wrote:
>>>> On Nov 26, 2018, at 8:39 PM, David Holmes <david.holmes at oracle.com> wrote:
>>>>> The ResourceObj assignment operator also should not be assuming the
>>>>> target of the assignment was stack/embeded allocated; that's just not
>>>>> a valid assumption.  The corresponding assertion has been removed.
>>>>
>>>> Isn't that enforcing a usage requirement of ResourceObj? C-heap/resource-area/arena allocated ResourceObj should only be used via pointers - assigning one to another makes no sense to me.
>>> I don’t see any reason for such an artificial restriction, and think it might prevent reasonable uses.
>>> I don’t have a specific use-case in mind, but the natural result is observed from doing the natural thing.
>>
>> What is the "natural result" here? I don't see (and Vladimir seems to share a similar view) how it makes any sense to assign non stack/embedded resource objects to each other?  And mixing assignment across different types of ResourceObj makes no sense to me at all. I'm not even sure there's a usecase for stack/embedded but that at least seem a consistent usage.
>>
>> David
> 
> The "natural result" is that the derived class copy-assign code does
> what it does.  There are things that can go badly wrong here regarding
> lifetimes of embedded object references.  But the asserted restriction
> on the allocated location of the object being copy-assigned to does
> nothing that I can see to prevent those potential problems.
> 
> Can you provide a use-case where the assertion actually does something
> useful?  I removed it because I don’t think such a thing exists.

AFAICS the only time it makes any sense to use assignment is with 
stack/embedded ResourceObj. The assert verifies that is the case.

This seems a very clear cut use of an assert to ensure an API is not 
being misused. Does it prevent all possible misuse - no. But that's not 
a reason to remove what it does do.

Do we ever need the assignment semantics? Maybe we should just prohibit 
it altogether?

David


More information about the hotspot-dev mailing list