RFR: 8339159: api/java_rmi/Naming/Rebind.html crashes with SEGV from UTF8::quoted_ascii_length call

[David Holmes]

On Fri, 6 Sep 2024 14:39:49 GMT, Axel Boldt-Christmas <aboldtch at openjdk.org> wrote:

>> src/hotspot/share/classfile/symbolTable.cpp line 181:
>> 
>>> 179:       NOT_PRODUCT(sym = ZapResourceArea ? nullptr : &value;)
>>> 180:       if (!SymbolTable::arena()->Afree(memory, alloc_size)) {
>>> 181:         log_trace_symboltable_helper(sym, "Leaked permanent symbol");
>> 
>> Thinking more about this I'm not sure the log message is really appropriate. IIUC we haven't "leaked" anything, we just can't fast-free it. I'm not sure what the true/false return from `Afree` is really meant to signify. No other callers of `Afree` even check the return value!
>
> Failing to Afree here leaks the memory for the lifetime of the arena (unusable for new allocations), while if it succeeds the memory can be reused during the arenas lifetime.  This arena seems to be created when we start the VM and is never reclaimed, so we effectively leak this memory.
> 
> I do not quite understand why we have a `SymbolTable::arena()`. We only seem to use the arena for the permanent symbols. But why not just allocate with malloc, and in the racy case, where we free a permanent (embedded symbol) node just call free.
> 
> Maybe there is a fear that permanent small malloc allocations would cause fragmentation. Or there is some other history here.

Thanks for taking a look @xmas92 .  As far as I can see Arena's don't actually try to free memory and shrink, so in that sense they always "leak". `Afree` has one fast path that if the thing being freed was the last thing allocated then it reclaims it by resetting the hwm - otherwise it is a no-op other than zapping the freed region. In any case we aren't leaking a symbol per-se just leaking part of the memory used by the `symbolTable`. Given no other code ever checks the result of `Afree` I am tempted to just delete the logging statement. Otherwise the fix presently in the PR will suffice.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/20865#discussion_r1749556721