RFR: 8301564: Non-C-heap allocated ResourceHashtable keys and values must have trivial destructor

Jorn Vernee jvernee at openjdk.org
Wed Feb 1 21:18:59 UTC 2023


On Tue, 31 Jan 2023 23:23:16 GMT, Ioi Lam <iklam at openjdk.org> wrote:

> To ensure we don't have memory leaks or other more serious memory management bugs, I added static asserts to check that the `K` and `V` types for `ResourceHashtableBase` must have trivial destructors if the table is not `C_HEAP` allocated. Thanks to @JornVernee for the original assert code.
> 
> The asserts actually found a problem with `ClassLoaderStatsClosure::StatsTable`. The space used by the `oop` in the freed hashtable may be trashed when `-XX:+CheckUnhandledOops` is enabled, so live objects that reuse the same space may be corrupted. (I tried but couldn't get it to fail).
> 
> I also had to change `CodeBuffer::SharedTrampolineRequests` because it's `V` type is `LinkListImpl<int>`, which has a non-trivial destructor. (Note: in this case, the destructor doesn't do anything; we can probably make it go away with C++-20. See https://stackoverflow.com/questions/40094871/sfinae-away-a-destructor)
> 
> Testing with tier1~4 + builds-tier5

This looks good, thanks.

I'll put this here for posterity: we discussed changing `ResourceHashtable` to always call the destructor of the key and value (sketch [here](https://github.com/openjdk/jdk/compare/master...JornVernee:jdk:Always_Dtor)). That would allow using `K` and `V` with non-trivial destructors as well. But, this discussion didn't reach a conclusion yet (in particular, there were some doubts expressed over whether a container should be calling destructors at all).

In the mean time, this patch improves upon the status quo.

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

Marked as reviewed by jvernee (Reviewer).

PR: https://git.openjdk.org/jdk/pull/12355


More information about the hotspot-dev mailing list