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. ------------- PR Comment: https://git.openjdk.org/jdk/pull/25267#issuecomment-2992826422
