On Thu, 19 Jun 2025 06:33:08 GMT, Thomas Stuefe <stu...@openjdk.org> wrote:
>> Coleen Phillimore has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Add a basic gtest. > > I feel apprehensive about this; the solution feels pretty complex and I am > not fully convinced this is the simplest solution for this problem. > > How much space to we lose in real life? Side note: I see the payload of the > jmethodID block in NMT is allocated with mtInternal, so we don't see it in > NMT. We should add jmethodIDs as an own category to NMT. > > A pragmatic alternative solution could be to do delete them, but delayed: > keep the last N methodblocks undeleted. It is rare that JNI accesses > jmethodIDs long after they have been deleted. Typically, the bad access > happens close after class unloading, e.g. because of concurrency problems in > customer code. > > We could then make the parameter N configurable, and thus give customers and > supporters a tool to check for these kind of errors. > > (I briefly wondered whether we could just mmap these blocks, and > uncommit/mprotect them on release, so that we stop paying the memory costs > but don't release the address space; but the coarser page size allocation > granularity would make this probably forbidding in terms of mem cost per > class) > @tstuefe I think your method would still have a race and still allow a stale > pointer crash, but just less likely, with more code to manage how to delete > these pointers. > > The memory leak for jmethodIDs has been observed in our testing, resulting in > native OOMs. We discussed a few things to solve this in the bugs, and this > was the best idea we had at the time. It is a big change though. Okay, fair enough. Thank you. ------------- PR Comment: https://git.openjdk.org/jdk/pull/25267#issuecomment-2995060097
