Re: RFR: 8333361: ubsan, test : libHeapMonitorTest.cpp:518:9: runtime error: null pointer passed as argument 2, which is declared to never be null
On Thu, 20 Jun 2024 11:58:23 GMT, Matthias Baesken wrote: > The following issue has been observed when running > serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorThreadTest (and some > other :tier1 tests) > on Linux with ubsan enabled binaries : > > test/hotspot/jtreg/serviceability/jvmti/HeapMonitor/libHeapMonitorTest.cpp:518:9: > runtime error: null pointer passed as argument 2, which is declared to never > be null > #0 0x80388020 in event_storage_augment_storage > test/hotspot/jtreg/serviceability/jvmti/HeapMonitor/libHeapMonitorTest.cpp:518 > #1 0x80388020 in event_storage_add > test/hotspot/jtreg/serviceability/jvmti/HeapMonitor/libHeapMonitorTest.cpp:557 > #2 0x85e695fc in JvmtiExport::post_sampled_object_alloc(JavaThread*, > oopDesc*) src/hotspot/share/prims/jvmtiExport.cpp:2926 > #3 0x85e558b8 in > JvmtiObjectAllocEventCollector::generate_call_for_allocated() > src/hotspot/share/prims/jvmtiExport.cpp:3074 > #4 0x85e56c14 in > JvmtiSampledObjectAllocEventCollector::~JvmtiSampledObjectAllocEventCollector() > src/hotspot/share/prims/jvmtiExport.cpp:3171 > #5 0x85e56c14 in > JvmtiSampledObjectAllocEventCollector::~JvmtiSampledObjectAllocEventCollector() > src/hotspot/share/prims/jvmtiExport.cpp:3166 > #6 0x862ace34 in > MemAllocator::Allocation::notify_allocation_jvmti_sampler() > src/hotspot/share/gc/shared/memAllocator.cpp:196 > #7 0x862af7a4 in MemAllocator::Allocation::~Allocation() > src/hotspot/share/gc/shared/memAllocator.cpp:87 > #8 0x862af7a4 in MemAllocator::allocate() const > src/hotspot/share/gc/shared/memAllocator.cpp:356 > #9 0x86dca4dc in CollectedHeap::array_allocate(Klass*, unsigned long, > int, bool, JavaThread*) > src/hotspot/share/gc/shared/collectedHeap.inline.hpp:41 > #10 0x86dca4dc in TypeArrayKlass::allocate_common(int, bool, > JavaThread*) src/hotspot/share/oops/typeArrayKlass.cpp:93 > #11 0x86dca4dc in TypeArrayKlass::allocate_common(int, bool, > JavaThread*) src/hotspot/share/oops/typeArrayKlass.cpp:89 > #12 0x857f35c8 in InterpreterRuntime::newarray(JavaThread*, > BasicType, int) src/hotspot/share/interpreter/interpreterRuntime.cpp:232 > #13 0x6b094cf4 () LGTM - Marked as reviewed by asteiner (Author). PR Review: https://git.openjdk.org/jdk/pull/19804#pullrequestreview-2130310413
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v8]
On Tue, 19 Mar 2024 20:15:18 GMT, Chris Plummer wrote: >> So should I also use HeapDumpGzipLevel the same way as HeapDumpPath ? Tehn >> we have to change the text in globals.hpp for HeapDumpGzipLevel as well >> because it mentions only the HeapDumpOnOutOfmemoryError case and not the >> jcmd case . > > I actually mentioned HeapDumpGzipLevel as another reason not to make this > change. I'm still not seeing it's value relative to the documentation > headaches it causes. > > @ansteiner commented that: > >> This is really helpful from a support point of view. > > I'd like to understand how. It seems to me that the person using the jcmd has > a lot more interest in where the file ends up than the person launching the > JVM, and can always specify the location with the jcmd. It seems odd to me > that the jcmd user would want to rely on HeapDumpPath when they can just > specify the path with the jcmd and know for sure where the file is going to > end up. In a cloud environment using containers, the HeapDumpPath is automatically set to a file system service to persist the heapdump. However, if a support engineer or DevOps detects high or increasing memory usage and wants to trigger a heapdump via jcmd, they would need to specify the filename. This requires retrieving the set HeapDumpPath from the app environment and copying it to the jcmd to use the persistent file system. This change can help avoid the need for an additional copy and paste step, which is prone to errors. - PR Review Comment: https://git.openjdk.org/jdk/pull/18190#discussion_r1531715298
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v4]
On Fri, 15 Mar 2024 06:59:42 GMT, Christoph Langer wrote: >> src/hotspot/share/runtime/globals.hpp line 565: >> >>> 563: "triggered by jcmd GC.heap_dump without specifying a path, " >>> \ >>> 564: "the path (filename or directory) of the dump file " >>> \ >>> 565: "(defaults to java_pid.hprof in the working >>> directory)") \ >> >> This incorrectly leads one to conclude that if HeapDumpPath is not specified >> and GC.heap_dump is used without specifying a path, the default will be >> java_pid.hprof in the working directory. That's not the case. The jcmd >> will produce an error because it requires that either HeapDumpPath be >> specified or a filename be specified as a jcmd argument (I'm not sure why >> the jcmd does not default to java_pid.hprof) >> >> Also, if you are cleaning up this text, I would suggest changing "is on" to >> "is enabled". Same for HeapDumpGzipLevel below. > > Yes, why not make java_pid.hprof in the working directory the default if > calling jcmd GC.heap_dump without specifying a path? Would there be any > problem with that approach? Could fill up the working directory if the user is not aware of this default. If you will specify the HeapDumpPath or the filename explicitly you should be aware that you will need enough disk space there. - PR Review Comment: https://git.openjdk.org/jdk/pull/18190#discussion_r1525976831
Re: RFR: JDK-8327769: jcmd GC.heap_dump without options should write to location given by -XX:HeapDumpPath, if set [v4]
On Thu, 14 Mar 2024 13:43:09 GMT, Matthias Baesken wrote: >> Currently jcmd command GC.heap_dump only works with an additionally provided >> file name. >> Syntax : GC.heap_dump [options] >> >> In case the JVM has the XX - flag HeapDumpPath set, we should support an >> additional mode where the is optional. >> In case the filename is NOT set, we take the HeapDumpPath (file or >> directory); >> >> new syntax : >> GC.heap_dump [options] .. has precedence over second option >> GC.heap_dump [options] …in case -XX: HeapDumpPath=p is set >> >> This would be a simplification e.g. for support cases where a filename or >> directory is set at JVM startup with -XX: HeapDumpPath=p and writing to the >> path is intended/recommended for usage also in the jcmd case. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > add test HeapDumpJcmdPresetPathTest This is really helpful from a support point of view. - Marked as reviewed by asteiner (Author). PR Review: https://git.openjdk.org/jdk/pull/18190#pullrequestreview-1936916215
