On Wed, 8 Feb 2023 15:39:17 GMT, Justin King <jck...@openjdk.org> wrote:

> > I read through your explanation, and through the [design 
> > docs](https://github.com/google/sanitizers/wiki/AddressSanitizerLeakSanitizerDesignDocument),
> >  but my question remains unanswered. See below.
> > > > Metaspace objects hold pointers to malloced memory. Metaspace Objects 
> > > > can leak, that's accepted. So these pointers would leak too. Would that 
> > > > not give us tons of false positives?
> > > 
> > > 
> > > No. LSan only cares that it can find pointers to malloc'ed memory, not 
> > > that it can find pointers to Metaspace objects. The Metaspace is 
> > > registered as a root region. It does however take into account ASan 
> > > poisoning, which we added earlier to Metaspace. LSan will not scan 
> > > poisoned memory regions that are part of root regions. We only poison in 
> > > Metaspace when deallocation occurs and unpoison when allocation occurs.
> > 
> > 
> > But they are not poisened.
> > Upon JVM end, the heap still exists, holding class loaders, which hold 
> > references to CLDs. Those hold metaspace arenas (living in Metaspace), 
> > which hold Metaspace chunks, which are registered LSAN root areas, which 
> > hold C-allocated sub structures. These chunks are not poisened since they 
> > have never been returned to the chunk pool. They are still alive.
> > To put it in other words: even I as a human cannot determine that this is 
> > not a C-heap leak. Technically, it is. But it is accepted since we accept 
> > these things leaking. Since we don't support unloading and re-loading the 
> > JVM, we accept that to have a faster shutdown.
> 
> Let's ignore ASan/poisoning for now, its an optimization. I shouldn't have 
> brought it up.
> 
> LSan leak checking, as implemented for Hotspot, happens before the JVM has 
> actually shutdown. It happens immediately after one thread wins the right to 
> begin shutting down and other threads are either waiting or doing other work. 
> Everything is still alive, including CollectedHeap, Metaspace, and CodeHeap. 
> We ignore leaks as a result of shutdown this way, because as you mentioned we 
> are not that concerned. Outside of Hotspot, LSan leak checking is typically 
> done at program exit. If you do that with Hotspot you will definitely get 
> false positives.
> 
> Note: If you want, feel free to run a build with `--enable-asan` and 
> `--enable-lsan` and run some of the test tiers. Some tests will fail due to 
> some existing incompatibilities with ASan (working on fixing those 
> separately), but some will fail with real leaks. I need to report these, just 
> have not gotten around to it yet. Plan was to do so after this integrated. 
> Most of the remaining ones are minor.
> 
> The only time false positives can happen is some of the tests which do some 
> interesting things and use a custom launcher, these don't inherit our options 
> and will attempt to leak check at exit. I am slowly going through and 
> correcting these. They should be rare at this point.
> 
> On a side note, once this discussion is complete and in favor of this change, 
> I should probably add some documentation related to sanitizers in Hotspot 
> including ASan, LSan, and UBSan.

Thanks for the explanation, @jcking . I'm out of time for this one, but since 
this PR is already integrated, you don't need my immediate feedback. I'll play 
around with LSAN when I have time. Finding leaked C-heap substructures from 
dead metaspace objects should certainly be useful.

Cheers, Thomas

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

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

Reply via email to