RFR (xs) 8145940: TempNewSymbol should have correct copy and assignment functions

Kim Barrett kim.barrett at oracle.com
Mon Jan 11 23:45:59 UTC 2016

On Jan 7, 2016, at 3:02 PM, Coleen Phillimore <coleen.phillimore at oracle.com> wrote:
> Summary: Add clear() to the assignment operator and add copy constructor.
> Ran all jtreg, colocated and non-colocated tests.   RunThese -jck with PrintSymbolTableSizeHistogram statistics:
>  Percent removed          1.35
>  Reference counts          194583
> clean:
>  Percent removed          1.53
>  Reference counts          194245
> Without a reference counting copy constructor, we could remove a TempNewSymbol's Symbol if a GC happens.  Consider:
> TempNewSymbol ts = SymbolTable::new_symbol("abc");
> // Hit GC
> The ref count for "abc" is 1 when created by new_symbol, and the destructor could run after the copy into ts, decrementing the reference count to 0 again.  GC could unlink that symbol from the symbol table.  Fortunately, we haven't seen this bug.
> open webrev at http://cr.openjdk.java.net/~coleenp/8145940/
> bug link https://bugs.openjdk.java.net/browse/JDK-8145940
> Thanks,
> Coleen

For the assignment operator, I think better is:

void operator=(TempNewSymbol s) {
  Symbol* tmp = s._temp;
  s._temp = _temp;
  _temp = tmp;

This is the well-known copy and swap idiom, with a by-hand "swap".


In the copy constructor, as a matter of style I would initialize _temp
in the initialer list, rather than by assignment in the body.

I think the updated description of TempNewSymbol isn't really right.
Because the conversion constructor doesn't increment the reference
count, it must *not* be used to capture some arbitrary reference to a
Symbol*. Only a new symbol or one obtained from one of the lookup
functions really should be managed by this class.

Regarding copy elision, there's a good discussion here:


More information about the hotspot-runtime-dev mailing list