RFR(S): 8165439: Convert Test_TempNewSymbol to GTest

Coleen Phillimore coleen.phillimore at oracle.com
Tue Sep 6 19:12:34 UTC 2016


On 9/5/16 7:55 PM, David Holmes wrote:
> Hi Kirill,
>
> On 6/09/2016 3:37 AM, Kirill Zhaldybin wrote:
>> Dear all,
>>
>> Could you please review this fix for 8165439?
>>
>> WebRev: 
>> http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8165439/webrev.00/
>> CR: https://bugs.openjdk.java.net/browse/JDK-8165439
>
> Conversion seems fine but I have to admit I'm baffled by the final test:
>
>  70   Symbol* xyz = SymbolTable::new_symbol("xyz", CATCH);
>   71   int xyzcount = xyz->refcount();
>   72   { // inner scope
>   73     TempNewSymbol s_inner = xyz;
>   74   }
>   75   ASSERT_EQ(xyz->refcount(), xyzcount - 1)
>   76           << "Should have been decremented by dtor in inner scope";
>
> this appears to expect a refcount of zero even though we still have 
> the original reference. I guess I have no idea how this TempNewSymbol 
> ref counting works :)

I think the last test was supposed to test the assignment operator so 
that s_inner and xyz had the same refcount, but then the TempNewSymbol 
destructor runs which decrements the refcount to both (since they were 
assigned).  This is very unsafe though.  I wish I'd put more comments in 
this.

Coleen

>
> Cheers,
> David
>
>
>> Thank you.
>>
>> Regards, Kirill



More information about the hotspot-runtime-dev mailing list