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